Geekly articles weekly

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

Quadro 1


Bem-vindo a todos os fãs de descartar o código de outra pessoa. :) Hoje em nosso laboratório, temos um novo material para uma pesquisa - o código-fonte do projeto AWS SDK for .NET. Na época, escrevemos um artigo sobre como verificar o AWS SDK para C ++. Então não havia nada de particularmente interessante. Vamos ver o valor do .NET da versão do AWS SDK. Mais uma vez, é uma ótima oportunidade para demonstrar as habilidades do analisador PVS-Studio e tornar o mundo um pouco melhor.

O SDK do Amazon Web Services (AWS) para .NET é um conjunto de ferramentas do desenvolvedor, destinadas à criação de aplicativos baseados no .NET na infraestrutura da AWS. Este conjunto permite simplificar significativamente o processo de escrita de código. O SDK inclui conjuntos API .NET para vários serviços da AWS, como Amazon S3, Amazon EC2, DynamoDB e outros. O código fonte do SDK está disponível no GitHub .

Como mencionei, no momento já escrevemos o artigo sobre como verificar o AWS SDK para C ++. O artigo acabou sendo pequeno - apenas alguns erros encontrados por 512 mil linhas de código. Desta vez, estamos lidando com um tamanho muito maior do código, que inclui cerca de 34 mil arquivos cs, e o número total de linhas de código (excluindo as em branco) é impressionante 5 milhões. Uma pequena parte do código (200 mil linhas em arquivos 664-cs) é acumulada nos testes, 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 um número 10 vezes maior de erros. Obviamente, essa é uma metodologia de cálculo muito imprecisa, que não leva em conta as peculiaridades linguísticas e muitos outros fatores, mas não acho que o leitor queira agora entrar em um raciocínio entediante. Em vez disso, sugiro passar para os resultados.

A verificação foi realizada usando o PVS-Studio 6.27. É simplesmente incrível, mas ainda assim o fato é que no AWS SDK for .NET o analisador conseguiu detectar 40 erros, dos quais vale a pena falar. Ele demonstra não apenas uma alta qualidade do código SDK (cerca de 4 erros por 512 KLOC), mas também uma qualidade comparável do analisador C # PVS-Studio em comparação com o C ++. Um ótimo resultado!

Autores do AWS SDK for .NET, você é um verdadeiro campeão! Com cada projeto, você demonstra uma tremenda qualidade do código. Pode ser um ótimo exemplo para outras equipes. No entanto, é claro, eu não seria desenvolvedor de um analisador estático se não desse meus 2 centavos. :) Já estamos trabalhando com uma equipe Lumberyard da Amazon no uso do PVS-Studio. Como é uma empresa muito grande com várias unidades em todo o mundo, é muito provável que a equipe do AWS SDK para .NET nunca tenha ouvido falar do PVS-Studio. De qualquer forma, não encontrei nenhum sinal de uso do analisador no código SDK, embora ele não diga nada. No entanto, pelo menos, a equipe usa o analisador incorporado no Visual Studio. É ótimo, mas as revisões de código sempre podem ser aprimoradas :).

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

Erro na lógica

Aviso do PVS-Studio: 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 atribuição repetida de valor para a mesma variável. A partir do código, fica claro que isso se deve ao erro que viola a lógica do trabalho do programa: o valor da variável this.linker.s3.region sempre será igual ao valor do valor da variável, independentemente da condição if (String.IsNullOrEmpty (value)) . A declaração de retorno foi perdida no corpo do bloco if . 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 leva a uma recursão infinita no acessador get da propriedade OnFailure . Em vez de retornar o valor de um campo privado onFailure, o acesso à propriedade OnFailure ocorre. Variante correta:

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

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

Outro comentário a essa construção é o uso do identificador, que corresponde completamente ao nome do tipo OnFailure . Do ponto de vista do compilador, é bastante aceitável, mas isso complica a percepção do código para uma pessoa.

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. No entanto, aqui ocorrerá uma recursão infinita ao acessar a propriedade SSES3, tanto para leitura quanto para atribuição. Variante correta:

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

Teste em consideração

Agora eu gostaria de citar uma tarefa de um desenvolvedor, usada com o método Copy-Paste. Dê uma olhada na aparência do código no editor do Visual Studio e tente encontrar um erro.

Quadro 3


Aviso 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: 91, 95. AWSSDK.AppSync.Net45 CreateApiKeyResponseUnmarshaller.cs 91

Reduzi o corpo do método UnmarshallException , removendo tudo o que não era necessário. Agora você pode ver que verificações idênticas se seguem:

 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. No entanto, muitas vezes esse padrão pode indicar problemas mais sérios no código, quando uma verificação necessária não será executada.

No código, existem vários erros semelhantes.

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 é isso?

Aviso do PVS-Studio: 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 : uma sequência de caracteres é verificada se ela se contém. 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 - null . De fato, como a condição attributeName.Contains (attributeName) é sempre verdadeira, é feita uma tentativa de retornar o valor por uma chave que pode não ser encontrada em um dicionário. Em vez de retornar nulo, uma exceção KeyNotFoundException será lançada.

Vamos tentar corrigir esse código. Para entender melhor como fazer isso, você deve procurar 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 novamente 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.

Mais um pequeno comentário para esse fragmento de código. Notei que os autores usam lock ao trabalhar com o dicionário _attributes . É claro que isso é necessário quando se tem um acesso multithread, mas a construção da trava é bastante lenta e complicada. Em vez de um dicionário , nesse caso, talvez, seria mais conveniente usar a versão segura do thread - ConcurrentDictionary . Dessa forma, não haverá necessidade de bloqueio. Embora, talvez eu não conheça as especificidades 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 estava preocupado com a sequência de verificação.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 fragmento de código acima. Eu deliberadamente não o reduzi para entender melhor a situação. Portanto, na primeira instrução if (e também na próxima), a variável specificIndexName é de alguma forma verificada. Dependendo dos resultados das verificações, a variável inferredIndexName está recebendo um novo valor. Agora, vamos olhar para a terceira declaração if . O corpo dessa instrução (lançamento da exceção) será executado caso indexNames.Count> 0, como a primeira parte de toda a condição, que é string.IsNullOrEmpty (inferredIndexName) seja sempre verdadeira. Talvez as variáveis especificadasIndexName e inferredIndexName sejam misturadas ou a terceira verificação precise 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 definitiva sobre as opções para corrigir esse código. De qualquer forma, o autor precisa dar uma olhada.

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

É um clássico. A variável conditionValues é usada sem uma verificação preliminar de null . Enquanto mais tarde no código, essa verificação é realizada. O código precisa 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) { .... } .... } }

Encontrei vários erros semelhantes 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 o caso é oposto ao 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 inclui a verificação do valor da variável de estado como nulo . No código abaixo, a variável é usada para cancelar a inscrição do evento PreemptExpiryTime , no entanto, uma verificação de nulo não é mais executada e o lançamento da exceção NullReferenceException se torna possível. Uma versão mais segura do 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; .... }

No código, há outros erros semelhantes:

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 não alternativa

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 é para o código de chamada. Eu verifiquei os casos de uso do método State19 . Ele está envolvido no preenchimento da matriz de manipuladores fsm_handler_table igualmente com outros métodos semelhantes (existem 28 deles com os nomes, respectivamente, começando de State1 para State28 ). Aqui é importante observar que, além do State19 , para alguns outros manipuladores, os avisos V3009 [CWE-393] também foram emitidos. Estes são os manipuladores: State23, State26, State27, State28 . Os avisos 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 da inicialização da matriz dos manipuladores:

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

Para concluir a imagem, vamos ver o código de um dos manipuladores para os quais o analisador não teve nenhuma reivindicação, por exemplo, State2 :

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

Aqui está como ocorre a chamada de manipuladores:

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

Como podemos ver, uma exceção será lançada no caso de retornar false . No nosso caso, para os manipuladores State19, State23, State26 State27 e State28 isso nunca acontecerá. Parece suspeito. Por outro lado, cinco manipuladores têm um comportamento semelhante (sempre retornará verdadeiro ), portanto, talvez tenha sido tão artificial e não seja o resultado de um erro de digitação.

Por que estou indo tão fundo nisso tudo? Essa situação é muito significativa no sentido de que o analisador estático geralmente pode indicar apenas uma construção suspeita. E mesmo uma pessoa (não uma máquina), que não tem conhecimento suficiente sobre o projeto, ainda não é capaz de dar uma resposta completa sobre a presença do erro, mesmo tendo passado algum tempo aprendendo o código. Um desenvolvedor deve revisar esse código.

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 o valor falso , essa variável obterá o valor verdadeiro em todos os casos ao longo do código. Portanto, verifique se (doLog) é sempre verdadeiro. Talvez, anteriormente no método, houvesse uma ramificação, na qual a variável doLog não recebesse nenhum valor. No momento da verificação, poderia conter o valor falso , recebido ao inicializar. 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. 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 todas as condições. Além disso, o analisador emitiu um aviso para este 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 pesquisando outros bugs do V3009 e o salvei para este caso. Assim, a ferramenta estava certa ao apontar o erro V3022 no código de chamada.

Copie e cole. Novamente

Aviso do PVS-Studio: 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 // <= )) { .... } .... }

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

Refatoração + desatenção?

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

Talvez a string de formato do método string.Format contivesse anteriormente o item de formato {0}, para o qual o argumento AWSConfigs.AWSRegionKey foi definido. A sequência foi alterada, o item de formato desapareceu, mas um desenvolvedor esqueceu de remover o argumento. O exemplo de código fornecido funciona sem erros (a exceção foi lançada no caso oposto - o item de formato sem o argumento), mas não parece bom. 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 insegura 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 comum de uma chamada não segura do manipulador de eventos. Um usuário pode cancelar a inscrição entre a verificação da variável mOnSyncSuccess por null e a chamada de um manipulador, para que seu valor se torne nulo . 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)); }

No código, há outros erros semelhantes:

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 bruta

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 bastante código, então eu não o forneci por inteiro, citando apenas sua declaração. Essa classe realmente não contém o método substituído GetHashCode, 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á problemas no momento, mas no futuro esse tipo de estratégia poderá mudar. Este erro é descrito na documentação em mais detalhes.

Conclusão

Esses são todos os bugs interessantes que consegui detectar no código do AWS SDK for .NET usando o analisador estático PVS-Studio. Gostaria de destacar mais uma vez a qualidade do projeto. Encontrei um número muito pequeno de erros para 5 milhões de linhas de código. Embora uma análise provavelmente mais completa dos avisos emitidos me permita adicionar mais alguns erros a esta lista. No entanto, também é bastante provável que eu tenha adicionado alguns dos avisos aos erros por nada. Conclusões inequívocas nesse caso são sempre feitas apenas por um desenvolvedor que esteja no contexto do código verificado.

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

More articles:


All Articles