Análise do código-fonte RPC da estrutura Apache Dubbo com o analisador estático PVS-Studio

O Apache Dubbo é um dos projetos Java mais populares no GitHub. E isso não é surpreendente. Foi criado há 8 anos e é amplamente utilizado como um ambiente RPC de alto desempenho. Obviamente, muitos dos erros em seu código foram corrigidos há muito tempo e a qualidade do código é mantida em um nível alto. No entanto, não há motivo para recusar a verificação de um projeto tão interessante usando o analisador de código estático PVS-Studio. Vamos ver o que conseguimos encontrar.

Sobre o PVS-Studio

O analisador de código estático do PVS-Studio existe há mais de 10 anos no mercado de TI e é uma solução de software multifuncional e de fácil implementação. No momento, o analisador suporta linguagens C, C ++, C #, Java e funciona nas plataformas Windows, Linux e macOS.

O PVS-Studio é uma solução B2B paga e é usada por um grande número de equipes em várias empresas. Se você deseja avaliar os recursos do analisador, faça o download do kit de distribuição e solicite uma chave de avaliação aqui .

Também existem opções para usar o PVS-Studio gratuitamente em projetos de código aberto ou se você é um estudante.

Apache Dubbo: o que é e o que come?

Atualmente, quase todos os grandes sistemas de software são distribuídos . Se em um sistema distribuído houver uma conexão interativa entre componentes remotos com um tempo de resposta curto e uma quantidade relativamente pequena de dados transmitidos, esse é um bom motivo para usar o ambiente RPC (chamada de procedimento remoto).

O Apache Dubbo é um ambiente RPC (chamada de procedimento remoto) baseado em Java de alto desempenho. Como em muitos sistemas RPC, o dubbo é baseado na idéia de criar um serviço para definir métodos que podem ser chamados remotamente com seus parâmetros e tipos de retorno. No lado do servidor, uma interface é implementada e o servidor dubbo é iniciado para processar chamadas do cliente. Há um esboço no lado do cliente que fornece os mesmos métodos que o servidor. O Dubbo oferece três recursos principais que incluem chamadas remotas de front-end, tolerância a falhas e balanceamento de carga, além de registro e descoberta automáticos de serviços.

Sobre a análise

A sequência de etapas para a análise é bastante simples e não requer muito tempo:

• Obtenha o Apache Dubbo com o GitHub ;
• Utilizou as instruções para iniciar o analisador Java e iniciou a análise;
• Recebi um relatório do analisador, analisei e destaquei casos interessantes.

Resultados da análise: 73 avisos dos níveis de confiança Alto e Médio (46 e 27, respectivamente) foram emitidos para mais de 4000 arquivos, o que é um bom indicador da qualidade do código.

Nem todos os avisos são erros. Essa é uma situação normal e, antes do uso regular do analisador, é necessária sua configuração. Então podemos esperar uma porcentagem bastante baixa de falsos positivos ( exemplo ).

Entre os avisos, 9 avisos (7 alto e 2 médio) por arquivos de teste não foram considerados.

Como resultado, um pequeno número de avisos permaneceu, mas entre eles também havia falsos positivos, pois eu não configurei o analisador. A análise de 73 avisos em um artigo é uma tarefa longa, boba e tediosa; portanto, os mais interessantes foram selecionados.

Byte assinado em Java

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

Um valor do tipo byte (um valor de -128 a 127) é comparado com um valor de 0xff (255). Nessa condição, não é levado em consideração que o tipo de byte em Java é significativo; portanto, a condição sempre será atendida, o que significa que não faz sentido.

Retorna o mesmo valor

A expressão V6007 'isPreferIPV6Address ()' é sempre falsa. NetUtils.java (236)

 private static Optional<InetAddress> toValidAddress(InetAddress address) { if (address instanceof Inet6Address) { Inet6Address v6Address = (Inet6Address) address; if (isPreferIPV6Address()) { // <= return Optional.ofNullable(normalizeV6Address(v6Address)); } } if (isValidV4Address(address)) { return Optional.of(address); } return Optional.empty(); } 

Método IsPreferIPV6Address .

 static boolean isPreferIPV6Address() { boolean preferIpv6 = Boolean.getBoolean("java.net.preferIPv6Addresses"); if (!preferIpv6) { return false; // <= } return false; // <= } 

O método isPreferIPV6Address retorna false em ambos os casos, provavelmente um dos casos retornou true conforme pretendido pelo programador, caso contrário, o método não faz sentido.

Verificações sem sentido

A expressão V6007 '! Mask [i] .equals (ipAddress [i])' é sempre verdadeira. NetUtils.java (476)

 public static boolean matchIpRange(....) throws UnknownHostException { .... for (int i = 0; i < mask.length; i++) { if ("*".equals(mask[i]) || mask[i].equals(ipAddress[i])) { continue; } else if (mask[i].contains("-")) { .... } else if (....) { continue; } else if (!mask[i].equals(ipAddress[i])) { // <= return false; } } return true; } 

Na primeira das condições if-else-if, ocorre uma verificação "*". Equals (mask [i]) || mascarar [i] .equals (endereço_IP [i]) . Se essa condição não for atendida, o código continuará para a próxima verificação se-mais-se, e ficamos cientes de que a máscara [i] e o endereço_IP [i] não são equivalentes. Mas em uma das seguintes verificações, se-senão-se, apenas verifique se a máscara [i] e o endereço IP [i] não são equivalentes. Como mask [i] e ipAddress [i] não recebem nenhum valor no código do método, a segunda verificação não faz sentido.

V6007 A expressão 'message.length> 0' sempre é verdadeira. DeprecatedTelnetCodec.java (302)

V6007 A expressão 'message! = Null' sempre é verdadeira. DeprecatedTelnetCodec.java (302)

 protected Object decode(.... , byte[] message) throws IOException { .... if (message == null || message.length == 0) { return NEED_MORE_INPUT; } .... //   message  ! .... if (....) { String value = history.get(index); if (value != null) { byte[] b1 = value.getBytes(); if (message != null && message.length > 0) { // <= byte[] b2 = new byte[b1.length + message.length]; System.arraycopy(b1, 0, b2, 0, b1.length); System.arraycopy(message, 0, b2, b1.length, message.length); message = b2; } else { message = b1; } } } .... } 

Verificar mensagem! = Nulo && message.length> 0 na linha 302 não faz sentido. Antes da verificação, a linha 302 verifica:

 if (message == null || message.length == 0) { return NEED_MORE_INPUT; } 

Se a condição de verificação não for atendida, saberemos que a mensagem não é nula e seu comprimento não é 0. Segue-se a partir desta informação que seu comprimento é maior que zero (já que o comprimento da cadeia não pode ser um número negativo). A mensagem da variável local antes da linha 302 não recebe nenhum valor, o que significa que na linha 302 o valor da variável de mensagem é usado, como no código acima. De tudo isso, podemos concluir que a expressão message! = Null && message.length> 0 sempre será verdadeira , o que significa que o código no bloco else nunca será executado.

Definindo o valor de um campo de referência não inicializado

A expressão V6007 '! ShouldExport ()' é sempre falsa. ServiceConfig.java (371)

 public synchronized void export() { checkAndUpdateSubConfigs(); if (!shouldExport()) { // <= return; } if (shouldDelay()) { .... } else { doExport(); } 

O método shouldExport da classe ServiceConfig chama o método getExport definido na mesma classe.

 private boolean shouldExport() { Boolean export = getExport(); // default value is true return export == null ? true : export; } .... @Override public Boolean getExport() { return (export == null && provider != null) ? provider.getExport() : export; } 

O método getExport chama o método getExport da classe abstrata AbstractServiceConfig , que retorna o valor do campo de exportação do tipo Boolean . Há também um método setExport para definir um valor de campo.

 protected Boolean export; .... public Boolean getExport() { return export; } .... public void setExport(Boolean export) { this.export = export; } 

O campo de exportação no código é definido apenas pelo método setExport . O método setExport é chamado apenas no método de construção da classe abstrata AbstractServiceBuilder (estendendo AbstractServiceConfig ) e somente se o campo de exportação não for nulo.

 @Override public void build(T instance) { .... if (export != null) { instance.setExport(export); } .... } 

Como todos os campos de referência são inicializados como nulos por padrão e o campo de exportação não recebeu nenhum valor em nenhum lugar do código, o método setExport nunca será chamado.

Como resultado, o método getExport da classe ServiceConfig que estende a classe AbstractServiceConfig sempre retornará nulo . O valor retornado é usado no método shouldExport da classe ServiceConfig , portanto, o método shouldExport sempre retornará true . Devido ao retorno true, o valor da expressão ! ShouldExport () sempre será falso. Acontece que nunca haverá um retorno do método de exportação da classe ServiceConfig antes da execução do código:

 if (shouldDelay()) { DELAY_EXPORT_EXECUTOR.schedule(this::doExport, getDelay(), ....); } else { doExport(); } 

Valor do parâmetro não negativo

V6009 A função 'substring' pode receber o valor '-1' enquanto se espera um valor não negativo. Argumento de inspeção: 2. AbstractEtcdClient.java (169)

 protected void createParentIfAbsent(String fixedPath) { int i = fixedPath.lastIndexOf('/'); if (i > 0) { String parentPath = fixedPath.substring(0, i); if (categories.stream().anyMatch(c -> fixedPath.endsWith(c))) { if (!checkExists(parentPath)) { this.doCreatePersistent(parentPath); } } else if (categories.stream().anyMatch(c -> parentPath.endsWith(c))) { String grandfather = parentPath .substring(0, parentPath.lastIndexOf('/')); // <= if (!checkExists(grandfather)) { this.doCreatePersistent(grandfather); } } } } 

O resultado da função lastIndexOf é passado pelo segundo parâmetro para a função de substring , cujo segundo parâmetro não deve ser um número negativo, embora lastIndexOf possa retornar -1 se não encontrar o valor na string. Se o segundo parâmetro do método de substring for menor que o primeiro (-1 <0), uma StringIndexOutOfBoundsException será lançada . Para corrigir o erro, você precisa verificar o resultado retornado pela função lastIndexOf e, se estiver correto (pelo menos não negativo), passá-lo para a função de substring .

Contador de ciclo não utilizado

V6016 Acesso suspeito ao elemento do objeto 'types' por um índice constante dentro de um loop. RpcUtils.java (153)

 public static Class<?>[] getParameterTypes(Invocation invocation) { if ($INVOKE.equals(invocation.getMethodName()) && invocation.getArguments() != null && invocation.getArguments().length > 1 && invocation.getArguments()[1] instanceof String[]) { String[] types = (String[]) invocation.getArguments()[1]; if (types == null) { return new Class<?>[0]; } Class<?>[] parameterTypes = new Class<?>[types.length]; for (int i = 0; i < types.length; i++) { parameterTypes[i] = ReflectUtils.forName(types[0]); // <= } return parameterTypes; } return invocation.getParameterTypes(); } 

O loop for usa o índice constante 0 para se referir ao elemento da matriz de tipos . Talvez fosse para usar a variável i como um índice para acessar os elementos da matriz, mas eles não ignoraram, como dizem.

Do-while sem sentido

V6019 Código inacessível detectado. É possível que haja um erro. GrizzlyCodecAdapter.java (136)

 @Override public NextAction handleRead(FilterChainContext context) throws IOException { .... do { savedReadIndex = frame.readerIndex(); try { msg = codec.decode(channel, frame); } catch (Exception e) { previousData = ChannelBuffers.EMPTY_BUFFER; throw new IOException(e.getMessage(), e); } if (msg == Codec2.DecodeResult.NEED_MORE_INPUT) { frame.readerIndex(savedReadIndex); return context.getStopAction(); } else { if (savedReadIndex == frame.readerIndex()) { previousData = ChannelBuffers.EMPTY_BUFFER; throw new IOException("Decode without read data."); } if (msg != null) { context.setMessage(msg); return context.getInvokeAction(); } else { return context.getInvokeAction(); } } } while (frame.readable()); // <= .... } 

A expressão na condição do loop do - while (frame.readable ()) é um código inacessível, pois a primeira iteração do loop sempre sai do método. No corpo do loop, várias verificações da variável msg são executadas usando if-else, e ambas, if e in else, sempre retornam um valor do método ou lançam uma exceção. É por isso que o corpo do ciclo será executado apenas uma vez, como resultado, o uso do loop do-while não faz sentido.

Copiar colar no comutador

V6067 Dois ou mais ramificações de caso executam as mesmas ações. JVMUtil.java (67), JVMUtil.java (71)

 private static String getThreadDumpString(ThreadInfo threadInfo) { .... if (i == 0 && threadInfo.getLockInfo() != null) { Thread.State ts = threadInfo.getThreadState(); switch (ts) { case BLOCKED: sb.append("\t- blocked on " + threadInfo.getLockInfo()); sb.append('\n'); break; case WAITING: // <= sb.append("\t- waiting on " + threadInfo.getLockInfo()); // <= sb.append('\n'); // <= break; // <= case TIMED_WAITING: // <= sb.append("\t- waiting on " + threadInfo.getLockInfo()); // <= sb.append('\n'); // <= break; // <= default: } } .... } 

O código de opção para WAITING e TIMED_WAITING contém um código de copiar e colar. Se você realmente precisar fazer as mesmas coisas, poderá simplificar o código excluindo o conteúdo no bloco de maiúsculas e minúsculas de WAITING . Como resultado, o mesmo código gravado em uma única cópia será executado para WAITING e TIMED_WAITING .

Conclusão

Qualquer pessoa interessada em usar RPC em Java provavelmente já ouviu falar do Apache Dubbo. Este é um projeto popular de código aberto com uma longa história e código escrito por muitos desenvolvedores. O código para este projeto é de excelente qualidade, mas o analisador estático do PVS-Studio conseguiu encontrar vários erros. A partir disso, podemos concluir que a análise estática é muito importante no desenvolvimento de projetos de médio e grande porte, por mais perfeito que seja o seu código.

Nota Essas verificações únicas demonstram os recursos de um analisador de código estático, mas são uma maneira completamente errada de usá-lo. Essa idéia é apresentada em mais detalhes aqui e aqui . Use a análise regularmente!

Obrigado aos desenvolvedores do Apache Dubbo por uma ferramenta tão boa. Espero que este artigo ajude você a melhorar o código. O artigo não descreve todas as seções suspeitas do código; portanto, é melhor para os desenvolvedores verificar o projeto por conta própria e avaliar os resultados.

Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: Valery Komarov. Análise do Apache Dubbo RPC Framework pelo analisador de código estático PVS-Studio .