Geekly articles weekly

Huawei Cloud: Berawan di PVS-Studio Hari Ini

Gambar 2

Saat ini semua orang tahu tentang layanan cloud. Banyak perusahaan telah memecahkan segmen pasar ini dan menciptakan layanan cloud mereka sendiri dari berbagai tujuan. Baru-baru ini tim kami juga tertarik dengan layanan ini dalam hal mengintegrasikan penganalisa kode PVS-Studio ke dalamnya. Kemungkinannya, pembaca reguler kami sudah menebak jenis proyek apa yang akan kami periksa kali ini. Pilihannya jatuh pada kode layanan cloud Huawei.

Pendahuluan


Jika Anda mengikuti posting tim PVS-Studio, Anda mungkin memperhatikan bahwa kami telah menggali jauh dalam teknologi cloud belakangan ini. Kami telah menerbitkan beberapa artikel yang membahas topik ini:


Tepat ketika saya sedang mencari proyek yang tidak biasa untuk artikel yang akan datang, saya mendapat email dengan tawaran pekerjaan dari Huawei . Setelah mengumpulkan beberapa informasi tentang perusahaan ini, ternyata mereka memiliki layanan cloud sendiri, tetapi yang utama adalah bahwa kode sumber layanan ini tersedia di GitHub. Ini adalah alasan utama untuk memilih perusahaan ini untuk artikel ini. Seperti kata seorang bijak Tiongkok: "Kecelakaan itu bukan karena kecelakaan."

Biarkan saya memberi Anda beberapa detail tentang analisa kami. PVS-Studio adalah penganalisa statis untuk deteksi bug dalam kode sumber program, ditulis dalam C, C ++, C #, dan Java. Penganalisa bekerja pada Windows, Linux, dan macOS. Selain plugin untuk lingkungan pengembangan klasik, seperti Visual Studio atau IntelliJ IDEA, penganalisa memiliki kemampuan untuk berintegrasi ke SonarQube dan Jenkins:


Analisis proyek


Ketika saya sedang melakukan riset untuk artikel tersebut, saya menemukan bahwa Huawei memiliki pusat pengembang dengan informasi, manual, dan sumber layanan cloud yang tersedia. Berbagai macam bahasa pemrograman digunakan untuk membuat layanan ini, tetapi bahasa seperti Go, Java dan Python adalah yang paling umum.

Sejak saya berspesialisasi di Jawa, proyek-proyek telah dipilih sesuai dengan pengetahuan dan keterampilan saya. Anda dapat menganalisis sumber proyek dalam artikel di huaweicloud repositori GitHub.

Untuk menganalisis proyek, saya hanya perlu melakukan beberapa hal:

  • Dapatkan proyek dari repositori;
  • Gunakan instruksi penganalisis Java start-up dan jalankan analisis pada setiap proyek.

Setelah menganalisis proyek, kami hanya memilih tiga di antaranya, yang ingin kami perhatikan. Karena ukuran proyek Java yang lain ternyata terlalu kecil.

Hasil analisis proyek (jumlah peringatan dan jumlah file):


Ada beberapa peringatan, yang memberi tahu kita tentang kode berkualitas tinggi, terlebih lagi karena tidak semua peringatan menunjukkan kesalahan nyata. Ini disebabkan oleh fakta bahwa penganalisa kadang-kadang kekurangan informasi untuk membedakan kode yang benar dari kode yang salah. Karena alasan ini, kami mengubah diagnostik penganalisa hari demi hari dengan mencari informasi dari pengguna. Anda boleh melihat artikel " Cara analis statis melawan positif palsu, dan mengapa mereka melakukannya ."

Saat menganalisis proyek ini, saya hanya mengambil peringatan paling banyak, yang akan saya bicarakan di artikel ini.

Urutan inisialisasi bidang


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 dalam konstruktor kelas UntrustedSSL , informasi tentang pengecualian ini dicatat di blok tangkap menggunakan LOG logger. Namun, karena urutan inisialisasi bidang statis, pada saat inisialisasi bidang INSTANCE , LOG belum diinisialisasi. Oleh karena itu, jika Anda mencatat informasi tentang pengecualian di konstruktor, itu akan menghasilkan NullPointerException . Pengecualian ini adalah alasan untuk pengecualian lain ExceptionInInitializerError , yang dilemparkan jika ada pengecualian ketika bidang statis telah diinisialisasi. Apa yang Anda butuhkan untuk menyelesaikan masalah ini adalah menempatkan inisialisasi LOG sebelum menginisialisasi INSTANCE .

Kesalahan ketik yang tidak mencolok


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 simbol satu '. Programmer menamai argumen metode ini sesuai dengan nama metode tersebut. Akibatnya, di jalur yang ditunjuk oleh penganalisis, bidang metricSchema ditetapkan untuk dirinya sendiri, dan argumen 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 ini adalah kesalahan sepele yang berkaitan dengan kecerobohan, oleh karena itu argumen penangguhan ditugaskan untuk dirinya sendiri. Akibatnya, bidang yang ditangguhkan tidak akan diberi nilai argumen yang diperoleh sebagaimana tersirat. Versi yang 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 tidak sama dengan nol . Karena kedua argumen metode removeFirewallRuleFromPolicy diperiksa oleh metode checkNotNull , pemeriksaan lebih lanjut untuk null tidak masuk akal. Namun, ekspresi, di mana argumen firewallPolicyId dan firewallRuleId diperiksa ulang untuk null , dilewatkan sebagai argumen pertama ke metode checkState .

Peringatan serupa juga 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 , metode mengembalikan nilai. Inilah sebabnya pemeriksaan yang ditunjukkan oleh penganalisis akan selalu benar yang, pada gilirannya, berarti bahwa pemeriksaan ini tidak ada artinya.

13 kelas lebih mirip:

  • 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 memanggil metode add dari bidang m.blockDeviceMapping , NullPointerException akan terjadi.

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

Konstruktor kelas TrackedFile menerima hasil dari metode FileId.get (path) statis sebagai argumen ketiga. Tetapi metode ini dapat mengembalikan nol :

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

Di konstruktor, 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)) { .... } }

Seperti yang dapat kita lihat, jika nol dilewatkan sebagai argumen ketiga ke metode, pengecualian akan terjadi.

Berikut ini adalah kasus serupa lainnya:

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

NPE lagi. Sejumlah pemeriksaan di operator bersyarat memungkinkan objek dataTmpFile nol untuk dereferensi lebih lanjut. Saya pikir ada dua kesalahan ketik di sini dan cek harus benar-benar terlihat 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("/")); }

Implikasinya adalah bahwa metode ini mendapatkan URL sebagai string, yang tidak divalidasi dengan cara apa pun. Kemudian, string ini terputus beberapa kali menggunakan metode lastIndexOf . Jika metode lastIndexOf tidak menemukan kecocokan dalam string, itu akan mengembalikan -1. Ini akan mengarah ke StringIndexOutOfBoundsException , karena argumen metode substring harus berupa angka non-negatif. Untuk operasi metode yang benar, kita harus menambahkan validasi argumen input atau memeriksa bahwa hasil metode lastIndexOf adalah angka non-negatif.

Berikut beberapa cuplikan lainnya dengan cara yang serupa:

  • 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, pembuatnya tidak memperhitungkan bahwa pemanggilan metode concat tidak akan mengubah string host karena ketidakmampuan objek tipe String . Untuk operasi metode yang benar, hasil dari metode concat harus ditetapkan ke variabel host di blok if . Versi yang 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 mengambil bagian dalam inisialisasi variabel url .

Argumen tidak menggunakan 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, pemrogram lupa untuk menggunakan argumen returnType , dan menetapkan nilainya ke bidang returnType . Itu sebabnya ketika memanggil metode getReturnType dari objek, dibuat oleh konstruktor ini, null akan dikembalikan secara default. Tetapi kemungkinan besar, programmer bermaksud 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); } .... }

Memiliki 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 harus melakukan tindakan yang berlawanan. Faktanya, kedua metode melakukan hal yang sama - membuat dan mengembalikan objek ServiceAction . Kemungkinan besar, metode menonaktifkan dibuat dengan menyalin kode metode aktifkan , tetapi tubuh metode tetap sama.

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, penulis memutuskan untuk memeriksa isi struktur tipe Peta untuk null . Untuk melakukan ini, metode get dipanggil dua kali dari argumen params . Hasil dari metode get diteruskan ke metode checkNotNull . Segalanya tampak logis, tetapi tidak seperti itu! Argumen params diperiksa untuk null jika . Setelah ini diharapkan argumen input mungkin nol , tetapi sebelum pemeriksaan ini, metode get sudah dipanggil dua kali dari params. Jika nol dilewatkan sebagai argumen untuk metode ini, pertama kali Anda memanggil metode get , pengecualian akan dilemparkan.

Situasi serupa terjadi di tiga tempat lain:

  • 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 saat ini tidak dapat melakukannya tanpa penggunaan layanan cloud. Sejumlah besar orang menggunakan layanan ini. Dalam pandangan ini, bahkan kesalahan kecil dalam suatu layanan dapat menyebabkan masalah bagi banyak orang dan juga kerugian tambahan, yang ditimbulkan oleh perusahaan untuk memperbaiki konsekuensi negatif dari kesalahan ini. Kelemahan manusia harus selalu diperhitungkan terutama karena cepat atau lambat semua orang melakukan kesalahan, seperti yang dijelaskan dalam artikel ini. Fakta ini mendukung penggunaan semua alat yang mungkin untuk meningkatkan kualitas kode.

PVS-Studio pasti akan memberi tahu perusahaan Huawei tentang hasil pemeriksaan layanan cloud mereka sehingga para pengembang Huawei dapat memikirkannya, karena satu kali penggunaan analisis kode statis yang dicakup oleh artikel ini ( 1 , 2 ) tidak dapat sepenuhnya menunjukkan semua kelebihannya. Anda dapat mengunduh penganalisa PVS-Studio di sini .

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

More articles:


All Articles