Apache Dubbo es uno de los proyectos Java más populares en GitHub. No es sorprendente Fue creado hace 8 años y se aplica ampliamente como un entorno RPC de alto rendimiento. Por supuesto, la mayoría de los errores en su código se han corregido durante mucho tiempo y la calidad del código se mantiene a un alto nivel. Sin embargo, no hay razón para dejar de verificar un proyecto tan interesante utilizando el analizador de código estático PVS-Studio. Veamos cómo resultó.
Sobre PVS-Studio
El analizador de código estático
PVS-Studio ha existido durante más de 10 años en el mercado de TI y es una solución de software multifuncional y fácil de introducir. Por el momento, el analizador es compatible con C, C ++, C #, Java y funciona en Windows, Linux y macOS.
PVS-Studio es una solución B2B y es utilizada por una gran cantidad de equipos en varias compañías. Si desea estimar las capacidades del analizador, puede descargar el distributivo y solicitar una clave de prueba
aquí .
También hay
opciones sobre cómo puede usar PVS-Studio de forma gratuita en proyectos de código abierto o si es un estudiante.
Apache Dubbo: qué es y características principales
Hoy en día, casi todos los grandes sistemas de software están
distribuidos . Si en un sistema distribuido hay una conexión interactiva entre componentes remotos con baja latencia y relativamente pocos datos para pasar, es una buena razón para usar un entorno
RPC (llamada a procedimiento remoto).
Apache Dubbo es un entorno
RPC (llamada a procedimiento remoto) de alto rendimiento con código fuente abierto basado en Java. Al igual que muchos otros sistemas RPC, dubbo se basa en la idea de crear un servicio que defina algunos métodos que se puedan llamar de forma remota con sus parámetros y tipos de valores de retorno. En el lado del servidor, se implementa una interfaz y se lanza el servidor dubbo para atender las consultas de los clientes. En el lado del cliente, hay un código auxiliar que proporciona los mismos métodos que el servidor. Dubbo sugiere tres funciones clave, que incluyen llamadas remotas de interfaz, tolerancia a fallas y equilibrio de carga, así como registro automático y descubrimiento de servicios.
Sobre el analisis
La secuencia de acciones del análisis es bastante simple y no requirió mucho tiempo en mi caso:
- Apache Dubbo descargado de GitHub ;
- Utilicé las instrucciones de inicio del analizador Java y ejecuté el análisis;
- Obtuve el informe del analizador, lo revisé y destaqué casos interesantes.
Los resultados del análisis: se emitieron 73 advertencias de niveles de certeza alto y medio (46 y 27, respectivamente) para más de 4000 archivos, lo que es un buen indicador de la calidad del código.
No todas las advertencias son errores. Es un resultado habitual, ya que antes de usar directamente el analizador, uno tiene que configurarlo. Después de eso, puede esperar un porcentaje bastante bajo de falsos positivos (
ejemplo ).
No consideré 9 advertencias (7 altas y 2 medias), relacionadas con archivos con pruebas.
Como resultado, tuve una pequeña cantidad de advertencias, que también incluyeron falsos positivos, porque no he configurado el analizador. Profundizar en las 73 advertencias con una descripción más detallada en el artículo es largo, ridículo y tedioso, así que elegí las más ingeniosas.
Firmar Byte en Java
V6007 La expresión 'endKey [i] <0xff' siempre es verdadera. OpciónUtil.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) {
Un valor del tipo de
byte (el valor de -128 a 127) se compara con el valor 0xff (255). En esta condición, no se tiene en cuenta que el tipo de
byte en Java está firmado, por lo tanto, la condición siempre será verdadera, lo que significa que no tiene sentido.
Devolución de los mismos valores
V6007 La expresión 'isPreferIPV6Address ()' siempre es falsa. NetUtils.java (236)
private static Optional<InetAddress> toValidAddress(InetAddress address) { if (address instanceof Inet6Address) { Inet6Address v6Address = (Inet6Address) address; if (isPreferIPV6Address()) {
El método
esPreferIPV6Address .
static boolean isPreferIPV6Address() { boolean preferIpv6 = Boolean.getBoolean("java.net.preferIPv6Addresses"); if (!preferIpv6) { return false;
El método
isPreferIPV6Address devuelve
falso en ambos casos. Lo más probable es que un desarrollador quisiera que un caso devuelva
verdadero, de lo contrario el método no tiene ningún sentido.
Controles sin sentido
V6007 La expresión '! Máscara [i] .equals (ipAddress [i])' siempre es verdadera. 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])) {
En la condición if-else-if, la marca
"*". Igual (máscara [i]) || Se realiza la máscara [i] .equals (ipAddress [i]) . Si no se cumple la condición, se produce la siguiente verificación en if-else-if que nos muestra que
mask [i] e
ipAddress [i] no son iguales. Pero en una de las siguientes comprobaciones en if-else-if se verifica que
mask [i] e
ipAddress [i] no son iguales. Como la
máscara [i] y
ipAddress [i ] no tienen ningún valor asignado, la segunda comprobación no tiene sentido.
V6007 La expresión 'message.length> 0' siempre es verdadera. DeprecatedTelnetCodec.java (302)
V6007 Expresión 'mensaje! = Nulo' siempre es verdadero. DeprecatedTelnetCodec.java (302)
protected Object decode(.... , byte[] message) throws IOException { .... if (message == null || message.length == 0) { return NEED_MORE_INPUT; } ....
El
mensaje de verificación
! = Nulo && mensaje.length> 0 en la línea 302 es redundante. Antes de la verificación en la línea 302, se realiza la siguiente verificación:
if (message == null || message.length == 0) { return NEED_MORE_INPUT; }
Si la condición de la verificación no cumple, sabremos que el
mensaje no es nulo y su longitud no es igual a 0. De esto se deduce que su longitud es mayor que 0 (ya que la longitud de una cadena no puede ser un número negativo) El
mensaje de la variable local no tiene asignado ningún valor antes de la línea 302, lo que significa que en la línea 302 el valor de la variable del
mensaje es el mismo que en el código anterior. Se puede concluir que la expresión
message! = Null && message.length> 0 siempre será verdadera, pero el código en el bloque else nunca se ejecutará.
Establecer el valor de un campo de referencia no inicializado
V6007 La expresión '! ShouldExport ()' siempre es falsa. ServiceConfig.java (371)
public synchronized void export() { checkAndUpdateSubConfigs(); if (!shouldExport()) {
El método
shouldExport de la clase
ServiceConfig llama al método
getExport , definido en la misma clase.
private boolean shouldExport() { Boolean export = getExport();
El método
getExport llama al método
getExport de la clase abstracta
AbstractServiceConfig , que devuelve el valor del campo de
exportación del tipo
booleano . También hay un método
setExport para establecer el valor del campo.
protected Boolean export; .... public Boolean getExport() { return export; } .... public void setExport(Boolean export) { this.export = export; }
El campo de exportación se establece en el código solo por el método
setExport . El método
setExport se llama solo en el método de compilación de la clase abstracta
AbstractServiceBuilder (que extiende
AbstractServiceConfig ) solo en caso de que el campo no sea nulo.
@Override public void build(T instance) { .... if (export != null) { instance.setExport(export); } .... }
Debido al hecho de que todos los campos de referencia de forma predeterminada se inicializan como
nulos y al campo de
exportación no se le asignó un valor, nunca se llamará al método
setExport .
Como resultado, el método
getExport de la clase
ServiceConfig , que expande la clase
AbstractServiceConfig siempre devolverá
nulo . El valor de retorno se usa en el método
shouldExport de la clase
ServiceConfig , por lo tanto, el método
shouldExport siempre devolverá
verdadero . Debido a que devuelve verdadero, el valor de la
expresión! ShouldExport () siempre será falso. Resulta que el campo de
exportación de la clase
ServiceConfig nunca se devolverá hasta que se ejecute el siguiente código:
if (shouldDelay()) { DELAY_EXPORT_EXECUTOR.schedule(this::doExport, getDelay(), ....); } else { doExport(); }
Valor del parámetro no negativo
V6009 La función 'subcadena' podría recibir el valor '-1' mientras se espera un valor no negativo. Inspeccionar argumento: 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('/'));
El resultado de la función
lastIndexOf se pasa como el segundo parámetro a la función de
subcadena , cuyo segundo parámetro no debe ser un número negativo, aunque
lastIndexOf puede devolver
-1 si no encuentra el valor buscado en la cadena. Si el segundo parámetro del método de
subcadena es menor que el primero (-1 <0), se
generará la excepción
StringIndexOutOfBoundsException . Para corregir el error, debemos verificar el resultado, devuelto por la función
lastIndexOf . Si es correcto (al menos, no negativo), páselo a la función de
subcadena .
Contador de bucle no utilizado
V6016 Acceso sospechoso al elemento del objeto 'tipos' por un índice constante dentro de un bucle. 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]);
En el bucle
for , se usa un índice constante
0 para acceder al elemento de los
tipos de matriz. Es posible que haya tenido la intención de usar la variable
i como índice para acceder a los elementos de la matriz. Pero los autores lo han dejado de lado.
No tiene sentido hacer
V6019 Código inalcanzable detectado. Es posible que haya un error presente. 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());
La expresión en la condición del bucle
do -
while (frame.readable ()) es un código inalcanzable, ya que el método sale durante la primera iteración del bucle. Se realizan varias verificaciones de la variable
msg en el cuerpo del bucle usando if-else. Al hacerlo, se devuelven los valores
if y
else del método o se lanzan excepciones. Esa es la razón por la cual el cuerpo del bucle se ejecuta solo una vez, por lo que el uso de este bucle no tiene sentido.
Copiar y pegar en el interruptor
V6067 Dos o más ramificaciones de casos realizan las mismas acciones. 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:
El código en el
interruptor para los casos
WAITING y
TIMED_WAITING contiene código de copiar y pegar. Si se deben realizar las mismas acciones, el código puede simplificarse eliminando el contenido del bloque de
caso WAITING . Como resultado, se ejecutará el mismo código para
WAITING y
TIMED_WAITING.Conclusión
Cualquier persona interesada en usar
RPC en Java, probablemente haya escuchado sobre Apache Dubbo. Es un proyecto de código abierto popular con una larga historia y código, escrito por muchos desarrolladores. El código de este proyecto es de gran calidad, aun así el analizador de código estático PVS-Studio logró encontrar un cierto número de errores. Esto lleva al hecho de que el análisis estático es muy importante al desarrollar proyectos de tamaño medio y grande, sin importar cuán perfecto sea su código.
Nota Estas comprobaciones únicas demuestran las capacidades de un analizador de código estático, pero representan una forma completamente incorrecta de su uso. Se detallan más detalles de esta idea
aquí y
aquí . ¡Usa el análisis regularmente!
Gracias a los desarrolladores de Apache Dubbo por una herramienta tan maravillosa. Espero que este artículo te ayude a mejorar el código. El artículo no describe todas las piezas sospechosas de código, por lo que es mejor que los desarrolladores verifiquen el proyecto y evalúen los resultados.