WildFly (sebelumnya JBoss Application Server) adalah server aplikasi JavaEE open source yang dibuat oleh JBoss pada Februari 2008. Tujuan utama proyek WildFly adalah menyediakan seperangkat alat yang biasanya dibutuhkan oleh aplikasi Java perusahaan. Dan karena server digunakan untuk mengembangkan aplikasi perusahaan, sangat penting untuk meminimalkan jumlah kesalahan dan kemungkinan kerentanan dalam kode. Sekarang WildFly sedang dikembangkan oleh sebuah perusahaan besar Red Hat, dan kualitas kode proyek dipertahankan pada tingkat yang cukup tinggi, tetapi penganalisis masih berhasil menemukan sejumlah kesalahan dalam proyek tersebut.
Nama saya Dmitry, dan baru-baru ini saya bergabung dengan tim PVS-Studio sebagai programmer Java. Seperti yang Anda ketahui, cara terbaik untuk berkenalan dengan penganalisis kode adalah dengan mencobanya dalam praktik, jadi diputuskan untuk memilih beberapa proyek yang menarik, memeriksanya dan menulis artikel tentangnya berdasarkan hasil. Inilah yang sedang Anda baca sekarang. :)
Analisis proyek
Untuk analisis, saya menggunakan kode sumber dari proyek WildFly yang diterbitkan di GitHub . Cloc menghitung 600 ribu baris kode Java dalam proyek tersebut, tidak termasuk kolom kosong dan komentar. Pencarian kesalahan dalam kode dilakukan oleh PVS-Studio . PVS-Studio adalah alat untuk menemukan bug dan potensi kerentanan dalam kode sumber program yang ditulis dalam C, C ++, C # dan Java. Plugin penganalisis untuk IntelliJ IDEA versi 7.09 digunakan.
Sebagai hasil dari pemeriksaan proyek, hanya 491 pemicu penganalisis yang diterima, yang menunjukkan tingkat kualitas kode WildFly yang baik. Dari jumlah tersebut, 113 tinggi dan 146 sedang. Pada saat yang sama, bagian positif yang layak jatuh pada diagnostik:
- V6002. Pernyataan switch tidak mencakup semua nilai enum.
- V6008. Potensi dereferensi nol.
- V6021. Nilai diberikan ke variabel 'x' tetapi tidak digunakan.
Saya belum mempertimbangkan pemicu diagnostik ini dalam artikel, karena sulit untuk memahami apakah sebenarnya kesalahan tersebut. Peringatan semacam itu lebih dipahami oleh penulis kode.
Selanjutnya, kita akan melihat 10 pemicu penganalisis yang menurut saya paling menarik. Tanyakan - mengapa 10? Hanya karena angkanya seperti itu. :)
Jadi, ayo pergi.
Peringatan N1 adalah pernyataan bersyarat yang tidak berguna
V6004 Pernyataan 'then' setara dengan pernyataan 'else'. WeldPortableExtensionProcessor.java (61), WeldPortableExtensionProcessor.java (65).
@Override
public void deploy(DeploymentPhaseContext
phaseContext) throws DeploymentUnitProcessingException {
final DeploymentUnit deploymentUnit = phaseContext.getDeploymentUnit();
// for war modules we require a beans.xml to load portable extensions
if (PrivateSubDeploymentMarker.isPrivate(deploymentUnit)) {
if (!WeldDeploymentMarker.isPartOfWeldDeployment(deploymentUnit)) {
return;
}
} else {
// if any deployments have a beans.xml we need
// to load portable extensions
// even if this one does not.
if (!WeldDeploymentMarker.isPartOfWeldDeployment(deploymentUnit)) {
return;
}
}
}Kode di cabang if dan else sama, dan operator bersyarat tidak masuk akal dalam bentuknya saat ini. Sulit untuk memikirkan alasan mengapa pengembang menulis metode ini dengan cara ini. Kemungkinan besar, kesalahan terjadi karena salin-tempel atau pemfaktoran ulang.
N2 Peringatan - Kondisi Duplikat
V6007 Ekspresi 'poolStatsSize> 0' selalu benar. PooledConnectionFactoryStatisticsService.java (85)
@Override
public void start(StartContext context) throws StartException {
....
if (poolStatsSize > 0) {
if (registration != null) {
if (poolStatsSize > 0) {
....
}
}
}
}Dalam hal ini terjadi duplikasi kondisi. Ini tidak akan mempengaruhi hasil program, tetapi memperburuk keterbacaan kode. Namun, mungkin saja pemeriksaan kedua seharusnya berisi beberapa kondisi lain yang lebih kuat.
Contoh lain dari diagnostik ini yang dipicu di WildFly:
- V6007 Ekspresi 'referralMode == null' selalu salah. DirContext.java (93)
- V6007 Ekspresi 'mBeanServer == null' selalu benar. WildFlyServerPlatform.java (82)
- V6007 Expression 'result! = Null' selalu benar. Baru mengembalikan referensi bukan-nol. JarCheck.java (84)
- 'Hasil' V6007 Expression selalu benar. MultipleAdminObject2Impl.java (147)
Peringatan N3 - referensi dengan referensi nol
V6008 Dereferensi nol 'tc'. ExternalPooledConnectionFactoryService.java (382)
private void createService(ServiceTarget serviceTarget,
ServiceContainer container) throws Exception {
....
for (TransportConfiguration tc : connectors) {
if (tc == null) {
throw MessagingLogger.ROOT_LOGGER.connectorNotDefined(tc.getName());
}
}
....
}Mereka jelas mengacau di sini. Pertama, kami memastikan bahwa referensi adalah null, dan kemudian kami memanggil metode getName pada referensi yang sangat null ini. Ini akan menghasilkan NullPointerException, bukan pengecualian yang diharapkan dari connectorNotDefined (....).
Peringatan N4 - kode yang sangat aneh
V6019 Kode tidak dapat dijangkau terdeteksi. Mungkin saja ada kesalahan. EJB3Subsystem12Parser.java (79)
V6037 Sebuah 'lemparan' tanpa syarat dalam satu loop. EJB3Subsystem12Parser.java (81)
protected void readAttributes(final XMLExtendedStreamReader reader)
throws XMLStreamException {
for (int i = 0; i < reader.getAttributeCount(); i++) {
ParseUtils.requireNoNamespaceAttribute(reader, i);
throw ParseUtils.unexpectedAttribute(reader, i);
}
}Desain yang sangat aneh, yang membuat dua diagnostik bereaksi sekaligus: V6019 dan V6037 . Di sini hanya satu iterasi loop yang dilakukan, setelah itu keluar dengan lemparan tanpa syarat . Dengan demikian, metode readAttributes melontarkan pengecualian jika pembaca berisi setidaknya satu atribut. Loop ini dapat diganti dengan kondisi yang setara:
if(reader.getAttributeCount() > 0) {
throw ParseUtils.unexpectedAttribute(reader, 0);
}Namun, Anda dapat menggali lebih dalam dan melihat metode requireNoNamespaceAttribute (....) :
public static void requireNoNamespaceAttribute
(XMLExtendedStreamReader reader, int index)
throws XMLStreamException {
if (!isNoNamespaceAttribute(reader, index)) {
throw unexpectedAttribute(reader, index);
}
}Diketahui bahwa metode ini secara internal menampilkan pengecualian yang sama. Kemungkinan besar, metode readAttributes harus memeriksa bahwa tidak ada atribut yang ditentukan yang termasuk dalam namespace apa pun, dan bukan atribut yang hilang. Saya ingin mengatakan bahwa konstruksi seperti itu muncul sebagai hasil dari pemfaktoran ulang kode dan melemparkan pengecualian ke dalam metode requireNoNamespaceAttribute . Hanya dengan melihat riwayat komit menunjukkan bahwa semua kode ini ditambahkan pada saat yang bersamaan.
Peringatan N5 - meneruskan parameter ke konstruktor
Parameter V6022 'mechanismName' tidak digunakan di dalam badan konstruktor. DigestAuthenticationMechanism.java (144)
public DigestAuthenticationMechanism(final String realmName,
final String domain,
final String mechanismName,
final IdentityManager identityManager,
boolean validateUri) {
this(Collections.singletonList(DigestAlgorithm.MD5),
Collections.singletonList(DigestQop.AUTH),
realmName, domain, new SimpleNonceManager(),
DEFAULT_NAME, identityManager, validateUri);
}Biasanya variabel dan parameter fungsi yang tidak digunakan bukanlah sesuatu yang penting: secara umum, mereka tetap ada setelah pemfaktoran ulang atau ditambahkan untuk mengimplementasikan fungsionalitas baru di masa mendatang. Namun, operasi ini tampaknya cukup mencurigakan bagi saya:
public DigestAuthenticationMechanism
(final List<DigestAlgorithm> supportedAlgorithms,
final List<DigestQop> supportedQops,
final String realmName,
final String domain,
final NonceManager nonceManager,
final String mechanismName,
final IdentityManager identityManager,
boolean validateUri) {....}Jika Anda melihat konstruktor kedua dari kelas tersebut, Anda dapat melihat bahwa string mechanizmName diasumsikan sebagai parameter keenam . Konstruktor pertama mengambil string dengan nama yang sama dengan parameter ketiga dan memanggil konstruktor kedua. Namun, string ini tidak digunakan, dan konstanta diteruskan ke konstruktor kedua. Mungkin pembuat kode di sini berencana untuk meneruskan nama mekanisme ke konstruktor daripada konstanta DEFAULT_NAME .
Peringatan N6 - garis duplikat
V6033 Item dengan kunci yang sama 'org.apache.activemq.artemis.core.remoting.impl.netty.
TransportConstants.NIO_REMOTING_THREADS_PROPNAME 'telah ditambahkan. LegacyConnectionFactoryService.java (145), LegacyConnectionFactoryService.java (139)
private static final Map<String, String>
PARAM_KEY_MAPPING = new HashMap<>();
....
static {
PARAM_KEY_MAPPING.put(
org.apache.activemq.artemis.core.remoting.impl.netty
.TransportConstants.NIO_REMOTING_THREADS_PROPNAME,
TransportConstants.NIO_REMOTING_THREADS_PROPNAME);
....
PARAM_KEY_MAPPING.put(
org.apache.activemq.artemis.core.remoting.impl.netty
.TransportConstants.NIO_REMOTING_THREADS_PROPNAME,
TransportConstants.NIO_REMOTING_THREADS_PROPNAME);
....
}Penganalisis melaporkan bahwa dua nilai dengan kunci yang sama telah ditambahkan ke kamus. Dalam kasus ini, kecocokan nilai kunci yang ditambahkan sepenuhnya diduplikasi. Nilai yang terekam adalah konstanta di kelas TransportConstants , dan mungkin ada salah satu dari dua opsi: penulis tidak sengaja menduplikasi kode, atau lupa mengubah nilai selama salin-tempel. Selama pemeriksaan sepintas, saya tidak dapat menemukan kunci dan nilai yang hilang, jadi saya berasumsi bahwa skenario pertama lebih mungkin.
Peringatan N7 - Variabel Hilang
V6046 Format salah. Jumlah item format yang berbeda diharapkan. Argumen yang hilang: 2. TxTestUtil.java (80)
public static void addSynchronization(TransactionManager tm,
TransactionCheckerSingletonRemote checker) {
try {
addSynchronization(tm.getTransaction(), checker);
} catch (SystemException se) {
throw new RuntimeException(String
.format("Can't obtain transaction for transaction manager '%s' "
+ "to enlist add test synchronization '%s'"), se);
}
}Variabel hilang (diinginkan). Seharusnya dua baris lain akan diganti ke dalam string yang diformat, tetapi penulis kode, tampaknya, lupa menambahkannya. Memformat string tanpa argumen yang sesuai akan memunculkan IllegalFormatException, bukan RuntimeException yang diinginkan developer. Pada prinsipnya, IllegalFormatException mewarisi dari RuntimeException , tetapi pesan yang diteruskan ke pengecualian akan hilang dalam keluaran, dan akan lebih sulit untuk memahami apa yang sebenarnya salah saat melakukan debug.
Peringatan N8 - perbandingan string dengan objek
V6058 Fungsi 'sama' membandingkan objek dari jenis yang tidak kompatibel: String, ModelNode. JaxrsIntegrationProcessor.java (563)
// Send value to RESTEasy only if it's not null, empty string, or the
// default value.
private boolean isTransmittable(AttributeDefinition attribute,
ModelNode modelNode) {
if (modelNode == null || ModelType
.UNDEFINED.equals(modelNode.getType())) {
return false;
}
String value = modelNode.asString();
if ("".equals(value.trim())) {
return false;
}
return !value.equals(attribute.getDefaultValue()); // <=
}Dalam kasus ini, string dibandingkan dengan objek, dan perbandingan seperti itu selalu salah. Artinya, jika nilai modelNode sama dengan atribut.getDefaultValue () diteruskan ke metode , perilaku metode akan berubah menjadi salah, dan nilai akan dikenali sebagai valid untuk dikirim meskipun ada komentar.
Kemungkinan besar, mereka lupa memanggil metode asString () untuk merepresentasikan attribute.getDefaultValue () sebagai string. Versi yang dikoreksi mungkin terlihat seperti ini:
return !value.equals(attribute.getDefaultValue().asString());WildFly memiliki satu lagi pemicu yang mirip dari diagnostik V6058 :
- V6058 Fungsi 'sama' membandingkan objek dari jenis yang tidak kompatibel: String, ObjectTypeAttributeDefinition. DataSourceDefinition.java (141)
Peringatan N9 - Pemeriksaan Terlambat
V6060 Referensi 'dataSourceController' digunakan sebelum diverifikasi terhadap null. AbstractDataSourceAdd.java (399), AbstractDataSourceAdd.java (297)
static void secondRuntimeStep(OperationContext context, ModelNode operation,
ManagementResourceRegistration datasourceRegistration,
ModelNode model, boolean isXa) throws OperationFailedException {
final ServiceController<?> dataSourceController =
registry.getService(dataSourceServiceName);
....
dataSourceController.getService()
....
if (dataSourceController != null) {....}
....
}Penganalisis menemukan bahwa objek digunakan dalam kode jauh sebelum diperiksa untuk null , dan jarak antara mereka sebanyak 102 baris kode! Ini sangat sulit untuk diperhatikan saat menganalisis kode secara manual.
Peringatan N10 - penguncian diperiksa dua kali
V6082 Penguncian dua kali tidak aman. Objek yang ditetapkan sebelumnya dapat diganti dengan objek lain. JspApplicationContextWrapper.java (74), JspApplicationContextWrapper.java (72)
private volatile ExpressionFactory factory;
....
@Override
public ExpressionFactory getExpressionFactory() {
if (factory == null) {
synchronized (this) {
if (factory == null) {
factory = delegate.getExpressionFactory();
for (ExpressionFactoryWrapper wrapper : wrapperList) {
factory = wrapper.wrap(factory, servletContext);
}
}
}
}
return factory;
}Ini menggunakan pola "penguncian yang diperiksa dua kali", dan situasi dapat terjadi ketika metode akan mengembalikan variabel yang diinisialisasi secara tidak lengkap.
Thread A memperhatikan bahwa nilai belum diinisialisasi, sehingga memperoleh kunci dan mulai menginisialisasi nilai. Dalam kasus ini, utas berhasil menulis objek ke bidang bahkan sebelum diinisialisasi hingga akhir. Thread B mendeteksi bahwa objek telah dibuat dan mengembalikannya, meskipun thread A belum memiliki waktu untuk melakukan semua pekerjaan dengan pabrik .
Akibatnya, sebuah objek dapat dikembalikan dari metode tersebut, di mana tidak semua tindakan yang direncanakan telah dilakukan.
kesimpulan
Terlepas dari kenyataan bahwa proyek sedang dikembangkan oleh perusahaan besar Red Hat dan kualitas kode dalam proyek berada pada tingkat yang tinggi, analisis statis yang dilakukan menggunakan PVS-Studio mampu mengungkap sejumlah kesalahan yang dalam satu atau lain cara dapat mempengaruhi pengoperasian server. Dan karena WildFly dirancang untuk membuat aplikasi perusahaan, kesalahan ini dapat menyebabkan konsekuensi yang sangat menyedihkan.
Saya mengundang semua orang untuk mengunduh PVS-Studio dan memeriksa proyek mereka dengannya. Untuk melakukan ini, Anda dapat meminta lisensi uji coba atau menggunakan salah satu kasus penggunaan gratis .
Jika Anda ingin berbagi artikel ini dengan audiens berbahasa Inggris, silakan gunakan tautan terjemahan: Dmitry Scherbakov. Memeriksa WildFly, Server Aplikasi JavaEE .