Geekly articles weekly

Procurando bugs no código-fonte do Amazon Web Services SDK para .NET

Quadro 1


Saudações a todos os fãs por criticarem o código de outra pessoa. :) Hoje, em nosso laboratório, novos materiais de pesquisa são o código-fonte do projeto AWS SDK para .NET. Ao mesmo tempo, escrevemos um artigo sobre como verificar o AWS SDK para C ++. Então não havia nada particularmente interessante. Vamos ver como a versão .NET do AWS SDK nos agradará. Uma boa oportunidade para demonstrar mais uma vez os recursos do analisador PVS-Studio, além de tornar o mundo um pouco mais perfeito.

O .NET SDK da Amazon Web Services (AWS) é um kit de ferramentas para desenvolvedores projetado para criar aplicativos baseados em .NET na estrutura da AWS e simplificar bastante o processo de gravação de código. O SDK inclui pacotes de API do .NET para vários serviços da AWS, como Amazon S3, Amazon EC2, DynamoDB e outros. O código fonte do SDK está hospedado no GitHub .

Como eu disse, escrevemos um artigo sobre como verificar o AWS SDK para C ++. O artigo acabou sendo pequeno - houve apenas alguns erros em 512 mil linhas de código. Desta vez, estamos lidando com uma quantidade muito maior de código, que inclui cerca de 34 mil arquivos cs, e o número total de linhas de código (excluindo vazio) é de impressionantes 5 milhões. Uma pequena parte do código (200 mil linhas em arquivos 664 cs) cai nos testes, eu não os considerei.

Se a qualidade do código .NET da versão do SDK for aproximadamente a mesma que a do C ++ (dois erros por 512 KLOC), devemos obter cerca de 10 vezes mais erros. É claro que esse é um método de cálculo muito impreciso que não leva em consideração os recursos da linguagem e muitos outros fatores, mas não creio que o leitor agora queira se aprofundar em discussões chatas. Em vez disso, proponho ir direto aos resultados.

A verificação foi realizada usando o PVS-Studio versão 6.27. Incrivelmente, o analisador conseguiu detectar cerca de 40 erros no código do AWS SDK para .NET que mereciam ser mencionados. Isso fala não apenas da alta qualidade do código SDK (cerca de 4 erros por 512 KLOC), mas também da qualidade comparável do trabalho em C # do analisador PVS-Studio em comparação com o C ++. Ótimo resultado!

Os autores do AWS SDK for .NET são ótimos. De projeto para projeto, eles demonstram uma incrível qualidade de código. Isso pode servir como um bom exemplo para outras equipes. Mas, é claro, eu não seria o desenvolvedor de um analisador estático se não tivesse inserido meus 5 copeques. :) Já estamos trabalhando com a equipe do Amazon Lumberyard no uso do PVS-Studio. Mas como essa é uma empresa muito grande com várias divisões em todo o mundo, é altamente provável que a equipe do AWS SDK for .NET nunca tenha ouvido falar do PVS-Studio. De qualquer forma, no código SDK não encontrei sinais de uso do analisador, embora isso não signifique nada. No entanto, a equipe, no mínimo, usa o analisador incorporado no Visual Studio. Isso é bom, mas as verificações de código podem ser aprimoradas :).

Como resultado, ainda consegui encontrar alguns erros no código SDK e, finalmente, é hora de compartilhá-los.

Erro na lógica

PVS-Studio Warning: V3008 [CWE-563] A variável 'this.linker.s3.region' recebe valores duas vezes sucessivos. Talvez isso seja um erro. Verifique as linhas: 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; } }

O analisador avisa sobre a reatribuição do valor da mesma variável. A partir do código, fica claro que isso se deve a um erro que viola a lógica da operação: o valor da variável this.linker.s3.region sempre será igual ao valor , independentemente da condição if (String.IsNullOrEmpty (value)) . No corpo do bloco if , o retorno foi ignorado. O código precisa ser corrigido da seguinte maneira:

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

Recursão infinita

Aviso do PVS-Studio: V3110 [CWE-674] Possível recursão infinita dentro da propriedade 'OnFailure'. AWSSDK.ElasticMapReduce.Net45 ResizeJobFlowStep.cs 171

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

Um exemplo clássico de erro de digitação que causa recursão infinita no acessador get da propriedade OnFailure . Em vez de retornar o valor do campo privado, onFailure refere-se à propriedade OnFailure . Opção corrigida:

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

Você pergunta: "Como funcionou?" Até agora, de jeito nenhum. A propriedade não é usada em nenhum lugar, mas é temporária. A certa altura, alguém começará a usá-lo e obterá definitivamente um resultado inesperado. Para evitar erros de digitação, é recomendável não usar identificadores que diferem apenas no caso da primeira letra.

Outra observação nesse design é o uso de um identificador que corresponde completamente ao nome do tipo OnFailure . Do ponto de vista do compilador, isso é bastante aceitável, mas dificulta a percepção humana do código.

Outro erro semelhante:

Aviso do PVS-Studio: V3110 [CWE-674] Possível recursão infinita dentro da propriedade 'SSES3'. AWSSDK.S3.Net45 InventoryEncryption.cs 37

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

A situação é idêntica à descrita acima. Somente aqui, uma recursão infinita ocorrerá ao acessar a propriedade SSES3 para leitura e gravação. Opção corrigida:

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

Teste de atenção plena

A seguir, uma tarefa de um programador interessado em usar a técnica Copiar e Colar. Veja como o código fica no editor do Visual Studio e tente encontrar o erro.

Quadro 3


PVS-Studio Warning: V3029 As expressões condicionais das instruções 'if' localizadas uma ao lado da outra são idênticas. Verifique as linhas: 91, 95. AWSSDK.AppSync.Net45 CreateApiKeyResponseUnmarshaller.cs 91

Reduzi o corpo do método UnmarshallException , removendo todos os desnecessários. Agora você pode ver que verificações idênticas seguem uma após a outra:

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

Pode parecer que o erro não é grave - apenas uma verificação extra. Porém, frequentemente, esse padrão pode indicar problemas mais sérios no código quando alguma verificação necessária não é executada.

Existem alguns erros mais semelhantes no código.

Avisos do PVS-Studio:

  • V3029 As expressões condicionais das instruções 'if' localizadas uma ao lado da outra são idênticas. Verifique as linhas: 75, 79. AWSSDK.CloudDirectory.Net45 CreateSchemaResponseUnmarshaller.cs 75
  • V3029 As expressões condicionais das instruções 'if' localizadas uma ao lado da outra são idênticas. Verifique as linhas: 105, 109. AWSSDK.CloudDirectory.Net45 GetSchemaAsJsonResponseUnmarshaller.cs 105
  • V3029 As expressões condicionais das instruções 'if' localizadas uma ao lado da outra são idênticas. Verifique as linhas: 201, 205. AWSSDK.CodeCommit.Net45 PostCommentForPullRequestResponseUnmarshaller.cs 201
  • V3029 As expressões condicionais das instruções 'if' localizadas uma ao lado da outra são idênticas. Verifique as linhas: 101, 105. AWSSDK.CognitoIdentityProvider.Net45 VerifySoftwareTokenResponseUnmarshaller.cs 101
  • V3029 As expressões condicionais das instruções 'if' localizadas uma ao lado da outra são idênticas. Verifique as linhas: 72, 76. AWSSDK.Glue.Net45 UpdateConnectionResponseUnmarshaller.cs 72
  • V3029 As expressões condicionais das instruções 'if' localizadas uma ao lado da outra são idênticas. Verifique as linhas: 123, 127. AWSSDK.Neptune.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 123
  • V3029 As expressões condicionais das instruções 'if' localizadas uma ao lado da outra são idênticas. Verifique as linhas: 167, 171. AWSSDK.Neptune.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 167
  • V3029 As expressões condicionais das instruções 'if' localizadas uma ao lado da outra são idênticas. Verifique as linhas: 127, 131. AWSSDK.RDS.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 127
  • V3029 As expressões condicionais das instruções 'if' localizadas uma ao lado da outra são idênticas. Verifique as linhas: 171, 175. AWSSDK.RDS.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 171
  • V3029 As expressões condicionais das instruções 'if' localizadas uma ao lado da outra são idênticas. Verifique as linhas: 99, 103. AWSSDK.Rekognition.Net45 RecognizeCelebritiesResponseUnmarshaller.cs 99

O que você é

PVS-Studio Warning: V3062 Um objeto 'attributeName' é usado como argumento para seu próprio método. Considere verificar o primeiro argumento real do método 'Contém'. 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; }

O analisador detectou um erro no método GetAttribute : a sequência de caracteres é verificada quanto ao seu conteúdo. A partir da descrição do método, segue-se que, se o nome do atributo (chave attributeName ) for encontrado (no dicionário _attributes ), o valor do atributo deverá ser retornado, caso contrário nulo . Na realidade, como a condição attributeName.Contains (attributeName) é sempre verdadeira, é feita uma tentativa de retornar um valor por uma chave que pode não ser encontrada no dicionário. Em vez de retornar nulo, uma KeyNotFoundException será lançada.

Vamos tentar corrigir esse código. Para entender melhor como fazer isso, dê uma olhada em outro 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 se o nome do atributo (chave attributeName ) existe no dicionário _attributes . Vamos voltar ao método GetAttribute e corrigir o erro:

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

Agora, o método faz exatamente o que é indicado na descrição.

E mais uma pequena observação para esse fragmento de código. Notei que os autores usam o bloqueio ao trabalhar com o dicionário _attributes . Claramente, isso é necessário com acesso multiencadeado, mas a construção de bloqueio é bastante lenta e complicada. Em vez de Dicionário , nesse caso, pode ser mais conveniente usar uma versão segura do encadeamento do dicionário - ConcurrentDictionary . Então a necessidade de bloqueio desaparece. Embora, talvez eu não conheça nenhum recurso do projeto.

Comportamento suspeito

Aviso do PVS-Studio: V3063 [CWE-571] Uma parte da expressão condicional sempre é verdadeira se for avaliada: 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); .... }

O analisador alertou a verificação string.IsNullOrEmpty (inferredIndexName) . De fato, a cadeia inferredIndexName é atribuída nula , o valor dessa variável não é alterado em lugar algum e, por algum motivo, é verificado se há uma cadeia nula ou vazia. Parece suspeito. Vamos dar uma olhada no trecho de código fornecido. Eu deliberadamente não o reduzi para uma melhor compreensão da situação. Portanto, na primeira instrução if (e também na próxima), a variável especificadaIndexName é verificada de alguma forma. Dependendo do resultado das verificações, a variável inferredIndexName recebe um novo valor. Agora vamos prestar atenção à terceira declaração if . O corpo dessa instrução (lançando uma exceção) será executado se indexNames.Count> 0 , pois a primeira parte da condição é string.IsNullOrEmpty (inferredIndexName) sempre é verdadeira. Talvez as variáveis especificadasIndexName e inferredIndexName estejam simplesmente confusas. Ou a terceira verificação deve ser sem outra , representando uma instrução if independente:

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

Nesse caso, é difícil dar uma resposta inequívoca sobre as opções para corrigir esse código. Mas é definitivamente necessário que o autor a inspecione.

NullReferenceException

Aviso do PVS-Studio: V3095 [CWE-476] O objeto 'conditionValues' foi usado antes de ser verificado com valor nulo. Verifique as linhas: 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) { .... } .... } }

Clássicos do gênero. A variável conditionValues ​​é usada sem primeiro verificar nulo . Além disso, ainda mais no código, essa verificação é realizada, mas é tarde demais. :)

O código pode ser corrigido da seguinte maneira:

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

Mais alguns erros semelhantes foram encontrados no código.

Avisos do PVS-Studio:

  • V3095 [CWE-476] O objeto 'ts.Listeners' foi usado antes de ser verificado como nulo. Verifique as linhas: 140, 143. AWSSDK.Core.Net45 Logger.Diagnostic.cs 140
  • V3095 [CWE-476] O objeto 'obj' foi usado antes de ser verificado com relação a nulo. Verifique as linhas: 743, 745. AWSSDK.Core.Net45 JsonMapper.cs 743
  • V3095 [CWE-476] O objeto 'multipartUploadMultipartUploadpartsList' foi usado antes de ser verificado como nulo. Verifique as linhas: 65, 67. AWSSDK.S3.Net45 CompleteMultipartUploadRequestMarshaller.cs 65

O aviso a seguir tem significado muito semelhante, mas a situação é o inverso do discutido acima.

Aviso do PVS-Studio: V3125 [CWE-476] O objeto 'state' foi usado após a verificação contra nulo. Verifique as linhas: 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; // <= .... }

Um dos fragmentos de código contém uma verificação da variável de estado para nulo . Além do código, a variável state é usada para cancelar a inscrição no evento PreemptExpiryTime , enquanto a verificação de nulo não é mais realizada e uma NullReferenceException é possível . Opção de código mais seguro:

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

Existem outros erros semelhantes no código.

Avisos do PVS-Studio:

  • V3125 [CWE-476] O objeto 'wrapRequest.Content' foi usado após a verificação contra nulo. Verifique as linhas: 395, 383. AWSSDK.Core.Net45 HttpHandler.cs 395
  • V3125 [CWE-476] O objeto 'datasetUpdates' foi usado após a verificação contra nulo. Verifique as linhas: 477, 437. Conjunto de dados AWSSDK.CognitoSync.Net45.cs 477
  • V3125 [CWE-476] O objeto 'cORSConfigurationCORSConfigurationcORSRulesListValue' foi usado depois que foi verificado como nulo. Verifique as linhas: 125, 111. AWSSDK.S3.Net45 PutCORSConfigurationRequestMarshaller.cs 125
  • V3125 [CWE-476] O objeto 'lifecycleConfigurationLifecycleConfigurationrulesListValue' foi usado depois que foi verificado como nulo. Verifique as linhas: 157, 68. AWSSDK.S3.Net45 PutLifecycleConfigurationRequestMarshaller.cs 157
  • V3125 [CWE-476] O objeto 'this.Key' foi usado após a verificação contra nulo. Verifique as linhas: 199, 183. AWSSDK.S3.Net45 S3PostUploadRequest.cs 199

Realidade incontestada

Aviso do PVS-Studio: V3009 [CWE-393] É estranho que esse método sempre retorne um e o mesmo valor de 'true'. AWSSDK.Core.Net45 Lexer.cs 651

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

O método sempre retorna verdadeiro . Vamos ver o quão crítico isso é para o código de chamada. Acompanhei o uso do método State19 . Ele participa do preenchimento da matriz de manipuladores fsm_handler_table junto com outros métodos semelhantes (há 28 deles com nomes, respectivamente, de State1 para State28 ). É importante observar aqui que, além do State19 , também foram emitidos avisos V3009 [CWE-393] para alguns outros manipuladores. Estes são os manipuladores: State23, State26, State27, State28 . Alertas emitidos pelo analisador para eles:

  • V3009 [CWE-393] É estranho que esse método sempre retorne um e o mesmo valor de 'true'. AWSSDK.Core.Net45 Lexer.cs 752
  • V3009 [CWE-393] É estranho que esse método sempre retorne um e o mesmo valor de 'true'. AWSSDK.Core.Net45 Lexer.cs 810
  • V3009 [CWE-393] É estranho que esse método sempre retorne um e o mesmo valor de 'true'. AWSSDK.Core.Net45 Lexer.cs 822
  • V3009 [CWE-393] É estranho que esse método sempre retorne um e o mesmo valor de 'true'. AWSSDK.Core.Net45 Lexer.cs 834

Aqui está a aparência da declaração e inicialização da matriz manipuladora:

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

Para garantir a integridade, vamos ver o código de um dos manipuladores, dos quais o analisador não teve queixas, por exemplo, State2 :

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

E é assim que os manipuladores são chamados:

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

Como você pode ver, uma exceção será lançada se false for retornado. No nosso caso, para os manipuladores State19, State23, State26, State27 e State28 isso nunca acontecerá. Parece suspeito. Por outro lado, até cinco manipuladores têm comportamento semelhante (sempre retornam verdadeiros ); talvez isso tenha sido planejado e não seja o resultado de um erro de digitação.

Por que insisti em tudo isso com tanto detalhe? Essa situação é muito indicativa no sentido de que um analisador estático geralmente é capaz de indicar apenas um projeto suspeito. E mesmo uma pessoa (não uma máquina) que não possui conhecimento suficiente sobre o projeto, tendo passado algum tempo estudando o código, ainda não é capaz de dar uma resposta exaustiva sobre a presença de um erro. O código deve ser estudado pelo desenvolvedor.

Verificações sem sentido

Aviso do PVS-Studio: V3022 [CWE-571] A expressão 'doLog' sempre é verdadeira. 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) // <= { .... } .... }

Preste atenção à variável doLog . Após a inicialização com false , no código essa variável será verdadeira em todos os casos. Portanto, a verificação if (doLog) é sempre verdadeira. Talvez houvesse uma ramificação anteriormente neste método na qual a variável doLog não recebesse nenhum valor e, no momento da verificação, ela poderia conter o valor falso obtido durante a inicialização. Mas agora não existe tal ramo.

Outro erro semelhante:

Aviso do PVS-Studio: A expressão V3022 '! Result' é sempre 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(....); } .... }

O analisador afirma que o valor da variável de resultado é sempre verdadeiro . Isso só é possível se o método PutValueHelper sempre retornar true . Dê uma olhada neste método:

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

De fato, o método retornará verdadeiro em qualquer condição. Além disso, o analisador emitiu um aviso para esse método. Aviso do PVS-Studio: V3009 [CWE-393] É estranho que esse método sempre retorne um e o mesmo valor de 'true'. SQLiteLocalStorage.cs 1016

Eu deliberadamente não citei esse aviso anteriormente quando estava considerando outros erros do V3009 , mas o salvei neste caso. Assim, o analisador estava certo ao indicar o erro V3022 no código de chamada.

Copie e cole. Novamente

PVS-Studio Warning: V3001 Existem sub-expressões idênticas 'this.token == JsonToken.String' à esquerda e à direita da '||' 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 // <= )) { .... } .... }

Compare duas vezes o campo this.token com o valor JsonToken.String da enumeração JsonToken . Provavelmente, uma das comparações deve conter um valor de enumeração diferente. Nesse caso, um erro grave foi cometido.

Refatoração + descuido?

Aviso do PVS-Studio: V3025 [CWE-685] Formato incorreto. É esperado um número diferente de itens de formato ao chamar a função 'Format'. Argumentos não usados: 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)); } .... }

Provavelmente, a string de formato para o método string.Format anteriormente continha o especificador de saída {0} , para o qual o argumento AWSConfigs.AWSRegionKey foi definido. Então a linha foi alterada, o especificador desapareceu, mas eles esqueceram de excluir o argumento. O fragmento de código acima funciona sem erros (uma exceção seria lançada no caso oposto - um especificador sem argumento), mas parece feio. O código deve ser corrigido da seguinte maneira:

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

Inseguro

Aviso do PVS-Studio: V3083 [CWE-367] Chamada não segura do evento 'mOnSyncSuccess', NullReferenceException é possível. Considere atribuir um evento a uma variável local antes de invocá-lo. AWSSDK.CognitoSync.PCL Dataset.cs 827

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

Uma situação bastante comum de uma chamada não segura para o manipulador de eventos. Entre a verificação da variável mOnSyncSuccess para null e a chamada do manipulador, um evento pode ser cancelado e seu valor se tornará zero. A probabilidade de tal cenário é pequena, mas ainda é melhor tornar o código mais seguro:

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

Existem outros erros semelhantes no código.

Avisos do PVS-Studio:

  • V3083 [CWE-367] Chamada não segura do evento 'mOnSyncFailure', NullReferenceException é possível. Considere atribuir um evento a uma variável local antes de invocá-lo. AWSSDK.CognitoSync.PCL Dataset.cs 839
  • V3083 [CWE-367] Chamada insegura do evento, NullReferenceException é possível. Considere atribuir um evento a uma variável local antes de invocá-lo. AWSSDK.Core.PCL AmazonServiceClient.cs 332
  • V3083 [CWE-367] Chamada insegura do evento, NullReferenceException é possível. Considere atribuir um evento a uma variável local antes de invocá-lo. AWSSDK.Core.PCL AmazonServiceClient.cs 344
  • V3083 [CWE-367] Chamada insegura do evento, NullReferenceException é possível. Considere atribuir um evento a uma variável local antes de invocá-lo. AWSSDK.Core.PCL AmazonServiceClient.cs 357
  • V3083 [CWE-367] Chamada não segura do evento 'mExceptionEvent', NullReferenceException é possível. Considere atribuir um evento a uma variável local antes de invocá-lo. AWSSDK.Core.PCL AmazonServiceClient.cs 366
  • V3083 [CWE-367] Chamada insegura do evento, NullReferenceException é possível. Considere atribuir um evento a uma variável local antes de invocá-lo. AWSSDK.Core.PCL AmazonWebServiceRequest.cs 78
  • V3083 [CWE-367] Chamada não segura do evento 'OnRead', NullReferenceException é possível. Considere atribuir um evento a uma variável local antes de invocá-lo. AWSSDK.Core.PCL EventStream.cs 97
  • V3083 [CWE-367] Chamada insegura do evento, NullReferenceException é possível. Considere atribuir um evento a uma variável local antes de invocá-lo. AWSSDK.Core.Android NetworkReachability.cs 57
  • V3083 [CWE-367] Chamada insegura do evento, NullReferenceException é possível. Considere atribuir um evento a uma variável local antes de invocá-lo. AWSSDK.Core.Android NetworkReachability.cs 94
  • V3083 [CWE-367] Chamada insegura do evento, NullReferenceException é possível. Considere atribuir um evento a uma variável local antes de invocá-lo. AWSSDK.Core.iOS NetworkReachability.cs 54

Classe inacabada

Aviso do PVS-Studio: V3126 O tipo 'JsonData' implementando a interface IEquatable <T> não substitui o método 'GetHashCode'. AWSSDK.Core.Net45 JsonData.cs 26

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

A classe JsonData contém muito código, então eu não o forneci por completo, limitando-me apenas à declaração. Essa classe realmente não contém um método GetHashCode substituído, o que não é seguro, pois pode levar a um comportamento incorreto ao usar o tipo JsonData para trabalhar, por exemplo, com coleções. Provavelmente não há nenhum problema no momento, mas a estratégia para usar esse tipo pode mudar no futuro. Este erro é descrito em mais detalhes na documentação .

Conclusão

Foram todos os erros interessantes que consegui detectar no código do AWS SDK for .NET usando o analisador estático PVS-Studio. Mais uma vez, enfatizo a qualidade do projeto. Encontrei um número muito pequeno de erros para 5 milhões de linhas de código. Embora, provavelmente, uma análise mais completa dos avisos emitidos me permita adicionar mais alguns erros a esta lista. Mas também é muito provável que, em vão, eu tenha atribuído alguns dos avisos descritos a erros. Nesse caso, conclusões inequívocas são sempre feitas apenas pelo desenvolvedor, que está no contexto do código que está sendo verificado.



Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: Sergey Khrenov. Procurando por erros no código-fonte do Amazon Web Services SDK para .NET

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

More articles:


All Articles