Os 10 principais erros encontrados em projetos Java em 2019


2019 está chegando ao fim e a equipe PVS-Studio está olhando para as realizações deste ano. No início de 2019, aprimoramos os recursos de diagnóstico do analisador adicionando suporte a Java, o que nos permitiu verificar e revisar projetos Java também. Encontramos muitos bugs durante este ano, e aqui estão nossos 10 principais bugs encontrados em projetos Java.


Não. 10: byte assinado


Fonte: Análise do Apache Dubbo RPC Framework pelo analisador de código estático PVS-Studio

A expressão V6007 'endKey [i] <0xff' sempre é verdadeira. OptionUtil.java (32)

public static final ByteSequence prefixEndOf(ByteSequence prefix) { byte[] endKey = prefix.getBytes().clone(); for (int i = endKey.length - 1; i >= 0; i--) { if (endKey[i] < 0xff) { // <= endKey[i] = (byte) (endKey[i] + 1); return ByteSequence.from(Arrays.copyOf(endKey, i + 1)); } } return ByteSequence.from(NO_PREFIX_END); } 

Muitos programadores acreditam que o tipo de byte não está assinado. Este é realmente o caso em muitas linguagens de programação. Por exemplo, isso é verdade para C #. Mas isso não é assim em Java.

Na condição endKey [i] <0xff , uma variável do tipo byte é comparada com o número 255 (0xff) representado na forma hexadecimal. O desenvolvedor provavelmente esqueceu que o intervalo do tipo de byte Java é [-128, 127]. Essa condição será sempre verdadeira e o loop for sempre processará apenas o último elemento da matriz endKey .

Não. 9: Dois em um


Fonte: PVS-Studio for Java pega a estrada. A próxima parada é a Elasticsearch

A expressão V6007 '(int) x <0' é sempre falsa. BCrypt.java (429)

V6025 Possivelmente o índice '(int) x' está fora dos limites. BCrypt.java (431)

 private static byte char64(char x) { if ((int)x < 0 || (int)x > index_64.length) return -1; return index_64[(int)x]; } 

Desconto! Um método - dois erros! O primeiro tem a ver com o tipo char : como não é assinado em Java, a condição (int) x <0 será sempre falsa. O segundo bug é a indexação comum além dos limites da matriz index_64 quando (int) x == index_64.length . Isso acontece devido à condição (int) x> index_64.length . O bug pode ser corrigido alterando o operador '>' para '> =': (int) x> = index_64.length .

Não. 8: Uma solução e suas implicações


Fonte: Analisando o código da plataforma CUBA com o PVS-Studio

A expressão V6007 'previousMenuItemFlatIndex> = 0' sempre é verdadeira. CubaSideMenuWidget.java (328)

 protected MenuItemWidget findNextMenuItem(MenuItemWidget currentItem) { List<MenuTreeNode> menuTree = buildVisibleTree(this); List<MenuItemWidget> menuItemWidgets = menuTreeToList(menuTree); int menuItemFlatIndex = menuItemWidgets.indexOf(currentItem); int previousMenuItemFlatIndex = menuItemFlatIndex + 1; if (previousMenuItemFlatIndex >= 0) { // <= return menuItemWidgets.get(previousMenuItemFlatIndex); } return null; } 

O autor do método findNextMenuItem queria se livrar do valor -1 retornado pelo método indexOf quando a lista menuItemWidgets não contém currentItem . Para fazer isso, o programador está adicionando 1 ao resultado de indexOf (a variável menuItemFlatIndex ) e gravando o valor resultante na variável previousMenuItemFlatIndex , que é usada no método. A adição de -1 prova ser uma solução ruim, pois leva a vários erros ao mesmo tempo:

  • A declaração nula de retorno nunca será executada porque a expressão previousMenuItemFlatIndex> = 0 sempre será verdadeira; portanto, o método findNextMenuItem sempre retornará de dentro da instrução if ;
  • uma IndexOutOfBoundsException será gerada quando a lista menuItemWidgets ficar vazia, pois o programa tentará acessar o primeiro elemento da lista vazia;
  • uma IndexOutOfBoundsException será gerada quando o argumento currentItem se tornar o último elemento da lista menuItemWidget .

Não. 7: Criando um arquivo do nada


Fonte: Huawei Cloud: Hoje está nublado no PVS-Studio

V6008 Dereferência nula potencial de 'dataTmpFile'. CacheManager.java (91)

 @Override public void putToCache(PutRecordsRequest putRecordsRequest) { .... if (dataTmpFile == null || !dataTmpFile.exists()) { try { dataTmpFile.createNewFile(); // <= } catch (IOException e) { LOGGER.error("Failed to create cache tmp file, return.", e); return; } } .... } 

Ao escrever o método putToCache , o programador fez um erro de digitação na condição dataTmpFile == null || ! dataTmpFile.exists () antes de criar um novo arquivo dataTmpFile.createNewFile () : eles escreveram o operador '==' em vez de '! ='. Esse erro de digitação levará ao lançamento de um NullPointerException ao chamar o método createNewFile . É assim que a condição se parece com o erro de digitação corrigido:

 if (dataTmpFile != null || !dataTmpFile.exists()) 

Você pode pensar: "Bem, tudo bem, podemos relaxar agora." Ainda não!

Agora que um bug foi corrigido, outro apareceu. Uma NullPointerException pode ser lançada ao chamar o método dataTmpFile.exists () . Para consertar isso, precisamos substituir o '||' operador com '&&'. Esta é a versão final da condição, depois de todas as correções:

 if (dataTmpFile != null && !dataTmpFile.exists()) 

Não. 6: Um erro lógico muito estranho


Fonte: PVS-Studio for Java

V6007 [CWE-570] A expressão '"0" .equals (text)' é sempre falsa. ConvertIntegerToDecimalPredicate.java 46

 public boolean satisfiedBy(@NotNull PsiElement element) { .... @NonNls final String text = expression.getText().replaceAll("_", ""); if (text == null || text.length() < 2) { return false; } if ("0".equals(text) || "0L".equals(text) || "0l".equals(text)) {// <= return false; } return text.charAt(0) == '0'; } 

O interessante desse método é que ele contém um erro lógico óbvio. Se o método satisfeitoBy não retornar após a primeira instrução if , isso significa que a sequência de texto tem pelo menos dois caracteres. Isso também significa que a primeira verificação "0" .equals (texto) na próxima instrução if não faz sentido. O que o programador realmente quis dizer com isso permanece um mistério.

Não. 5: Que reviravolta!


Fonte: PVS-Studio visita o Apache Hive

V6034 A mudança no valor de 'bitShiftsInWord - 1' pode ser inconsistente com o tamanho do tipo: 'bitShiftsInWord - 1' = [-1 ... 30]. UnsignedInt128.java (1791)

 private void shiftRightDestructive(int wordShifts, int bitShiftsInWord, boolean roundUp) { if (wordShifts == 0 && bitShiftsInWord == 0) { return; } assert (wordShifts >= 0); assert (bitShiftsInWord >= 0); assert (bitShiftsInWord < 32); if (wordShifts >= 4) { zeroClear(); return; } final int shiftRestore = 32 - bitShiftsInWord; // check this because "123 << 32" will be 123. final boolean noRestore = bitShiftsInWord == 0; final int roundCarryNoRestoreMask = 1 << 31; final int roundCarryMask = (1 << (bitShiftsInWord - 1)); // <= .... } 

Com os argumentos de entrada wordShifts = 3 e bitShiftsInWord = 0 , a variável roundCarryMask , que armazena o resultado do deslocamento bit a bit (1 << (bitShiftsInWord - 1)) , se tornará um número negativo. O desenvolvedor provavelmente não esperava isso.

Não. 4: Podemos ver as exceções, por favor?


Fonte: PVS-Studio visita o Apache Hive

V6051 O uso da instrução 'return' no bloco 'final' pode levar à perda de exceções não tratadas. ObjectStore.java (9080)

 private List<MPartitionColumnStatistics> getMPartitionColumnStatistics(....) throws NoSuchObjectException, MetaException { boolean committed = false; try { .... /*some actions*/ committed = commitTransaction(); return result; } catch (Exception ex) { LOG.error("Error retrieving statistics via jdo", ex); if (ex instanceof MetaException) { throw (MetaException) ex; } throw new MetaException(ex.getMessage()); } finally { if (!committed) { rollbackTransaction(); return Lists.newArrayList(); } } } 

A declaração do método getMPartitionColumnStatistics é enganosa, pois diz que poderia gerar uma exceção. Na realidade, qualquer exceção gerada no bloco try , a variável confirmada será sempre falsa ; portanto, a instrução de retorno no bloco final retornará um valor, enquanto todas as exceções lançadas serão perdidas, incapazes de serem processadas fora do método. Portanto, nenhuma das exceções lançadas neste método poderá abandoná-lo.

Não. 3: Hocus-pocus, ou tentando obter uma nova máscara


Fonte: PVS-Studio visita o Apache Hive

V6034 A mudança no valor de 'j' pode ser inconsistente com o tamanho do tipo: 'j' = [0 ... 63]. IoTrace.java (272)

 public void logSargResult(int stripeIx, boolean[] rgsToRead) { .... for (int i = 0, valOffset = 0; i < elements; ++i, valOffset += 64) { long val = 0; for (int j = 0; j < 64; ++j) { int ix = valOffset + j; if (rgsToRead.length == ix) break; if (!rgsToRead[ix]) continue; val = val | (1 << j); // <= } .... } .... } 

Esse bug também tem a ver com uma mudança bit a bit, mas não apenas isso. A variável j é usada como um contador no intervalo [0 ... 63] no loop for interno. Este contador participa de um deslocamento bit a bit 1 << j . Tudo parece bom, mas é aqui que o literal inteiro '1' do tipo int (um valor de 32 bits) entra em ação. Por esse motivo, o deslocamento bit a bit começará a retornar os valores retornados anteriormente quando o valor da variável j exceder 31. Se esse comportamento não for o que o programador desejava, o valor 1 deverá ser representado por muito tempo : 1L << j ou (long) 1 << j .

Não. 2: ordem de inicialização


Fonte: Huawei Cloud: Hoje está nublado no PVS-Studio

O ciclo de inicialização da classe V6050 está presente. A inicialização de 'INSTANCE' aparece antes da inicialização de 'LOG'. UntrustedSSL.java (32), UntrustedSSL.java (59), UntrustedSSL.java (33)

 public class UntrustedSSL { private static final UntrustedSSL INSTANCE = new UntrustedSSL(); private static final Logger LOG = LoggerFactory.getLogger(UntrustedSSL.class); .... private UntrustedSSL() { try { .... } catch (Throwable t) { LOG.error(t.getMessage(), t); // <= } } } 

A ordem na qual os campos são declarados em uma classe faz diferença, pois eles são inicializados na mesma ordem em que são declarados. Esquecer esse fato leva a erros esquivos como o descrito acima.

O analisador indica que o campo estático LOG é desreferenciado no construtor no momento em que é inicializado com o valor nulo , o que leva a lançar uma série de exceções NullPointerException -> ExceptionInInitializerError .

Mas por que o campo estático LOG tem o valor nulo no momento de chamar o construtor?

O ExceptionInInitializerError é a pista. O construtor está inicializando o campo estático INSTANCE , que é declarado antes do campo LOG . É por isso que o campo LOG ainda não foi inicializado quando o construtor é chamado. Para que esse código funcione corretamente, o campo LOG deve ser inicializado antes da chamada.

Primeiro: programação orientada para copiar e colar


Fonte: Código Apache Hadoop Qualidade: Teste de Produção VS

V6072 Dois fragmentos de código semelhantes foram encontrados. Talvez seja um erro de digitação e a variável 'localFiles' deva ser usada em vez de 'localArchives'. LocalDistributedCacheManager.java (183), LocalDistributedCacheManager.java (178), LocalDistributedCacheManager.java (176), LocalDistributedCacheManager.java (181)

 public synchronized void setup(JobConf conf, JobID jobId) throws IOException { .... // Update the configuration object with localized data. if (!localArchives.isEmpty()) { conf.set(MRJobConfig.CACHE_LOCALARCHIVES, StringUtils .arrayToString(localArchives.toArray(new String[localArchives // <= .size()]))); } if (!localFiles.isEmpty()) { conf.set(MRJobConfig.CACHE_LOCALFILES, StringUtils .arrayToString(localFiles.toArray(new String[localArchives // <= .size()]))); } .... } 

O primeiro lugar em nossa lista dos 10 principais é concedido para copiar e colar, ou melhor, um bug resultante do uso descuidado dessa técnica. A segunda instrução if se parece muito com uma cópia da primeira, com algumas das variáveis ​​modificadas:

  • localArchives foi alterado para localFiles ;
  • MRJobConfig.CACHE_LOCALARCHIVES foi alterado para MRJobConfig.CACHE_LOCALFILES .

Mas o programador conseguiu cometer um erro, mesmo nesta operação simples: a instrução if na linha apontada pelo analisador ainda está usando a variável localArchives em vez da variável localFiles aparentemente pretendida.

Conclusão


A correção de erros encontrados nos estágios de desenvolvimento posteriores ou após o lançamento consome uma grande quantidade de recursos. O analisador estático PVS-Studio permite detectar erros no estágio de codificação, tornando-o muito mais fácil e barato. Muitas empresas já facilitaram a vida de seus desenvolvedores, começando a usar o analisador regularmente. Se você realmente gosta do seu trabalho, experimente o PVS-Studio .

Não vamos parar por isso e planejamos continuar melhorando e aprimorando nossa ferramenta. Fique atento a novos diagnósticos e artigos com bugs ainda mais interessantes no próximo ano.

Vejo você como aventuras! Primeiro, você derrotou os 10 principais bugs no projeto C # em 2019 e agora também lida com Java! Bem-vindo ao próximo nível - o artigo sobre os melhores erros nos projetos C ++ em 2019 .

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


All Articles