Geekly articles weekly

Suchen nach Fehlern im Amazon Web Services SDK-Quellcode für .NET

Bild 1


Willkommen bei allen Fans, die den Code eines anderen wegwerfen. :) Heute haben wir in unserem Labor ein neues Material für eine Forschung - den Quellcode des AWS SDK for .NET-Projekts. Zu dieser Zeit haben wir einen Artikel über das Überprüfen des AWS SDK auf C ++ geschrieben. Dann gab es nichts besonders Interessantes. Mal sehen, was .NET der AWS SDK-Version wert ist. Es ist wieder einmal eine großartige Gelegenheit, die Fähigkeiten des PVS-Studio-Analysators zu demonstrieren und die Welt ein bisschen besser zu machen.

Das Amazon Web Services (AWS) SDK für .NET ist eine Reihe von Entwicklertools, mit denen Anwendungen basierend auf .NET in der AWS-Infrastruktur erstellt werden können. Dieser Satz ermöglicht es, den Prozess des Code-Schreibens erheblich zu vereinfachen. Das SDK enthält Sets API .NET für verschiedene AWS-Dienste wie Amazon S3, Amazon EC2, DynamoDB und andere. SDK-Quellcode ist auf GitHub verfügbar.

Wie bereits erwähnt, haben wir zu diesem Zeitpunkt bereits den Artikel über das Überprüfen des AWS SDK auf C ++ geschrieben. Der Artikel erwies sich als klein - nur ein paar Fehler pro 512 Tausend Codezeilen. Dieses Mal haben wir es mit einer viel größeren Größe des Codes zu tun, die ungefähr 34.000 cs-Dateien enthält, und die Gesamtzahl der Codezeilen (ohne leere) beträgt beeindruckende 5 Millionen. Ein kleiner Teil des Codes (200.000 Zeilen in 664-cs-Dateien) fällt für Tests an, ich habe sie nicht berücksichtigt.

Wenn die Qualität des .NET-Codes der SDK-Version ungefähr der von C ++ entspricht (zwei Fehler pro 512 KLOC), sollten wir ungefähr zehnmal mehr Fehler erhalten. Natürlich ist dies eine sehr ungenaue Berechnungsmethode, die die sprachlichen Besonderheiten und viele andere Faktoren nicht berücksichtigt, aber ich denke nicht, dass der Leser jetzt auf langweilige Überlegungen eingehen möchte. Stattdessen schlage ich vor, mit den Ergebnissen fortzufahren.

Die Prüfung wurde mit PVS-Studio 6.27 durchgeführt. Es ist einfach unglaublich, aber dennoch ist es dem AWS SDK für .NET gelungen, 40 Fehler zu erkennen, über die es sich zu sprechen lohnt. Es zeigt nicht nur eine hohe Qualität des SDK-Codes (ca. 4 Fehler pro 512 KLOC), sondern auch eine vergleichbare Qualität des C # PVS-Studio-Analysators im Vergleich zu C ++. Ein tolles Ergebnis!

Autoren des AWS SDK für .NET, Sie sind echte Champions! Mit jedem Projekt demonstrieren Sie eine enorme Qualität des Codes. Es kann ein gutes Beispiel für andere Teams sein. Natürlich wäre ich kein Entwickler eines statischen Analysators, wenn ich nicht meine 2 Cent geben würde. :) Wir arbeiten bereits mit einem Lumberyard-Team von Amazon an der Verwendung von PVS-Studio. Da es sich um ein sehr großes Unternehmen mit einer Reihe von Einheiten auf der ganzen Welt handelt, ist es sehr wahrscheinlich, dass das AWS SDK-Team für .NET noch nie von PVS-Studio gehört hat. Wie auch immer, ich habe keine Anzeichen für die Verwendung unseres Analysators im SDK-Code gefunden, obwohl er nichts aussagt. Zumindest verwendet das Team jedoch den in Visual Studio integrierten Analysator. Es ist großartig, aber Code-Reviews können immer verbessert werden :).

Infolgedessen habe ich es geschafft, einige Fehler im SDK-Code zu finden, und schließlich ist es Zeit, sie zu teilen.

Fehler in der Logik

PVS-Studio-Warnung: V3008 [CWE-563] Der Variablen 'this.linker.s3.region' werden zweimal nacheinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 116, 114. AWSSDK.DynamoDBv2.Net45 S3Link.cs 116

public string Region { get { .... } set { if (String.IsNullOrEmpty(value)) { this.linker.s3.region = "us-east-1"; } this.linker.s3.region = value; } }

Der Analysator warnt vor einer wiederholten Wertzuweisung an dieselbe Variable. Aus dem Code wird deutlich, dass dies auf den Fehler zurückzuführen ist, der die Logik der Programmarbeit verletzt: Der Wert der Variablen this.linker.s3.region ist unabhängig von der Bedingung immer gleich dem Wert des Variablenwerts if (String.IsNullOrEmpty (Wert)) . Die return- Anweisung wurde im Hauptteil des if- Blocks übersehen. Der Code muss wie folgt korrigiert werden:

 public string Region { get { .... } set { if (String.IsNullOrEmpty(value)) { this.linker.s3.region = "us-east-1"; return; } this.linker.s3.region = value; } }

Unendliche Rekursion

PVS-Studio-Warnung: V3110 [CWE-674] Mögliche unendliche Rekursion innerhalb der Eigenschaft 'OnFailure'. AWSSDK.ElasticMapReduce.Net45 ResizeJobFlowStep.cs 171

 OnFailure? onFailure = null; public OnFailure? OnFailure { get { return this.OnFailure; } // <= set { this.onFailure = value; } }

Ein klassisches Beispiel für einen Tippfehler, der zu einer unendlichen Rekursion im get- Accessor der OnFailure- Eigenschaft führt. Anstatt den Wert eines privaten Felds onFailure zurückzugeben, erfolgt der Zugriff auf die Eigenschaft OnFailure . Richtige Variante:

 public OnFailure? OnFailure { get { return this.onFailure; } set { this.onFailure = value; } }

Sie fragen sich vielleicht: "Wie hat es funktioniert?" Bisher - nein wie. Die Eigenschaft wird nirgendwo anders verwendet, dies ist jedoch nur vorübergehend. Irgendwann wird jemand damit anfangen und sicherlich ein unerwartetes Ergebnis erhalten. Um solche Tippfehler zu vermeiden, wird empfohlen, keine Bezeichner zu verwenden, die sich nur im Fall des ersten Buchstabens unterscheiden.

Ein weiterer Kommentar zu dieser Konstruktion ist die Verwendung des Bezeichners, der vollständig mit dem Namen des OnFailure- Typs übereinstimmt . Aus Sicht des Compilers ist dies durchaus akzeptabel, dies erschwert jedoch die Wahrnehmung von Code für eine Person.

Ein weiterer ähnlicher Fehler:

PVS-Studio-Warnung: V3110 [CWE-674] Mögliche unendliche Rekursion innerhalb der Eigenschaft 'SSES3'. AWSSDK.S3.Net45 InventoryEncryption.cs 37

 private SSES3 sSES3; public SSES3 SSES3 { get { return this.SSES3; } set { this.SSES3 = value; } }

Die Situation ist identisch mit der oben beschriebenen. Hier tritt jedoch eine unendliche Rekursion auf, wenn auf die Eigenschaft SSES3 sowohl zum Lesen als auch zum Zuweisen zugegriffen wird . Richtige Variante:

 public SSES3 SSES3 { get { return this.sSES3; } set { this.sSES3 = value; } }

Test unter Berücksichtigung

Jetzt möchte ich eine Aufgabe eines Entwicklers zitieren, die mit der Copy-Paste-Methode erstellt wurde. Sehen Sie sich an, wie Code im Visual Studio-Editor aussieht, und versuchen Sie, einen Fehler zu finden.

Bild 3


PVS-Studio-Warnung: V3029 Die bedingten Ausdrücke der nebeneinander angeordneten if-Anweisungen sind identisch. Überprüfen Sie die Zeilen: 91, 95. AWSSDK.AppSync.Net45 CreateApiKeyResponseUnmarshaller.cs 91

Ich habe den Hauptteil der Methode UnmarshallException reduziert , nachdem ich alles entfernt habe, was nicht benötigt wird. Jetzt können Sie sehen, dass identische Prüfungen aufeinander folgen:

 public override AmazonServiceException UnmarshallException(....) { .... if (errorResponse.Code != null && errorResponse.Code.Equals("LimitExceededException")) { return new LimitExceededException(errorResponse.Message, innerException, errorResponse.Type, errorResponse.Code, errorResponse.RequestId, statusCode); } if (errorResponse.Code != null && errorResponse.Code.Equals("LimitExceededException")) { return new LimitExceededException(errorResponse.Message, innerException, errorResponse.Type, errorResponse.Code, errorResponse.RequestId, statusCode); } .... }

Es scheint, dass der Fehler nicht grob ist - nur eine zusätzliche Überprüfung. Trotzdem kann ein solches Muster häufig auf schwerwiegendere Probleme im Code hinweisen, wenn eine erforderliche Überprüfung nicht durchgeführt wird.

Im Code gibt es mehrere ähnliche Fehler.

PVS-Studio-Warnungen:

  • V3029 Die bedingten Ausdrücke der nebeneinander angeordneten if-Anweisungen sind identisch. Überprüfen Sie die Zeilen: 75, 79. AWSSDK.CloudDirectory.Net45 CreateSchemaResponseUnmarshaller.cs 75
  • V3029 Die bedingten Ausdrücke der nebeneinander angeordneten if-Anweisungen sind identisch. Überprüfen Sie die Zeilen: 105, 109. AWSSDK.CloudDirectory.Net45 GetSchemaAsJsonResponseUnmarshaller.cs 105
  • V3029 Die bedingten Ausdrücke der nebeneinander angeordneten if-Anweisungen sind identisch. Überprüfen Sie die Zeilen: 201, 205. AWSSDK.CodeCommit.Net45 PostCommentForPullRequestResponseUnmarshaller.cs 201
  • V3029 Die bedingten Ausdrücke der nebeneinander angeordneten if-Anweisungen sind identisch. Überprüfen Sie die Zeilen: 101, 105. AWSSDK.CognitoIdentityProvider.Net45 VerifySoftwareTokenResponseUnmarshaller.cs 101
  • V3029 Die bedingten Ausdrücke der nebeneinander angeordneten if-Anweisungen sind identisch. Überprüfen Sie die Zeilen: 72, 76. AWSSDK.Glue.Net45 UpdateConnectionResponseUnmarshaller.cs 72
  • V3029 Die bedingten Ausdrücke der nebeneinander angeordneten if-Anweisungen sind identisch. Überprüfen Sie die Zeilen: 123, 127. AWSSDK.Neptune.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 123
  • V3029 Die bedingten Ausdrücke der nebeneinander angeordneten if-Anweisungen sind identisch. Überprüfen Sie die Zeilen: 167, 171. AWSSDK.Neptune.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 167
  • V3029 Die bedingten Ausdrücke der nebeneinander angeordneten if-Anweisungen sind identisch. Überprüfen Sie die Zeilen: 127, 131. AWSSDK.RDS.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 127
  • V3029 Die bedingten Ausdrücke der nebeneinander angeordneten if-Anweisungen sind identisch. Überprüfen Sie die Zeilen: 171, 175. AWSSDK.RDS.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 171
  • V3029 Die bedingten Ausdrücke der nebeneinander angeordneten if-Anweisungen sind identisch. Überprüfen Sie die Zeilen: 99, 103. AWSSDK.Rekognition.Net45 RecognizeCelebritiesResponseUnmarshaller.cs 99

Was ist das

PVS-Studio-Warnung: V3062 Ein Objekt 'attributeName' wird als Argument für seine eigene Methode verwendet. Überprüfen Sie das erste tatsächliche Argument der Methode 'Enthält'. AWSSDK.MobileAnalytics.Net45 CustomEvent.cs 261

 /// <summary> /// Dictionary that stores attribute for this event only. /// </summary> private Dictionary<string,string> _attributes = new Dictionary<string,string>(); /// <summary> /// Gets the attribute. /// </summary> /// <param name="attributeName">Attribute name.</param> /// <returns>The attribute. Return null of attribute doesn't /// exist.</returns> public string GetAttribute(string attributeName) { if(string.IsNullOrEmpty(attributeName)) { throw new ArgumentNullException("attributeName"); } string ret = null; lock(_lock) { if(attributeName.Contains(attributeName)) // <= ret = _attributes[attributeName]; } return ret; }

Der Analysator hat einen Fehler in der GetAttribute- Methode festgestellt : Eine Zeichenfolge wird überprüft, ob sie sich selbst enthält. Aus der Beschreibung der Methode folgt, dass, wenn der Attributname ( Attributname- Schlüssel) gefunden wird (im Wörterbuch _attributes ), der Attributwert zurückgegeben werden sollte, andernfalls - null . Da die Bedingung attributeName.Contains (attributeName) immer wahr ist, wird versucht, den Wert durch einen Schlüssel zurückzugeben, der möglicherweise nicht in einem Wörterbuch gefunden wird. Anstatt null zurückzugeben, wird dann eine Ausnahme KeyNotFoundException ausgelöst.

Versuchen wir, diesen Code zu beheben. Um besser zu verstehen, wie das geht, sollten Sie sich eine andere Methode ansehen:

 /// <summary> /// Determines whether this instance has attribute the specified /// attributeName. /// </summary> /// <param name="attributeName">Attribute name.</param> /// <returns>Return true if the event has the attribute, else /// false.</returns> public bool HasAttribute(string attributeName) { if(string.IsNullOrEmpty(attributeName)) { throw new ArgumentNullException("attributeName"); } bool ret = false; lock(_lock) { ret = _attributes.ContainsKey(attributeName); } return ret; }

Diese Methode prüft, ob der Attributname ( Attributname- Schlüssel) im Wörterbuch _attributes vorhanden ist . Kehren wir noch einmal zur GetAttribute- Methode zurück und beheben Sie den Fehler:

 public string GetAttribute(string attributeName) { if(string.IsNullOrEmpty(attributeName)) { throw new ArgumentNullException("attributeName"); } string ret = null; lock(_lock) { if(_attributes.ContainsKey(attributeName)) ret = _attributes[attributeName]; } return ret; }

Jetzt macht die Methode genau das, was in der Beschreibung angegeben ist.

Noch ein kleiner Kommentar zu diesem Codefragment. Mir ist aufgefallen, dass die Autoren bei der Arbeit mit dem _attributes- Wörterbuch die Sperre verwenden . Es ist klar, dass dies bei einem Multithread-Zugriff erforderlich ist, aber die Schlosskonstruktion ist ziemlich langsam und umständlich. Anstelle eines Wörterbuchs wäre es in diesem Fall möglicherweise bequemer, eine thread-sichere Version des Wörterbuchs - ConcurrentDictionary - zu verwenden. Auf diese Weise ist keine Sperre erforderlich . Obwohl ich vielleicht nicht über die Besonderheiten des Projekts Bescheid weiß.

Verdächtiges Verhalten

PVS-Studio-Warnung: V3063 [CWE-571] Ein Teil des bedingten Ausdrucks ist immer wahr, wenn er ausgewertet wird: string.IsNullOrEmpty (inferredIndexName). AWSSDK.DynamoDBv2.PCL ContextInternal.cs 802

 private static string GetQueryIndexName(....) { .... string inferredIndexName = null; if (string.IsNullOrEmpty(specifiedIndexName) && indexNames.Count == 1) { inferredIndexName = indexNames[0]; } else if (indexNames.Contains(specifiedIndexName, StringComparer.Ordinal)) { inferredIndexName = specifiedIndexName; } else if (string.IsNullOrEmpty(inferredIndexName) && // <= indexNames.Count > 0) throw new InvalidOperationException("Local Secondary Index range key conditions are used but no index could be inferred from model. Specified index name = " + specifiedIndexName); .... }

Der Analysator war besorgt über die Prüfzeichenfolge.IsNullOrEmpty (inferredIndexName) . In der Tat wird der Zeichenfolge inferredIndexName null zugewiesen, dann wird der Wert dieser Variablen nirgendwo geändert und aus irgendeinem Grund auf null oder eine leere Zeichenfolge überprüft. Sieht verdächtig aus. Schauen wir uns das obige Codefragment genauer an. Ich habe es bewusst nicht reduziert, um die Situation besser zu verstehen. In der ersten if- Anweisung (und auch in der nächsten) wird die Variable specIndexName irgendwie überprüft. Abhängig von den Ergebnissen der Überprüfungen erhält die Variable inferredIndexName einen neuen Wert. Schauen wir uns nun die dritte if- Anweisung an. Der Hauptteil dieser Anweisung ( Auslösen der Ausnahme) wird ausgeführt, wenn indexNames.Count> 0 als erster Teil der gesamten Bedingung, nämlich string.IsNullOrEmpty (inferredIndexName), immer wahr ist. Möglicherweise sind die angegebenen VariablenIndexName und inferredIndexName verwechselt, oder die dritte Prüfung muss ohne andere erfolgen , was eine eigenständige if- Anweisung darstellt:

 if (string.IsNullOrEmpty(specifiedIndexName) && indexNames.Count == 1) { inferredIndexName = indexNames[0]; } else if (indexNames.Contains(specifiedIndexName, StringComparer.Ordinal)) { inferredIndexName = specifiedIndexName; } if (string.IsNullOrEmpty(inferredIndexName) && indexNames.Count > 0) throw new InvalidOperationException(....);

In diesem Fall ist es schwierig, eine eindeutige Antwort auf Optionen zur Behebung dieses Codes zu geben. Auf jeden Fall muss der Autor es überprüfen.

NullReferenceException

PVS-Studio-Warnung: V3095 [CWE-476] Das Objekt 'conditionValues' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 228, 238. AWSSDK.Core.Net45 JsonPolicyWriter.cs 228

 private static void writeConditions(....) { .... foreach (....) { IList<string> conditionValues = keyEntry.Value; if (conditionValues.Count == 0) // <= continue; .... if (conditionValues != null && conditionValues.Count != 0) { .... } .... } }

Es ist ein Klassiker. Die Variable conditionValues wird ohne vorläufige Prüfung auf null verwendet . Während später im Code diese Überprüfung durchgeführt wird. Der Code muss wie folgt korrigiert werden:

 private static void writeConditions(....) { .... foreach (....) { IList<string> conditionValues = keyEntry.Value; if (conditionValues != null && conditionValues.Count == 0) continue; .... if (conditionValues != null && conditionValues.Count != 0) { .... } .... } }

Ich habe mehrere ähnliche Fehler im Code gefunden.

PVS-Studio-Warnungen:

  • V3095 [CWE-476] Das Objekt 'ts.Listeners' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 140, 143. AWSSDK.Core.Net45 Logger.Diagnostic.cs 140
  • V3095 [CWE-476] Das Objekt 'obj' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 743, 745. AWSSDK.Core.Net45 JsonMapper.cs 743
  • V3095 [CWE-476] Das Objekt 'multipartUploadMultipartUploadpartsList' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 65, 67. AWSSDK.S3.Net45 CompleteMultipartUploadRequestMarshaller.cs 65

Die folgende Warnung ist in ihrer Bedeutung sehr ähnlich, aber der Fall ist dem oben diskutierten entgegengesetzt.

PVS-Studio-Warnung: V3125 [CWE-476] Das ' state' -Objekt wurde verwendet, nachdem es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 139, 127. AWSSDK.Core.Net45 RefreshingAWSCredentials.cs 139

 private void UpdateToGeneratedCredentials( CredentialsRefreshState state) { string errorMessage; if (ShouldUpdate) { .... if (state == null) errorMessage = "Unable to generate temporary credentials"; else .... throw new AmazonClientException(errorMessage); } state.Expiration -= PreemptExpiryTime; // <= .... }

Eines der Codefragmente umfasst das Überprüfen des Werts der Statusvariablen auf Null . Im folgenden Code wird die Variable zum Abbestellen des PreemptExpiryTime- Ereignisses verwendet. Es wird jedoch keine Überprüfung auf Null mehr durchgeführt und das Auslösen der Ausnahme NullReferenceException wird möglich. Eine sicherere Version des Codes:

 private void UpdateToGeneratedCredentials( CredentialsRefreshState state) { string errorMessage; if (ShouldUpdate) { .... if (state == null) errorMessage = "Unable to generate temporary credentials"; else .... throw new AmazonClientException(errorMessage); } if (state != null) state.Expiration -= PreemptExpiryTime; .... }

Im Code gibt es andere ähnliche Fehler:

PVS-Studio-Warnungen:

  • V3125 [CWE-476] Das Objekt 'wrapRequest.Content' wurde verwendet, nachdem es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 395, 383. AWSSDK.Core.Net45 HttpHandler.cs 395
  • V3125 [CWE-476] Das Objekt 'datasetUpdates' wurde verwendet, nachdem es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 477, 437. AWSSDK.CognitoSync.Net45 Dataset.cs 477
  • V3125 [CWE-476] Das Objekt 'cORSConfigurationCORSConfigurationcORSRulesListValue' wurde verwendet, nachdem es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 125, 111. AWSSDK.S3.Net45 PutCORSConfigurationRequestMarshaller.cs 125
  • V3125 [CWE-476] Das Objekt 'lifecycleConfigurationLifecycleConfigurationrulesListValue' wurde verwendet, nachdem es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 157, 68. AWSSDK.S3.Net45 PutLifecycleConfigurationRequestMarshaller.cs 157
  • V3125 [CWE-476] Das Objekt 'this.Key' wurde verwendet, nachdem es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 199, 183. AWSSDK.S3.Net45 S3PostUploadRequest.cs 199

Nicht alternative Realität

PVS-Studio-Warnung: V3009 [CWE-393] Es ist seltsam, dass diese Methode immer ein und denselben Wert von 'true' zurückgibt. AWSSDK.Core.Net45 Lexer.cs 651

 private static bool State19 (....) { while (....) { switch (....) { case '"': .... return true; case '\\': .... return true; default: .... continue; } } return true; }

Die Methode gibt immer true zurück . Mal sehen, wie wichtig es für den aufrufenden Code ist. Ich habe die Fälle der Verwendung der State19- Methode überprüft. Es ist daran beteiligt, das Array der Handler fsm_handler_table gleichermaßen mit anderen ähnlichen Methoden zu füllen (es gibt 28 davon mit den Namen, beginnend von State1 bis State28 ). Hierbei ist zu beachten, dass neben State19 für einige andere Handler auch die Warnungen V3009 [CWE-393] ausgegeben wurden. Dies sind Handler: State23, State26, State27, State28 . Die vom Analysator für sie ausgegebenen Warnungen:

  • V3009 [CWE-393] Es ist seltsam, dass diese Methode immer ein und denselben Wert von 'true' zurückgibt. AWSSDK.Core.Net45 Lexer.cs 752
  • V3009 [CWE-393] Es ist seltsam, dass diese Methode immer ein und denselben Wert von 'true' zurückgibt. AWSSDK.Core.Net45 Lexer.cs 810
  • V3009 [CWE-393] Es ist seltsam, dass diese Methode immer ein und denselben Wert von 'true' zurückgibt. AWSSDK.Core.Net45 Lexer.cs 822
  • V3009 [CWE-393] Es ist seltsam, dass diese Methode immer ein und denselben Wert von 'true' zurückgibt. AWSSDK.Core.Net45 Lexer.cs 834

So sehen die Deklaration und die Array-Initialisierung von Handlern aus:

 private static StateHandler[] fsm_handler_table; .... private static void PopulateFsmTables () { fsm_handler_table = new StateHandler[28] { State1, State2, .... State19, .... State23, .... State26, State27, State28 };

Um das Bild zu vervollständigen, sehen wir uns den Code eines der Handler an, für die der Analysator keine Ansprüche hatte, z. B. State2 :

 private static bool State2 (....) { .... if (....) { return true; } switch (....) { .... default: return false; } }

So erfolgt der Aufruf von Handlern:

 public bool NextToken () { .... while (true) { handler = fsm_handler_table[state - 1]; if (! handler (fsm_context)) // <= throw new JsonException (input_char); .... } .... }

Wie wir sehen können, wird eine Ausnahme ausgelöst, wenn false zurückgegeben wird . In unserem Fall wird dies für die Handler State19, State23, State26 State27 und State28 niemals passieren. Sieht verdächtig aus. Auf der anderen Seite haben fünf Handler ein ähnliches Verhalten (geben immer true zurück ). Vielleicht wurde es so erfunden und ist nicht das Ergebnis eines Tippfehlers.

Warum gehe ich so tief in all das? Diese Situation ist insofern von großer Bedeutung, als der statische Analysator häufig nur eine verdächtige Konstruktion anzeigen kann. Und selbst eine Person (keine Maschine), die nicht über ausreichende Kenntnisse über das Projekt verfügt, kann immer noch keine vollständige Antwort auf das Vorhandensein des Fehlers geben, selbst wenn sie Zeit damit verbracht hat, Code zu lernen. Ein Entwickler sollte diesen Code überprüfen.

Sinnlose Schecks

PVS-Studio-Warnung: V3022 [CWE-571] Der Ausdruck 'doLog' ist immer wahr. AWSSDK.Core.Net45 StoredProfileAWSCredentials.cs 235

 private static bool ValidCredentialsExistInSharedFile(....) { .... var doLog = false; try { if (....) { return true; } else { doLog = true; } } catch (InvalidDataException) { doLog = true; } if (doLog) // <= { .... } .... }

Achten Sie auf die Variable doLog . Nach der Initialisierung mit dem falschen Wert erhält diese Variable in allen Fällen den wahren Wert weiter entlang des Codes. Daher ist die Überprüfung, ob (doLog) immer wahr ist. Möglicherweise gab es früher in der Methode einen Zweig, in dem der Variablen doLog kein Wert zugewiesen wurde. Zum Zeitpunkt der Überprüfung kann es den falschen Wert enthalten, der beim Initialisieren empfangen wurde. Aber jetzt gibt es keinen solchen Zweig.

Ein weiterer ähnlicher Fehler:

PVS-Studio-Warnung: V3022 Ausdruck '! Ergebnis' ist immer falsch. AWSSDK.CognitoSync.PCL SQLiteLocalStorage.cs 353

 public void PutValue(....) { .... bool result = PutValueHelper(....); if (!result) <= { _logger.DebugFormat("{0}", @"Cognito Sync - SQLiteStorage - Put Value Failed"); } else { UpdateLastModifiedTimestamp(....); } .... }

Der Analysator behauptet, dass der Wert der Ergebnisvariablen immer wahr ist. Dies ist nur möglich, wenn die Methode PutValueHelper immer true zurückgibt . Schauen Sie sich diese Methode an:

 private bool PutValueHelper(....) { .... if (....)) { return true; } if (record == null) { .... return true; } else { .... return true; } }

In der Tat wird die Methode unter allen Bedingungen true zurückgeben . Darüber hinaus hat der Analysator eine Warnung für diese Methode ausgegeben. PVS-Studio-Warnung: V3009 [CWE-393] Es ist seltsam, dass diese Methode immer ein und denselben Wert von 'true' zurückgibt. SQLiteLocalStorage.cs 1016

Ich habe diese Warnung absichtlich nicht früher zitiert, als ich nach anderen Fehlern V3009 gefragt und sie für diesen Fall gespeichert habe. Daher hat das Tool zu Recht auf den Fehler V3022 im aufrufenden Code hingewiesen .

Kopieren-Einfügen. Wieder

PVS-Studio-Warnung: V3001 Links und rechts vom '||' befinden sich identische Unterausdrücke 'this.token == JsonToken.String'. Betreiber. AWSSDK.Core.Net45 JsonReader.cs 343

 public bool Read() { .... if ( (this.token == JsonToken.ObjectEnd || this.token == JsonToken.ArrayEnd || this.token == JsonToken.String || // <= this.token == JsonToken.Boolean || this.token == JsonToken.Double || this.token == JsonToken.Int || this.token == JsonToken.UInt || this.token == JsonToken.Long || this.token == JsonToken.ULong || this.token == JsonToken.Null || this.token == JsonToken.String // <= )) { .... } .... }

Das Feld this.token wird zweimal mit dem Wert JsonToken.String der Aufzählung JsonToken verglichen . Wahrscheinlich sollte einer der Vergleiche einen anderen Aufzählungswert enthalten. Wenn ja, wurde hier ein schwerwiegender Fehler gemacht.

Refactoring + Unaufmerksamkeit?

PVS-Studio-Warnung: V3025 [CWE-685] Falsches Format. Beim Aufrufen der Funktion 'Format' wird eine andere Anzahl von Formatelementen erwartet. Nicht verwendete Argumente: AWSConfigs.AWSRegionKey. AWSSDK.Core.Net45 AWSRegion.cs 116

 public InstanceProfileAWSRegion() { .... if (region == null) { throw new InvalidOperationException( string.Format(CultureInfo.InvariantCulture, "EC2 instance metadata was not available or did not contain region information.", AWSConfigs.AWSRegionKey)); } .... }

Möglicherweise enthielt die Formatzeichenfolge für die Methode string.Format zuvor das Formatelement {0}, für das das Argument AWSConfigs.AWSRegionKey festgelegt wurde. Dann wurde die Zeichenfolge geändert, das Formatelement wurde entfernt, aber ein Entwickler hat vergessen, das Argument zu entfernen. Das angegebene Codebeispiel funktioniert fehlerfrei (die Ausnahme wurde im umgekehrten Fall ausgelöst - das Formatelement ohne Argument), sieht aber nicht gut aus. Der Code sollte wie folgt korrigiert werden:

 if (region == null) { throw new InvalidOperationException( "EC2 instance metadata was not available or did not contain region information."); }

Unsicher

PVS-Studio-Warnung: V3083 [CWE-367] Unsicherer Aufruf des Ereignisses 'mOnSyncSuccess', NullReferenceException ist möglich. Überlegen Sie, ob Sie einer lokalen Variablen ein Ereignis zuweisen möchten, bevor Sie sie aufrufen. AWSSDK.CognitoSync.PCL Dataset.cs 827

 protected void FireSyncSuccessEvent(List<Record> records) { if (mOnSyncSuccess != null) { mOnSyncSuccess(this, new SyncSuccessEventArgs(records)); } }

Eine häufige Situation eines unsicheren Aufrufs des Ereignishandlers. Ein Benutzer kann sich zwischen der Überprüfung der Variablen mOnSyncSuccess auf null und dem Aufruf eines Handlers abmelden, sodass sein Wert null wird . Die Wahrscheinlichkeit eines solchen Szenarios ist gering, aber es ist immer noch besser, Code sicherer zu machen:

 protected void FireSyncSuccessEvent(List<Record> records) { mOnSyncSuccess?.Invoke(this, new SyncSuccessEventArgs(records)); }

Im Code gibt es andere ähnliche Fehler:

PVS-Studio-Warnungen:

  • V3083 [CWE-367] Unsicherer Aufruf des Ereignisses 'mOnSyncFailure', NullReferenceException ist möglich. Überlegen Sie, ob Sie einer lokalen Variablen ein Ereignis zuweisen möchten, bevor Sie sie aufrufen. AWSSDK.CognitoSync.PCL Dataset.cs 839
  • V3083 [CWE-367] Unsicherer Aufruf des Ereignisses, NullReferenceException ist möglich. Überlegen Sie, ob Sie einer lokalen Variablen ein Ereignis zuweisen möchten, bevor Sie sie aufrufen. AWSSDK.Core.PCL AmazonServiceClient.cs 332
  • V3083 [CWE-367] Unsicherer Aufruf des Ereignisses, NullReferenceException ist möglich. Überlegen Sie, ob Sie einer lokalen Variablen ein Ereignis zuweisen möchten, bevor Sie sie aufrufen. AWSSDK.Core.PCL AmazonServiceClient.cs 344
  • V3083 [CWE-367] Unsicherer Aufruf des Ereignisses, NullReferenceException ist möglich. Überlegen Sie, ob Sie einer lokalen Variablen ein Ereignis zuweisen möchten, bevor Sie sie aufrufen. AWSSDK.Core.PCL AmazonServiceClient.cs 357
  • V3083 [CWE-367] Unsicherer Aufruf des Ereignisses 'mExceptionEvent', NullReferenceException ist möglich. Überlegen Sie, ob Sie einer lokalen Variablen ein Ereignis zuweisen möchten, bevor Sie sie aufrufen. AWSSDK.Core.PCL AmazonServiceClient.cs 366
  • V3083 [CWE-367] Unsicherer Aufruf des Ereignisses, NullReferenceException ist möglich. Überlegen Sie, ob Sie einer lokalen Variablen ein Ereignis zuweisen möchten, bevor Sie sie aufrufen. AWSSDK.Core.PCL AmazonWebServiceRequest.cs 78
  • V3083 [CWE-367] Unsicherer Aufruf des Ereignisses 'OnRead', NullReferenceException ist möglich. Überlegen Sie, ob Sie einer lokalen Variablen ein Ereignis zuweisen möchten, bevor Sie sie aufrufen. AWSSDK.Core.PCL EventStream.cs 97
  • V3083 [CWE-367] Unsicherer Aufruf des Ereignisses, NullReferenceException ist möglich. Überlegen Sie, ob Sie einer lokalen Variablen ein Ereignis zuweisen möchten, bevor Sie sie aufrufen. AWSSDK.Core.Android NetworkReachability.cs 57
  • V3083 [CWE-367] Unsicherer Aufruf des Ereignisses, NullReferenceException ist möglich. Überlegen Sie, ob Sie einer lokalen Variablen ein Ereignis zuweisen möchten, bevor Sie sie aufrufen. AWSSDK.Core.Android NetworkReachability.cs 94
  • V3083 [CWE-367] Unsicherer Aufruf des Ereignisses, NullReferenceException ist möglich. Überlegen Sie, ob Sie einer lokalen Variablen ein Ereignis zuweisen möchten, bevor Sie sie aufrufen. AWSSDK.Core.iOS NetworkReachability.cs 54

Rohklasse

PVS-Studio-Warnung: V3126 Der Typ 'JsonData', der die IEquatable <T> -Schnittstelle implementiert, überschreibt die 'GetHashCode'-Methode nicht. AWSSDK.Core.Net45 JsonData.cs 26

 public class JsonData : IJsonWrapper, IEquatable<JsonData> { .... }

Die JsonData- Klasse enthält ziemlich viel Code, daher habe ich sie nicht vollständig angegeben und nur ihre Deklaration zitiert. Diese Klasse enthält wirklich nicht die überschriebene Methode GetHashCode, die unsicher ist, da sie zu fehlerhaftem Verhalten führen kann, wenn der Typ JsonData beispielsweise für die Arbeit mit Sammlungen verwendet wird. Wahrscheinlich gibt es im Moment kein Problem, aber in Zukunft könnte sich diese Art von Strategie ändern. Dieser Fehler wird in der Dokumentation ausführlicher beschrieben.

Fazit

Dies sind alles interessante Fehler, die ich im Code des AWS SDK für .NET mithilfe des statischen Analysators PVS-Studio feststellen konnte. Ich möchte noch einmal die Qualität des Projekts hervorheben. Ich habe eine sehr kleine Anzahl von Fehlern für 5 Millionen Codezeilen gefunden. Obwohl eine wahrscheinlich gründlichere Analyse der ausgegebenen Warnungen es mir ermöglichen würde, dieser Liste einige weitere Fehler hinzuzufügen. Trotzdem ist es auch sehr wahrscheinlich, dass ich einige der Warnungen umsonst zu Fehlern hinzugefügt habe. Eindeutige Schlussfolgerungen werden in diesem Fall immer nur von einem Entwickler gezogen, der sich im Kontext des geprüften Codes befindet.

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

More articles:


All Articles