Analisis kode sumber RPC kerangka kerja Dubbo Apache dengan penganalisa statis PVS-Studio

Apache Dubbo adalah salah satu proyek Java paling populer di GitHub. Dan ini tidak mengejutkan. Itu dibuat 8 tahun yang lalu dan banyak digunakan sebagai lingkungan RPC berkinerja tinggi. Tentu saja, sebagian besar kesalahan dalam kodenya telah lama diperbaiki dan kualitas kodenya dijaga pada tingkat tinggi. Namun, tidak ada alasan untuk menolak untuk memeriksa proyek yang menarik menggunakan penganalisis kode statis PVS-Studio. Mari kita lihat apa yang berhasil kita temukan.

Tentang PVS-Studio

Alat analisis kode statis PVS-Studio telah ada selama lebih dari 10 tahun di pasar TI dan merupakan solusi perangkat lunak yang multifungsi dan mudah diimplementasikan. Saat ini, alat analisa mendukung C, C ++, C #, bahasa Java dan bekerja pada platform Windows, Linux dan macOS.

PVS-Studio adalah solusi B2B berbayar dan digunakan oleh sejumlah besar tim di berbagai perusahaan. Jika Anda ingin mengevaluasi kemampuan penganalisa, maka unduh kit distribusi dan minta kunci percobaan di sini .

Ada juga opsi untuk menggunakan PVS-Studio secara gratis di proyek sumber terbuka atau jika Anda seorang pelajar.

Apache Dubbo: apa itu dan apa yang dimakannya?

Saat ini, hampir semua sistem perangkat lunak besar didistribusikan . Jika dalam sistem terdistribusi ada koneksi interaktif antara komponen jarak jauh dengan waktu respons singkat dan jumlah data yang ditransmisikan relatif kecil, maka ini adalah alasan yang baik untuk menggunakan lingkungan RPC (panggilan prosedur jarak jauh).

Apache Dubbo adalah lingkungan RPC berbasis Java (panggilan prosedur) berkinerja tinggi berkinerja tinggi. Seperti banyak sistem RPC, dubbo didasarkan pada gagasan untuk menciptakan layanan untuk mendefinisikan metode yang dapat disebut jarak jauh dengan parameter mereka dan mengembalikan tipe data. Di sisi server, antarmuka diimplementasikan dan server dubbo diluncurkan untuk memproses panggilan klien. Ada tulisan rintisan di sisi klien yang menyediakan metode yang sama seperti server. Dubbo menawarkan tiga fitur utama yang mencakup panggilan jarak jauh front-end, toleransi kesalahan dan penyeimbangan muatan, serta pendaftaran otomatis dan penemuan layanan.

Tentang analisis

Urutan langkah-langkah untuk analisis ini cukup sederhana dan tidak memerlukan banyak waktu:

• Mendapat Apache Dubbo dengan GitHub ;
• Saya menggunakan instruksi untuk memulai penganalisis Java dan memulai analisis;
• Saya menerima laporan analisa, menganalisisnya dan menyoroti kasus-kasus menarik.

Hasil analisis: 73 peringatan tingkat kepercayaan Tinggi dan Sedang (masing-masing 46 dan 27) dikeluarkan untuk 4000+ file, yang merupakan indikator kualitas kode yang baik.

Tidak semua peringatan adalah kesalahan. Ini adalah situasi yang normal, dan sebelum menggunakan penganalisa secara teratur, konfigurasinya diperlukan. Maka kita dapat mengharapkan persentase positif palsu yang cukup rendah ( contoh )

Di antara peringatan, 9 peringatan (7 Tinggi dan 2 Medium) per file uji tidak dipertimbangkan.

Akibatnya, sejumlah kecil peringatan tetap ada, tetapi di antara mereka ada juga positif palsu, karena saya tidak mengonfigurasi analisanya. Untuk mengurutkan 73 peringatan dalam sebuah artikel adalah tugas yang panjang, konyol dan membosankan, jadi yang paling menarik dipilih.

Byte yang ditandatangani di Java

Ekspresi V6007 'endKey [i] <0xff' selalu benar. 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); } 

Nilai tipe byte (nilai dari -128 hingga 127) dibandingkan dengan nilai 0xff (255). Dalam kondisi ini, tidak diperhitungkan bahwa tipe byte di Jawa adalah signifikan, oleh karena itu kondisi akan selalu terpenuhi, yang berarti bahwa itu tidak masuk akal.

Kembalikan nilai yang sama

Ekspresi V6007 'isPreferIPV6Address ()' selalu salah. 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(); } 

IsPreferIPV6Address Method.

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

Metode isPreferIPV6Address mengembalikan false dalam kedua kasus, kemungkinan besar salah satu kasus seharusnya dikembalikan benar seperti yang dimaksudkan oleh programmer, jika tidak metode ini tidak masuk akal.

Cek tidak berarti

Ekspresi V6007 '! Mask [i]. Equals (ipAddress [i])' selalu benar. 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; } 

Dalam kondisi if-else-if yang pertama, tanda "*" akan muncul . Persamaan (mask [i]) || mask [i] .equals (ipAddress [i]) . Jika kondisi ini tidak terpenuhi, kode dilanjutkan ke pemeriksaan berikutnya di if-else-if, dan kami menyadari bahwa mask [i] dan ipAddress [i] tidak setara. Tetapi salah satu dari cek berikut ini di if-else-if hanya memeriksa bahwa mask [i] dan ipAddress [i] tidak setara. Karena mask [i] dan ipAddress [i] tidak diberi nilai apa pun dalam kode metode, pemeriksaan kedua tidak masuk akal.

Ekspresi V6007 'message.length> 0' selalu benar. DeprecatedTelnetCodec.java (302)

V6007 Ekspresi 'message! = Null' selalu benar. 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; } } } .... } 

Memeriksa pesan! = Null && message.length> 0 on line 302 tidak masuk akal. Sebelum cek, baris 302 memeriksa:

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

Jika kondisi verifikasi tidak terpenuhi, maka kita akan tahu bahwa pesan tidak nol dan panjangnya tidak 0. Ini mengikuti informasi ini bahwa panjangnya lebih besar dari nol (karena panjang string tidak boleh berupa angka negatif). Pesan variabel lokal sebelum baris 302 tidak diberikan nilai apa pun, yang berarti bahwa di baris 302 nilai variabel pesan digunakan, seperti dalam kode di atas. Dari semua ini kita dapat menyimpulkan bahwa pesan ekspresi ! = Null && message.length> 0 akan selalu benar , yang berarti bahwa kode di blok lain tidak akan pernah dieksekusi.

Menetapkan nilai bidang referensi yang tidak diinisialisasi

Ekspresi V6007 '! ShouldExport ()' selalu salah. ServiceConfig.java (371)

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

Metode shouldExport dari kelas ServiceConfig memanggil metode getExport yang didefinisikan di kelas yang sama.

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

Metode getExport memanggil metode getExport dari kelas abstrak AbstractServiceConfig , yang mengembalikan nilai bidang ekspor tipe Boolean . Ada juga metode setExport untuk mengatur nilai bidang.

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

Bidang ekspor dalam kode hanya ditetapkan oleh metode setExport . Metode setExport dipanggil hanya dalam metode membangun kelas abstrak AbstractServiceBuilder (memperluas AbstractServiceConfig ) dan hanya jika bidang ekspor tidak nol.

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

Karena kenyataan bahwa semua bidang referensi diinisialisasi ke nol secara default dan bahwa bidang ekspor belum diberi nilai apa pun di dalam kode, metode setExport tidak akan pernah dipanggil.

Akibatnya, metode getExport dari kelas ServiceConfig yang memperpanjang kelas AbstractServiceConfig akan selalu mengembalikan nol . Nilai yang dikembalikan digunakan dalam metode shouldExport dari kelas ServiceConfig , sehingga metode shouldExport akan selalu mengembalikan true . Karena mengembalikan true, nilai ekspresi ! ShouldExport () akan selalu salah. Ternyata tidak akan pernah ada pengembalian dari metode ekspor kelas ServiceConfig sebelum eksekusi kode:

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

Nilai parameter non-negatif

V6009 Fungsi 'substring' dapat menerima nilai '-1' sementara nilai non-negatif diharapkan. Periksa argumen: 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); } } } } 

Hasil dari fungsi lastIndexOf dilewatkan oleh parameter kedua ke fungsi substring , parameter kedua yang seharusnya tidak menjadi angka negatif, meskipun lastIndexOf dapat mengembalikan -1 jika tidak menemukan nilai dalam string. Jika parameter kedua dari metode substring kurang dari yang pertama (-1 <0), maka StringIndexOutOfBoundsException akan dibuang . Untuk memperbaiki kesalahan, Anda perlu memeriksa hasil yang dikembalikan oleh fungsi lastIndexOf , dan jika itu benar (setidaknya tidak negatif), kemudian meneruskannya ke fungsi substring .

Penghitung siklus yang tidak digunakan

V6016 Akses mencurigakan ke elemen objek 'types' dengan indeks konstan di dalam satu 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(); } 

Untuk loop menggunakan indeks konstan 0 untuk merujuk ke elemen dari array jenis . Mungkin itu dimaksudkan untuk menggunakan variabel i sebagai indeks untuk mengakses elemen-elemen array, tetapi mereka tidak mengabaikannya, seperti yang mereka katakan.

Tidak berarti apa-apa

V6019 Kode tidak terjangkau terdeteksi. Mungkin saja ada kesalahan. 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()); // <= .... } 

Ekspresi dalam kondisi loop do - while (frame.readable ()) adalah kode yang tidak dapat dijangkau, karena iterasi pertama dari loop selalu keluar dari metode. Di badan loop, beberapa pemeriksaan variabel msg dilakukan menggunakan if-else, dan keduanya di if dan di lain selalu mengembalikan nilai dari metode atau melemparkan pengecualian. Karena inilah tubuh siklus akan dieksekusi hanya sekali, sebagai akibatnya, menggunakan do-while loop tidak masuk akal.

Salin tempel di sakelar

V6067 Dua atau lebih cabang kasus melakukan tindakan yang sama. 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: } } .... } 

Kode sakelar untuk WAITING dan TIMED_WAITING berisi kode salin-tempel. Jika Anda benar-benar perlu melakukan hal yang sama, Anda dapat menyederhanakan kodenya dengan menghapus konten pada blok kasus untuk menunggu . Akibatnya, kode yang sama yang direkam dalam satu salinan akan dieksekusi untuk WAITING dan TIMED_WAITING .

Kesimpulan

Siapa pun yang tertarik menggunakan RPC di Jawa mungkin pernah mendengar tentang Apache Dubbo. Ini adalah proyek open source yang populer dengan sejarah panjang dan kode yang ditulis oleh banyak pengembang. Kode untuk proyek ini berkualitas sangat baik, tetapi penganalisis statis PVS-Studio berhasil menemukan sejumlah kesalahan. Dari sini kita dapat menyimpulkan bahwa analisis statis sangat penting ketika mengembangkan proyek-proyek menengah dan besar, tidak peduli seberapa sempurna kode Anda.

Catatan Pemeriksaan satu kali seperti itu menunjukkan kemampuan penganalisa kode statis, tetapi merupakan cara yang sepenuhnya salah untuk menggunakannya. Ide ini disajikan secara lebih rinci di sini dan di sini . Gunakan analisis secara teratur!

Terima kasih kepada pengembang Apache Dubbo untuk alat yang hebat ini. Saya harap artikel ini membantu Anda meningkatkan kode. Artikel ini tidak menjelaskan semua bagian kode yang mencurigakan, jadi lebih baik bagi pengembang untuk memeriksa sendiri proyek dan mengevaluasi hasilnya.

Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Valery Komarov. Analisis Kerangka Apache Dubbo RPC oleh PVS-Studio Static Code Analyzer .