Les bibliothèques .NET Core sont l'un des projets C # les plus populaires sur GitHub. Sans surprise, étant donné sa grande popularité et sa convivialité. Il est d'autant plus intéressant d'essayer de découvrir quels coins sombres peuvent être trouvés dans le code source de ces bibliothèques, ce que nous allons essayer de faire en utilisant l'analyseur statique PVS-Studio. Pensez-vous que vous avez finalement réussi à découvrir quelque chose d'intéressant?
Je suis allé à cet article pendant plus d'un an et demi. À un moment donné, l'idée m'est venue à l'esprit que les bibliothèques .NET Core sont une friandise, et les vérifier sera intéressant. Plusieurs fois j'ai vérifié le projet, l'analyseur a trouvé des endroits de plus en plus intéressants, mais cela n'a pas dépassé un simple défilement de la liste des avertissements. Et voilà , c'est fait! Le projet est vérifié, l'article est devant vous.
En savoir plus sur le projet et l'analyse
Si vous avez hâte de vous lancer dans l'analyse du code - vous pouvez sauter cette section, mais j'aimerais vraiment que vous le lisiez - ici, je parle un peu plus du projet et de l'analyseur, ainsi que de la façon dont j'ai analysé et reproduit les erreurs.
Projet audité
Il serait probablement possible de ne pas dire ce que sont les CoreFX (bibliothèques .NET Core), mais, si vous ne les avez pas entendus, la description est ci-dessous. Je ne l'ai pas reformulé et je l'ai pris
sur la page du projet sur GitHub , où vous pouvez également télécharger les sources.
Description:
ce dépôt contient l'implémentation de la bibliothèque (appelée "CoreFX") pour .NET Core. Il comprend System.Collections, System.IO, System.Xml et de nombreux autres composants. Le référentiel .NET Core Runtime correspondant (appelé "CoreCLR") contient l'implémentation d'exécution pour .NET Core. Il comprend RyuJIT, le GC .NET et de nombreux autres composants. Le code de bibliothèque spécifique au runtime (System.Private.CoreLib) réside dans le référentiel CoreCLR. Il doit être construit et versionné en tandem avec le runtime. Le reste de CoreFX est indépendant de l'implémentation du runtime et peut être exécuté sur n'importe quel runtime .NET compatible (par exemple CoreRT) .
Analyseur et méthode d'analyse utilisés
J'ai vérifié le code source à l'aide de l'
analyseur statique PVS-Studio . En général, PVS-Studio peut analyser non seulement le code C #, mais aussi C, C ++, Java. Jusqu'à présent, l'analyse du code C # ne fonctionne que sous Windows, tandis que le code en C, C ++, Java peut être analysé sous Windows, Linux, macOS.
J'utilise généralement le plugin PVS-Studio pour Visual Studio pour tester des projets C # (les versions 2010-2019 sont prises en charge), car c'est probablement la façon la plus simple et la plus pratique d'analyser: ouvrir une solution, démarrer l'analyse, travailler avec une liste d'avertissements. Avec CoreFX, cependant, les choses sont devenues un peu plus compliquées.
Le fait est que le projet n'a pas un seul fichier .sln, par conséquent, l'ouvrir dans Visual Studio et effectuer une analyse complète à l'aide du plug-in PVS-Studio échouera. Probablement, c'est bien - je ne sais pas vraiment comment Visual Studio traiterait une solution de cette taille.
Cependant, l'analyse n'a posé aucun problème, car le kit de distribution PVS-Studio comprend une version en ligne de commande de l'analyseur pour les projets MSBuild (et, en fait, .sln). Tout ce qui m'était demandé était d'écrire un petit script qui exécuterait «PVS-Studio_Cmd.exe» pour chaque .sln dans le répertoire CoreFX et mettrait les résultats d'analyse dans un répertoire séparé (indiqué par l'indicateur de lancement de l'analyseur).
Voila! - à la sortie, j'ai un ensemble de journaux, qui ont beaucoup de choses intéressantes. Si vous le souhaitez, les journaux peuvent être combinés à l'aide de l'utilitaire PlogConverter, fourni avec le kit de distribution. Mais il était plus pratique pour moi de travailler avec des journaux individuels, donc je n'ai pas commencé à les combiner.
Pour décrire certaines erreurs, je me réfère à la documentation de docs.microsoft.com et aux packages NuGet disponibles en téléchargement sur nuget.org. J'avoue que le code décrit dans la documentation / trouvé dans les packages peut légèrement différer de celui analysé. Néanmoins, il sera très étrange si, par exemple, la documentation ne contient pas de description des exceptions générées pour un certain nombre de données d'entrée, et qu'elles apparaissent dans la nouvelle version du package - convenez que ce sera une surprise douteuse. La reproduction des erreurs dans les packages de NuGet sur les mêmes données d'entrée utilisées pour le débogage des bibliothèques montre que le problème n'est pas nouveau et, plus important encore, qu'il peut être «touché» sans construire le projet à partir de la source.
Ainsi, en supposant la possibilité d'une mauvaise synchronisation théorique du code, je trouve permis de se référer à la description des méthodes correspondantes sur docs.microsoft.com et de reproduire les problèmes sur les packages de nuget.org.
Je note également que la description des liens fournis, ainsi que les informations (commentaires) dans les packages (dans d'autres versions) peuvent changer lors de la rédaction de l'article.
Autres projets éprouvés
Soit dit en passant, ce n'est pas un article unique, nous écrivons d'autres articles sur la vérification des projets, dont une liste
peut être trouvée ici . De plus, sur le site, nous avons collecté non seulement des articles sur l'analyse des projets, mais aussi divers articles techniques sur C, C ++, C #, Java, ainsi que des notes intéressantes. Vous pouvez trouver tout cela sur le
blog .
Mon collègue avait précédemment testé les bibliothèques .NET Core en 2015. Les résultats de l'analyse précédente peuvent être trouvés dans l'article correspondant: "
Vérification du Nouvel An des bibliothèques .NET Core (CoreFX) ."
Erreurs découvertes, lieux suspects et intéressants
Comme toujours, pour plus d'intérêt, je vous suggère de rechercher d'abord les erreurs dans les fragments par vous-même, puis de lire l'avertissement de l'analyseur et la description du problème.
Pour plus de commodité, j'ai explicitement séparé les fragments en question les uns des autres à l'aide des étiquettes du formulaire
Problème N - il est plus facile de comprendre où se termine la description d'une erreur et où commence l'analyse de l'autre. Oui, et se référer à des fragments spécifiques est également plus facile.
Numéro 1abstract public class Principal : IDisposable { .... public void Save(PrincipalContext context) { .... if ( context.ContextType == ContextType.Machine || _ctx.ContextType == ContextType.Machine) { throw new InvalidOperationException( SR.SaveToNotSupportedAgainstMachineStore); } if (context == null) { Debug.Assert(this.unpersisted == true); throw new InvalidOperationException(SR.NullArguments); } .... } .... }
PVS-Studio Warning :
V3095 L'objet 'context' a été utilisé avant d'être vérifié par rapport à null. Vérifier les lignes: 340, 346. Principal.cs 340
Les développeurs indiquent explicitement que la valeur
nulle pour le paramètre de
contexte n'est pas valide et souhaitent le souligner avec une exception de type
InvalidOperationException . Cependant, un peu plus haut, dans la condition précédente, il y a une déréférence inconditionnelle du lien de
contexte -
context.ContextType . Par conséquent, si la valeur du
contexte est
nulle , une exception de type
NullReferenceException sera
levée à la place de la valeur
InvalidOperationExcetion attendue.
Essayons de reproduire le problème. Connectez la bibliothèque appropriée (
System.DirectoryServices.AccountManagement ) au projet et exécutez le code suivant:
GroupPrincipal groupPrincipal = new GroupPrincipal(new PrincipalContext(ContextType.Machine)); groupPrincipal.Save(null);
GroupPrincipal est le successeur de la classe abstraite
Principal , qui contient l'implémentation de la méthode
Save dont nous avons besoin. Nous exécutons le code pour l'exécution et voyons ce qu'il fallait prouver.
Pour le plaisir, vous pouvez essayer de télécharger le package approprié à partir de NuGet et essayer de répéter le problème de la même manière. J'ai installé la version 4.5.0 du package et obtenu le résultat attendu.
Numéro 2 private SearchResultCollection FindAll(bool findMoreThanOne) { searchResult = null; DirectoryEntry clonedRoot = null; if (_assertDefaultNamingContext == null) { clonedRoot = SearchRoot.CloneBrowsable(); } else { clonedRoot = SearchRoot.CloneBrowsable(); } .... }
PVS-Studio Warning :
V3004 L'
instruction 'then' est Ă©quivalente Ă l'instruction 'else'. DirectorySearcher.cs 629
Indépendamment de la vérité de la condition
_assertDefaultNamingContext == null , les mêmes actions seront effectuées, depuis
lors et
sinon les branches de l'
instruction if ont les mĂŞmes corps. Soit il devrait y avoir une autre action dans une branche, soit vous pouvez omettre l'
instruction if afin de ne pas confondre les programmeurs et l'analyseur.
Numéro 3 public class DirectoryEntry : Component { .... public void RefreshCache(string[] propertyNames) { .... object[] names = new object[propertyNames.Length]; for (int i = 0; i < propertyNames.Length; i++) names[i] = propertyNames[i]; .... if (_propertyCollection != null && propertyNames != null) .... .... } .... }
PVS-Studio Warning :
V3095 L'objet 'propertyNames' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 990, 1004. DirectoryEntry.cs 990
Encore une fois, nous voyons une procédure étrange. La méthode a un
propertyNames! = Null check, i.e. les développeurs s'assurent que la méthode retourne
null . Voici juste au-dessus, vous pouvez observer plusieurs accès à cette référence potentiellement nulle -
propertyNames.Length et
propertyNames [i] . Le résultat est tout à fait prévisible - une exception de type
NullReferenceExcepption se produira si une référence nulle est passée à la méthode.
Quelle coĂŻncidence que
RefreshCache soit une méthode publique dans une classe publique. Essayez de répéter le problème? Pour ce faire, connectez au projet la bibliothèque souhaitée -
System.DirectoryServices - et Ă©crivez le code comme ceci:
DirectoryEntry de = new DirectoryEntry(); de.RefreshCache(null);
Exécutez le code pour l'exécution et voyez l'image attendue.
Pour le plaisir, vous pouvez essayer de reproduire le problème sur la version finale du package NuGet. Nous connectons le package
System.DirectoryServices au projet NuGet (j'ai utilisé la version 4.5.0) et exécutons le code déjà familier pour l'exécution. Le résultat est plus faible.
Numéro 4Maintenant, nous allons partir de l'opposé - essayez d'abord d'écrire du code qui utilise une instance de la classe, puis regardez à l'intérieur. Examinons la structure de
System.Drawing.CharacterRange de la bibliothèque
System.Drawing.Common et le package NuGet du mĂŞme nom.
Le code utilisé sera le suivant:
CharacterRange range = new CharacterRange(); bool eq = range.Equals(null); Console.WriteLine(eq);
Juste au cas où, pour rafraîchir la mémoire, nous nous tournerons vers
docs.microsoft.com pour rappeler la valeur de retour attendue de l'expression
obj.Equals (null) :
Les instructions suivantes doivent être vraies pour toutes les implémentations de la méthode Equals (Object) . Dans la liste, x, y et z représentent des références d'objet qui ne sont pas nulles.....x.Equals (null) renvoie false.Pensez-vous que le texte «False» sera affiché dans la console? Bien sûr que non, ce serait trop facile. :) Nous exécutons le code et regardons le résultat.
Telle était la conclusion lors de l'exécution du code ci-dessus à l'aide du package NuGet
System.Drawing.Common version 4.5.1. Nous exécutons le même code avec la version de débogage de la bibliothèque et voyons ce qui suit:
Examinons maintenant le code source - l'implémentation de la méthode
Equals dans la structure
CharacterRange et l'avertissement de l'analyseur:
public override bool Equals(object obj) { if (obj.GetType() != typeof(CharacterRange)) return false; CharacterRange cr = (CharacterRange)obj; return ((_first == cr.First) && (_length == cr.Length)); }
Avertissement PVS-Studio :
V3115 Passer la
méthode 'null' à 'Equals' ne devrait pas entraîner 'NullReferenceException'. CharacterRange.cs 56
Nous voyons ce que nous devions prouver - le paramètre
obj est traité de manière incorrecte, à cause de laquelle une exception du type
NullReferenceException se produit lors de l'appel de la méthode d'instance
GetType dans l'expression conditionnelle.
Numéro 5En explorant cette bibliothèque, considérez un autre endroit intéressant - la méthode
Icon.Save . Avant l'étude, voir la description de la méthode.
Il n'y a pas de description de la méthode:
Nous nous tournons vers docs.microsoft.com - "
Icon.Save (Stream) Method ". Cependant, il n'y a également aucune restriction sur les valeurs d'entrée et aucune information sur les exceptions générées.
Passons maintenant Ă la recherche de code.
public sealed partial class Icon : MarshalByRefObject, ICloneable, IDisposable, ISerializable { .... public void Save(Stream outputStream) { if (_iconData != null) { outputStream.Write(_iconData, 0, _iconData.Length); } else { .... if (outputStream == null) throw new ArgumentNullException("dataStream"); .... } } .... }
PVS-Studio Warning :
V3095 L'objet 'outputStream' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 654, 672. Icon.Windows.cs 654
Encore une fois, l'histoire que nous connaissons déjà est le déréférencement possible d'une référence nulle, car le paramètre de méthode est déréférencé sans vérifier la valeur
nulle . Encore une fois, une bonne combinaison de circonstances - à la fois la classe et la méthode - est publique, ce qui signifie que vous pouvez essayer de reproduire le problème.
La tâche est simple: amener l'exécution du code à l'expression
outputStream.Write (_iconData, 0, _iconData.Length); tout en conservant la valeur de la variable
outputStream -
null . Pour cela, il suffit que la condition
_iconData! = Null soit satisfaite.
Regardons le constructeur public le plus simple:
public Icon(string fileName) : this(fileName, 0, 0) { }
Il délègue simplement le travail à un autre constructeur. Eh bien, regardez plus loin - le constructeur utilisé ici.
public Icon(string fileName, int width, int height) : this() { using (FileStream f = new FileStream(fileName, FileMode.Open, FileAccess.Read, FileShare.Read)) { Debug.Assert(f != null, "File.OpenRead returned null instead of throwing an exception"); _iconData = new byte[(int)f.Length]; f.Read(_iconData, 0, _iconData.Length); } Initialize(width, height); }
Le voici, ce dont vous avez besoin. Après avoir appelé ce constructeur, si nous lisons avec succès les données du fichier et si aucun plantage ne se produit dans la méthode
Initialize , le champ
_iconData contiendra un lien vers un objet, ce dont nous avons besoin.
Il s'avère que pour reproduire le problème, vous devez créer une instance de la classe
Icon avec l'icône réelle, puis appeler la méthode
Save , en passant
null comme argument, ce que nous ferons. Le code peut ressembler, par exemple, comme suit:
Icon icon = new Icon(@"D:\document.ico"); icon.Save(null);
Le résultat de l'exécution est attendu.
Numéro 6Nous continuons l'examen et allons à la bibliothèque
System.Management . Essayez de trouver 3 différences entre les actions effectuées dans le
cas CimType.UInt32 et le reste du
cas .
private static string ConvertToNumericValueAndAddToArray(....) { string retFunctionName = string.Empty; enumType = string.Empty; switch(cimType) { case CimType.UInt8: case CimType.SInt8: case CimType.SInt16: case CimType.UInt16: case CimType.SInt32: arrayToAdd.Add(System.Convert.ToInt32( numericValue, (IFormatProvider)CultureInfo.InvariantCulture .GetFormat(typeof(int)))); retFunctionName = "ToInt32"; enumType = "System.Int32"; break; case CimType.UInt32: arrayToAdd.Add(System.Convert.ToInt32( numericValue, (IFormatProvider)CultureInfo.InvariantCulture .GetFormat(typeof(int)))); retFunctionName = "ToInt32"; enumType = "System.Int32"; break; } return retFunctionName; }
Bien sûr, il n'y a pas de différences, ce qui met en garde l'analyseur.
Avertissement PVS-Studio :
V3139 Deux ou plusieurs branches de cas effectuent les mĂŞmes actions. WMIGenerator.cs 5220
Ce style de code n'est pas très clair pour moi personnellement. S'il n'y a pas d'erreur ici, je pense que cela ne valait pas la peine de diffuser la même logique dans différents cas.
Numéro 7Bibliothèque
Microsoft.CSharp .
private static IList<KeyValuePair<string, object>> QueryDynamicObject(object obj) { .... List<string> names = new List<string>(mo.GetDynamicMemberNames()); names.Sort(); if (names != null) { .... } .... }
Avertissement PVS-Studio :
V3022 L' expression 'names! = Null' est toujours vraie. DynamicDebuggerProxy.cs 426
Je pourrais probablement ignorer cet avertissement ainsi que de nombreux
messages similaires Ă©mis par les
diagnostics V3022 et
V3063 . Il y avait de nombreux (très nombreux) contrôles étranges, mais cela s'est en quelque sorte enfoncé dans mon âme. Il est possible qu'avant de comparer les
noms de variables locales avec
null à cette variable, non seulement une référence à l'objet nouvellement créé soit écrite, elle appelle également la méthode d'instance
Sort . Ce n'est pas une erreur, bien sûr, mais l'endroit est intéressant, quant à moi.
Numéro 8Voici un autre morceau de code intéressant.
private static void InsertChildNoGrow(Symbol child) { .... while (sym?.nextSameName != null) { sym = sym.nextSameName; } Debug.Assert(sym != null && sym.nextSameName == null); sym.nextSameName = child; .... }
PVS-Studio Warning :
V3042 Possible NullReferenceException. Le '?.' et '.' les opérateurs sont utilisés pour accéder aux membres de l'objet 'sym' SymbolStore.cs 56
Voyez ce que la chose est ici. Le cycle se termine lorsque l'une des deux conditions est remplie:
- sym == null ;
- sym.nextSameName == null .
Il n'y a pas de problème avec la deuxième condition, qui ne peut pas être dite à propos de la première, car ci-dessous est un appel inconditionnel au champ d'instance
nextSameName et, si
sym est
nul , une exception de type
NullReferenceException sera levée
pendant l'appel .
«Êtes-vous aveugle? Il y a aussi un appel Ă
Debug.Assert , où il est vérifié que
sym! = Null "- quelqu'un peut s'opposer. Mais c'est tout du sel! Lorsque vous travaillez dans la version Release de
Debug.Assert, rien ne vous aidera, et avec l'état décrit ci-dessus, tout ce que nous obtenons est une
NullReferenceException . De plus, j'ai déjà vu une erreur similaire dans un autre projet de Microsoft -
Roslyn , où il y avait une situation très similaire avec
Debug.Assert . Un peu distrait par Roslyn avec votre permission.
Le problème peut être reproduit à l'aide des bibliothèques
Microsoft.CodeAnalysis ou directement dans Visual Studio à l'aide de Syntax Visualizer. Sur Visual Studio version 16.1.6 + Syntax Visualizer 1.0, ce problème se reproduit toujours.
Pour reproduire, le code suivant suffit:
class C1<T1, T2> { void foo() { T1 val = default; if (val is null) { } } }
Ensuite, dans Syntax Visualizer, vous devez rechercher le nœud de l'arborescence de syntaxe de type
ConstantPatternSyntax , correspondant Ă
null dans le code, et demander
TypeSymbol pour cela.
Après cela, Visual Studio redémarrera. Si nous allons dans l'Observateur d'événements, nous trouverons des informations sur les problèmes dans les bibliothèques:
Application: devenv.exe Framework Version: v4.0.30319 Description: The process was terminated due to an unhandled exception. Exception Info: System.Resources.MissingManifestResourceException at System.Resources.ManifestBasedResourceGroveler .HandleResourceStreamMissing(System.String) at System.Resources.ManifestBasedResourceGroveler.GrovelForResourceSet( System.Globalization.CultureInfo, System.Collections.Generic.Dictionary'2 <System.String,System.Resources.ResourceSet>, Boolean, Boolean, System.Threading.StackCrawlMark ByRef) at System.Resources.ResourceManager.InternalGetResourceSet( System.Globalization.CultureInfo, Boolean, Boolean, System.Threading.StackCrawlMark ByRef) at System.Resources.ResourceManager.InternalGetResourceSet( System.Globalization.CultureInfo, Boolean, Boolean) at System.Resources.ResourceManager.GetString(System.String, System.Globalization.CultureInfo) at Roslyn.SyntaxVisualizer.DgmlHelper.My. Resources.Resources.get_SyntaxNodeLabel() ....
Et sur le problème avec devenv.exe:
Faulting application name: devenv.exe, version: 16.1.29102.190, time stamp: 0x5d1c133b Faulting module name: KERNELBASE.dll, version: 10.0.18362.145, time stamp: 0xf5733ace Exception code: 0xe0434352 Fault offset: 0x001133d2 ....
Ayant des versions de débogage des bibliothèques Roslyn, vous pouvez trouver l'endroit où l'exception s'est produite:
private Conversion ClassifyImplicitBuiltInConversionSlow( TypeSymbol source, TypeSymbol destination, ref HashSet<DiagnosticInfo> useSiteDiagnostics) { Debug.Assert((object)source != null); Debug.Assert((object)destination != null); if ( source.SpecialType == SpecialType.System_Void || destination.SpecialType == SpecialType.System_Void) { return Conversion.NoConversion; } .... }
Ici, comme dans le code ci-dessus des bibliothèques .NET Core, il y a aussi une vérification via
Debug.Assert , qui, cependant, n'a aidé en aucune façon lors de l'utilisation des versions de sortie des bibliothèques.
Numéro 9Distrait un peu - et cela suffit, revenons aux bibliothèques .NET Core. Le package
System.IO.IsolatedStorage contient le code intéressant suivant.
private bool ContainsUnknownFiles(string directory) { .... return (files.Length > 2 || ( (!IsIdFile(files[0]) && !IsInfoFile(files[0]))) || (files.Length == 2 && !IsIdFile(files[1]) && !IsInfoFile(files[1])) ); }
Avertissement PVS-Studio :
V3088 L'expression a été placée entre parenthèses deux fois: ((expression)). Une paire de parenthèses n'est pas nécessaire ou une erreur d'impression est présente. IsolatedStorageFile.cs 839
Dire que le formatage du code prête à confusion, c'est ne rien dire. En regardant brièvement ce code, je dirais que l'opérande gauche du premier opérateur || -
files.Length> 2 , celui de droite est celui entre parenthèses. Au moins, le code est formaté comme ceci. En y regardant de plus près, vous pouvez comprendre que ce n'est pas le cas. En fait, l'opérande de droite est
((!! IsIdFile (fichiers [0]) &&! IsInfoFile (fichiers [0]))) . À mon avis, ce code est assez déroutant.
Numéro 10Dans la version de PVS-Studio 7.03, la règle de diagnostic V3138 a été ajoutée, qui recherche les erreurs dans les lignes interpolées. Plus précisément, dans les lignes les plus susceptibles d'être interpolées, mais à cause du caractère
$ manquant, elles ne le sont pas. Les bibliothèques
System.Net ont trouvé plusieurs réponses intéressantes à cette règle de diagnostic.
internal static void CacheCredential(SafeFreeCredentials newHandle) { try { .... } catch (Exception e) { if (!ExceptionCheck.IsFatal(e)) { NetEventSource.Fail(null, "Attempted to throw: {e}"); } } }
Avertissement PVS-Studio : le littéral de chaîne
V3138 contient une expression interpolée potentielle. Envisagez d'inspecter: e. SSPIHandleCache.cs 42
Il est très probable que le deuxième argument de la méthode
Fail soit une chaîne interpolée, dans laquelle la représentation de chaîne de l'exception
e serait substituée. Cependant, en raison du caractère
$ manquant, aucune représentation sous forme de chaîne de l'exception n'est levée.
Numéro 11J'ai rencontré un autre cas similaire.
public static async Task<string> GetDigestTokenForCredential(....) { .... if (NetEventSource.IsEnabled) NetEventSource.Error(digestResponse, "Algorithm not supported: {algorithm}"); .... }
Avertissement PVS-Studio : le littéral de chaîne
V3138 contient une expression interpolée potentielle. Pensez à inspecter: algorithme. AuthenticationHelper.Digest.cs 58
La situation est similaire à celle décrite ci-dessus, le symbole
$ est à nouveau ignoré - la mauvaise ligne va à la méthode
Error .
Numéro 12Package
System.Net.Mail . La méthode est petite, je vais l'apporter dans son intégralité pour que l'erreur soit un peu plus intéressante.
internal void SetContent(Stream stream) { if (stream == null) { throw new ArgumentNullException(nameof(stream)); } if (_streamSet) { _stream.Close(); _stream = null; _streamSet = false; } _stream = stream; _streamSet = true; _streamUsedOnce = false; TransferEncoding = TransferEncoding.Base64; }
PVS-Studio Warning :
V3008 La variable '_streamSet' reçoit des valeurs successives deux fois. C'est peut-être une erreur. Vérifiez les lignes: 123, 119. MimePart.cs 123
La double affectation de la valeur de la variable
_streamSet semble étrange (d'abord - sous la condition, puis - à l'extérieur). La même histoire avec la mise à zéro de la variable
_stream . Par conséquent,
_stream sera toujours défini sur
stream et
_streamSet sur
true .
Numéro 13Un endroit intéressant de la bibliothèque
System.Linq.Expressions , auquel l'analyseur a immédiatement émis 2 avertissements. Dans ce cas, c'est plus une fonctionnalité qu'un bug, mais néanmoins, la méthode est très intéressante ...
Avertissements de PVS-Studio :
- V3010 La valeur de retour de la fonction 'GetType' doit être utilisée. Instruction.cs 36
- V3080 Déréférence nulle possible. Pensez à inspecter «o». Instruction.cs 36
Il n'y a probablement rien Ă commenter.
Numéro 14Regardons un autre cas avec lequel nous travaillerons "de l'extérieur". Tout d'abord, nous allons écrire le code, identifier les problèmes, puis regarder à l'intérieur. Pour étudier, prenez la bibliothèque
System.Configuration.ConfigurationManager et le package NuGet du même nom. J'ai utilisé la version 4.5.0 du package. Nous travaillerons avec la classe
System.Configuration.CommaDelimitedStringCollection .
Faisons quelque chose de pas très délicat. Par exemple, créez un objet, extrayez sa représentation sous forme de chaîne, obtenez la longueur de cette chaîne et imprimez-la. Code pertinent:
CommaDelimitedStringCollection collection = new CommaDelimitedStringCollection(); Console.WriteLine(collection.ToString().Length);
Au cas où, regardez la description de la méthode
ToString :
Rien d'extraordinaire - la représentation sous forme de chaîne de l'objet est simplement renvoyée. Au cas où, je regarde également docs.microsoft.com - "
CommaDelimitedStringCollection.ToString Method ". Cela ne semble rien de spécial non plus.
Ok, exécutez le code pour l'exécution, ii ...
Hmm, de façon inattendue. Eh bien, essayons d'ajouter un élément à la collection, puis obtenons sa représentation sous forme de chaîne. «Complètement par accident» nous ajouterons une chaîne vide :). Le code changera et ressemblera à ceci:
CommaDelimitedStringCollection collection = new CommaDelimitedStringCollection(); collection.Add(String.Empty); Console.WriteLine(collection.ToString().Length);
Nous le lançons et voyons ...
Quoi encore?! Eh bien, regardons enfin l'implémentation de la méthode
ToString de la classe
CommaDelimitedStringCollection . Le code est présenté ci-dessous:
public override string ToString() { if (Count <= 0) return null; StringBuilder sb = new StringBuilder(); foreach (string str in this) { ThrowIfContainsDelimiter(str);
Avertissements de PVS-Studio :
- V3108 Il n'est pas recommandé de renvoyer 'null' à partir de la méthode 'ToSting ()'. StringAttributeCollection.cs 57
- V3108 Il n'est pas recommandé de renvoyer 'null' à partir de la méthode 'ToSting ()'. StringAttributeCollection.cs 71
Ici, nous voyons 2 endroits où l'implémentation actuelle de
ToString peut retourner
null . Rappelez-vous ce que Microsoft conseille lors de l'implémentation de la méthode
ToString , pour laquelle nous nous tournons Ă nouveau vers docs.microsoft.com - "
Object.ToString Method ":
Notes aux héritiers .... Les remplacements de la méthode ToString () doivent suivre ces directives:- ....
- Votre remplacement ToString () ne doit pas renvoyer vide ou une chaîne nulle .
- ....
En fait, c'est ce à quoi PVS-Studio met en garde. Les deux extraits de code ci-dessus que nous avons écrits pour reproduire le problème atteignent des points de sortie différents - les premier et deuxième endroits où renvoie
null , respectivement. Creuser un peu plus profondément.
Le premier cas.
Count - une propriété de la classe de base
StringCollection . Aucun élément n'ayant été ajouté,
Count == 0 , la condition
Count <= 0 est remplie,
null est renvoyé.
Dans le deuxième cas, nous avons ajouté un élément en utilisant la méthode d'instance
CommaDelimitedStringCollection.Add pour cela.
public new void Add(string value) { ThrowIfReadOnly(); ThrowIfContainsDelimiter(value); _modified = true; base.Add(value.Trim()); }
Les vérifications de la méthode
ThrowIf ... réussissent et l'élément est ajouté à la collection de base. Par conséquent, la valeur de
Count devient égale à 1. Maintenant, nous revenons à la méthode
ToString . La valeur de l'expression
Count <= 0 est
fausse , par conséquent, il n'y a aucun moyen de sortir de la méthode et le code continue de s'exécuter. La traversée de la collection interne commence et 2 éléments sont ajoutés au
StringBuilder - une chaîne vide et une virgule. Par conséquent, il s'avère que
sb ne contient qu'une virgule, la valeur de la propriété
Length , respectivement, est Ă©gale Ă un. La valeur de l'expression
sb.Length> 0 est
vraie , la soustraction et l'Ă©criture dans
sb.Length sont effectuées , maintenant la valeur de
sb.Length est 0. Cela conduit au fait que la méthode retourne à nouveau
null .
Numéro 15De façon assez inattendue, je voulais utiliser la classe
System.Configuration.ConfigurationProperty . Prenez le constructeur avec le plus de paramètres:
public ConfigurationProperty( string name, Type type, object defaultValue, TypeConverter typeConverter, ConfigurationValidatorBase validator, ConfigurationPropertyOptions options, string description);
Voyons la description du dernier paramètre:
La description du constructeur sur docs.microsoft.com dit la même chose. Eh bien, regardons comment ce paramètre est utilisé dans le corps du constructeur:
public ConfigurationProperty(...., string description) { ConstructorInit(name, type, options, validator, typeConverter); SetDefaultValue(defaultValue); }
Et le paramètre n'est pas utilisé.
Avertissement PVS-Studio : le paramètre constructeur
V3117 'description' n'est pas utilisé. ConfigurationProperty.cs 62
Ils ne l'utilisent probablement pas exprès, mais la description du paramètre correspondant prête à confusion.
Numéro 16J'ai rencontré un autre endroit similaire. Essayez de trouver l'erreur vous-même, le code du constructeur est ci-dessous.
internal SectionXmlInfo( string configKey, string definitionConfigPath, string targetConfigPath, string subPath, string filename, int lineNumber, object streamVersion, string rawXml, string configSource, string configSourceStreamName, object configSourceStreamVersion, string protectionProviderName, OverrideModeSetting overrideMode, bool skipInChildApps) { ConfigKey = configKey; DefinitionConfigPath = definitionConfigPath; TargetConfigPath = targetConfigPath; SubPath = subPath; Filename = filename; LineNumber = lineNumber; StreamVersion = streamVersion; RawXml = rawXml; ConfigSource = configSource; ConfigSourceStreamName = configSourceStreamName; ProtectionProviderName = protectionProviderName; OverrideModeSetting = overrideMode; SkipInChildApps = skipInChildApps; }
PVS-Studio :
V3117 Constructor parameter 'configSourceStreamVersion' is not used. SectionXmlInfo.cs 16
, :
internal object ConfigSourceStreamVersion { set { } }
, . , / , .
Issue 17,
System.Runtime.WindowsRuntime.UI.Xaml NuGet .
public struct RepeatBehavior : IFormattable { .... public override string ToString() { return InternalToString(null, null); } .... }
PVS-Studio :
V3108 It is not recommended to return 'null' from 'ToSting()' method. RepeatBehavior.cs 113
, —
ToString null . - , ,
RepeatBehavior.ToString , - . , Microsoft.
, ,
ToString null —
InternalToString .
internal string InternalToString(string format, IFormatProvider formatProvider) { switch (_Type) { case RepeatBehaviorType.Forever: return "Forever"; case RepeatBehaviorType.Count: StringBuilder sb = new StringBuilder(); sb.AppendFormat( formatProvider, "{0:" + format + "}x", _Count); return sb.ToString(); case RepeatBehaviorType.Duration: return _Duration.ToString(); default: return null; } }
, ,
switch default ,
InternalToString null , ,
null ToString .
RepeatBehavior — ,
ToString — , .
RepeatBehavior ,
ToString , ,
_Type RepeatBehaviorType.Forever ,
RepeatBehaviorType.Count RepeatBehaviorType.Duration .
_Type — , :
public struct RepeatBehavior : IFormattable { .... private RepeatBehaviorType _Type; .... public RepeatBehaviorType Type { get { return _Type; } set { _Type = value; } } .... }
. , ,
RepeatBehaviorType .
public enum RepeatBehaviorType { Count, Duration, Forever }
,
RepeatBehaviorType — , .
switch . , , ,
default .
System.Runtime.WindowsRuntime.UI.Xaml ( 4.3.0) .
RepeatBehavior behavior = new RepeatBehavior() { Type = (RepeatBehaviorType)666 }; Console.WriteLine(behavior.ToString() is null);
True ,
ToString null , ..
_Type case default . , , .
, ,
docs.microsoft.com , ,
null .
Issue 18System.Private.DataContractSerialization .
private static class CharType { public const byte None = 0x00; public const byte FirstName = 0x01; public const byte Name = 0x02; public const byte Whitespace = 0x04; public const byte Text = 0x08; public const byte AttributeText = 0x10; public const byte SpecialWhitespace = 0x20; public const byte Comment = 0x40; } private static byte[] s_charType = new byte[256] { .... CharType.None, CharType.None| CharType.Comment| CharType.Comment| CharType.Whitespace| CharType.Text| CharType.SpecialWhitespace, CharType.None| CharType.Comment| CharType.Comment| CharType.Whitespace| CharType.Text| CharType.SpecialWhitespace, CharType.None, CharType.None, CharType.None| CharType.Comment| CharType.Comment| CharType.Whitespace, CharType.None, .... };
PVS-Studio :
- V3001 There are identical sub-expressions 'CharType.Comment' to the left and to the right of the '|' opérateur. XmlUTF8TextReader.cs 56
- V3001 There are identical sub-expressions 'CharType.Comment' to the left and to the right of the '|' opérateur. XmlUTF8TextReader.cs 58
- V3001 There are identical sub-expressions 'CharType.Comment' to the left and to the right of the '|' opérateur. XmlUTF8TextReader.cs 64
CharType.Comment| CharType.Comment . ,
(CharType.Comment | CharType.Comment) == CharType.Comment . ,
CharType.Comment , .
Issue 19.
XmlBinaryWriterSession.TryAdd docs.microsoft.com — "
XmlBinaryWriterSession.TryAdd(XmlDictionaryString, Int32) Method ":
Returns: true if the string could be added; otherwise, false.:
public virtual bool TryAdd(XmlDictionaryString value, out int key) { IntArray keys; if (value == null) throw System.Runtime .Serialization .DiagnosticUtility .ExceptionUtility .ThrowHelperArgumentNull(nameof(value)); if (_maps.TryGetValue(value.Dictionary, out keys)) { key = (keys[value.Key] - 1); if (key != -1) {
PVS-Studio :
V3009 It's odd that this method always returns one and the same value of 'true'. XmlBinaryWriterSession.cs 29
,
true , ,
false .
Issue 20, —
false :
internal virtual bool OnHandleReference(....) { if (xmlWriter.depth < depthToCheckCyclicReference) return false; if (canContainCyclicReference) { if (_byValObjectsInScope.Contains(obj)) throw ....; _byValObjectsInScope.Push(obj); } return false; }
PVS-Studio :
V3009 It's odd that this method always returns one and the same value of 'false'. XmlObjectSerializerWriteContext.cs 415
, ! , , — , , , …
, , . :)
Issue 21System.Security.Cryptography.Algorithms .
public override byte[] GenerateMask(byte[] rgbSeed, int cbReturn) { using (HashAlgorithm hasher = (HashAlgorithm)CryptoConfig.CreateFromName(_hashNameValue)) { byte[] rgbCounter = new byte[4]; byte[] rgbT = new byte[cbReturn]; uint counter = 0; for (int ib = 0; ib < rgbT.Length;) {
PVS-Studio :
V3080 Possible null dereference. Consider inspecting 'hasher'. PKCS1MaskGenerationMethod.cs 37
,
hasher.TransformBlock hasher null ,
NullReferenceException . .
, ,
hasher null ,
CreateFromName .
public static object CreateFromName(string name) { return CreateFromName(name, null); }
— .
CreateFromName , .
public static object CreateFromName(string name, params object[] args) { .... if (retvalType == null) { return null; } .... if (cons == null) { return null; } .... if (candidates.Count == 0) { return null; } .... if (rci == null || typeof(Delegate).IsAssignableFrom(rci.DeclaringType)) { return null; } .... return retval; }
, ,
null . , , , ,
NullReferenceException .
— , . . .
public class PKCS1MaskGenerationMethod : ....
:
1, 3 .
public . , — .
2 . — , — , . , .
4 .
CreateFromName null — , .
5, 6 .
cbReturn > 0 (, , ).
cbReturn > 0 ib < rgbT.Length .
7 .
Helpres.ConvertIntToByteArray .
, , , :
- rgbCeed — new byte[] { 0, 1, 2, 3 };
- cbReturn — 42.
, «»
CryptoConfig.CreateFromName ,
_hashNameValue . , , - :
public string HashName { get { return _hashNameValue; } set { _hashNameValue = value ?? DefaultHash; } }
''
HashName ( —
_hashNameValue ),
null CreateFromName . (, ), .
,
NullReferenceException , :
PKCS1MaskGenerationMethod tempObj = new PKCS1MaskGenerationMethod(); tempObj.HashName = "Dummy"; tempObj.GenerateMask(new byte[] { 1, 2, 3 }, 42);
, :
NuGet 4.3.1.
, , docs.microsoft.com — "
PKCS1MaskGenerationMethod.GenerateMask(Byte[], Int32) Method ".
, 2 «» :
OutOfMemoryException .
NullReferenceException rgbSeed.Length . ,
hasher ,
rgbSeed.Length .
Issue 22.
public class SignatureDescription { .... public string FormatterAlgorithm { get; set; } public string DeformatterAlgorithm { get; set; } public SignatureDescription() { } .... public virtual AsymmetricSignatureDeformatter CreateDeformatter( AsymmetricAlgorithm key) { AsymmetricSignatureDeformatter item = (AsymmetricSignatureDeformatter) CryptoConfig.CreateFromName(DeformatterAlgorithm); item.SetKey(key);
PVS-Studio :
- V3080 Possible null dereference. Consider inspecting 'item'. SignatureDescription.cs 31
- V3080 Possible null dereference. Consider inspecting 'item'. SignatureDescription.cs 38
,
FormatterAlgorithm DeformatterAlgorithm ,
CryptoConfig.CreateFromName null CreateDeformatter CreateFormatter . ,
SetKey NullReferenceException . , , :
SignatureDescription signature = new SignatureDescription() { DeformatterAlgorithm = "Dummy", FormatterAlgorithm = "Dummy" }; signature.CreateDeformatter(null);
CreateDeformatter ,
CreateFormatter NullReferenceException .
Issue 23System.Private.Xml .
public override void WriteBase64(byte[] buffer, int index, int count) { if (!_inAttr && (_inCDataSection || StartCDataSection())) _wrapped.WriteBase64(buffer, index, count); else _wrapped.WriteBase64(buffer, index, count); }
PVS-Studio :
V3004 The 'then' statement is equivalent to the 'else' statement. QueryOutputWriterV1.cs 242
,
then else if . , ,
if .
Issue 24 internal void Depends(XmlSchemaObject item, ArrayList refs) { .... if (content is XmlSchemaSimpleTypeRestriction) { baseType = ((XmlSchemaSimpleTypeRestriction)content).BaseType; baseName = ((XmlSchemaSimpleTypeRestriction)content).BaseTypeName; } else if (content is XmlSchemaSimpleTypeList) { .... } else if (content is XmlSchemaSimpleTypeRestriction) { baseName = ((XmlSchemaSimpleTypeRestriction)content).BaseTypeName; } else if (t == typeof(XmlSchemaSimpleTypeUnion)) { .... } .... }
PVS-Studio :
V3003 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 381, 396. ImportContext.cs 381
if-else-if —
content is XmlSchemaSimpleTypeRestriction . —
then - . ,
then - ( ), , , .
Issue 25, .
public bool MatchesXmlType(IList<XPathItem> seq, int indexType) { XmlQueryType typBase = GetXmlType(indexType); XmlQueryCardinality card; switch (seq.Count) { case 0: card = XmlQueryCardinality.Zero; break; case 1: card = XmlQueryCardinality.One; break; default: card = XmlQueryCardinality.More; break; } if (!(card <= typBase.Cardinality)) return false; typBase = typBase.Prime; for (int i = 0; i < seq.Count; i++) { if (!CreateXmlType(seq[0]).IsSubtypeOf(typBase)) return false; } return true; }
— !
— PVS-Studio :
V3102 Suspicious access to element of 'seq' object by a constant index inside a loop. XmlQueryRuntime.cs 738
for ,
i < seq.Count . ,
seq . (
seq[i] ), — (
seq[0] ).
Issue 26, .
public override void WriteValue(string value) { WriteValue(value); }
PVS-Studio :
V3110 Possible infinite recursion inside 'WriteValue' method. XmlAttributeCache.cs 166
, .
Issue 27 public IList<XPathNavigator> DocOrderDistinct(IList<XPathNavigator> seq) { if (seq.Count <= 1) return seq; XmlQueryNodeSequence nodeSeq = (XmlQueryNodeSequence)seq; if (nodeSeq == null) nodeSeq = new XmlQueryNodeSequence(seq); return nodeSeq.DocOrderDistinct(_docOrderCmp); }
PVS-Studio :
V3095 The 'seq' object was used before it was verified against null. Check lines: 880, 884. XmlQueryRuntime.cs 880
null , -
Count NullReferenceException .
nodeSeq ,
seq , — .
seq —
null , - .
seq —
null , :
- InvalidCastException , ;
- , nodeSeq , null .
Issue 284 , . , , .
PVS-Studio :
- V3117 Constructor parameter 'securityUrl' is not used. XmlSecureResolver.cs 15
- V3117 Constructor parameter 'strdata' is not used. XmlEntity.cs 18
- V3117 Constructor parameter 'location' is not used. Compilation.cs 58
- V3117 Constructor parameter 'access' is not used. XmlSerializationILGen.cs 38
( , ). Quoi? . , .
public XmlSecureResolver(XmlResolver resolver, string securityUrl) { _resolver = resolver; }
, docs.microsoft.com — "
XmlSecureResolver Constructors "
securityUrl :
The URL used to create the PermissionSet that will be applied to the underlying XmlResolver. The XmlSecureResolver calls PermitOnly() on the created PermissionSet before calling GetEntity(Uri, String, Type) on the underlying XmlResolver.Issue 29System.Private.Uri , Microsoft
ToString . "
Object.ToString Method ":
Your ToString() override should not throw an exception .:
public override string ToString() { if (_username.Length == 0 && _password.Length > 0) { throw new UriFormatException(SR.net_uri_BadUserPassword); } .... }
PVS-Studio :
V3108 It is not recommended to throw exceptions from 'ToSting()' method. UriBuilder.cs 406
,
UserName Password _username _password ,
ToString , . :
UriBuilder uriBuilder = new UriBuilder() { UserName = String.Empty, Password = "Dummy" }; String stringRepresentation = uriBuilder.ToString(); Console.WriteLine(stringRepresentation);
, — , docs.microsoft.com — "
UriBuilder.ToString Method ".
Issue 30,
System.Data.Common .
private ArrayList _tables; private DataTable GetTable(string tableName, string ns) { .... if (_tables.Count == 0) return (DataTable)_tables[0]; .... }
PVS-Studio :
V3106 Possibly index is out of bound. The '0' index is pointing beyond '_tables' bound. XMLDiffLoader.cs 277
? , ?
ArgumentOutOfRangeException ? , , . , .
Issue 31 internal XmlNodeOrder ComparePosition(XPathNodePointer other) { RealFoliate(); other.RealFoliate(); Debug.Assert(other != null); .... }
PVS-Studio :
V3095 The 'other' object was used before it was verified against null. Check lines: 1095, 1096. XPathNodePointer.cs 1095
other != null Debug.Assert ,
ComparePosition null . , .
other RealFoliate . ,
other null ,
NullReferenceException Assert .
Issue 32 private PropertyDescriptorCollection GetProperties(Attribute[] attributes) { .... foreach (Attribute attribute in attributes) { Attribute attr = property.Attributes[attribute.GetType()]; if ( (attr == null && !attribute.IsDefaultAttribute()) || !attr.Match(attribute)) { match = false; break; } } .... }
PVS-Studio :
V3080 Possible null dereference. Consider inspecting 'attr'. DbConnectionStringBuilder.cs 534
if .
Match — .
attr == null ,
null — () . , || ,
attr —
null ,
NullReferenceException .
, :
- attr — null . &&.
- !attribute.IsDefaultAttribute() — false . && — false .
- || false , .
- attr — null , Match .
Issue 33 private int ReadOldRowData( DataSet ds, ref DataTable table, ref int pos, XmlReader row) { .... if (table == null) { row.Skip();
PVS-Studio :
V3021 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless XMLDiffLoader.cs 301
if , —
table == null . then- — -1, — .
table . , .
Issue 34System.ComponentModel.TypeConverter . , :
Removes the last character from the formatted string. (Remove last character in virtual string). On exit the out param contains the position where the operation was actually performed. This position is relative to the test string. The MaskedTextResultHint out param gives more information about the operation result. Returns true on success, false otherwise.: ,
true , —
false . , .
public bool Remove(out int testPosition, out MaskedTextResultHint resultHint) { .... if (lastAssignedPos == INVALID_INDEX) { .... return true;
PVS-Studio :
V3009 It's odd that this method always returns one and the same value of 'true'. MaskedTextProvider.cs 1529
, —
true .
Issue 35 public void Clear() { if (_table != null) { .... } if (_table.fInitInProgress && _delayLoadingConstraints != null) { .... } .... }
PVS-Studio :
V3125 The '_table' object was used after it was verified against null. Check lines: 437, 423. ConstraintCollection.cs 437
_table != null —
_table null . , .
_table null —
_table .fInitInProgress .
Issue 36,
System.Runtime.Serialization.Formatters .
private void Write(....) { .... if (memberNameInfo != null) { .... _serWriter.WriteObjectEnd(memberNameInfo, typeNameInfo); } else if ((objectInfo._objectId == _topId) && (_topName != null)) { _serWriter.WriteObjectEnd(topNameInfo, typeNameInfo); .... } else if (!ReferenceEquals(objectInfo._objectType, Converter.s_typeofString)) { _serWriter.WriteObjectEnd(typeNameInfo, typeNameInfo); } }
PVS-Studio :
V3038 The argument was passed to method several times. It is possible that other argument should be passed instead. BinaryObjectWriter.cs 262
_serWriter.WriteObjectEnd —
typeNameInfo . , . ,
WriteObjectEnd .
internal void WriteObjectEnd(NameInfo memberNameInfo, NameInfo typeNameInfo) { }
… . :)
Issue 37 internal void WriteSerializationHeader( int topId, int headerId, int minorVersion, int majorVersion) { var record = new SerializationHeaderRecord( BinaryHeaderEnum.SerializedStreamHeader, topId, headerId, minorVersion, majorVersion); record.Write(this); }
, , . , .
PVS-Studio :
V3066 Possible incorrect order of arguments passed to 'SerializationHeaderRecord' constructor: 'minorVersion' and 'majorVersion'. BinaryFormatterWriter.cs 111
SerializationHeaderRecord .
internal SerializationHeaderRecord( BinaryHeaderEnum binaryHeaderEnum, int topId, int headerId, int majorVersion, int minorVersion) { _binaryHeaderEnum = binaryHeaderEnum; _topId = topId; _headerId = headerId; _majorVersion = majorVersion; _minorVersion = minorVersion; }
,
majorVersion ,
minorVersion ;
minorVersion ,
majorVersion . . ( ?) — , .
Issue 38 internal ObjectManager( ISurrogateSelector selector, StreamingContext context, bool checkSecurity, bool isCrossAppDomain) { _objects = new ObjectHolder[DefaultInitialSize]; _selector = selector; _context = context; _isCrossAppDomain = isCrossAppDomain; }
PVS-Studio :
V3117 Constructor parameter 'checkSecurity' is not used. ObjectManager.cs 33
checkSecurity . . , , , .
Issue 39, . 1 1 . :
:
private void EnlargeArray() { int newLength = _values.Length * 2; if (newLength < 0) { if (newLength == int.MaxValue) { throw new SerializationException(SR.Serialization_TooManyElements); } newLength = int.MaxValue; } FixupHolder[] temp = new FixupHolder[newLength]; Array.Copy(_values, 0, temp, 0, _count); _values = temp; }
PVS-Studio :
- V3022 Expression 'newLength == int.MaxValue' is always false. ObjectManager.cs 1423
- V3022 Expression 'newLength == int.MaxValue' is always false. ObjectManager.cs 1511
- V3022 Expression 'newLength == int.MaxValue' is always false. ObjectManager.cs 1558
, —
temp (
FixupHolder ,
long object ). - copy-paste…
Issue 40System.Data.Odbc .
public string UnquoteIdentifier(....) { .... if (!string.IsNullOrEmpty(quotePrefix) || quotePrefix != " ") { .... } .... }
PVS-Studio :
V3022 Expression '!string.IsNullOrEmpty(quotePrefix) || quotePrefix != " "' is always true. OdbcCommandBuilder.cs 338
,
true . . ,
quotePrefix — . .
||, ,
true , ( )
true . . ,
false . , ,
true , —
false ,
true .
, ,
string.IsNullOrEmpty(quotePrefix) —
true , , :
- quotePrefix == null ;
- quotePrefix.Length == 0 .
quotePrefix != " " , . , —
true ,
quotePrefix .
Issue 41:
private sealed class PendingGetConnection { public PendingGetConnection( long dueTime, DbConnection owner, TaskCompletionSource<DbConnectionInternal> completion, DbConnectionOptions userOptions) { DueTime = dueTime; Owner = owner; Completion = completion; } public long DueTime { get; private set; } public DbConnection Owner { get; private set; } public TaskCompletionSource<DbConnectionInternal> Completion { get; private set; } public DbConnectionOptions UserOptions { get; private set; } }
PVS-Studio :
V3117 Constructor parameter 'userOptions' is not used. DbConnectionPool.cs 26
, —
userOptions , , . , .
Issue 42, 2 . .
private DataTable ExecuteCommand(....) { .... foreach (DataRow row in schemaTable.Rows) { resultTable.Columns .Add(row["ColumnName"] as string, (Type)row["DataType"] as Type); } .... }
PVS-Studio :
- V3051 An excessive type cast. The object is already of the 'Type' type. DbMetaDataFactory.cs 176
- V3051 An excessive type cast. The object is already of the 'Type' type. OdbcMetaDataFactory.cs 1109
(Type)row[«DataType»] as Type . , —
as .
row[«DataType»] —
null , ''
Add .
row[«DataType»] ,
Type ,
InvalidCastException . , ? La question est ouverte.
Issue 43System.Runtime.InteropServices.RuntimeInformation .
public static string FrameworkDescription { get { if (s_frameworkDescription == null) { string versionString = (string)AppContext.GetData("FX_PRODUCT_VERSION"); if (versionString == null) { .... versionString = typeof(object).Assembly .GetCustomAttribute< AssemblyInformationalVersionAttribute>() ?.InformationalVersion; .... int plusIndex = versionString.IndexOf('+'); .... } .... } .... } }
PVS-Studio :
V3105 The 'versionString' variable was used after it was assigned through null-conditional operator. NullReferenceException is possible. RuntimeInformation.cs 29
NullReferenceException IndexOf versionString . '?.',
NullReferenceException InfromationalVersion . ,
GetCustomAttribute<...> null , , —
IndexOf ,
versionString null .
Issue 44System.ComponentModel.Composition . 2 :
public static bool CanSpecialize(....) { .... object[] genericParameterConstraints = ....; GenericParameterAttributes[] genericParameterAttributes = ....;
PVS-Studio :
- V3125 The 'genericParameterConstraints' object was used after it was verified against null. Check lines: 603, 589. GenericSpecializationPartCreationInfo.cs 603
- V3125 The 'genericParameterAttributes' object was used after it was verified against null. Check lines: 604, 594. GenericSpecializationPartCreationInfo.cs 604
genericParameterAttributes != null genericParameterConstraints != null . ,
null — , .
null , — . , -
null , ? , ,
NullReferenceException .
Issue 45. , — , . NuGet prerelease ( 4.6.0-preview6.19303.8). , , :
LazyMemberInfo lazyMemberInfo = new LazyMemberInfo(); var eq = lazyMemberInfo.Equals(null); Console.WriteLine(eq);
Equals , docs.microsoft.com .NET Core, .NET Framework. ("
LazyMemberInfo.Equals(Object) Method ") — —
true false , .
:
, :
LazyMemberInfo lazyMemberInfo = new LazyMemberInfo(); var eq = lazyMemberInfo.Equals(typeof(String)); Console.WriteLine(eq);
.
, .
Equals .
public override bool Equals(object obj) { LazyMemberInfo that = (LazyMemberInfo)obj;
PVS-Studio :
V3115 Passing 'null' to 'Equals' method should not result in 'NullReferenceException'. LazyMemberInfo.cs 116
, ,
that._memberType . , , —
(LazyMemberInfo)obj . .
InvalidCastException , , .
NullReferenceException ? ,
LazyMemberInfo — , , .
null NullReferenceException . — . .
Issue 46- , ,
System.Drawing.Common ,
TriState .
public override bool Equals(object o) { TriState state = (TriState)o; return _value == state._value; }
PVS-Studio :
V3115 Passing 'null' to 'Equals' method should not result in 'NullReferenceException'. TriState.cs 53
, .
Issue 47System.Text.Json .
, ,
ToString null ? .
public override string ToString() { switch (TokenType) { case JsonTokenType.None: case JsonTokenType.Null: return string.Empty; case JsonTokenType.True: return bool.TrueString; case JsonTokenType.False: return bool.FalseString; case JsonTokenType.Number: case JsonTokenType.StartArray: case JsonTokenType.StartObject: {
null , .
PVS-Studio :
V3108 It is not recommended to return 'null' from 'ToSting()' method. JsonElement.cs 1460
GetString() . :
public string GetString() { CheckValidInstance(); return _parent.GetString(_idx, JsonTokenType.String); }
—
GetString :
internal string GetString(int index, JsonTokenType expectedType) { .... if (tokenType == JsonTokenType.Null) { return null; } .... }
,
null — ,
ToString .
Issue 48:
internal JsonPropertyInfo CreatePolymorphicProperty(....) { JsonPropertyInfo runtimeProperty = CreateProperty(property.DeclaredPropertyType, runtimePropertyType, property.ImplementedPropertyType, property?.PropertyInfo, Type, options); property.CopyRuntimeSettingsTo(runtimeProperty); return runtimeProperty; }
PVS-Studio :
V3042 Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'property' object JsonClassInfo.AddProperty.cs 179
CreateProperty property :
property.DeclaredPropertyType ,
property.ImplementedPropertyType ,
property?.PropertyInfo . , '?.'.
property null , ,
NullReferenceException .
Issue 49System.Security.Cryptography.Xml , . copy-paste, .
:
public void Write(StringBuilder strBuilder, DocPosition docPos, AncestralNamespaceContextManager anc) { docPos = DocPosition.BeforeRootElement; foreach (XmlNode childNode in ChildNodes) { if (childNode.NodeType == XmlNodeType.Element) { CanonicalizationDispatcher.Write( childNode, strBuilder, DocPosition.InRootElement, anc); docPos = DocPosition.AfterRootElement; } else { CanonicalizationDispatcher.Write(childNode, strBuilder, docPos, anc); } } }
:
public void WriteHash(HashAlgorithm hash, DocPosition docPos, AncestralNamespaceContextManager anc) { docPos = DocPosition.BeforeRootElement; foreach (XmlNode childNode in ChildNodes) { if (childNode.NodeType == XmlNodeType.Element) { CanonicalizationDispatcher.WriteHash( childNode, hash, DocPosition.InRootElement, anc); docPos = DocPosition.AfterRootElement; } else { CanonicalizationDispatcher.WriteHash(childNode, hash, docPos, anc); } } }
PVS-Studio :
- V3061 Parameter 'docPos' is always rewritten in method body before being used. CanonicalXmlDocument.cs 37
- V3061 Parameter 'docPos' is always rewritten in method body before being used. CanonicalXmlDocument.cs 54
docPos , , , , , .
Issue 50System.Data.SqlClient .
private bool IsBOMNeeded(MetaType type, object value) { if (type.NullableType == TdsEnums.SQLXMLTYPE) { Type currentType = value.GetType(); if (currentType == typeof(SqlString)) { if (!((SqlString)value).IsNull && ((((SqlString)value).Value).Length > 0)) { if ((((SqlString)value).Value[0] & 0xff) != 0xff) return true; } } else if ((currentType == typeof(string)) && (((String)value).Length > 0)) { if ((value != null) && (((string)value)[0] & 0xff) != 0xff) return true; } else if (currentType == typeof(SqlXml)) { if (!((SqlXml)value).IsNull) return true; } else if (currentType == typeof(XmlDataFeed)) { return true;
PVS-Studio :
V3095 The 'value' object was used before it was verified against null. Check lines: 8696, 8708. TdsParser.cs 8696
value != null . , ,
value .
value null — .
Issue 51, , .
protected virtual TDSMessageCollection CreateQueryResponse(....) { .... if (....) { .... } else if ( lowerBatchText.Contains("name") && lowerBatchText.Contains("state") && lowerBatchText.Contains("databases") && lowerBatchText.Contains("db_name"))
PVS-Studio :
V3053 An excessive expression. Examine the substrings 'name' and 'db_name'. QueryEngine.cs 151
,
lowerBatchText.Contains(«name») lowerBatchText.Contains(«db_name») . ,
«db_name» ,
«name» .
«name» ,
«db_name» . ,
lowerBatchText.Contains(«name») . ,
«name» .
Issue 52System.Net.Requests .
protected override PipelineInstruction PipelineCallback( PipelineEntry entry, ResponseDescription response, ....) { if (NetEventSource.IsEnabled) NetEventSource.Info(this, $"Command:{entry?.Command} Description:{response?.StatusDescription}");
PVS-Studio :
V3125 The 'entry' object was used after it was verified against null. Check lines: 270, 227. FtpControlStream.cs 270
entry?.Command response?.Description . '.' '?.',
NullReferenceException , -
null . . , ,
null response ( ,
response == null ),
entry . ,
entry —
null , ,
entry.Command ( '.', '?.') .
, — , , .
? . :)
Issue 53-
System.Collections.Immutable .
System.Collections.Immutable.ImmutableArray<T> .
IStructuralEquatable.Equals IStructuralComparable.CompareTo .
IStructuralEquatable.Equals . , , :
bool IStructuralEquatable.Equals(object other, IEqualityComparer comparer) { var self = this; Array otherArray = other as Array; if (otherArray == null) { var theirs = other as IImmutableArray; if (theirs != null) { otherArray = theirs.Array; if (self.array == null && otherArray == null) { return true; } else if (self.array == null) { return false; } } } IStructuralEquatable ours = self.array; return ours.Equals(otherArray, comparer); }
? — , . :)
PVS-Studio :
V3125 The 'ours' object was used after it was verified against null. Check lines: 1212, 1204. ImmutableArray_1.cs 1212
Equals ours ,
return , ,
NullReferenceException . ? , .
bool IStructuralEquatable.Equals(object other, IEqualityComparer comparer) { .... if (....) { .... if (....) { .... if (self.array == null && otherArray == null) { .... } else if (self.array == null) { .... } } } IStructuralEquatable ours = self.array; return ours.Equals(otherArray, comparer); }
,
ours self.array .
self.array == null . ,
self.array ,
ours ,
null . , . ? . .
bool IStructuralEquatable.Equals(object other, IEqualityComparer comparer) { var self = this;
1 .
self.array == this.array (-
self = this ). ,
this.array == null .
2 .
if , , .
if , ,
other Array other null .
as otherArray ,
if .
3 . .
if (,
theirs != null ).
then -, 5
self.array == null 4. ,
if 3, :
- other null ;
- other IImmutableArray .
5 .
self.array == null , , ,
NullReferenceException .
, .
:
this.array — null .
— :
- other — null ;
- other Array ;
- other Array , IImmutableArray .
array — , :
internal T[] array;
ImmutableArray<T> — , ( ),
array , —
null . , .
, , , .
, . , , , .
1 .
var comparer = EqualityComparer<String>.Default; ImmutableArray<String> immutableArray = new ImmutableArray<string>(); ((IStructuralEquatable)immutableArray).Equals(null, comparer);
2 .
var comparer = EqualityComparer<String>.Default; ImmutableArray<String> immutableArray = new ImmutableArray<string>(); ((IStructuralEquatable)immutableArray).Equals(new string[] { }, comparer);
3 .
var comparer = EqualityComparer<String>.Default; ImmutableArray<String> immutableArray = new ImmutableArray<string>(); ((IStructuralEquatable)immutableArray).Equals(typeof(Object), comparer);
, .
Issue 54, , . :) . , .
int IStructuralComparable.CompareTo(object other, IComparer comparer) { var self = this; Array otherArray = other as Array; if (otherArray == null) { var theirs = other as IImmutableArray; if (theirs != null) { otherArray = theirs.Array; if (self.array == null && otherArray == null) { return 0; } else if (self.array == null ^ otherArray == null) { throw new ArgumentException( SR.ArrayInitializedStateNotEqual, nameof(other)); } } } if (otherArray != null) { IStructuralComparable ours = self.array; return ours.CompareTo(otherArray, comparer);
PVS-Studio :
V3125 The 'ours' object was used after it was verified against null. Check lines: 1265, 1251. ImmutableArray_1.cs 1265
, .
:
Object other = ....; var comparer = Comparer<String>.Default; ImmutableArray<String> immutableArray = new ImmutableArray<string>(); ((IStructuralComparable)immutableArray).CompareTo(other, comparer);
, ,
NullReferenceException .
:
other —
new String[]{ } ;
:
, , .
Issue 55System.Net.HttpListener , , , . copy-paste. , , :
public override IAsyncResult BeginRead(byte[] buffer, ....) { if (NetEventSource.IsEnabled) { NetEventSource.Enter(this); NetEventSource.Info(this, "buffer.Length:" + buffer.Length + " size:" + size + " offset:" + offset); } if (buffer == null) { throw new ArgumentNullException(nameof(buffer)); } .... }
PVS-Studio :
V3095 The 'buffer' object was used before it was verified against null. Check lines: 51, 53. HttpRequestStream.cs 51
ArgumentNullException buffer == null ,
null — . ,
NetEventSource.IsEnabled —
true ,
buffer —
null ,
NullReferenceException buffer.Length . ,
buffer == null .
PVS-Studio, :
- V3095 The 'buffer' object was used before it was verified against null. Check lines: 49, 51. HttpResponseStream.cs 49
- V3095 The 'buffer' object was used before it was verified against null. Check lines: 74, 75. HttpResponseStream.cs 74
Issue 56System.Transactions.Local .
internal override void EnterState(InternalTransaction tx) { if (tx._outcomeSource._isoLevel == IsolationLevel.Snapshot) { throw TransactionException.CreateInvalidOperationException( TraceSourceType.TraceSourceLtm, SR.CannotPromoteSnapshot, null, tx == null ? Guid.Empty : tx.DistributedTxId); } .... }
PVS-Studio :
V3095 The 'tx' object was used before it was verified against null. Check lines: 3282, 3285. TransactionState.cs 3282
InvalidOperationException .
tx ,
null ,
NullReferenceException tx.DistributedTxId . , ,
tx —
null ,
if tx —
tx._outcomeSource._isoLevel .
Issue 57System.Runtime.Caching .
internal void SetLimit(int cacheMemoryLimitMegabytes) { long cacheMemoryLimit = cacheMemoryLimitMegabytes; cacheMemoryLimit = cacheMemoryLimit << MEGABYTE_SHIFT; _memoryLimit = 0;
PVS-Studio :
V3022 Expression 'cacheMemoryLimit != 0 && _memoryLimit != 0' is always false. CacheMemoryMonitor.cs 250
, , —
cacheMemoryLimit != 0 && _memoryLimit != 0 —
false .
_memoryLimit 0 (
if ), &&
false , ,
false .
Issue 58System.Diagnostics.TraceSource .
public override object Pop() { StackNode n = _stack.Value; if (n == null) { base.Pop(); } _stack.Value = n.Prev; return n.Value; }
PVS-Studio :
V3125 The 'n' object was used after it was verified against null. Check lines: 115, 111. CorrelationManager.cs 115
. -
n == null ,
null — . —
NullReferenceException —
n.Prev .
n null ,
base.Pop() .
Issue 59System.Drawing.Primitives . . :
public static string ToHtml(Color c) { string colorString = string.Empty; if (c.IsEmpty) return colorString; if (ColorUtil.IsSystemColor(c)) { switch (c.ToKnownColor()) { case KnownColor.ActiveBorder: colorString = "activeborder"; break; case KnownColor.GradientActiveCaption: case KnownColor.ActiveCaption: colorString = "activecaption"; break; case KnownColor.AppWorkspace: colorString = "appworkspace"; break; case KnownColor.Desktop: colorString = "background"; break; case KnownColor.Control: colorString = "buttonface"; break; case KnownColor.ControlLight: colorString = "buttonface"; break; case KnownColor.ControlDark: colorString = "buttonshadow"; break; case KnownColor.ControlText: colorString = "buttontext"; break; case KnownColor.ActiveCaptionText: colorString = "captiontext"; break; case KnownColor.GrayText: colorString = "graytext"; break; case KnownColor.HotTrack: case KnownColor.Highlight: colorString = "highlight"; break; case KnownColor.MenuHighlight: case KnownColor.HighlightText: colorString = "highlighttext"; break; case KnownColor.InactiveBorder: colorString = "inactiveborder"; break; case KnownColor.GradientInactiveCaption: case KnownColor.InactiveCaption: colorString = "inactivecaption"; break; case KnownColor.InactiveCaptionText: colorString = "inactivecaptiontext"; break; case KnownColor.Info: colorString = "infobackground"; break; case KnownColor.InfoText: colorString = "infotext"; break; case KnownColor.MenuBar: case KnownColor.Menu: colorString = "menu"; break; case KnownColor.MenuText: colorString = "menutext"; break; case KnownColor.ScrollBar: colorString = "scrollbar"; break; case KnownColor.ControlDarkDark: colorString = "threeddarkshadow"; break; case KnownColor.ControlLightLight: colorString = "buttonhighlight"; break; case KnownColor.Window: colorString = "window"; break; case KnownColor.WindowFrame: colorString = "windowframe"; break; case KnownColor.WindowText: colorString = "windowtext"; break; } } else if (c.IsNamedColor) { if (c == Color.LightGray) {
-, … ? , , .
:
switch (c.ToKnownColor()) { .... case KnownColor.Control: colorString = "buttonface"; break; case KnownColor.ControlLight: colorString = "buttonface"; break; .... }
PVS-Studio :
V3139 Two or more case-branches perform the same actions. ColorTranslator.cs 302
, , - . , ,
case , . copy-paste , .
.
ToHtml «buttonface» , ():
- SystemColors.Control ;
- SystemColors.ControlLight .
ARGB, :
- SystemColors.Control — (255, 240, 240, 240) ;
- SystemColors.ControlLight — (255, 227, 227, 227) .
(
«buttonface» ) —
FromHtml ,
Control (255, 240, 240, 240) .
FromHtml ControlLight ? Oui , ( ) . :
s_htmlSysColorTable["threedhighlight"] = ColorUtil.FromKnownColor(KnownColor.ControlLight);
,
FromHtml ControlLight (255, 227, 227, 227) «threedhighlight» . ,
case KnownColor.ControlLight .
Issue 60System.Text.RegularExpressions .
internal virtual string TextposDescription() { var sb = new StringBuilder(); int remaining; sb.Append(runtextpos); if (sb.Length < 8) sb.Append(' ', 8 - sb.Length); if (runtextpos > runtextbeg) sb.Append(RegexCharClass.CharDescription(runtext[runtextpos - 1])); else sb.Append('^'); sb.Append('>'); remaining = runtextend - runtextpos; for (int i = runtextpos; i < runtextend; i++) { sb.Append(RegexCharClass.CharDescription(runtext[i])); } if (sb.Length >= 64) { sb.Length = 61; sb.Append("..."); } else { sb.Append('$'); } return sb.ToString(); }
PVS-Studio :
V3137 The 'remaining' variable is assigned but is not used by the end of the function. RegexRunner.cs 612
remaining - , . , - , , , . - .
Issue 61 public void AddRange(char first, char last) { _rangelist.Add(new SingleRange(first, last)); if (_canonical && _rangelist.Count > 0 && first <= _rangelist[_rangelist.Count - 1].Last) { _canonical = false; } }
PVS-Studio :
V3063 A part of conditional expression is always true if it is evaluated: _rangelist.Count > 0. RegexCharClass.cs 523
, —
_rangelist.Count > 0 —
true , . ,
_rangelist , , —
_rangelist.Add(....) — .
Issue 62V3128 System.Drawing.Common System.Transactions.Local .
private class ArrayEnumerator : IEnumerator { private object[] _array; private object _item; private int _index; private int _startIndex; private int _endIndex; public ArrayEnumerator(object[] array, int startIndex, int count) { _array = array; _startIndex = startIndex; _endIndex = _index + count; _index = _startIndex; } .... }
PVS-Studio :
V3128 The '_index' field is used before it is initialized in constructor. PrinterSettings.Windows.cs 1679
_endIndex —
_index , —
default(int) ,
0 .
_index . —
_index , .
Issue 63 internal class TransactionTable { .... private int _timerInterval; .... internal TransactionTable() {
PVS-Studio :
V3128 The '_timerInterval' field is used before it is initialized in constructor. TransactionTable.cs 151
.
_timerInterval (
default(int) )
_timer ,
_timerInterval .
Issue 64, . , .
copy-paste , .
private bool ProcessNotifyConnection(....) { .... WeakReference reference = (WeakReference)( LdapConnection.s_handleTable[referralFromConnection]); if ( reference != null && reference.IsAlive && null != ((LdapConnection)reference.Target)._ldapHandle) { .... } .... }
PVS-Studio () : VXXXX TODO_MESSAGE. LdapSessionOptions.cs 974
,
reference.IsAlive ,
WeakReference ,
reference.Target null . —
_ldapHandle NullReferenceException . , IsAlive Microsoft. docs.microsoft.com — "
WeakReference.IsAlive Property ":
Because an object could potentially be reclaimed for garbage collection immediately after the IsAlive property returns true, using this property is not recommended unless you are testing only for a false return value., ? , ! , . , , , , , . ( ), , , - . , , , .
,
V3022 V3063 . , :
String str = null; if (str == null) ....
, , .
lock statement this .. —
V3090 ; —
V3083 ; ,
IDisposable ,
Dispose /
Close —
V3072 ; .
( — , - ) , . , , . , - .
, , —
.
. — , . , , .
— , , , . , , .
, — . / , . , PVS-Studio.
Conclusion
, — ! , . - , — .
, ,
PVS-Studio , . - —
support@viva64.com . :)
!
PS .NET Core
, ! — . , . , , — , , / (
).
, : Sergey Vasiliev.
Checking the .NET Core Libraries Source Code by the PVS-Studio Static Analyzer