Apache Dubbo ist eines der beliebtesten Java-Projekte auf GitHub. Und das ist nicht überraschend. Es wurde vor 8 Jahren erstellt und wird häufig als Hochleistungs-RPC-Umgebung verwendet. Natürlich sind die meisten Fehler in seinem Code seit langem behoben und die Qualität des Codes wird auf einem hohen Niveau gehalten. Es gibt jedoch keinen Grund, die Überprüfung eines so interessanten Projekts mit dem statischen Code-Analysator PVS-Studio abzulehnen. Mal sehen, was wir gefunden haben.
Über PVS-Studio
Der statische Code-Analysator
PVS-Studio existiert seit mehr als 10 Jahren auf dem IT-Markt und ist eine multifunktionale und einfach zu implementierende Softwarelösung. Derzeit unterstützt der Analyzer die Sprachen C, C ++, C # und Java und funktioniert auf Windows-, Linux- und MacOS-Plattformen.
PVS-Studio ist eine kostenpflichtige B2B-Lösung und wird von einer Vielzahl von Teams in verschiedenen Unternehmen eingesetzt. Wenn Sie die Funktionen des Analysators bewerten möchten, laden Sie das Verteilungskit herunter und fordern Sie hier einen Testschlüssel
an .
Es gibt auch
Optionen, um PVS-Studio kostenlos in Open Source-Projekten oder als Student zu verwenden.
Apache Dubbo: Was ist das und was isst es?
Derzeit werden fast alle großen Softwaresysteme
vertrieben . Wenn in einem verteilten System eine interaktive Verbindung zwischen Remote-Komponenten mit einer kurzen Antwortzeit und einer relativ geringen Menge übertragener Daten besteht, ist dies ein guter Grund, die
RPC- Umgebung (Remote Procedure Call) zu verwenden.
Apache Dubbo ist eine leistungsstarke Open-Source-Java-basierte
RPC- Umgebung (Remote Procedure Call). Wie bei vielen RPC-Systemen basiert Dubbo auf der Idee, einen Service zum Definieren von Methoden zu erstellen, die mit ihren Parametern und Rückgabedatentypen remote aufgerufen werden können. Auf der Serverseite wird eine Schnittstelle implementiert und der Dubbo-Server gestartet, um Clientaufrufe zu verarbeiten. Auf der Clientseite gibt es einen Stub, der dieselben Methoden wie der Server bereitstellt. Dubbo bietet drei Hauptfunktionen, darunter Front-End-Remote-Anrufe, Fehlertoleranz und Lastausgleich sowie die automatische Registrierung und Erkennung von Diensten.
Über die Analyse
Die Abfolge der Schritte für die Analyse ist recht einfach und erfordert nicht viel Zeit:
- Habe Apache Dubbo mit GitHub ;
- Ich habe die Anweisungen zum Starten des Java-Analysators verwendet und die Analyse gestartet.
- Ich erhielt einen Analysatorbericht, analysierte ihn und hob interessante Fälle hervor.
Analyseergebnisse: 73 Warnungen des Konfidenzniveaus Hoch und Mittel (46 bzw. 27) wurden für mehr als 4000 Dateien ausgegeben, was ein guter Indikator für die Qualität des Codes ist.
Nicht alle Warnungen sind Fehler. Dies ist eine normale Situation, und vor der regelmäßigen Verwendung des Analysators ist dessen Konfiguration erforderlich. Dann können wir einen relativ geringen Prozentsatz an falsch positiven Ergebnissen erwarten (
Beispiel ).
Unter den Warnungen wurden 9 Warnungen (7 hoch und 2 mittel) pro Testdatei nicht berücksichtigt.
Infolgedessen blieb eine kleine Anzahl von Warnungen übrig, aber unter ihnen gab es auch falsch positive Ergebnisse, da ich den Analysator nicht konfiguriert habe. 73 Warnungen in einem Artikel zu sortieren ist eine lange, dumme und mühsame Aufgabe, daher wurden die interessantesten ausgewählt.
Signiertes Byte in Java
V6007 Der Ausdruck 'endKey [i] <0xff' ist immer wahr. 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) {
Ein Wert vom Typ
Byte (ein Wert von -128 bis 127) wird mit einem Wert von 0xff (255) verglichen. In dieser Bedingung wird nicht berücksichtigt, dass der
Bytetyp in Java von Bedeutung ist, daher wird die Bedingung immer erfüllt, was bedeutet, dass dies keinen Sinn ergibt.
Geben Sie den gleichen Wert zurück
V6007 Ausdruck 'isPreferIPV6Address ()' ist immer falsch. NetUtils.java (236)
private static Optional<InetAddress> toValidAddress(InetAddress address) { if (address instanceof Inet6Address) { Inet6Address v6Address = (Inet6Address) address; if (isPreferIPV6Address()) {
IsPreferIPV6Address- Methode.
static boolean isPreferIPV6Address() { boolean preferIpv6 = Boolean.getBoolean("java.net.preferIPv6Addresses"); if (!preferIpv6) { return false;
Die
isPreferIPV6Address- Methode gibt in beiden Fällen
false zurück. Höchstwahrscheinlich sollte einer der Fälle
true zurückgegeben haben, wie vom Programmierer beabsichtigt, andernfalls ist die Methode nicht sinnvoll.
Sinnlose Schecks
V6007 Ausdruck '! Mask [i] .equals (ipAddress [i])' ist immer wahr. 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])) {
In der ersten der if-else-if-Bedingungen erfolgt eine
"*" - Prüfung
. Gleich (Maske [i]) || mask [i] .equals (ipAddress [i]) . Wenn diese Bedingung nicht erfüllt ist, fährt der Code mit der nächsten Prüfung in if-else-if fort, und wir stellen fest, dass
mask [i] und
ipAddress [i] nicht gleichwertig sind. Eine der folgenden Überprüfungen in if-else-if überprüft jedoch nur, ob
mask [i] und
ipAddress [i] nicht gleichwertig sind. Da
mask [i] und
ipAddress [i] im Methodencode keine Werte zugewiesen bekommen, ist die zweite Prüfung nicht sinnvoll.
V6007 Der Ausdruck 'message.length> 0' ist immer wahr. DeprecatedTelnetCodec.java (302)
V6007 Ausdruck 'message! = Null' ist immer wahr. DeprecatedTelnetCodec.java (302)
protected Object decode(.... , byte[] message) throws IOException { .... if (message == null || message.length == 0) { return NEED_MORE_INPUT; } ....
Nachricht prüfen! = Null && message.length> 0 in Zeile 302 macht keinen Sinn. Vor der Prüfung prüft Zeile 302:
if (message == null || message.length == 0) { return NEED_MORE_INPUT; }
Wenn die Überprüfungsbedingung nicht erfüllt ist, wissen wir, dass die
Nachricht nicht
null und ihre Länge nicht 0 ist. Aus diesen Informationen folgt, dass ihre Länge größer als Null ist (da die
Länge der Zeichenfolge keine negative Zahl sein kann). Der lokalen Variablennachricht vor Zeile 302 werden keine Werte zugewiesen, was bedeutet, dass in Zeile 302 der Wert der
Nachrichtenvariablen wie im obigen Code verwendet wird. Daraus können wir schließen, dass der Ausdruck
message! = Null && message.length> 0 immer
wahr ist , was bedeutet, dass der Code im
else- Block niemals ausgeführt wird.
Festlegen des Werts eines nicht initialisierten Referenzfelds
V6007 Ausdruck '! ShouldExport ()' ist immer falsch. ServiceConfig.java (371)
public synchronized void export() { checkAndUpdateSubConfigs(); if (!shouldExport()) {
Die
shouldExport- Methode der
ServiceConfig- Klasse ruft die in derselben Klasse definierte
getExport- Methode auf.
private boolean shouldExport() { Boolean export = getExport();
Die Methode
getExport ruft die Methode
getExport der abstrakten Klasse
AbstractServiceConfig auf, die den Wert des
Exportfelds vom Typ
Boolean zurückgibt. Es gibt auch eine
setExport- Methode zum Festlegen des Werts eines Feldes.
protected Boolean export; .... public Boolean getExport() { return export; } .... public void setExport(Boolean export) { this.export = export; }
Das
Exportfeld im Code wird nur von der
setExport- Methode festgelegt. Die
setExport- Methode
wird nur in der
Erstellungsmethode der abstrakten Klasse
AbstractServiceBuilder (Erweiterung von
AbstractServiceConfig ) aufgerufen und nur, wenn das
Exportfeld nicht null ist.
@Override public void build(T instance) { .... if (export != null) { instance.setExport(export); } .... }
Aufgrund der Tatsache, dass alle Referenzfelder standardmäßig auf
Null initialisiert sind und dem
Exportfeld an keiner Stelle im Code ein Wert zugewiesen wurde, wird die
setExport- Methode niemals aufgerufen.
Infolgedessen gibt die
getExport- Methode der
ServiceConfig- Klasse, die die
AbstractServiceConfig- Klasse erweitert, immer
null zurück . Der zurückgegebene Wert wird in der
shouldExport- Methode der
ServiceConfig- Klasse verwendet, sodass die
shouldExport- Methode immer
true zurückgibt . Aufgrund der Rückgabe von true ist der Wert des Ausdrucks
! ShouldExport () immer false. Es stellt sich heraus, dass die
Exportmethode der
ServiceConfig- Klasse vor der Codeausführung niemals zurückgegeben wird:
if (shouldDelay()) { DELAY_EXPORT_EXECUTOR.schedule(this::doExport, getDelay(), ....); } else { doExport(); }
Nicht negativer Parameterwert
V6009 Die Funktion 'Teilzeichenfolge' könnte den Wert '-1' empfangen, während ein nicht negativer Wert erwartet wird. Argument überprüfen: 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('/'));
Das Ergebnis der Funktion
lastIndexOf wird vom zweiten Parameter an die
Teilzeichenfolgenfunktion übergeben , deren zweiter Parameter keine negative Zahl sein sollte, obwohl
lastIndexOf -1 zurückgeben kann, wenn der Wert in der Zeichenfolge nicht gefunden wird. Wenn der zweite Parameter der
Teilstring- Methode kleiner als der erste ist (-1 <0), wird eine
StringIndexOutOfBoundsException ausgelöst . Um den Fehler zu beheben, müssen Sie das von der Funktion
lastIndexOf zurückgegebene Ergebnis überprüfen. Wenn es korrekt (zumindest nicht negativ) ist, übergeben Sie es an die
Teilzeichenfolgenfunktion .
Unbenutzter Zykluszähler
V6016 Verdächtiger Zugriff auf ein Element vom Typ 'Typ' durch einen konstanten Index innerhalb einer Schleife. 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]);
Die
for- Schleife verwendet den konstanten Index
0 , um auf das Element des Typarrays zu verweisen. Vielleicht sollte die Variable
i als Index für den Zugriff auf die Elemente des Arrays verwendet werden, aber sie haben nicht übersehen, wie sie sagen.
Sinnlose Pause
V6019 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. 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());
Der Ausdruck in der Schleifenbedingung
do -
while (frame.readable ()) ist ein nicht erreichbarer Code, da die erste Iteration der Schleife die Methode immer beendet. Im Hauptteil der Schleife werden mehrere Überprüfungen der Variablen
msg mit if-else durchgeführt, und sowohl in
if als auch in
else wird immer ein Wert aus der Methode zurückgegeben oder eine Ausnahme ausgelöst. Aus diesem Grund wird der Hauptteil des Zyklus nur einmal ausgeführt, weshalb die Verwendung der do-while-Schleife keinen Sinn ergibt.
Kopieren Einfügen in Schalter
V6067 Zwei oder mehr
Fallzweige führen dieselben Aktionen aus. 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:
Der
Switch- Code für
WAITING und
TIMED_WAITING enthält einen Code zum
Kopieren und Einfügen. Wenn Sie wirklich die gleichen Dinge tun müssen, können Sie den Code vereinfachen, indem Sie den Inhalt im
case- Block für
WAITING löschen . Infolgedessen wird für
WAITING und
TIMED_WAITING derselbe Code ausgeführt, der in einer einzelnen Kopie
aufgezeichnet ist .
Fazit
Jeder, der sich für die Verwendung von
RPC in Java interessiert, hat wahrscheinlich von Apache Dubbo gehört. Dies ist ein beliebtes Open Source-Projekt mit einer langen Geschichte und Code, die von vielen Entwicklern geschrieben wurden. Der Code für dieses Projekt ist von ausgezeichneter Qualität, aber der statische Analysator PVS-Studio konnte eine Reihe von Fehlern feststellen. Daraus können wir schließen, dass die statische Analyse bei der Entwicklung mittlerer und großer Projekte sehr wichtig ist, unabhängig davon, wie perfekt Ihr Code ist.
Hinweis Solche einmaligen Überprüfungen demonstrieren die Fähigkeiten eines statischen Code-Analysators, sind jedoch eine völlig falsche Verwendung. Diese Idee wird
hier und
hier ausführlicher vorgestellt. Verwenden Sie die Analyse regelmäßig!
Vielen Dank an die Apache Dubbo-Entwickler für dieses großartige Tool. Ich hoffe, dieser Artikel hilft Ihnen, den Code zu verbessern. Der Artikel beschreibt nicht alle verdächtigen Codeabschnitte. Daher ist es für Entwickler besser, das Projekt selbst zu überprüfen und die Ergebnisse auszuwerten.
Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Valery Komarov.
Analyse des Apache Dubbo RPC Frameworks mit dem PVS-Studio Static Code Analyzer .