Grüße an alle Fans, die den Code eines anderen kritisiert haben. :) Heute ist in unserem Labor neues Forschungsmaterial der Quellcode des AWS SDK-Projekts für .NET. Einmal haben wir einen Artikel über das Überprüfen des AWS SDK auf C ++ geschrieben. Dann gab es nichts besonders Interessantes. Mal sehen, wie uns die .NET-Version des AWS SDK gefallen wird. Eine gute Gelegenheit, die Fähigkeiten des PVS-Studio-Analysators erneut zu demonstrieren und die Welt ein wenig perfekter zu machen.
Das .NET SDK von Amazon Web Services (AWS) ist ein Entwickler-Toolkit, mit dem .NET-basierte Anwendungen in der AWS-Infrastruktur erstellt und das Schreiben von Code erheblich vereinfacht werden können. Das SDK enthält .NET API-Suites für verschiedene AWS-Dienste wie Amazon S3, Amazon EC2, DynamoDB und andere. Der Quellcode für das SDK wird auf
GitHub gehostet .
Wie gesagt, wir haben einmal
einen Artikel über das Überprüfen des AWS SDK auf C ++ geschrieben. Der Artikel erwies sich als klein - es gab nur ein paar Fehler in 512.000 Codezeilen. Dieses Mal haben wir es mit einer viel größeren Menge an Code zu tun, einschließlich ungefähr 34.000 cs-Dateien, und die Gesamtzahl der Codezeilen (ohne leere) beträgt beeindruckende 5 Millionen. Ein kleiner Teil des Codes (200 Tausend Zeilen in 664 cs-Dateien) fällt auf die Tests, 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. Dies ist natürlich eine sehr ungenaue Berechnungsmethode, bei der Sprachmerkmale und viele andere Faktoren nicht berücksichtigt werden, aber ich glaube nicht, dass der Leser jetzt in langweilige Diskussionen eintauchen möchte. Stattdessen schlage ich vor, direkt zu den Ergebnissen zu gehen.
Die Überprüfung wurde mit PVS-Studio Version 6.27 durchgeführt. Unglaublicherweise konnte der Analysator etwa 40 erwähnenswerte Fehler im AWS SDK für .NET-Code erkennen. Dies zeigt nicht nur die hohe Qualität des SDK-Codes (ca. 4 Fehler pro 512 KLOC), sondern auch die vergleichbare Qualität der C # -Arbeit des PVS-Studio-Analysators im Vergleich zu C ++. Tolles Ergebnis!
Die Autoren des AWS SDK für .NET sind großartig. Von Projekt zu Projekt zeigen sie eine erstaunliche Codequalität. Dies kann ein gutes Beispiel für andere Teams sein. Aber natürlich wäre ich nicht der Entwickler eines statischen Analysators, wenn ich meine 5 Kopeken nicht eingesetzt hätte. :) Wir arbeiten bereits mit dem Amazon Lumberyard-Team an der Verwendung von PVS-Studio. Da es sich jedoch um ein sehr großes Unternehmen mit einer Reihe von Abteilungen auf der ganzen Welt handelt, ist es sehr wahrscheinlich, dass das AWS SDK für .NET-Team noch nie von PVS-Studio gehört hat. Auf jeden Fall habe ich im SDK-Code keine Anzeichen für die Verwendung unseres Analysators gefunden, obwohl dies nichts bedeutet. Das Team
verwendet jedoch mindestens
den in Visual Studio integrierten Analysator. Das ist gut, aber Codeprüfungen können
verbessert werden :).
Infolgedessen konnte ich immer noch einige Fehler im SDK-Code finden, und schließlich ist es Zeit, sie zu teilen.
Fehler in der LogikPVS-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 davor, den Wert derselben Variablen neu zuzuweisen. Aus dem Code wird deutlich, dass dies auf einen Fehler zurückzuführen ist, der gegen die Funktionslogik verstößt: Der Wert der Variablen
this.linker.s3.region ist unabhängig von der
if- Bedingung
(String.IsNullOrEmpty (Wert)) immer gleich
value . Im Hauptteil des
if- Blocks wurde die
Rückgabe übersprungen. 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 RekursionPVS-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; }
Ein klassisches Tippfehlerbeispiel, das im
get- Accessor der OnFailure- Eigenschaft eine unendliche Rekursion verursacht. Anstatt den Wert des privaten Felds zurückzugeben, verweist
onFailure auf die
OnFailure- Eigenschaft. Korrigierte Option:
public OnFailure? OnFailure { get { return this.onFailure; } set { this.onFailure = value; } }
Sie fragen: "Wie hat es funktioniert?" Bisher auf keinen Fall. Die Eigenschaft wird nirgendwo verwendet, ist aber nur vorübergehend. Irgendwann wird jemand damit anfangen und definitiv ein unerwartetes Ergebnis erzielen. Um solche Tippfehler zu vermeiden, wird empfohlen, keine Bezeichner zu verwenden, die sich nur im Großbuchstaben unterscheiden.
Ein weiterer Hinweis zu diesem Entwurf ist die Verwendung eines Bezeichners, der vollständig mit dem Namen des
OnFailure- Typs
übereinstimmt . Aus Sicht des Compilers ist dies durchaus akzeptabel, erschwert jedoch die menschliche Wahrnehmung des Codes.
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. Nur hier tritt beim Zugriff auf die
SSES3- Eigenschaft
zum Lesen und Schreiben eine unendliche Rekursion auf. Korrigierte Option:
public SSES3 SSES3 { get { return this.sSES3; } set { this.sSES3 = value; } }
AchtsamkeitstestDas Folgende ist eine Aufgabe eines Programmierers, der die Copy-Paste-Technik verwenden möchte. Sehen Sie sich an, wie der Code im Visual Studio-Editor aussieht, und versuchen Sie, den Fehler zu finden.
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
UnmarshallException- Methode reduziert und alles Unnötige entfernt. Jetzt können Sie sehen, dass identische Prüfungen nacheinander 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. Ein solches Muster kann jedoch häufig auf schwerwiegendere Probleme im Code hinweisen, wenn eine erforderliche Überprüfung nicht durchgeführt wird.
Es gibt einige ähnliche Fehler im Code.
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 bist duPVS-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
Der Analysator hat einen Fehler in der
GetAttribute- Methode
festgestellt : Die Zeichenfolge wird auf die Tatsache überprüft, dass sie sich selbst enthält. Aus der Beschreibung der Methode folgt, dass, wenn der Attributname (
Attributname- Schlüssel) gefunden wird (im
_attributes- Wörterbuch), der Attributwert zurückgegeben werden sollte, andernfalls
null . Da die Bedingung
attributeName.Contains (attributeName) immer wahr ist, wird in der Realität versucht, einen Wert durch einen Schlüssel zurückzugeben, der möglicherweise nicht im Wörterbuch gefunden wird. Anstatt
null zurückzugeben, wird dann eine KeyNotFoundException ausgelöst.
Versuchen wir, diesen Code zu beheben. Schauen Sie sich eine andere Methode an, um besser zu verstehen, wie das geht:
Diese Methode prüft, ob der Attributname (
Attributname- Schlüssel) im
_attributes- Wörterbuch vorhanden ist.
Kehren wir 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.
Und noch eine kleine Bemerkung zu diesem Codefragment. Ich habe festgestellt, dass Autoren bei der Arbeit mit dem
_attributes- Wörterbuch die
Sperre verwenden . Dies ist natürlich bei Multithread-Zugriff erforderlich, aber das
Sperrkonstrukt ist ziemlich langsam und umständlich. In diesem Fall ist es möglicherweise bequemer, anstelle von
Dictionary eine thread-sichere Version des Wörterbuchs zu verwenden -
ConcurrentDictionary . Dann verschwindet die Notwendigkeit einer
Sperre . Obwohl ich vielleicht keine Funktionen des Projekts kenne.
Verdächtiges VerhaltenPVS-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) &&
Der Analysator hat die
Prüfung string.IsNullOrEmpty (inferredIndexName) alarmiert. 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. Es sieht verdächtig aus. Schauen wir uns das angegebene Code-Snippet 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 VariableIndexName auf irgendeine Weise überprüft. Abhängig vom Ergebnis der Überprüfungen erhält die Variable
inferredIndexName einen neuen Wert. Achten wir nun auf die dritte
if-Anweisung . Der Hauptteil dieser Anweisung (
Auslösen einer Ausnahme) wird ausgeführt, wenn
indexNames.Count> 0 ist , da der erste Teil der Bedingung
string.IsNullOrEmpty (inferredIndexName) ist. Möglicherweise sind die
angegebenen VariablenIndexName und
inferredIndexName einfach verwirrt. Oder die dritte Prüfung sollte ohne
andere sein und eine eigenständige
if-Anweisung darstellen :
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 die Optionen zum Korrigieren dieses Codes zu geben. Aber es ist definitiv notwendig, dass der Autor es inspiziert.
NullReferenceExceptionPVS-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)
Klassiker des Genres. Die Variable
conditionValues wird verwendet, ohne vorher nach
null zu
suchen . Darüber hinaus wird im Code eine solche Überprüfung durchgeführt, aber es ist zu spät. :) :)
Der Code kann wie folgt festgelegt werden:
private static void writeConditions(....) { .... foreach (....) { IList<string> conditionValues = keyEntry.Value; if (conditionValues != null && conditionValues.Count == 0) continue; .... if (conditionValues != null && conditionValues.Count != 0) { .... } .... } }
Einige weitere ähnliche Fehler wurden 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 hat eine sehr ähnliche Bedeutung, aber die Situation ist umgekehrt wie oben beschrieben.
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 enthält eine Überprüfung der Statusvariablen auf
null . Weiter
unten im Code wird die Statusvariable verwendet, um das
PreemptExpiryTime- Ereignis
abzubestellen , während die Prüfung auf
Null nicht mehr durchgeführt wird und eine
NullReferenceException möglich ist . Sicherere Codeoption:
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; .... }
Es gibt andere ähnliche Fehler im Code.
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
Unbestrittene RealitätPVS-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 kritisch dies für den aufrufenden Code ist. Ich habe die Verwendung der
State19- Methode verfolgt. Er beteiligt sich am Füllen des Arrays von
fsm_handler_table-Handlern zusammen mit anderen ähnlichen Methoden (es gibt 28 von ihnen mit Namen von
State1 bis
State28 ). Hierbei ist zu beachten, dass neben
State19 auch Warnungen
V3009 [CWE-393] für einige andere Handler ausgegeben wurden. Dies sind die Handler:
State23, State26, State27, State28 . Vom Analysator für sie ausgegebene 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 sieht die Deklaration und Initialisierung des Handler-Arrays aus:
private static StateHandler[] fsm_handler_table; .... private static void PopulateFsmTables () { fsm_handler_table = new StateHandler[28] { State1, State2, .... State19, .... State23, .... State26, State27, State28 };
Der Vollständigkeit
halber sehen wir uns den Code eines der Handler an, bei dem der Analysator keine Beschwerden hatte, z. B.
State2 :
private static bool State2 (....) { .... if (....) { return true; } switch (....) { .... default: return false; } }
Und so heißen die Handler:
public bool NextToken () { .... while (true) { handler = fsm_handler_table[state - 1]; if (! handler (fsm_context))
Wie Sie 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. Es sieht verdächtig aus. Auf der anderen Seite haben bis zu fünf Handler ein ähnliches Verhalten (geben immer
true zurück ). Vielleicht war dies beabsichtigt und nicht das Ergebnis eines Tippfehlers.
Warum habe ich mich so ausführlich mit all dem befasst? Diese Situation ist sehr bezeichnend in dem Sinne, dass ein statischer Analysator oft nur ein verdächtiges Design anzeigen kann. Und selbst eine Person (keine Maschine), die nicht über ausreichende Kenntnisse über das Projekt verfügt und Zeit damit verbracht hat, den Code zu studieren, kann immer noch keine erschöpfende Antwort auf das Vorhandensein eines Fehlers geben. Der Code muss vom Entwickler studiert werden.
Sinnlose SchecksPVS-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
false wird diese Variable im Code in allen Fällen
true . Die
if (doLog) -Prüfung ist also immer wahr. Möglicherweise gab es früher in dieser Methode einen Zweig, in dem der
doLog- Variablen kein Wert zugewiesen wurde.
Zum Zeitpunkt der Überprüfung konnte sie dann den
falschen Wert enthalten, der während der Initialisierung erhalten 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
PutValueHelper- Methode 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 gab der Analysator eine Warnung für diese Methode aus.
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 andere Fehler von
V3009 in Betracht
gezogen habe , aber ich habe sie für diesen Fall gespeichert. Somit hat der Analysator zu Recht den Fehler
V3022 im aufrufenden Code angezeigt.
Kopieren-Einfügen. WiederPVS-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 ||
Vergleichen Sie das Feld
this.token doppelt mit dem Wert
JsonToken.String der
JsonToken- Aufzählung. Wahrscheinlich sollte einer der Vergleiche einen anderen Aufzählungswert enthalten. Wenn ja, wurde ein schwerwiegender Fehler gemacht.
Refactoring + Nachlässigkeit?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)); } .... }
Wahrscheinlich enthielt die
Formatzeichenfolge für die Methode
string.Format zuvor den Ausgabespezifizierer
{0} , für den das Argument
AWSConfigs.AWSRegionKey festgelegt wurde. Dann wurde die Zeile geändert, der Bezeichner war weg, aber sie haben vergessen, das Argument zu löschen. Das obige Codefragment funktioniert fehlerfrei (eine Ausnahme würde im umgekehrten Fall ausgelöst - ein Bezeichner ohne Argument), sieht aber hässlich aus. Der Code sollte wie folgt festgelegt werden:
if (region == null) { throw new InvalidOperationException( "EC2 instance metadata was not available or did not contain region information."); }
UnsicherPVS-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 ziemlich häufige Situation eines unsicheren Aufrufs des Ereignishandlers. Zwischen dem Überprüfen der Variablen
mOnSyncSuccess auf
null und dem Aufrufen des
Handlers kann ein Ereignis abgemeldet werden, und sein Wert wird Null. Die Wahrscheinlichkeit eines solchen Szenarios ist gering, aber es ist immer noch besser, den Code sicherer zu machen:
protected void FireSyncSuccessEvent(List<Record> records) { mOnSyncSuccess?.Invoke(this, new SyncSuccessEventArgs(records)); }
Es gibt andere ähnliche Fehler im Code.
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
Unvollendete KlassePVS-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 viel Code, daher habe ich ihn nicht vollständig angegeben und mich darauf beschränkt, ihn nur zu deklarieren. Diese Klasse enthält wirklich keine überschriebene
GetHashCode- Methode, was unsicher ist, da dies zu fehlerhaftem Verhalten führen kann, wenn der Typ
JsonData beispielsweise für Sammlungen verwendet wird. Derzeit gibt es wahrscheinlich kein Problem, aber die Strategie für die Verwendung dieses Typs kann sich in Zukunft ändern. Dieser Fehler wird in der
Dokumentation ausführlicher beschrieben.
FazitDas sind all die interessanten Fehler, die ich im AWS SDK für .NET-Code mithilfe des statischen Analysators PVS-Studio festgestellt habe.
Ich betone noch einmal die Qualität des Projekts. Ich habe eine sehr kleine Anzahl von Fehlern für 5 Millionen Codezeilen gefunden. Obwohl eine gründlichere Analyse der ausgegebenen Warnungen es mir wahrscheinlich ermöglichen würde, dieser Liste einige weitere Fehler hinzuzufügen. Es ist aber auch sehr wahrscheinlich, dass ich einige der beschriebenen Warnungen vergeblich auf Fehler zurückgeführt habe. In diesem Fall trifft immer nur der Entwickler, der sich im Kontext des zu überprüfenden Codes befindet, eindeutige Schlussfolgerungen.
Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Sergey Khrenov. Suchen nach Fehlern im Amazon Web Services SDK-Quellcode für .NET