Salutations à tous les fans de critiquer le code de quelqu'un d'autre. :) Aujourd'hui, dans notre laboratoire, le nouveau matériel de recherche est le code source du projet AWS SDK pour .NET. À un moment donné, nous avons écrit un article sur la vérification du kit SDK AWS pour C ++. Ensuite, il n'y avait rien de particulièrement intéressant. Voyons comment la version .NET du kit AWS SDK nous plaira. Une bonne occasion de démontrer une fois de plus les capacités de l'analyseur PVS-Studio, ainsi que de rendre le monde un peu plus parfait.
Le Kit de développement logiciel (SDK) .NET Amazon Web Services (AWS) est une boîte à outils de développeur conçue pour créer des applications basées sur .NET dans l'infrastructure AWS et simplifier considérablement le processus d'écriture de code. Le SDK comprend des suites d'API .NET pour divers services AWS tels qu'Amazon S3, Amazon EC2, DynamoDB et autres. Le code source du SDK est hébergé sur
GitHub .
Comme je l'ai dit, à un moment donné, nous avons écrit
un article sur la vérification du kit SDK AWS pour C ++. L'article s'est avéré être petit - il n'y a eu que quelques erreurs sur 512 000 lignes de code. Cette fois, nous avons affaire à une quantité de code beaucoup plus importante, dont environ 34 000 fichiers cs, et le nombre total de lignes de code (à l'exclusion des lignes vides) est impressionnant de 5 millions. Une petite partie du code (200 000 lignes dans 664 fichiers cs) tombe sur les tests, je ne les ai pas considérés.
Si la qualité du code .NET de la version SDK est approximativement la même que celle de C ++ (deux erreurs par 512 KLOC), alors nous devrions obtenir environ 10 fois plus d'erreurs. C'est, bien sûr, une méthode de calcul très inexacte qui ne prend pas en compte les caractéristiques du langage et de nombreux autres facteurs, mais je ne pense pas que le lecteur veuille maintenant se plonger dans des discussions ennuyeuses. Au lieu de cela, je propose d'aller directement aux résultats.
La vérification a été effectuée à l'aide de PVS-Studio version 6.27. Incroyablement, l'analyseur a pu détecter environ 40 erreurs dans le code AWS SDK for .NET qui méritaient d'être mentionnées. Cela indique non seulement la haute qualité du code SDK (environ 4 erreurs pour 512 KLOC), mais également la qualité comparable du travail C # de l'analyseur PVS-Studio par rapport à C ++. Excellent résultat!
Les auteurs du kit AWS SDK pour .NET sont excellents. De projet en projet, ils démontrent une qualité de code incroyable. Cela peut servir de bon exemple pour d'autres équipes. Mais, bien sûr, je ne serais pas le développeur d'un analyseur statique si je n'avais pas inséré mes 5 kopecks. :) Nous travaillons déjà avec l'équipe d'Amazon Lumberyard sur l'utilisation de PVS-Studio. Mais comme il s'agit d'une très grande entreprise avec un tas de divisions à travers le monde, il est très probable que l'équipe AWS SDK for .NET n'ait jamais entendu parler de PVS-Studio. En tout cas, dans le code SDK, je n'ai trouvé aucun signe d'utilisation de notre analyseur, bien que cela ne signifie rien. Cependant, l'équipe, au minimum,
utilise l' analyseur intégré à Visual Studio. C'est bien, mais les vérifications de code peuvent être
améliorées :).
En conséquence, j'ai quand même réussi à trouver des erreurs dans le code du SDK, et enfin, il est temps de les partager.
Erreur de logiqueAvertissement PVS-Studio: V3008 [CWE-563] La variable 'this.linker.s3.region' reçoit des valeurs successives deux fois. C'est peut-être une erreur. Vérifiez les lignes: 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; } }
L'analyseur avertit de la réaffectation de la valeur de la même variable. À partir du code, il devient clair que cela est dû à une erreur qui viole la logique de fonctionnement: la valeur de la variable
this.linker.s3.region sera toujours égale à la
valeur , quelle que soit la condition
if (String.IsNullOrEmpty (value)) . Dans le corps du bloc
if , le
retour a été ignoré. Le code doit être corrigé comme suit:
public string Region { get { .... } set { if (String.IsNullOrEmpty(value)) { this.linker.s3.region = "us-east-1"; return; } this.linker.s3.region = value; } }
Récursion infiniePVS-Studio Warning: V3110 [CWE-674] Possible récursion infinie à l'intérieur de la propriété 'OnFailure'. AWSSDK.ElasticMapReduce.Net45 ResizeJobFlowStep.cs 171
OnFailure? onFailure = null; public OnFailure? OnFailure { get { return this.OnFailure; }
Un exemple de faute de frappe classique qui provoque une récursion infinie dans l'
accesseur get de la propriété
OnFailure . Au lieu de renvoyer la valeur du champ privé,
onFailure fait référence à la propriété
OnFailure . Option corrigée:
public OnFailure? OnFailure { get { return this.onFailure; } set { this.onFailure = value; } }
Vous demandez: "Comment cela a-t-il fonctionné?" Jusqu'à présent, aucun moyen. La propriété n'est utilisée nulle part, mais elle est temporaire. À un moment donné, quelqu'un commencera à l'utiliser et obtiendra certainement un résultat inattendu. Pour éviter de telles fautes de frappe, il est recommandé de ne pas utiliser d'identificateurs qui ne diffèrent que dans la première casse.
Une autre note à cette conception est l'utilisation d'un identifiant qui correspond complètement au nom du type
OnFailure . Du point de vue du compilateur, c'est tout à fait acceptable, mais cela rend la perception humaine du code plus difficile.
Une autre erreur similaire:
Avertissement PVS-Studio: V3110 [CWE-674] Possible récursion infinie à l'intérieur de la propriété 'SSES3'. AWSSDK.S3.Net45 InventoryEncryption.cs 37
private SSES3 sSES3; public SSES3 SSES3 { get { return this.SSES3; } set { this.SSES3 = value; } }
La situation est identique à celle décrite ci-dessus. Seulement ici, une récursion infinie se produira lors de l'accès à la propriété
SSES3 pour la lecture et l'écriture. Option corrigée:
public SSES3 SSES3 { get { return this.sSES3; } set { this.sSES3 = value; } }
Test de pleine conscienceCe qui suit est une tâche d'un programmeur désireux d'utiliser la technique du copier-coller. Regardez à quoi ressemble le code dans l'éditeur Visual Studio et essayez de trouver l'erreur.
Avertissement PVS-Studio: V3029 Les expressions conditionnelles des instructions «if» situées côte à côte sont identiques. Vérifiez les lignes: 91, 95. AWSSDK.AppSync.Net45 CreateApiKeyResponseUnmarshaller.cs 91
J'ai réduit le corps de la méthode
UnmarshallException , supprimant tous les éléments inutiles. Vous pouvez maintenant voir que des vérifications identiques se succèdent:
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); } .... }
Il peut sembler que l'erreur n'est pas grossière - juste une vérification supplémentaire. Mais souvent, un tel modèle peut indiquer des problèmes plus graves dans le code lorsque certaines vérifications nécessaires ne sont pas effectuées.
Il y a quelques erreurs plus similaires dans le code.
Avertissements de PVS-Studio:- V3029 Les expressions conditionnelles des instructions «if» situées côte à côte sont identiques. Vérifiez les lignes: 75, 79. AWSSDK.CloudDirectory.Net45 CreateSchemaResponseUnmarshaller.cs 75
- V3029 Les expressions conditionnelles des instructions «if» situées côte à côte sont identiques. Vérifiez les lignes: 105, 109. AWSSDK.CloudDirectory.Net45 GetSchemaAsJsonResponseUnmarshaller.cs 105
- V3029 Les expressions conditionnelles des instructions «if» situées côte à côte sont identiques. Vérifiez les lignes: 201, 205. AWSSDK.CodeCommit.Net45 PostCommentForPullRequestResponseUnmarshaller.cs 201
- V3029 Les expressions conditionnelles des instructions «if» situées côte à côte sont identiques. Vérifiez les lignes: 101, 105. AWSSDK.CognitoIdentityProvider.Net45 VerifySoftwareTokenResponseUnmarshaller.cs 101
- V3029 Les expressions conditionnelles des instructions «if» situées côte à côte sont identiques. Vérifiez les lignes: 72, 76. AWSSDK.Glue.Net45 UpdateConnectionResponseUnmarshaller.cs 72
- V3029 Les expressions conditionnelles des instructions «if» situées côte à côte sont identiques. Vérifiez les lignes: 123, 127. AWSSDK.Neptune.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 123
- V3029 Les expressions conditionnelles des instructions «if» situées côte à côte sont identiques. Vérifiez les lignes: 167, 171. AWSSDK.Neptune.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 167
- V3029 Les expressions conditionnelles des instructions «if» situées côte à côte sont identiques. Vérifiez les lignes: 127, 131. AWSSDK.RDS.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 127
- V3029 Les expressions conditionnelles des instructions «if» situées côte à côte sont identiques. Vérifiez les lignes: 171, 175. AWSSDK.RDS.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 171
- V3029 Les expressions conditionnelles des instructions «if» situées côte à côte sont identiques. Vérifiez les lignes: 99, 103. AWSSDK.Rekognition.Net45 RecognizeCelebritiesResponseUnmarshaller.cs 99
Tu es quoiPVS-Studio Warning: V3062 Un objet 'attributeName' est utilisé comme argument de sa propre méthode. Pensez à vérifier le premier argument réel de la méthode «Contient». AWSSDK.MobileAnalytics.Net45 CustomEvent.cs 261
L'analyseur a détecté une erreur dans la méthode
GetAttribute : la chaîne est vérifiée pour le fait qu'elle se contient. De la description de la méthode, il s'ensuit que si le nom d'attribut (clé
attributeName ) est trouvé (dans le dictionnaire
_attributes ), alors la valeur d'attribut doit être retournée, sinon
null . En réalité, comme la condition
attributeName.Contains (attributeName) est toujours vraie, une tentative est faite pour renvoyer une valeur par une clé qui peut ne pas être trouvée dans le dictionnaire. Ensuite, au lieu de retourner
null, une exception KeyNotFoundException sera levée.
Essayons de corriger ce code. Pour mieux comprendre comment procéder, jetez un œil à une autre méthode:
Cette méthode vérifie si le nom d'attribut (clé
attributeName )
existe dans le dictionnaire
_attributes .
Revenons à la méthode
GetAttribute et
corrigeons l'erreur:
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; }
Maintenant, la méthode fait exactement ce qui est indiqué dans la description.
Et encore une petite remarque sur ce fragment de code. J'ai remarqué que les auteurs utilisent le
verrouillage lorsqu'ils travaillent avec le dictionnaire
_attributes . De toute évidence, cela est nécessaire avec un accès multi-thread, mais la construction du
verrou est plutôt lente et lourde. Au lieu de
Dictionary dans ce cas, il pourrait être plus pratique d'utiliser une version thread-safe du dictionnaire -
ConcurrentDictionary . Ensuite, le besoin de
verrouillage disparaît. Bien que, je ne connaisse peut-être aucune caractéristique du projet.
Comportement suspectAvertissement PVS-Studio: V3063 [CWE-571] Une partie de l'expression conditionnelle est toujours vraie si elle est évaluée: 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) &&
L'analyseur a alerté la
vérification de string.IsNullOrEmpty (inferredIndexName) . En effet, la chaîne
inferredIndexName est affectée
null , puis la valeur de cette variable n'est modifiée nulle part, puis pour une raison quelconque, elle est vérifiée pour
null ou une chaîne vide. Ça a l'air suspect. Examinons de près l'extrait de code donné. Je ne l'ai délibérément pas réduit pour une meilleure compréhension de la situation. Ainsi, dans la première
instruction if (et également dans la suivante), la variable
specifiedIndexName est vérifiée d'une manière ou d'une autre. Selon le résultat des vérifications, la variable
inferredIndexName reçoit une nouvelle valeur. Prenons maintenant la troisième
instruction if . Le corps de cet opérateur (levant une exception) sera exécuté si
indexNames.Count> 0 , car la première partie de la condition est
string.IsNullOrEmpty (inferredIndexName) est toujours vrai. Peut-être que les variables
specifiedIndexName et
inferredIndexName sont simplement confondues. Ou le troisième chèque doit être sans
autre , représentant une
instruction if autonome:
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(....);
Dans ce cas, il est difficile de donner une réponse définitive sur les options de correction de ce code. Mais il est absolument nécessaire que l'auteur l'inspecte.
NullReferenceExceptionAvertissement PVS-Studio: V3095 [CWE-476] L'objet 'conditionValues' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 228, 238. AWSSDK.Core.Net45 JsonPolicyWriter.cs 228
private static void writeConditions(....) { .... foreach (....) { IList<string> conditionValues = keyEntry.Value; if (conditionValues.Count == 0)
Classiques du genre. La variable
conditionValues est utilisée sans d'abord vérifier la valeur
null . De plus, plus loin dans le code, une telle vérification est effectuée, mais il est trop tard. :)
Le code peut être corrigé comme suit:
private static void writeConditions(....) { .... foreach (....) { IList<string> conditionValues = keyEntry.Value; if (conditionValues != null && conditionValues.Count == 0) continue; .... if (conditionValues != null && conditionValues.Count != 0) { .... } .... } }
Quelques erreurs similaires ont été trouvées dans le code.
Avertissements de PVS-Studio:- V3095 [CWE-476] L'objet 'ts.Listeners' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 140, 143. AWSSDK.Core.Net45 Logger.Diagnostic.cs 140
- V3095 [CWE-476] L'objet 'obj' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 743, 745. AWSSDK.Core.Net45 JsonMapper.cs 743
- V3095 [CWE-476] L'objet 'multipartUploadMultipartUploadpartsList' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 65, 67. AWSSDK.S3.Net45 CompleteMultipartUploadRequestMarshaller.cs 65
L'avertissement suivant est très similaire dans sa signification, mais la situation est l'inverse de celle discutée ci-dessus.
Avertissement PVS-Studio: V3125 [CWE-476] L'objet 'état' a été utilisé après avoir été vérifié par rapport à null. Vérifiez les lignes: 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;
L'un des fragments de code contient une vérification de la valeur de la variable d'
état pour l'égalité
nulle . En outre, selon le code, la variable d'
état est utilisée pour se désinscrire de l'événement
PreemptExpiryTime , tandis que la vérification de
null n'est plus effectuée et une
NullReferenceException est possible . Option de code plus sécurisée:
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; .... }
Il existe d'autres erreurs similaires dans le code.
Avertissements de PVS-Studio:- V3125 [CWE-476] L'objet 'wrappedRequest.Content' a été utilisé après avoir été vérifié par rapport à null. Vérifiez les lignes: 395, 383. AWSSDK.Core.Net45 HttpHandler.cs 395
- V3125 [CWE-476] L'objet 'datasetUpdates' a été utilisé après avoir été vérifié par rapport à null. Vérifiez les lignes: 477, 437. AWSSDK.CognitoSync.Net45 Dataset.cs 477
- V3125 [CWE-476] L'objet 'cORSConfigurationCORSConfigurationcORSRulesListValue' a été utilisé après avoir été vérifié par rapport à null. Vérifiez les lignes: 125, 111. AWSSDK.S3.Net45 PutCORSConfigurationRequestMarshaller.cs 125
- V3125 [CWE-476] L'objet 'lifecycleConfigurationLifecycleConfigurationrulesListValue' a été utilisé après avoir été vérifié par rapport à null. Vérifiez les lignes: 157, 68. AWSSDK.S3.Net45 PutLifecycleConfigurationRequestMarshaller.cs 157
- V3125 [CWE-476] L'objet 'this.Key' a été utilisé après avoir été vérifié par rapport à null. Vérifiez les lignes: 199, 183. AWSSDK.S3.Net45 S3PostUploadRequest.cs 199
Une réalité incontestéeAvertissement PVS-Studio: V3009 [CWE-393] Il est étrange que cette méthode renvoie toujours une seule et même valeur de 'true'. AWSSDK.Core.Net45 Lexer.cs 651
private static bool State19 (....) { while (....) { switch (....) { case '"': .... return true; case '\\': .... return true; default: .... continue; } } return true; }
La méthode renvoie toujours
true . Voyons à quel point cela est critique pour le code appelant. J'ai suivi l'utilisation de la méthode
State19 . Il participe au remplissage du tableau des gestionnaires
fsm_handler_table avec d'autres méthodes similaires (il y en a 28 avec des noms, respectivement, de
State1 à
State28 ). Il est important de noter ici qu'en plus de
State19 , des avertissements
V3009 [CWE-393] ont également été émis pour certains autres gestionnaires. Ce sont les gestionnaires:
State23, State26, State27, State28 . Alertes émises par l'analyseur pour eux:
- V3009 [CWE-393] Il est étrange que cette méthode renvoie toujours une seule et même valeur de 'true'. AWSSDK.Core.Net45 Lexer.cs 752
- V3009 [CWE-393] Il est étrange que cette méthode renvoie toujours une seule et même valeur de 'true'. AWSSDK.Core.Net45 Lexer.cs 810
- V3009 [CWE-393] Il est étrange que cette méthode renvoie toujours une seule et même valeur de 'true'. AWSSDK.Core.Net45 Lexer.cs 822
- V3009 [CWE-393] Il est étrange que cette méthode renvoie toujours une seule et même valeur de 'true'. AWSSDK.Core.Net45 Lexer.cs 834
Voici à quoi ressemble la déclaration et l'initialisation du tableau de gestionnaires:
private static StateHandler[] fsm_handler_table; .... private static void PopulateFsmTables () { fsm_handler_table = new StateHandler[28] { State1, State2, .... State19, .... State23, .... State26, State27, State28 };
Pour être complet, voyons le code de l'un des gestionnaires, auquel l'analyseur ne s'est pas plaint, par exemple,
State2 :
private static bool State2 (....) { .... if (....) { return true; } switch (....) { .... default: return false; } }
Et voici comment les gestionnaires sont appelés:
public bool NextToken () { .... while (true) { handler = fsm_handler_table[state - 1]; if (! handler (fsm_context))
Comme vous pouvez le voir, une exception sera levée si
false est retourné. Dans notre cas, pour les gestionnaires
State19, State23, State26, State27 et
State28, cela ne se produira jamais. Ça a l'air suspect. D'un autre côté, jusqu'à cinq gestionnaires ont un comportement similaire (retournent toujours
true ), donc peut-être que cela était prévu et n'est pas le résultat d'une faute de frappe.
Pourquoi ai-je insisté sur tout cela avec autant de détails? Cette situation est très indicative dans le sens où un analyseur statique n'est souvent capable d'indiquer qu'une conception suspecte. Et même une personne (pas une machine) qui n'a pas une connaissance suffisante du projet, ayant passé du temps à étudier le code, n'est toujours pas en mesure de donner une réponse exhaustive sur la présence d'une erreur. Le code doit être étudié par le développeur.
Des contrôles dénués de sensAvertissement PVS-Studio: V3022 [CWE-571] L'expression 'doLog' est toujours vraie. 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)
Faites attention à la variable
doLog . Après l'initialisation avec
false , alors dans le code cette variable deviendra
vraie dans tous les cas. La vérification
if (doLog) est donc toujours vraie. Il y avait peut-être une branche plus tôt dans cette méthode dans laquelle la variable
doLog n'avait pas de valeur, puis au moment de la vérification, elle pouvait contenir la
fausse valeur obtenue lors de l'initialisation. Mais maintenant, il n'y a plus de branche.
Une autre erreur similaire:
Avertissement PVS-Studio: V3022 L' expression '! Résultat' est toujours fausse. 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(....); } .... }
L'analyseur prétend que la valeur de la variable de
résultat est toujours
vraie . Cela n'est possible que si la méthode
PutValueHelper renvoie toujours
true . Jetez un œil à cette méthode:
private bool PutValueHelper(....) { .... if (....)) { return true; } if (record == null) { .... return true; } else { .... return true; } }
En effet, la méthode retournera
vrai dans n'importe quelle condition. De plus, l'analyseur a émis un avertissement pour cette méthode.
Avertissement PVS-Studio: V3009 [CWE-393] Il est étrange que cette méthode renvoie toujours une seule et même valeur de 'true'. SQLiteLocalStorage.cs 1016
Je n'ai délibérément pas cité cet avertissement plus tôt lorsque je considérais d'autres erreurs du
V3009 , mais je l'ai enregistré pour ce cas. Ainsi, l'analyseur avait raison d'indiquer l'erreur
V3022 dans le code appelant.
Copiez-collez. Encore une foisAvertissement PVS-Studio: V3001 Il existe des sous-expressions identiques 'this.token == JsonToken.String' à gauche et à droite de '||' opérateur. AWSSDK.Core.Net45 JsonReader.cs 343
public bool Read() { .... if ( (this.token == JsonToken.ObjectEnd || this.token == JsonToken.ArrayEnd || this.token == JsonToken.String ||
Comparez deux fois le champ
this.token avec la valeur
JsonToken.String de l'énumération
JsonToken . L'une des comparaisons devrait probablement contenir une valeur d'énumération différente. Si oui, alors une grave erreur a été commise.
Refactorisation + insouciance?Avertissement PVS-Studio: V3025 [CWE-685] Format incorrect. Un nombre différent d'éléments de format est attendu lors de l'appel de la fonction 'Format'. Arguments non utilisés: 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)); } .... }
Probablement, la chaîne de format de la méthode
string.Format contenait précédemment le spécificateur de sortie
{0} , pour lequel l'argument
AWSConfigs.AWSRegionKey a été défini. Ensuite, la ligne a été modifiée, le spécificateur a disparu, mais ils ont oublié de supprimer l'argument. Le fragment de code ci-dessus fonctionne sans erreur (une exception serait levée dans le cas contraire - un spécificateur sans argument), mais il a l'air moche. Le code doit être fixé comme suit:
if (region == null) { throw new InvalidOperationException( "EC2 instance metadata was not available or did not contain region information."); }
DangereuxAvertissement PVS-Studio: V3083 [CWE-367] L'appel non sécurisé de l'événement 'mOnSyncSuccess', NullReferenceException est possible. Pensez à affecter un événement à une variable locale avant de l'invoquer. AWSSDK.CognitoSync.PCL Dataset.cs 827
protected void FireSyncSuccessEvent(List<Record> records) { if (mOnSyncSuccess != null) { mOnSyncSuccess(this, new SyncSuccessEventArgs(records)); } }
Une situation assez courante d'un appel non sécurisé au gestionnaire d'événements. Entre la vérification de la variable
mOnSyncSuccess pour
null et l'appel du gestionnaire, un événement peut être désabonné et sa valeur deviendra nulle. La probabilité d'un tel scénario est faible, mais il vaut toujours mieux sécuriser le code:
protected void FireSyncSuccessEvent(List<Record> records) { mOnSyncSuccess?.Invoke(this, new SyncSuccessEventArgs(records)); }
Il existe d'autres erreurs similaires dans le code.
Avertissements de PVS-Studio:- V3083 [CWE-367] L'appel non sécurisé de l'événement 'mOnSyncFailure', NullReferenceException est possible. Pensez à affecter un événement à une variable locale avant de l'invoquer. AWSSDK.CognitoSync.PCL Dataset.cs 839
- V3083 [CWE-367] L'appel d'événement à risque, NullReferenceException est possible. Pensez à affecter un événement à une variable locale avant de l'invoquer. AWSSDK.Core.PCL AmazonServiceClient.cs 332
- V3083 [CWE-367] L'appel d'événement à risque, NullReferenceException est possible. Pensez à affecter un événement à une variable locale avant de l'invoquer. AWSSDK.Core.PCL AmazonServiceClient.cs 344
- V3083 [CWE-367] L'appel d'événement à risque, NullReferenceException est possible. Pensez à affecter un événement à une variable locale avant de l'invoquer. AWSSDK.Core.PCL AmazonServiceClient.cs 357
- V3083 [CWE-367] L'appel non sécurisé de l'événement 'mExceptionEvent', NullReferenceException est possible. Pensez à affecter un événement à une variable locale avant de l'invoquer. AWSSDK.Core.PCL AmazonServiceClient.cs 366
- V3083 [CWE-367] L'appel d'événement à risque, NullReferenceException est possible. Pensez à affecter un événement à une variable locale avant de l'invoquer. AWSSDK.Core.PCL AmazonWebServiceRequest.cs 78
- V3083 [CWE-367] L'appel non sûr de l'événement 'OnRead', NullReferenceException est possible. Pensez à affecter un événement à une variable locale avant de l'invoquer. AWSSDK.Core.PCL EventStream.cs 97
- V3083 [CWE-367] L'appel d'événement à risque, NullReferenceException est possible. Pensez à affecter un événement à une variable locale avant de l'invoquer. AWSSDK.Core.Android NetworkReachability.cs 57
- V3083 [CWE-367] L'appel d'événement à risque, NullReferenceException est possible. Pensez à affecter un événement à une variable locale avant de l'invoquer. AWSSDK.Core.Android NetworkReachability.cs 94
- V3083 [CWE-367] L'appel d'événement à risque, NullReferenceException est possible. Pensez à affecter un événement à une variable locale avant de l'invoquer. AWSSDK.Core.iOS NetworkReachability.cs 54
Classe inachevéeAvertissement PVS-Studio: le type V3126 implémentant l'interface IEquatable <T> de type 'JsonData' ne remplace pas la méthode 'GetHashCode'. AWSSDK.Core.Net45 JsonData.cs 26
public class JsonData : IJsonWrapper, IEquatable<JsonData> { .... }
La classe
JsonData contient beaucoup de code, donc je ne l'ai pas donné dans son intégralité, me limitant à le déclarer. Cette classe ne contient vraiment pas de méthode
GetHashCode substituée, ce qui n'est pas sûr car elle peut conduire à un comportement erroné lors de l'utilisation du type
JsonData pour fonctionner, par exemple, avec des collections. Il n'y a probablement pas de problème à l'heure actuelle, mais la stratégie d'utilisation de ce type peut changer à l'avenir. Cette erreur est décrite plus en détail dans la
documentation .
ConclusionC'est toutes les erreurs intéressantes que j'ai réussi à détecter dans le code AWS SDK for .NET à l'aide de l'analyseur statique PVS-Studio. Encore une fois, je souligne la qualité du projet. J'ai trouvé un très petit nombre d'erreurs pour 5 millions de lignes de code. Bien que, probablement, une analyse plus approfondie des avertissements émis me permette d'ajouter quelques erreurs de plus à cette liste. Mais il est également très probable que j'ai en vain attribué certains des avertissements décrits à des erreurs. Dans ce cas, des conclusions non ambiguës sont toujours faites uniquement par le développeur, qui se trouve dans le contexte du code en cours de vérification.
Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: Sergey Khrenov. Recherche d'erreurs dans le code source du SDK Amazon Web Services pour .NET