Geekly articles weekly

Huawei Cloud: hari ini berawan di PVS-Studio

Gambar 2

Di abad ini, semua orang telah mendengar layanan cloud. Banyak perusahaan telah menguasai segmen pasar ini dan menciptakan layanan cloud mereka di berbagai arah. Tim kami juga baru-baru ini tertarik pada layanan ini dalam hal mengintegrasikan penganalisa kode PVS-Studio dengan mereka. Kami pikir pembaca reguler kami sudah menebak jenis proyek apa yang akan kami periksa kali ini. Pilihannya jatuh pada kode layanan cloud dari Huawei.

Pendahuluan


Jika Anda mengikuti tim PVS-Studio, Anda mungkin memperhatikan bahwa baru-baru ini kami sangat tertarik dengan teknologi cloud. Kami telah menerbitkan beberapa artikel yang berkaitan dengan topik ini:


Jadi, ketika saya mengambil proyek lain yang menarik untuk artikel yang akan datang, saya menerima tawaran pekerjaan dari Huawei melalui pos. Ketika mengumpulkan informasi tentang perusahaan ini, ternyata mereka memiliki layanan cloud sendiri, tetapi yang utama adalah bahwa kode sumber layanan ini tersedia secara bebas di GitHub. Ini adalah alasan utama untuk memilih perusahaan ini untuk artikel ini. Seperti kata seorang bijak Tiongkok: "Kecelakaan tidak disengaja."

Sekarang sedikit tentang analisa kami. PVS-Studio adalah penganalisa kode statis yang mengidentifikasi kesalahan dan kerentanan dalam kode sumber program yang ditulis dalam C, C ++, C # dan Java. Alat analisa bekerja pada platform Windows, Linux dan macOS. Selain plugin untuk lingkungan pengembangan klasik seperti Visual Studio atau IntelliJ IDEA, penganalisa memiliki kemampuan untuk berintegrasi dengan SonarQube dan Jenkins:


Analisis proyek


Dalam proses mencari informasi untuk artikel tersebut, saya menemukan bahwa Huawei memiliki pusat pengembang tempat Anda dapat memperoleh informasi, panduan, dan kode sumber untuk layanan cloud mereka. Berbagai bahasa pemrograman digunakan untuk membuat layanan ini, tetapi bahasa seperti Go, Java, dan Python ternyata menjadi yang paling populer.

Karena saya berspesialisasi dalam Jawa, proyek-proyek tersebut dipilih sesuai dengan itu. Sumber proyek yang dianalisis dalam artikel dapat diperoleh di huaweicloud repositori GitHub.

Untuk menganalisis proyek, saya hanya perlu melakukan beberapa tindakan:

  • Dapatkan proyek dari repositori;
  • Gunakan instruksi untuk memulai penganalisis Java dan memulai analisis pada masing-masing proyek.

Setelah menganalisis proyek, kami berhasil memilih hanya tiga yang ingin saya perhatikan. Ini disebabkan oleh fakta bahwa ukuran proyek Jawa yang tersisa terlalu kecil.

Hasil analisis proyek (jumlah peringatan dan jumlah file):


Ada beberapa peringatan, jadi secara umum kita dapat mengatakan tentang kualitas kode yang baik. Selain itu, tidak semua operasi adalah kesalahan yang valid. Ini disebabkan oleh fakta bahwa penganalisa kadang-kadang tidak memiliki informasi yang cukup untuk membedakan kode yang benar dari yang salah. Oleh karena itu, diagnostik penganalisa ditingkatkan setiap hari dengan bantuan informasi yang diterima dari pengguna. Lihat juga artikel " Bagaimana dan mengapa analisa statis melawan positif palsu ."

Dalam proses menganalisis proyek, saya memilih peringatan yang paling menarik, yang akan saya bahas dalam artikel ini.

Urutan inisialisasi lapangan


Siklus inisialisasi Kelas V6050 hadir. Inisialisasi 'INSTANCE' muncul sebelum inisialisasi '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); // <= } } }

Jika ada pengecualian terjadi dalam konstruktor kelas UntrustedSSL , informasi tentang pengecualian ini dicatat di blok tangkap menggunakan LOG logger. Namun, karena urutan inisialisasi bidang statis, LOG pada saat inisialisasi bidang INSTANCE belum diinisialisasi. Oleh karena itu, ketika mencatat informasi pengecualian di konstruktor, NullPointerException akan dilempar. Pengecualian ini adalah penyebab pengecualian ExceptionInInitializerError lain yang dilemparkan jika pengecualian terjadi saat menginisialisasi bidang statis. Semua yang diperlukan untuk menyelesaikan masalah yang dijelaskan adalah menempatkan inisialisasi LOG sebelum inisialisasi INSTANCE .

Kesalahan ketik yang tak terlihat


V6005 Variabel 'this.metricSchema' ditugaskan untuk dirinya sendiri. OpenTSDBSchema.java (72)

 public class OpenTSDBSchema { @JsonProperty("metric") private List<SchemaField> metricSchema; .... public void setMetricsSchema(List<SchemaField> metricsSchema) { this.metricSchema = metricSchema; // <= } public void setMetricSchema(List<SchemaField> metricSchema) { this.metricSchema = metricSchema; } .... }

Kedua metode mengatur bidang metricSchema , tetapi nama metode berbeda dengan karakter seseorang. Programmer menamai argumen metode ini sesuai dengan nama metode tersebut. Akibatnya, pada baris yang ditunjukkan oleh penganalisa, bidang metricSchema ditugaskan untuk dirinya sendiri, dan argumen dari metode metricsSchema tidak digunakan.

V6005 Variabel 'menangguhkan' ditugaskan untuk dirinya sendiri. SuspendTransferTaskRequest.java (77)

 public class SuspendTransferTaskRequest { .... private boolean suspend; .... public void setSuspend(boolean suspend) { suspend = suspend; } .... }

Berikut adalah kesalahan dangkal yang terkait dengan kecerobohan, yang menyebabkan penugasan argumen itu sendiri terjadi. Akibatnya, nilai argumen yang tiba tidak akan ditugaskan ke bidang menangguhkan , seperti tersirat. Dengan benar:

 public void setSuspend(boolean suspend) { this.suspend = suspend; }

Kondisi yang ditentukan sebelumnya


Seperti yang sering terjadi, aturan V6007 mendahului dalam hal jumlah peringatan.

Ekspresi V6007 'firewallPolicyId == null' selalu salah. FirewallPolicyServiceImpl.java (125)

 public FirewallPolicy removeFirewallRuleFromPolicy(String firewallPolicyId, String firewallRuleId) { checkNotNull(firewallPolicyId); checkNotNull(firewallRuleId); checkState(!(firewallPolicyId == null && firewallRuleId == null), "Either a Firewall Policy or Firewall Rule identifier must be set"); .... }

Dalam metode ini, argumen diperiksa untuk null oleh metode checkNotNull :

 @CanIgnoreReturnValue public static <T> T checkNotNull(T reference) { if (reference == null) { throw new NullPointerException(); } else { return reference; } }

Setelah memeriksa argumen dengan metode checkNotNull , Anda dapat 100% yakin bahwa argumen yang diteruskan ke metode ini bukan nol . Karena kedua argumen untuk metode removeFirewallRuleFromPolicy diperiksa oleh metode checkNotNull , tidak masuk akal untuk memeriksa argumen untuk nilai null lagi. Namun, ekspresi dilewatkan ke metode checkState sebagai argumen pertama, di mana argumen firewallPolicyId dan firewallRuleId diperiksa ulang untuk null .

Peringatan serupa dikeluarkan untuk firewallRuleId :

  • Ekspresi V6007 'firewallRuleId == null' selalu salah. FirewallPolicyServiceImpl.java (125)

Ekspresi V6007 'filteringParams! = Null' selalu benar. NetworkPolicyServiceImpl.java (60)

 private Invocation<NetworkServicePolicies> buildInvocation(Map<String, String> filteringParams) { .... if (filteringParams == null) { return servicePoliciesInvocation; } if (filteringParams != null) { // <= .... } return servicePoliciesInvocation; }

Dalam metode ini, jika argumen filteringParams adalah nol , maka metode keluar dan mengembalikan nilai. Oleh karena itu, tes yang ditunjukkan oleh penganalisa akan selalu benar, yang berarti bahwa tes ini tidak masuk akal.

Serupa terjadi di 13 kelas:

  • Ekspresi V6007 'filteringParams! = Null' selalu benar. PolicyRuleServiceImpl.java (58)
  • Ekspresi V6007 'filteringParams! = Null' selalu benar. GroupServiceImpl.java (58)
  • Ekspresi V6007 'filteringParams! = Null' selalu benar. ExternalSegmentServiceImpl.java (57)
  • Ekspresi V6007 'filteringParams! = Null' selalu benar. L3policyServiceImpl.java (57)
  • Ekspresi V6007 'filteringParams! = Null' selalu benar. PolicyRuleSetServiceImpl.java (58)
  • dan seterusnya ...

Referensi kosong


V6008 Potensi null dereference dari 'm.blockDeviceMapping'. NovaServerCreate.java (390)

 @Override public ServerCreateBuilder blockDevice(BlockDeviceMappingCreate blockDevice) { if (blockDevice != null && m.blockDeviceMapping == null) { m.blockDeviceMapping = Lists.newArrayList(); } m.blockDeviceMapping.add(blockDevice); // <= return this; }

Dalam metode ini, inisialisasi bidang referensi m.blockDeviceMapping tidak akan terjadi jika argumen blockDevice adalah nol . Bidang ini diinisialisasi hanya dalam metode ini, jadi ketika metode add dipanggil, bidang m.blockDeviceMapping akan melempar NullPointerException .

V6008 Potensi dereferensi nol dari 'FileId.get (path)' dalam fungsi '<init>'. TrackedFile.java (140), TrackedFile.java (115)

 public TrackedFile(FileFlow<?> flow, Path path) throws IOException { this(flow, path, FileId.get(path), ....); }

Hasil dari metode statis FileId.get (path) diteruskan ke konstruktor dari kelas TrackedFile sebagai argumen ketiga. Tetapi metode ini dapat mengembalikan nol :

 public static FileId get(Path file) throws IOException { if (!Files.exists(file)) { return null; } .... }

Dalam konstruktor yang dipanggil melalui ini , argumen id tidak berubah sampai digunakan pertama kali:

 public TrackedFile(...., ...., FileId id, ....) throws IOException { .... FileId newId = FileId.get(path); if (!id.equals(newId)) { .... } }

Oleh karena itu, jika nol dilewatkan ke metode sebagai argumen ketiga, pengecualian akan terjadi.

Situasi serupa terjadi dalam kasus lain:

  • V6008 Potensi null dereference dari 'buffer'. PublishingQueue.java (518)

V6008 Potensi null dereference dari '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 ; } } .... }

Dan lagi NPE. Sejumlah pemeriksaan dalam pernyataan bersyarat memungkinkan objek dataTmpFile nol untuk dereferencing lebih lanjut. Saya pikir ada dua kesalahan ketik di sini, dan cek harus benar-benar seperti ini:

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

Substring dan angka negatif


V6009 Fungsi 'substring' dapat menerima nilai '-1' sementara nilai non-negatif diharapkan. Periksa argumen: 2. RemoveVersionProjectIdFromURL.java (37)

 @Override public String apply(String url) { String urlRmovePojectId = url.substring(0, url.lastIndexOf("/")); return urlRmovePojectId.substring(0, urlRmovePojectId.lastIndexOf("/")); }

Dapat dipahami bahwa URL diteruskan ke metode ini sebagai string, yang tidak divalidasi dengan cara apa pun. Setelah string ini dipangkas beberapa kali menggunakan metode lastIndexOf . Jika lastIndexOf tidak menemukan kecocokan dalam string, itu akan mengembalikan -1. Ini akan melempar StringIndexOutOfBoundsException , karena argumen ke metode substring harus berupa angka non-negatif. Agar metode berfungsi dengan benar, Anda harus menambahkan validasi argumen input atau memverifikasi bahwa hasil dari metodeIndexOf terakhir bukan angka negatif.

Berikut adalah beberapa tempat lain di mana ini terjadi:

  • V6009 Fungsi 'substring' dapat menerima nilai '-1' sementara nilai non-negatif diharapkan. Periksa argumen: 2. RemoveProjectIdFromURL.java (37)
  • V6009 Fungsi 'substring' dapat menerima nilai '-1' sementara nilai non-negatif diharapkan. Periksa argumen: 2. RemoveVersionProjectIdFromURL.java (38)

Hasil yang terlupakan


V6010 Nilai kembali fungsi 'concat' diperlukan untuk dimanfaatkan. AKSK.java (278)

 public static String buildCanonicalHost(URL url) { String host = url.getHost(); int port = url.getPort(); if (port > -1) { host.concat(":" + Integer.toString(port)); } return host; }

Saat menulis kode ini, tidak diperhitungkan bahwa memanggil metode concat tidak akan mengubah string host karena ketidakmampuan objek-objek dari tipe String . Agar metode berfungsi dengan benar, Anda harus menetapkan hasil metode concat ke variabel host di blok if . Dengan benar:

 if (port > -1) { host = host.concat(":" + Integer.toString(port)); }

Variabel yang tidak digunakan


'URL' V6021 variabel tidak digunakan. TriggerV2Service.java (95)

 public ActionResponse deleteAllTriggersForFunction(String functionUrn) { checkArgument(!Strings.isNullOrEmpty(functionUrn), ....); String url = ClientConstants.FGS_TRIGGERS_V2 + ClientConstants.URI_SEP + functionUrn; return deleteWithResponse(uri(triggersUrlFmt, functionUrn)).execute(); }

Dalam metode ini, variabel url tidak digunakan setelah inisialisasi. Kemungkinan besar, variabel url harus diteruskan ke metode uri sebagai argumen kedua alih-alih functionUrn , karena variabel functionUrn terlibat dalam inisialisasi variabel url .

Argumen tidak digunakan dalam konstruktor


V6022 Parameter 'returnType' tidak digunakan di dalam tubuh konstruktor. HttpRequest.java (68)

 public class HttpReQuest<R> { .... Class<R> returnType; .... public HttpRequest(...., Class<R> returnType) // <= { this.endpoint = endpoint; this.path = path; this.method = method; this.entity = entity; } .... public Class<R> getReturnType() { return returnType; } .... }

Dalam konstruktor ini, mereka lupa menggunakan argumen returnType dan menetapkan nilainya ke bidang returnType . Oleh karena itu, ketika memanggil metode getReturnType , objek yang dibuat oleh konstruktor ini akan mengembalikan nilai default nol , meskipun itu kemungkinan besar dimaksudkan untuk mendapatkan objek yang sebelumnya diteruskan ke konstruktor.

Fungsionalitas yang sama


V6032 Aneh bahwa tubuh metode 'aktifkan' sepenuhnya setara dengan tubuh metode lain 'nonaktifkan'. ServiceAction.java (32), ServiceAction.java (36)

 public class ServiceAction implements ModelEntity { private String binary; private String host; private ServiceAction(String binary, String host) { this.binary = binary; this.host = host; } public static ServiceAction enable(String binary, String host) { // <= return new ServiceAction(binary, host); } public static ServiceAction disable(String binary, String host) { // <= return new ServiceAction(binary, host); } .... }

Kehadiran dua metode yang identik bukanlah kesalahan, tetapi fakta bahwa dua metode melakukan tindakan yang sama setidaknya aneh. Melihat nama-nama metode di atas, kita dapat mengasumsikan bahwa mereka melakukan tindakan yang berlawanan. Faktanya, kedua metode melakukan hal yang sama - mereka membuat dan mengembalikan objek ServiceAction . Kemungkinan besar, metode menonaktifkan dibuat dengan menyalin kode metode aktifkan , tetapi lupa untuk mengubah tubuh metode.

Lupa memeriksa hal utama


V6060 Referensi 'params' digunakan sebelum diverifikasi terhadap nol. DomainService.java (49), DomainService.java (46)

 public Domains list(Map<String, String> params) { Preconditions.checkNotNull(params.get("page_size"), ....); Preconditions.checkNotNull(params.get("page_number"), ....); Invocation<Domains> domainInvocation = get(Domains.class, uri("/domains")); if (params != null) { // <= .... } return domainInvocation.execute(this.buildExecutionOptions(Domains.class)); }

Dalam metode ini, kami memutuskan untuk memeriksa konten struktur tipe Map untuk ketimpangan nol . Untuk melakukan ini, argumen params memanggil metode get dua kali, yang hasilnya diteruskan ke metode checkNotNull . Segalanya tampak logis, tetapi bagaimanapun caranya! Jika , argumen params diperiksa untuk null . Setelah itu dapat diasumsikan bahwa argumen input mungkin nol , tetapi sebelum pemeriksaan ini, metode get dipanggil params dua kali. Jika null dilewatkan sebagai argumen untuk metode ini, maka pertama kali metode get dipanggil, pengecualian akan dilemparkan.

Situasi serupa terjadi di tiga tempat lagi:

  • V6060 Referensi 'params' digunakan sebelum diverifikasi terhadap nol. DomainService.java (389), DomainService.java (387)
  • V6060 Referensi 'params' digunakan sebelum diverifikasi terhadap nol. DomainService.java (372), DomainService.java (369)
  • V6060 Referensi 'params' digunakan sebelum diverifikasi terhadap nol. DomainService.java (353), DomainService.java (350)

Kesimpulan


Perusahaan besar modern saat ini tidak dapat melakukannya tanpa menggunakan layanan cloud. Sejumlah besar orang menggunakan layanan ini, sehingga kesalahan sekecil apa pun dalam layanan dapat menyebabkan masalah bagi banyak orang, serta biaya tambahan bagi perusahaan untuk memperbaiki konsekuensi dari kesalahan ini. Dalam mengembangkannya, selalu perlu memperhitungkan faktor manusia, karena siapa pun cepat atau lambat membuat kesalahan, dan artikel ini adalah contohnya. Itu sebabnya Anda harus menggunakan semua alat yang mungkin untuk meningkatkan kualitas kode.

PVS-Studio tentu saja akan memberi tahu Huawei tentang hasil pemeriksaan layanan cloud mereka sehingga pengembang perusahaan ini dapat mempelajarinya lebih detail.

Penggunaan satu kali analisis kode statis yang ditunjukkan dalam artikel tidak dapat menunjukkan semua kelebihannya. Informasi lebih rinci tentang cara menggunakan analisis statis dengan benar dapat ditemukan di sini dan di sini . Anda dapat mengunduh penganalisa PVS-Studio di sini .



Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Valery Komarov. Huawei Cloud: Berawan di PVS-Studio Hari Ini .

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

More articles:


All Articles