Geekly articles weekly

Búsqueda de errores en el código fuente del SDK de Amazon Web Services para .NET

Imagen 1


Bienvenido a todos los fanáticos de destrozar el código de otra persona. :) Hoy en nuestro laboratorio, tenemos un nuevo material para una investigación: el código fuente del AWS SDK para el proyecto .NET. En ese momento, escribimos un artículo sobre la comprobación de AWS SDK para C ++. Entonces no había nada particularmente interesante. Veamos qué vale .NET de la versión AWS SDK. Una vez más, es una gran oportunidad para demostrar las habilidades del analizador PVS-Studio y hacer que el mundo sea un poco mejor.

El SDK de Amazon Web Services (AWS) para .NET es un conjunto de herramientas de desarrollador destinadas a crear aplicaciones basadas en .NET en la infraestructura de AWS. Este conjunto permite simplificar significativamente el proceso de escritura de código. El SDK incluye conjuntos API .NET para varios servicios de AWS, como Amazon S3, Amazon EC2, DynamoDB y otros. El código fuente del SDK está disponible en GitHub .

Como mencioné, en ese momento ya hemos escrito el artículo sobre la comprobación de AWS SDK para C ++. El artículo resultó ser pequeño: solo se encontraron un par de errores por cada 512 mil líneas de código. Esta vez estamos tratando con un tamaño mucho más grande del código, que incluye aproximadamente 34 mil archivos cs, y el número total de líneas de código (excluyendo las en blanco) es impresionante de 5 millones. Una pequeña parte del código (200 mil líneas en archivos 664-cs) se acumula en las pruebas, no las he considerado.

Si la calidad del código .NET de la versión del SDK es aproximadamente la misma que la de C ++ (dos errores por 512 KLOC), entonces deberíamos obtener un número de errores aproximadamente 10 veces mayor. Por supuesto, esta es una metodología de cálculo muy inexacta, que no tiene en cuenta las peculiaridades lingüísticas y muchos otros factores, pero no creo que el lector ahora quiera entrar en un razonamiento aburrido. En cambio, sugiero pasar a los resultados.

La verificación se realizó con PVS-Studio 6.27. Es simplemente increíble, pero el hecho es que en el SDK de AWS para .NET el analizador logró detectar 40 errores, de los que valdría la pena hablar. Demuestra no solo una alta calidad del código SDK (aproximadamente 4 errores por 512 KLOC), sino también una calidad comparable del analizador C # PVS-Studio en comparación con C ++. ¡Un gran resultado!

Autores de AWS SDK para .NET, ¡son verdaderos campeones! Con cada proyecto, demuestra una calidad tremenda del código. Puede ser un gran ejemplo para otros equipos. Sin embargo, por supuesto, no sería un desarrollador de un analizador estático si no diera mis 2 centavos. :) Ya estamos trabajando con un equipo de Lumberyard de Amazon en el uso de PVS-Studio. Dado que es una empresa muy grande con un montón de unidades en todo el mundo, es muy probable que el equipo de AWS SDK para .NET nunca haya oído hablar de PVS-Studio. De todos modos, no he encontrado ningún signo de uso de nuestro analizador en el código SDK, aunque no dice nada. Sin embargo, al menos, el equipo usa el analizador integrado en Visual Studio. Es genial, pero las revisiones de código siempre se pueden mejorar :).

Como resultado, logré encontrar algunos errores en el código SDK y, finalmente, es hora de compartirlos.

Error en la logica

Advertencia de PVS-Studio: V3008 [CWE-563] La variable 'this.linker.s3.region' tiene valores asignados dos veces sucesivamente. Quizás esto sea un error. Líneas de verificación: 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; } }

El analizador advierte sobre la asignación de valores repetidos a la misma variable. Del código queda claro que esto se debe al error que viola la lógica del trabajo del programa: el valor de la variable this.linker.s3.region siempre será igual al valor del valor de la variable, independientemente de la condición if (String.IsNullOrEmpty (valor)) . se perdió la declaración de retorno en el cuerpo del bloque if . El código debe repararse de la siguiente manera:

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

Recursión infinita

Advertencia de PVS-Studio: V3110 [CWE-674] Posible recursión infinita dentro de la propiedad 'OnFailure'. AWSSDK.ElasticMapReduce.Net45 ResizeJobFlowStep.cs 171

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

Un ejemplo clásico de un error tipográfico, que conduce a una recursión infinita en el get getor de la propiedad OnFailure . En lugar de devolver el valor de un campo privado en Failure, se realiza el acceso a la propiedad OnFailure . Variante correcta:

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

Puede preguntar: "¿Cómo funcionó?" Hasta ahora, no cómo. La propiedad no se usa en ningún otro lugar, pero esto es temporal. En un momento, alguien comenzará a usarlo y ciertamente recibirá un resultado inesperado. Para evitar estos errores tipográficos, se recomienda no utilizar identificadores que difieran solo en el caso de la primera letra.

Otro comentario a esta construcción es el uso del identificador, que coincide completamente con el nombre del tipo OnFailure . Desde el punto de vista del compilador, es bastante aceptable, pero esto complica la percepción del código para una persona.

Otro error similar:

Advertencia de PVS-Studio: V3110 [CWE-674] Posible recursión infinita dentro de la propiedad 'SSES3'. AWSSDK.S3.Net45 InventoryEncryption.cs 37

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

La situación es idéntica a la descrita anteriormente. Sin embargo, aquí ocurrirá una recursión infinita al acceder a la propiedad SSES3 tanto para leer como para asignar. Variante correcta:

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

Prueba en consideración

Ahora me gustaría citar una tarea de un desarrollador, tomada con el método Copy-Paste. Eche un vistazo a la apariencia del código en el editor de Visual Studio e intente encontrar un error.

Cuadro 3


Advertencia de PVS-Studio: V3029 Las expresiones condicionales de las declaraciones 'if' ubicadas una junto a la otra son idénticas. Líneas de verificación: 91, 95. AWSSDK.AppSync.Net45 CreateApiKeyResponseUnmarshaller.cs 91

Reduje el cuerpo del método UnmarshallException , habiendo eliminado todo lo que no es necesario. Ahora puede ver que verificaciones idénticas se suceden:

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

Puede parecer que el error no es áspero, solo una comprobación adicional. Sin embargo, a menudo dicho patrón puede indicar problemas más serios en el código, cuando no se realizará una verificación necesaria.

En el código, hay varios errores similares.

Advertencias de PVS-Studio:

  • V3029 Las expresiones condicionales de las declaraciones 'if' ubicadas una junto a la otra son idénticas. Verificación de líneas: 75, 79. AWSSDK.CloudDirectory.Net45 CreateSchemaResponseUnmarshaller.cs 75
  • V3029 Las expresiones condicionales de las declaraciones 'if' ubicadas una junto a la otra son idénticas. Líneas de verificación: 105, 109. AWSSDK.CloudDirectory.Net45 GetSchemaAsJsonResponseUnmarshaller.cs 105
  • V3029 Las expresiones condicionales de las declaraciones 'if' ubicadas una junto a la otra son idénticas. Líneas de verificación: 201, 205. AWSSDK.CodeCommit.Net45 PostCommentForPullRequestResponseUnmarshaller.cs 201
  • V3029 Las expresiones condicionales de las declaraciones 'if' ubicadas una junto a la otra son idénticas. Verificación de líneas: 101, 105. AWSSDK.CognitoIdentityProvider.Net45 VerifySoftwareTokenResponseUnmarshaller.cs 101
  • V3029 Las expresiones condicionales de las declaraciones 'if' ubicadas una junto a la otra son idénticas. Líneas de verificación: 72, 76. AWSSDK.Glue.Net45 UpdateConnectionResponseUnmarshaller.cs 72
  • V3029 Las expresiones condicionales de las declaraciones 'if' ubicadas una junto a la otra son idénticas. Líneas de verificación: 123, 127. AWSSDK.Neptune.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 123
  • V3029 Las expresiones condicionales de las declaraciones 'if' ubicadas una junto a la otra son idénticas. Líneas de verificación: 167, 171. AWSSDK.Neptune.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 167
  • V3029 Las expresiones condicionales de las declaraciones 'if' ubicadas una junto a la otra son idénticas. Líneas de verificación: 127, 131. AWSSDK.RDS.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 127
  • V3029 Las expresiones condicionales de las declaraciones 'if' ubicadas una junto a la otra son idénticas. Líneas de verificación: 171, 175. AWSSDK.RDS.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 171
  • V3029 Las expresiones condicionales de las declaraciones 'if' ubicadas una junto a la otra son idénticas. Verifique las líneas: 99, 103. AWSSDK.Rekognition.Net45 RecognizeCelebritiesResponseUnmarshaller.cs 99

Que es

Advertencia de PVS-Studio: V3062 Se utiliza un objeto 'attributeName' como argumento para su propio método. Considere verificar el primer argumento real del método 'Contiene'. AWSSDK.MobileAnalytics.Net45 CustomEvent.cs 261

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

El analizador ha detectado un error en el método GetAttribute : se verifica si una cadena se contiene a sí misma. De la descripción del método se deduce que si se encuentra el nombre del atributo (clave del nombre del atributo ) (en el diccionario _atributos ), el valor del atributo debe ser devuelto, de lo contrario - nulo . De hecho, como la condición attributeName.Contains (attributeName) siempre es verdadera, se intenta devolver el valor mediante una clave que podría no encontrarse en un diccionario. Luego, en lugar de devolver nulo, se generará una excepción KeyNotFoundException .

Intentemos arreglar este código. Para entender mejor cómo hacer esto, debe buscar otro método:

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

Este método verifica si el nombre del atributo (key attribute key) existe en el diccionario _attributes . Volvamos al método GetAttribute nuevamente y arreglemos el error:

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

Ahora el método hace exactamente lo que se indica en la descripción.

Un pequeño comentario más a este fragmento de código. Noté que los autores usan el bloqueo cuando trabajan con el diccionario _attributes . Está claro que esto es necesario cuando se tiene un acceso multiproceso, pero la construcción de la cerradura es bastante lenta y engorrosa. En lugar de un diccionario , en este caso, tal vez, sería más conveniente utilizar la versión segura del subproceso del diccionario: ConcurrentDictionary . De esta manera, no habrá necesidad de bloqueo. Aunque, tal vez no sé sobre los detalles del proyecto.

Comportamiento sospechoso

Advertencia de PVS-Studio: V3063 [CWE-571] Una parte de la expresión condicional siempre es verdadera si se evalúa: string.IsNullOrEmpty (inferredIndexName). AWSSDK.DynamoDBv2.PCL ContextInternal.cs 802

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

El analizador estaba preocupado por la cadena de verificación. IsNullOrEmpty (inferredIndexName) . De hecho, la cadena inferredIndexName se asigna como nula , luego el valor de esta variable no se cambia en ningún lado, luego, por alguna razón, se verifica para una cadena nula o vacía. Parece sospechoso Echemos un vistazo de cerca al fragmento de código anterior. Deliberadamente no lo reduje para entender mejor la situación. Entonces, en la primera instrucción if (y también en la siguiente), la variable especificada IndexIndexName se verifica de alguna manera. Dependiendo de los resultados de las comprobaciones, la variable inferredIndexName está obteniendo un nuevo valor. Ahora veamos la tercera declaración if . El cuerpo de esta declaración (lanzamiento de la excepción) se realizará en caso de que indexNames.Count> 0, como la primera parte de la condición completa, que es string.IsNullOrEmpty (inferredIndexName) siempre es verdadero. Quizás, las variables especificadas como IndexIndex y inferredIndexName se mezclan o la tercera verificación tiene que ser sin otra cosa , lo que representa una declaración if independiente:

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

En este caso, es difícil dar una respuesta definitiva sobre las opciones para corregir este código. De todos modos, el autor necesita verificarlo.

NullReferenceException

Advertencia de PVS-Studio: V3095 [CWE-476] El objeto 'conditionValues' se usó antes de que se verificara como nulo. Líneas de verificación: 228, 238. AWSSDK.Core.Net45 JsonPolicyWriter.cs 228

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

Es un clasico. La variable conditionValues se usa sin una comprobación preliminar de nulo . Mientras más tarde en el código se realiza esta verificación. El código debe corregirse de la siguiente manera:

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

Encontré varios errores similares en el código.

Advertencias de PVS-Studio:

  • V3095 [CWE-476] El objeto 'ts.Listeners' se usó antes de que se verificara como nulo. Líneas de verificación: 140, 143. AWSSDK.Core.Net45 Logger.Diagnostic.cs 140
  • V3095 [CWE-476] El objeto 'obj' se usó antes de que se verificara como nulo. Líneas de verificación: 743, 745. AWSSDK.Core.Net45 JsonMapper.cs 743
  • V3095 [CWE-476] El objeto 'multipartUploadMultipartUploadpartsList' se usó antes de que se verificara contra nulo. Verifique las líneas: 65, 67. AWSSDK.S3.Net45 CompleteMultipartUploadRequestMarshaller.cs 65

La siguiente advertencia es muy similar en significado, pero el caso es opuesto al discutido anteriormente.

Advertencia de PVS-Studio: V3125 [CWE-476] El objeto 'estado' se usó después de que se verificó contra nulo. Líneas de verificación: 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; // <= .... }

Uno de los fragmentos de código incluye verificar el valor de la variable de estado para nulo . En el siguiente código, la variable se utiliza para cancelar la suscripción del evento PreemptExpiryTime , sin embargo, ya no se realiza una comprobación de nulo y es posible lanzar la excepción NullReferenceException . Una versión más segura del código:

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

En el código, hay otros errores similares:

Advertencias de PVS-Studio:

  • V3125 [CWE-476] El objeto 'wrapRequest.Content' se usó después de que se verificó contra nulo. Líneas de verificación: 395, 383. AWSSDK.Core.Net45 HttpHandler.cs 395
  • V3125 [CWE-476] El objeto 'datasetUpdates' se usó después de que se verificó contra nulo. Líneas de verificación: 477, 437. AWSSDK.CognitoSync.Net45 Dataset.cs 477
  • V3125 [CWE-476] El objeto 'cORSConfigurationCORSConfigurationcORSRulesListValue' se usó después de que se verificó contra nulo. Líneas de verificación: 125, 111. AWSSDK.S3.Net45 PutCORSConfigurationRequestMarshaller.cs 125
  • V3125 [CWE-476] El objeto 'lifecycleConfigurationLifecycleConfigurationrulesListValue' se usó después de que se verificó contra nulo. Líneas de verificación: 157, 68. AWSSDK.S3.Net45 PutLifecycleConfigurationRequestMarshaller.cs 157
  • V3125 [CWE-476] El objeto 'this.Key' se usó después de que se verificó contra nulo. Líneas de verificación: 199, 183. AWSSDK.S3.Net45 S3PostUploadRequest.cs 199

Realidad no alternativa

Advertencia de PVS-Studio: V3009 [CWE-393] Es extraño que este método siempre devuelva el mismo valor de 'verdadero'. AWSSDK.Core.Net45 Lexer.cs 651

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

El método siempre devuelve verdadero . Veamos cuán crítico es para el código de llamada. Revisé los casos de usar el método State19 . Está involucrado en llenar la matriz de manejadores fsm_handler_table por igual con otros métodos similares (hay 28 de ellos con los nombres, respectivamente, comenzando por State1 a State28 ). Aquí es importante tener en cuenta que, además de State19 , para algunos otros manejadores también se emitieron las advertencias V3009 [CWE-393]. Estos son controladores: Estado23, Estado26, Estado27, Estado28 . Las advertencias, emitidas por el analizador para ellos:

  • V3009 [CWE-393] Es extraño que este método siempre devuelva el mismo valor de 'verdadero'. AWSSDK.Core.Net45 Lexer.cs 752
  • V3009 [CWE-393] Es extraño que este método siempre devuelva el mismo valor de 'verdadero'. AWSSDK.Core.Net45 Lexer.cs 810
  • V3009 [CWE-393] Es extraño que este método siempre devuelva el mismo valor de 'verdadero'. AWSSDK.Core.Net45 Lexer.cs 822
  • V3009 [CWE-393] Es extraño que este método siempre devuelva el mismo valor de 'verdadero'. AWSSDK.Core.Net45 Lexer.cs 834

Así es como se ve la declaración y la inicialización de los controladores:

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

Para completar la imagen, veamos el código de uno de los controladores a los que el analizador no ha tenido ningún reclamo, por ejemplo, State2 :

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

Así es como ocurre la llamada de los manejadores:

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

Como podemos ver, se lanzará una excepción en caso de devolver falso . En nuestro caso, para los manejadores State19, State23, State26 State27 y State28 esto nunca sucederá. Parece sospechoso Por otro lado, cinco controladores tienen un comportamiento similar (siempre devolverá verdadero ), por lo que tal vez fue tan artificial y no es el resultado de un error tipográfico.

¿Por qué estoy yendo tan profundo en todo esto? Esta situación es muy significativa en el sentido de que el analizador estático a menudo solo puede indicar una construcción sospechosa. E incluso una persona (no una máquina), que no tiene suficiente conocimiento sobre el proyecto, aún no puede dar una respuesta completa sobre la presencia del error, incluso después de haber pasado tiempo aprendiendo código. Un desarrollador debe revisar este código.

Verificaciones sin sentido

Advertencia de PVS-Studio: V3022 [CWE-571] La expresión 'doLog' siempre es verdadera. 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) // <= { .... } .... }

Presta atención a la variable doLog . Después de la inicialización con el valor falso , esta variable obtendrá el valor verdadero en todos los casos a lo largo del código. Por lo tanto, la comprobación if (doLog) siempre es verdadera. Quizás, anteriormente en el método había una rama, en la cual a la variable doLog no se le asignó ningún valor. En el momento de la comprobación, podría contener el valor falso , recibido al inicializar. Pero ahora no existe tal rama.

Otro error similar:

Advertencia de PVS-Studio: V3022 La expresión '! Resultado' siempre es falsa. 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(....); } .... }

El analizador afirma que el valor de la variable de resultado siempre es verdadero. Solo es posible en caso de que el método PutValueHelper siempre devuelva verdadero . Echa un vistazo a este método:

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

De hecho, el método volverá verdadero en todas las condiciones. Además, el analizador ha emitido una advertencia para este método. Advertencia de PVS-Studio: V3009 [CWE-393] Es extraño que este método siempre devuelva el mismo valor de 'verdadero'. SQLiteLocalStorage.cs 1016

Deliberadamente, no cité esta advertencia antes cuando estaba investigando otros errores V3009 y la guardé para este caso. Por lo tanto, la herramienta tenía razón al señalar el error V3022 en el código de llamada.

Copiar y pegar. De nuevo

Advertencia de PVS-Studio: V3001 Hay subexpresiones idénticas 'this.token == JsonToken.String' a la izquierda y a la derecha de '||' operador AWSSDK.Core.Net45 JsonReader.cs 343

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

El campo this.token se compara dos veces con el valor JsonToken.String de la enumeración JsonToken . Probablemente, una de las comparaciones debe contener otro valor de enumeración. Si es así, se ha cometido un grave error aquí.

¿Refactorización + falta de atención?

Advertencia de PVS-Studio: V3025 [CWE-685] Formato incorrecto. Se espera un número diferente de elementos de formato al llamar a la función 'Formatear'. Argumentos no utilizados: 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)); } .... }

Quizás, la cadena de formato para el método string.Format contenía previamente el elemento de formato {0}, para el cual se estableció el argumento AWSConfigs.AWSRegionKey . Luego se cambió la cadena, el elemento de formato desapareció, pero un desarrollador olvidó eliminar el argumento. El ejemplo de código dado funciona sin errores (la excepción se produjo en el caso contrario, el elemento de formato sin el argumento), pero no se ve bien. El código debe corregirse de la siguiente manera:

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

Inseguro

Advertencia de PVS-Studio: V3083 [CWE-367] Invocación insegura del evento 'mOnSyncSuccess', es posible NullReferenceException. Considere asignar un evento a una variable local antes de invocarlo. AWSSDK.CognitoSync.PCL Dataset.cs 827

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

Una situación común de una llamada insegura del controlador de eventos. Un usuario puede darse de baja entre la comprobación de la variable mOnSyncSuccess para nulo y la llamada de un controlador, por lo que su valor se convertirá en nulo . La probabilidad de tal escenario es pequeña, pero aún es mejor hacer que el código sea más seguro:

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

En el código, hay otros errores similares:

Advertencias de PVS-Studio:

  • V3083 [CWE-367] Invocación insegura del evento 'mOnSyncFailure', NullReferenceException es posible. Considere asignar un evento a una variable local antes de invocarlo. AWSSDK.CognitoSync.PCL Dataset.cs 839
  • V3083 [CWE-367] Invocación de evento insegura, NullReferenceException es posible. Considere asignar un evento a una variable local antes de invocarlo. AWSSDK.Core.PCL AmazonServiceClient.cs 332
  • V3083 [CWE-367] Invocación de evento insegura, NullReferenceException es posible. Considere asignar un evento a una variable local antes de invocarlo. AWSSDK.Core.PCL AmazonServiceClient.cs 344
  • V3083 [CWE-367] Invocación de evento insegura, NullReferenceException es posible. Considere asignar un evento a una variable local antes de invocarlo. AWSSDK.Core.PCL AmazonServiceClient.cs 357
  • V3083 [CWE-367] Invocación insegura del evento 'mExceptionEvent', NullReferenceException es posible. Considere asignar un evento a una variable local antes de invocarlo. AWSSDK.Core.PCL AmazonServiceClient.cs 366
  • V3083 [CWE-367] Invocación de evento insegura, NullReferenceException es posible. Considere asignar un evento a una variable local antes de invocarlo. AWSSDK.Core.PCL AmazonWebServiceRequest.cs 78
  • V3083 [CWE-367] La ​​invocación insegura del evento 'OnRead', NullReferenceException es posible. Considere asignar un evento a una variable local antes de invocarlo. AWSSDK.Core.PCL EventStream.cs 97
  • V3083 [CWE-367] Invocación de evento insegura, NullReferenceException es posible. Considere asignar un evento a una variable local antes de invocarlo. AWSSDK.Core.Android NetworkReachability.cs 57
  • V3083 [CWE-367] Invocación de evento insegura, NullReferenceException es posible. Considere asignar un evento a una variable local antes de invocarlo. AWSSDK.Core.Android NetworkReachability.cs 94
  • V3083 [CWE-367] Invocación de evento insegura, NullReferenceException es posible. Considere asignar un evento a una variable local antes de invocarlo. AWSSDK.Core.iOS NetworkReachability.cs 54

Clase cruda

Advertencia de PVS-Studio: V3126 Tipo 'JsonData' que implementa la interfaz IEquatable <T> no anula el método 'GetHashCode'. AWSSDK.Core.Net45 JsonData.cs 26

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

La clase JsonData contiene bastante código, así que no lo di en su totalidad, citando solo su declaración. Esta clase realmente no contiene el método anulado GetHashCode, que no es seguro, ya que puede conducir a un comportamiento erróneo al usar el tipo JsonData para trabajar, por ejemplo, con colecciones. Probablemente, no hay ningún problema en este momento, pero en el futuro este tipo de estrategia podría cambiar. Este error se describe en la documentación con más detalle.

Conclusión

Todos estos son errores interesantes que pude detectar en el código de AWS SDK para .NET usando el analizador estático PVS-Studio. Me gustaría destacar una vez más la calidad del proyecto. Encontré un número muy pequeño de errores para 5 millones de líneas de código. Aunque probablemente un análisis más exhaustivo de las advertencias emitidas me permitiría agregar algunos errores más a esta lista. Sin embargo, también es bastante probable que agregue algunas de las advertencias a los errores por nada. Las conclusiones inequívocas en este caso siempre son hechas solo por un desarrollador que esté en el contexto del código verificado.

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

More articles:


All Articles