Huawei Cloud: Heute ist es im PVS-Studio bewölkt

Heute kennt jeder Cloud-Dienste. Viele Unternehmen haben dieses Marktsegment geknackt und eigene Cloud-Dienste für verschiedene Zwecke entwickelt. In letzter Zeit hat sich unser Team auch für diese Dienstleistungen interessiert, um den PVS-Studio Code Analyzer in diese zu integrieren. Wahrscheinlich haben unsere regelmäßigen Leser bereits geraten, welche Art von Projekt wir diesmal prüfen werden. Die Wahl fiel auf den Code der Huawei Cloud Services.

Einleitung

Wenn Sie PVS-Studio-Teambeiträgen folgen, haben Sie wahrscheinlich bemerkt, dass wir uns in letzter Zeit intensiv mit Cloud-Technologien befasst haben. Wir haben bereits mehrere Artikel zu diesem Thema veröffentlicht:

• PVS-Studio in den Wolken: Azure DevOps
• PVS-Studio in den Wolken: Travis CI
• PVS-Studio in den Wolken: CircleCI
• PVS-Studio in den Wolken: GitLab CI / CD

Gerade als ich nach einem ungewöhnlichen Projekt für den kommenden Artikel suchte, erhielt ich eine E-Mail mit einem Stellenangebot von Huawei . Nachdem einige Informationen über dieses Unternehmen gesammelt wurden, stellte sich heraus, dass sie ihre eigenen Cloud-Dienste hatten. Hauptsache, der Quellcode dieser Dienste ist auf GitHub verfügbar. Dies war der Hauptgrund für die Wahl dieses Unternehmens für diesen Artikel. Wie ein chinesischer Weiser sagte: "Die Unfälle sind nicht zufällig."

Lassen Sie mich einige Details über unseren Analysator geben. PVS-Studio ist ein statischer Analysator zur Fehlererkennung im Quellcode von Programmen, der in C, C ++, C # und Java geschrieben wurde. Der Analyzer funktioniert unter Windows, Linux und macOS. Neben Plugins für klassische Entwicklungsumgebungen wie Visual Studio oder IntelliJ IDEA kann der Analyzer auch in SonarQube und Jenkins integriert werden:

• Integration der PVS-Studio-Analyseergebnisse in SonarQube
• PVS-Studio läuft in Jenkins

Projektanalyse

Als ich nach dem Artikel recherchierte, stellte ich fest, dass Huawei über ein Entwicklercenter mit verfügbaren Informationen, Handbüchern und Quellen für ihre Cloud-Dienste verfügt. Für die Erstellung dieser Dienste wurde eine Vielzahl von Programmiersprachen verwendet, wobei Sprachen wie Go, Java und Python am häufigsten verwendet wurden.

Da ich mich auf Java spezialisiere, wurden die Projekte nach meinen Kenntnissen und Fähigkeiten ausgewählt. Sie können die im Artikel analysierten Projektquellen in einem GitHub-Repository huaweicloud abrufen .

Um Projekte zu analysieren, brauchte ich nur ein paar Dinge zu tun:

• Holen Sie sich Projekte aus dem Repository;
• Verwenden Sie die Anweisungen zum Starten von Java Analyzer und führen Sie die Analyse für jedes Projekt aus.

Nach der Analyse der Projekte haben wir nur drei ausgewählt, auf die wir gerne eingehen möchten. Dies liegt an der Tatsache, dass die Größe der restlichen Java-Projekte zu gering ausfiel.

Projektanalyseergebnisse (Anzahl der Warnungen und Anzahl der Dateien):

• huaweicloud-sdk-java : 31 - Hoch, 2 - Mittel und 16 - Niedrig, 2700+ Dateien.
• huaweicloud-dis-agent : 7 - Hoch, 6 - Mittel und 6 - Niedrig, über 100 Dateien.
• huaweicloud-sdk-java-dis : 15 - Hoch, 6 - Mittel und 16 - Niedrig, 270+ Dateien.

Es gab nur wenige Warnungen, die auf eine hohe Codequalität hinweisen, zumal nicht alle Warnungen auf echte Fehler hindeuten. Dies liegt an der Tatsache, dass dem Analysator manchmal Informationen fehlen, um den richtigen Code von dem fehlerhaften zu unterscheiden. Aus diesem Grund optimieren wir die Diagnose des Analysators Tag für Tag und greifen dabei auf die Informationen der Benutzer zurück. Gerne können Sie den Artikel " Die Art und Weise, wie statische Analysegeräte gegen falsche Positive kämpfen, und warum sie dies tun" lesen .

Bei der Analyse des Projekts habe ich nur die häufigsten Warnungen herausgegriffen, über die ich in diesem Artikel sprechen werde.

Felder Initialisierungsreihenfolge

V6050 Klasseninitialisierungszyklus ist vorhanden. Die Initialisierung von 'INSTANCE' erscheint vor der Initialisierung von 'LOG'. UntrustedSSL.java (32), UntrustedSSL.java (59), UntrustedSSL.java (33)

public class UntrustedSSL { private static final UntrustedSSL INSTANCE = new UntrustedSSL(); private static final Logger LOG = LoggerFactory.getLogger(UntrustedSSL.class); .... private UntrustedSSL() { try { .... } catch (Throwable t) { LOG.error(t.getMessage(), t); // <= } } } 

Wenn der Klassenkonstruktor UntrustedSSL eine Ausnahme enthält , werden die Informationen zu dieser Ausnahme im catch- Block mithilfe des LOG- Protokolls protokolliert. Aufgrund der Initialisierungsreihenfolge statischer Felder ist LOG zum Zeitpunkt der Initialisierung des INSTANCE- Felds noch nicht initialisiert. Wenn Sie Informationen zu der Ausnahme im Konstruktor protokollieren, führt dies daher zu einer NullPointerException . Diese Ausnahme ist der Grund für eine weitere Ausnahme ExceptionInInitializerError , die ausgelöst wird, wenn bei der Initialisierung des statischen Felds eine Ausnahme aufgetreten ist. Um dieses Problem zu lösen, müssen Sie die LOG- Initialisierung vor der INSTANCE- Initialisierung platzieren .

Unauffälliger Tippfehler

V6005 Die Variable 'this.metricSchema' ist sich selbst zugeordnet. OpenTSDBSchema.java (72)

 public class OpenTSDBSchema { @JsonProperty("metric") private List<SchemaField> metricSchema; .... public void setMetricsSchema(List<SchemaField> metricsSchema) { this.metricSchema = metricSchema; // <= } public void setMetricSchema(List<SchemaField> metricSchema) { this.metricSchema = metricSchema; } .... } 

Beide Methoden legen das metricSchema- Feld fest, aber die Methodennamen unterscheiden sich durch das Symbol. Der Programmierer benannte die Argumente dieser Methoden gemäß dem Namen der Methode. In der Zeile, auf die der Analysator zeigt, wird daher das metricSchema- Feld sich selbst zugewiesen, und das Argument der metricsSchema- Methode wird nicht verwendet.

V6005 Die Variable 'suspend' ist sich selbst zugeordnet. SuspendTransferTaskRequest.java (77)

 public class SuspendTransferTaskRequest { .... private boolean suspend; .... public void setSuspend(boolean suspend) { suspend = suspend; } .... } 

Hier ist ein trivialer Fehler im Zusammenhang mit Nachlässigkeit, aufgrund dessen das Argument suspend sich selbst zugewiesen wird. Infolgedessen wird dem Suspend- Feld nicht der implizite Wert des erhaltenen Arguments zugewiesen. Die richtige Version:

 public void setSuspend(boolean suspend) { this.suspend = suspend; } 

Vorausbestimmung der Bedingungen

Wie so oft bricht die V6007- Regel in Bezug auf die Anzahl der Warnungen vor.

V6007 Der Ausdruck 'firewallPolicyId == null' ist immer falsch. FirewallPolicyServiceImpl.java (125)

 public FirewallPolicy removeFirewallRuleFromPolicy(String firewallPolicyId, String firewallRuleId) { checkNotNull(firewallPolicyId); checkNotNull(firewallRuleId); checkState(!(firewallPolicyId == null && firewallRuleId == null), "Either a Firewall Policy or Firewall Rule identifier must be set"); .... } 

In dieser Methode werden Argumente von der checkNotNull- Methode auf null geprüft :

 @CanIgnoreReturnValue public static <T> T checkNotNull(T reference) { if (reference == null) { throw new NullPointerException(); } else { return reference; } } 

Nachdem Sie das Argument mit der checkNotNull- Methode überprüft haben , können Sie zu 100% sicher sein, dass das an diese Methode übergebene Argument nicht gleich null ist . Da beide Argumente der removeFirewallRuleFromPolicy- Methode von der checkNotNull- Methode überprüft werden, ist ihre weitere Überprüfung auf null nicht sinnvoll. Der Ausdruck, in dem die Argumente firewallPolicyId und firewallRuleId erneut auf null überprüft werden, wird jedoch als erstes Argument an die checkState- Methode übergeben.

Eine ähnliche Warnung wird auch für firewallRuleId ausgegeben:

• V6007 Der Ausdruck 'firewallRuleId == null' ist immer falsch. FirewallPolicyServiceImpl.java (125)

V6007 Der Ausdruck ' filtersParams ! = Null' ist immer wahr. NetworkPolicyServiceImpl.java (60)

 private Invocation<NetworkServicePolicies> buildInvocation(Map<String, String> filteringParams) { .... if (filteringParams == null) { return servicePoliciesInvocation; } if (filteringParams != null) { // <= .... } return servicePoliciesInvocation; } 

Wenn in dieser Methode das filteringParams- Argument null ist , gibt die Methode einen Wert zurück. Aus diesem Grund ist die Prüfung, auf die der Analysator zeigt, immer wahr, was wiederum bedeutet, dass diese Prüfung bedeutungslos ist.

13 weitere Klassen sind ähnlich:

• V6007 Der Ausdruck 'filtersParams! = Null' ist immer wahr. PolicyRuleServiceImpl.java (58)
• V6007 Der Ausdruck 'filtersParams! = Null' ist immer wahr. GroupServiceImpl.java (58)
• V6007 Der Ausdruck 'filtersParams! = Null' ist immer wahr. ExternalSegmentServiceImpl.java (57)
• V6007 Der Ausdruck 'filtersParams! = Null' ist immer wahr. L3policyServiceImpl.java (57)
• V6007 Der Ausdruck 'filtersParams! = Null' ist immer wahr. PolicyRuleSetServiceImpl.java (58)
• und so weiter ...

Nullreferenz

V6008 Potentielle Null-Dereferenzierung von 'm.blockDeviceMapping'. NovaServerCreate.java (390)

 @Override public ServerCreateBuilder blockDevice(BlockDeviceMappingCreate blockDevice) { if (blockDevice != null && m.blockDeviceMapping == null) { m.blockDeviceMapping = Lists.newArrayList(); } m.blockDeviceMapping.add(blockDevice); // <= return this; } 

Bei dieser Methode wird die Initialisierung des Referenzfelds m.blockDeviceMapping nicht durchgeführt, wenn das Argument blockDevice null ist . Dieses Feld wird nur in dieser Methode initialisiert. Wenn Sie also die Methode add aus dem Feld m.blockDeviceMapping aufrufen , tritt eine NullPointerException auf.

V6008 Potentielle Null-Dereferenzierung von 'FileId.get (path)' in Funktion '<init>'. TrackedFile.java (140), TrackedFile.java (115)

 public TrackedFile(FileFlow<?> flow, Path path) throws IOException { this(flow, path, FileId.get(path), ....); } 

Der Konstruktor der TrackedFile- Klasse erhält als drittes Argument das Ergebnis der statischen FileId.get (path) -Methode. Diese Methode kann jedoch null zurückgeben :

 public static FileId get(Path file) throws IOException { if (!Files.exists(file)) { return null; } .... } 

Im über diesen aufgerufenen Konstruktor ändert sich das Argument id erst bei seiner ersten Verwendung:

 public TrackedFile(...., ...., FileId id, ....) throws IOException { .... FileId newId = FileId.get(path); if (!id.equals(newId)) { .... } } 

Wie wir sehen können, tritt eine Ausnahme auf, wenn Null als drittes Argument an die Methode übergeben wird.

Hier ist ein ähnlicher Fall:

• V6008 Potentielle Null-Dereferenzierung von 'Buffer'. PublishingQueue.java (518)

V6008 Mögliche Null-Dereferenzierung von 'dataTmpFile'. CacheManager.java (91)

 @Override public void putToCache(PutRecordsRequest putRecordsRequest) { .... if (dataTmpFile == null || !dataTmpFile.exists()) { try { dataTmpFile.createNewFile(); // <= } catch (IOException e) { LOGGER.error("Failed to create cache tmp file, return.", e); return ; } } .... } 

Nochmal NPE. Eine Reihe von Überprüfungen im bedingten Operator ermöglicht dem Nullobjekt dataTmpFile eine weitere Dereferenzierung. Ich denke, hier gibt es zwei Tippfehler und die Prüfung sollte tatsächlich so aussehen:

 if (dataTmpFile != null && !dataTmpFile.exists()) 

Teilstrings und negative Zahlen

V6009 Die Funktion 'substring' kann den Wert '-1' empfangen, wenn ein nicht negativer Wert erwartet wird. Überprüfen Sie das Argument: 2. RemoveVersionProjectIdFromURL.java (37)

 @Override public String apply(String url) { String urlRmovePojectId = url.substring(0, url.lastIndexOf("/")); return urlRmovePojectId.substring(0, urlRmovePojectId.lastIndexOf("/")); } 

Dies impliziert, dass diese Methode eine URL als Zeichenfolge erhält, die in keiner Weise validiert wird. Später wird diese Zeichenfolge mit der lastIndexOf- Methode mehrmals abgeschnitten . Wenn die Methode lastIndexOf keine Übereinstimmung in der Zeichenfolge findet, wird -1 zurückgegeben. Dies führt zu einer StringIndexOutOfBoundsException , da die Argumente der substring- Methode nicht negative Zahlen sein müssen. Für die korrekte Funktionsweise der Methode muss eine Eingabeargumentvalidierung hinzugefügt oder überprüft werden, ob die Ergebnisse der lastIndexOf- Methode keine negativen Zahlen sind.

Hier sind einige andere Snippets mit ähnlichen Eigenschaften:

• V6009 Die Funktion 'substring' kann den Wert '-1' empfangen, wenn ein nicht negativer Wert erwartet wird. Überprüfen Sie das Argument: 2. RemoveProjectIdFromURL.java (37)
• V6009 Die Funktion 'substring' kann den Wert '-1' empfangen, wenn ein nicht negativer Wert erwartet wird. Überprüfen Sie das Argument: 2. RemoveVersionProjectIdFromURL.java (38)

Vergessenes Ergebnis

V6010 Der Rückgabewert der Funktion 'concat' muss verwendet werden. AKSK.java (278)

 public static String buildCanonicalHost(URL url) { String host = url.getHost(); int port = url.getPort(); if (port > -1) { host.concat(":" + Integer.toString(port)); } return host; } 

Beim Schreiben dieses Codes hat der Autor nicht berücksichtigt, dass ein Aufruf der concat- Methode den Host- String aufgrund der Unveränderlichkeit der Objekte vom Typ String nicht ändert. Damit die Methode korrekt funktioniert, muss das Ergebnis der concat- Methode der Hostvariablen im if- Block zugewiesen werden. Die richtige Version:

 if (port > -1) { host = host.concat(":" + Integer.toString(port)); } 

Nicht verwendete Variablen

V6021 Variable 'url' wird nicht verwendet. TriggerV2Service.java (95)

 public ActionResponse deleteAllTriggersForFunction(String functionUrn) { checkArgument(!Strings.isNullOrEmpty(functionUrn), ....); String url = ClientConstants.FGS_TRIGGERS_V2 + ClientConstants.URI_SEP + functionUrn; return deleteWithResponse(uri(triggersUrlFmt, functionUrn)).execute(); } 

In dieser Methode wird die URL- Variable nach ihrer Initialisierung nicht mehr verwendet. Höchstwahrscheinlich muss die URL- Variable als zweites Argument anstelle von functionUrn an die uri- Methode übergeben werden , da die functionUrn- Variable an der Initialisierung der URL- Variable beteiligt ist.

Das Argument hat den Konstruktor nicht verwendet

V6022 Der Parameter 'returnType' wird im Konstruktorkörper nicht verwendet. HttpRequest.java (68)

 public class HttpReQuest<R> { .... Class<R> returnType; .... public HttpRequest(...., Class<R> returnType) // <= { this.endpoint = endpoint; this.path = path; this.method = method; this.entity = entity; } .... public Class<R> getReturnType() { return returnType; } .... } 

In diesem Konstruktor hat der Programmierer vergessen, das returnType- Argument zu verwenden, und weist dem returnType- Feld seinen Wert zu. Aus diesem Grund wird beim Aufrufen der Methode getReturnType aus dem von diesem Konstruktor erstellten Objekt standardmäßig null zurückgegeben. Am wahrscheinlichsten war es jedoch, dass der Programmierer beabsichtigte, das zuvor an den Konstruktor übergebene Objekt abzurufen.

Gleiche Funktionalität

V6032 Es ist merkwürdig, dass der Hauptteil der Methode 'enable' dem Hauptteil einer anderen Methode ' disable ' vollständig entspricht. ServiceAction.java (32), ServiceAction.java (36)

 public class ServiceAction implements ModelEntity { private String binary; private String host; private ServiceAction(String binary, String host) { this.binary = binary; this.host = host; } public static ServiceAction enable(String binary, String host) { // <= return new ServiceAction(binary, host); } public static ServiceAction disable(String binary, String host) { // <= return new ServiceAction(binary, host); } .... } 

Zwei identische Methoden zu haben, ist kein Fehler, aber die Tatsache, dass zwei Methoden dieselbe Aktion ausführen, ist zumindest seltsam. Wenn wir uns die Namen der oben genannten Methoden ansehen, können wir davon ausgehen, dass sie die entgegengesetzten Aktionen ausführen sollten. Tatsächlich tun beide Methoden dasselbe: Erstellen Sie das ServiceAction- Objekt und geben Sie es zurück. Höchstwahrscheinlich wurde die Methode disable durch Kopieren des Codes der Methode enable erstellt, der Inhalt der Methode blieb jedoch unverändert.

Ich habe vergessen, die Hauptsache zu überprüfen

V6060 Die Referenz 'params' wurde verwendet, bevor sie gegen null verifiziert wurde. DomainService.java (49), DomainService.java (46)

 public Domains list(Map<String, String> params) { Preconditions.checkNotNull(params.get("page_size"), ....); Preconditions.checkNotNull(params.get("page_number"), ....); Invocation<Domains> domainInvocation = get(Domains.class, uri("/domains")); if (params != null) { // <= .... } return domainInvocation.execute(this.buildExecutionOptions(Domains.class)); } 

Bei dieser Methode hat der Autor beschlossen, den Inhalt einer Struktur vom Typ Map auf null zu prüfen. Zu diesem Zweck wird die Methode get vom Argument params zweimal aufgerufen . Das Ergebnis der get- Methode wird an die checkNotNull- Methode übergeben. Alles scheint logisch, aber so ist es nicht! Das Argument params wird in if auf null geprüft. Danach wird erwartet, dass das Eingabeargument möglicherweise null ist , aber vor dieser Überprüfung wurde die get- Methode bereits zweimal von params aufgerufen. Wenn null als Argument an diese Methode übergeben wird, wird beim ersten Aufruf der get- Methode eine Ausnahme ausgelöst.

Eine ähnliche Situation tritt an drei anderen Orten auf:

• V6060 Die Referenz 'params' wurde verwendet, bevor sie gegen null verifiziert wurde. DomainService.java (389), DomainService.java (387)
• V6060 Die Referenz 'params' wurde verwendet, bevor sie gegen null verifiziert wurde. DomainService.java (372), DomainService.java (369)
• V6060 Die Referenz 'params' wurde verwendet, bevor sie gegen null verifiziert wurde. DomainService.java (353), DomainService.java (350)

Fazit

Die heutigen großen Unternehmen können nicht auf die Nutzung von Cloud-Diensten verzichten. Eine große Anzahl von Menschen nutzt diese Dienste. Aus dieser Sicht kann bereits ein kleiner Fehler in einem Service zu Problemen für viele Menschen sowie zu zusätzlichen Verlusten führen, die von einem Unternehmen angehäuft werden, um die nachteiligen Folgen dieses Fehlers zu beheben. Menschliche Fehler sollten immer berücksichtigt werden, zumal früher oder später jeder Fehler macht, wie in diesem Artikel beschrieben. Diese Tatsache untermauert den Einsatz aller möglichen Tools zur Verbesserung der Codequalität.

PVS-Studio wird das Huawei-Unternehmen auf jeden Fall über die Ergebnisse der Überprüfung seiner Cloud-Dienste informieren, damit sich die Huawei-Entwickler mit ihnen befassen können, da die einmalige Verwendung der in diesem Artikel ( 1 , 2 ) beschriebenen statischen Code-Analyse nicht vollständig demonstrieren kann all seine vorteile. Sie können den PVS-Studio Analyzer hier herunterladen.