Geekly articles weekly

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

Bild 2

In diesem Jahrhundert hat jeder von Cloud-Diensten gehört. Viele Unternehmen haben dieses Marktsegment gemeistert und ihre Cloud-Services in verschiedene Richtungen entwickelt. Unser Team hat sich kürzlich auch für diese Dienste interessiert, um den PVS-Studio Code Analyzer in diese zu integrieren. Wir glauben, dass unsere regelmäßigen Leser bereits erraten, welche Art von Projekt wir diesmal prüfen werden. Die Wahl fiel auf den Code der Cloud-Dienste von Huawei.

Einleitung


Wenn Sie dem PVS-Studio-Team folgen, haben Sie wahrscheinlich bemerkt, dass wir uns in letzter Zeit sehr für Cloud-Technologien interessiert haben. Wir haben bereits mehrere Artikel zu diesem Thema veröffentlicht:


Als ich gerade ein weiteres interessantes Projekt für den kommenden Artikel aufnahm, erhielt ich per E-Mail ein Stellenangebot von Huawei . Beim Sammeln von Informationen über dieses Unternehmen stellte sich heraus, dass es über eigene Cloud-Dienste verfügt. Hauptsache, der Quellcode dieser Dienste ist auf GitHub frei verfügbar. Dies war der Hauptgrund für die Wahl dieses Unternehmens für diesen Artikel. Wie ein chinesischer Weiser sagte: "Unfälle sind nicht zufällig."

Nun ein wenig über unseren Analysator. PVS-Studio ist ein statischer Code-Analysator, der Fehler und Schwachstellen im Quellcode von Programmen identifiziert, die in C, C ++, C # und Java geschrieben wurden. Der Analyzer läuft auf den Plattformen 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:


Projektanalyse


Bei der Suche nach Informationen für den Artikel habe ich festgestellt, dass Huawei über ein Entwicklercenter verfügt, in dem Sie Informationen, Handbücher und den Quellcode für die Cloud-Dienste abrufen können. Eine Vielzahl von Programmiersprachen wurden verwendet, um diese Dienste zu erstellen, jedoch erwiesen sich Sprachen wie Go, Java und Python als die beliebtesten.

Da ich mich auf Java spezialisiere, wurden die Projekte entsprechend ausgewählt. Quellen der im Artikel analysierten Projekte finden Sie im GitHub-Repository huaweicloud .

Um die Projekte zu analysieren, musste ich nur ein paar Aktionen ausführen:

  • Holen Sie sich Projekte aus dem Repository;
  • Befolgen Sie die Anweisungen zum Starten des Java Analyzers und starten Sie die Analyse für jedes Projekt.

Nach der Analyse der Projekte haben wir nur drei herausgegriffen, auf die ich gerne eingehen möchte. Dies ist darauf zurückzuführen, dass die verbleibenden Java-Projekte zu klein waren.

Projektanalyseergebnisse (Anzahl der Warnungen und Anzahl der Dateien):


Da es nur wenige Warnungen gibt, können wir im Allgemeinen über die gute Qualität des Codes sprechen. Darüber hinaus sind nicht alle Operationen gültige Fehler. Dies liegt an der Tatsache, dass der Analysator manchmal nicht über genügend Informationen verfügt, um den richtigen Code vom falschen zu unterscheiden. Aus diesem Grund wird die Analysatordiagnose täglich mithilfe der vom Benutzer erhaltenen Informationen verbessert. Siehe auch den Artikel " Wie und warum statische Analysegeräte gegen falsch positive Ergebnisse vorgehen ".

Bei der Analyse von Projekten habe ich die interessantesten Warnungen ausgewählt, auf die ich in diesem Artikel eingehen werde.

Reihenfolge der Feldinitialisierung


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 im Konstruktor der UntrustedSSL- Klasse eine Ausnahme auftritt, werden Informationen zu dieser Ausnahme im catch-Block mithilfe des LOG- Protokolls protokolliert. Aufgrund der Initialisierungsreihenfolge statischer Felder ist das LOG zum Zeitpunkt der Initialisierung des INSTANCE- Felds jedoch noch nicht initialisiert. Daher wird beim Protokollieren von Ausnahmeinformationen im Konstruktor eine NullPointerException ausgelöst. Diese Ausnahme ist die Ursache für eine andere ExceptionInInitializerError-Ausnahme , die ausgelöst wird, wenn beim Initialisieren des statischen Felds eine Ausnahme aufgetreten ist. Um das beschriebene Problem zu lösen, muss lediglich die LOG- Initialisierung vor der INSTANCE- Initialisierung platziert werden.

Unsichtbarer 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 eigene Zeichen. Der Programmierer benannte die Argumente dieser Methoden gemäß dem Namen der Methode. In der Zeile, auf die der Analysator zeigt, wird 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; } .... }

Hierbei handelt es sich um einen banalen Fehler im Zusammenhang mit Nachlässigkeit, aufgrund dessen die Zuordnung des Arguments zu sich selbst erfolgt. Infolgedessen wird der Wert des angekommenen Arguments nicht wie impliziert dem Suspend- Feld zugewiesen. Richtig:

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

Vorausbestimmte 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 die Argumente von der checkNotNull- Methode auf null überprü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 null ist . Da beide Argumente für die removeFirewallRuleFromPolicy- Methode von der checkNotNull- Methode überprüft werden, ist es nicht sinnvoll, die Argumente erneut auf Nullwerte zu überprüfen. Als erstes Argument wird jedoch ein Ausdruck an die checkState- Methode übergeben, wobei die Argumente firewallPolicyId und firewallRuleId erneut auf null überprüft werden.

Eine ähnliche Warnung wird 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 Argument filtersParams null ist , wird die Methode beendet und gibt einen Wert zurück. Daher ist der Test, auf den der Analysator zeigt, immer wahr, was bedeutet, dass dieser Test keinen Sinn ergibt.

Ähnliches tritt in 13 Klassen auf:

  • 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)
  • usw...

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 den Wert null hat . Dieses Feld wird nur in dieser Methode initialisiert. Wenn also die Methode add aufgerufen wird, löst das Feld m.blockDeviceMapping eine NullPointerException aus .

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), ....); }

Das Ergebnis der statischen Methode FileId.get (path) wird als drittes Argument an den Konstruktor der TrackedFile- Klasse übergeben. Diese Methode kann jedoch null zurückgeben :

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

In dem hierdurch aufgerufenen Konstruktor ändert sich das Argument id erst, wenn es zum ersten Mal verwendet wird:

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

Wenn daher null als drittes Argument an die Methode übergeben wird, tritt eine Ausnahme auf.

Eine ähnliche Situation tritt in einem anderen Fall auf:

  • 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 ; } } .... }

Und wieder NPE. Durch eine Reihe von Überprüfungen in der bedingten Anweisung kann das dataTmpFile- Objekt für die weitere Dereferenzierung auf Null gesetzt werden. Ich denke, hier gibt es zwei Tippfehler, und der Check 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("/")); }

Es versteht sich, dass die URL als Zeichenfolge an diese Methode übergeben wird, die in keiner Weise validiert wird. Nachdem dieser String mit der lastIndexOf- Methode mehrmals gekürzt wurde . Wenn lastIndexOf keine Übereinstimmung in der Zeichenfolge findet, wird -1 zurückgegeben. Dadurch wird eine StringIndexOutOfBoundsException ausgelöst , da die Argumente für die Teilzeichenfolgemethode nicht negative Zahlen sein müssen. Damit die Methode ordnungsgemäß funktioniert, müssen Sie das Eingabeargument validieren oder sicherstellen , dass die Ergebnisse der lastIndexOf- Methoden keine negativen Zahlen sind.

Hier sind einige andere Orte, an denen dies passiert:

  • 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 wurde nicht berücksichtigt, dass der Aufruf der concat- Methode die Hostzeichenfolge aufgrund der Unveränderlichkeit von Objekten des Typs String nicht ändert. Damit die Methode ordnungsgemäß funktioniert, müssen Sie das Ergebnis der concat- Methode der Hostvariablen im if- Block zuweisen. Richtig:

 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(); }

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

Argument wird im 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 haben sie vergessen, das returnType- Argument zu verwenden und dessen Wert dem returnType- Feld zuzuweisen . Wenn die Methode getReturnType aufgerufen wird , gibt das von diesem Konstruktor erstellte Objekt daher den Standardwert null zurück , obwohl höchstwahrscheinlich beabsichtigt war, das Objekt abzurufen , das zuvor an den Konstruktor übergeben wurde.

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); } .... }

Das Vorhandensein von zwei identischen Methoden 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 annehmen, dass sie die entgegengesetzten Aktionen ausführen. Tatsächlich tun beide Methoden dasselbe: Sie erstellen ein ServiceAction- Objekt und geben es zurück. Die Methode disable wurde höchstwahrscheinlich durch Kopieren des Codes der Methode enable erstellt , hat jedoch vergessen, den Methodentext zu ändern.

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 haben wir beschlossen, den Inhalt einer Struktur vom Typ Map auf Nullungleichheit zu prüfen. Dazu ruft das Argument params die Methode get zweimal auf, deren Ergebnis an die Methode checkNotNull übergeben wird. Alles scheint logisch, aber egal wie! In if wird das Argument params auf null geprüft. Danach kann angenommen werden, dass das Eingabeargument null ist , aber vor dieser Überprüfung wurde die Methode get bereits zweimal für 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 weiteren Stellen 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


Moderne Großunternehmen können heutzutage einfach nicht auf die Nutzung von Cloud-Diensten verzichten. Eine große Anzahl von Menschen nutzt diese Dienste, so dass selbst der kleinste Fehler in dem Dienst für viele Menschen zu Problemen sowie zu zusätzlichen Kosten für das Unternehmen führen kann, um die Folgen dieses Fehlers zu beheben. Bei der Entwicklung muss immer der Faktor Mensch berücksichtigt werden, da jeder früher oder später Fehler macht und dieser Artikel ein Beispiel dafür ist. Aus diesem Grund sollten Sie alle möglichen Tools verwenden, um die Qualität des Codes zu verbessern.

PVS-Studio wird Huawei mit Sicherheit über die Ergebnisse der Überprüfung seiner Cloud-Dienste informieren, damit die Entwickler dieses Unternehmens diese genauer untersuchen können.

Die im Artikel gezeigte einmalige Verwendung der statischen Code-Analyse kann nicht alle Vorteile zeigen. Nähere Informationen zur korrekten Verwendung der statischen Analyse finden Sie hier und hier . Sie können den PVS-Studio Analyzer hier herunterladen.



Wenn Sie diesen Artikel mit einem englischsprachigen Publikum teilen möchten, verwenden Sie bitte den Link zur Übersetzung: Valery Komarov. Huawei Cloud: Heute ist es im PVS-Studio bewölkt .

Source: https://habr.com/ru/post/de477566/

More articles:


All Articles