أفضل 10 أخطاء في مشاريع Java لعام 2019


يقترب عام 2019 من نهايته ، ويقوم فريق PVS-Studio بتلخيص نتائج السنة المنتهية ولايتها. في بداية عام 2019 ، قمنا بتوسيع قدرات المحلل من خلال دعم لغة جافا. لذلك ، تم تجديد قائمة منشوراتنا حول التحقق من المشروعات المفتوحة بمراجعات لمشاريع Java. تم العثور على الكثير من الأخطاء خلال العام ، وقررنا إعداد أفضل 10 أهمها.


المركز العاشر: بايت مبدع


المصدر: تحليل الشفرة المصدرية لإطار RPC Apache Dubbo بواسطة محلل ثابت PVS-Studio

تعبير V6007 'endKey [i] <0xff' صحيح دائمًا. 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); } 

يعتقد العديد من المبرمجين أنه لن يتم توقيع نوع اسمه بايت . وبالفعل ، هذا هو الحال تمامًا بلغات مختلفة. على سبيل المثال ، في C # ، نوع البايت غير موقع. في Java ، هذا ليس هو الحال.

في الشرط endKey [i] <0xff ، يقارن مؤلف الطريقة متغير نوع البايت بالرقم 255 (0xff) الممثلة في التمثيل السداسي عشر. على ما يبدو ، عند كتابة الطريقة ، نسي المطور أن نطاق قيم نوع البايت في Java هو [-128 ، 127]. هذا الشرط دائمًا صحيح ، لذلك ستعمل حلقة for دائمًا على معالجة العنصر الأخير من مجموعة endKey فقط .

المركز التاسع: اثنان في واحد


المصدر: يتم إرسال PVS-Studio for Java إلى المسار. المحطة التالية هي Elasticsearch

تعبير V6007 '(int) x <0' غير صحيح دائمًا. BCrypt.java (429)

V6025 ربما يكون الفهرس '(int) x' خارج الحدود. BCrypt.java (431)

 private static byte char64(char x) { if ((int)x < 0 || (int)x > index_64.length) return -1; return index_64[(int)x]; } 

اليوم لدينا عرض خاص! خطأين في طريقة واحدة في وقت واحد. سبب الخطأ الأول هو نوع char ، الذي لم يتم توقيعه في Java ، وهذا هو سبب كون الشرط (int) x <0 خاطئًا دائمًا. الخطأ الثاني هو الخروج العادي من حدود الصفيف index_64 عندما (int) x == index_64.length . هذا الموقف ممكن بسبب الشرط (int) x> index_64.length . للتخلص من الخروج عن حدود الصفيف ، من الضروري استبدال الشرط '>' بـ '> ='. الحالة الصحيحة ستكون: (int) x> = index_64.length .

المركز الثامن: القرار وتبعاته


المصدر: تحليل كود منصة CUBA باستخدام PVS-Studio

تعبير V6007 'previousMenuItemFlatIndex> = 0' صحيح دائمًا. CubaSideMenuWidget.java (328)

 protected MenuItemWidget findNextMenuItem(MenuItemWidget currentItem) { List<MenuTreeNode> menuTree = buildVisibleTree(this); List<MenuItemWidget> menuItemWidgets = menuTreeToList(menuTree); int menuItemFlatIndex = menuItemWidgets.indexOf(currentItem); int previousMenuItemFlatIndex = menuItemFlatIndex + 1; if (previousMenuItemFlatIndex >= 0) { // <= return menuItemWidgets.get(previousMenuItemFlatIndex); } return null; } 

يريد مؤلف طريقة findNextMenuItem التخلص من -1 التي يتم إرجاعها بواسطة طريقة indexOf إذا لم تكن قائمة menuItemWidgets تحتوي على currentItem . للقيام بذلك ، يضيف واحدًا إلى indexOf نتيجة ( menuItemFlatIndex متغير) ويخزن القيمة الناتجة في المتغير السابق MenuItemFlatIndex ، والذي يُستخدم أكثر في الطريقة. هذا الحل للمشكلة -1 غير ناجح لأنه يؤدي إلى العديد من الأخطاء في وقت واحد:

  • لن يتم تنفيذ الإرجاع رمز فارغ ، لأن التعبير السابق MenuItemFlatIndex > = 0 صحيح دائمًا ، مما يعني أن الإرجاع من أسلوب findNextMenuItem سيحدث دائمًا داخل if ؛
  • سيتم طرح IndexOutOfBoundsException عندما تكون قائمة menuItemWidgets فارغة ، لأنه سيتم الوصول إلى العنصر الأول من القائمة الفارغة ؛
  • سيحدث استثناء IndexOutOfBoundsException عندما تكون الوسيطة currentItem هي الأخيرة في قائمة menuItemWidget .

المركز السابع: إنشاء ملف من لا شيء


المصدر: Huawei Cloud: إنه غائم في PVS-Studio اليوم

V6008 dereference خالية من "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; } } .... } 

عند كتابة طريقة putToCache ، قام المبرمج بعمل خطأ مطبعي في بيانات الحالة .TmpFile == null || ! dataTmpFile.exists () قبل إنشاء ملف dataTmpFile.createNewFile () جديد . الخطأ المطبعي هو استخدام العامل '==' بدلاً من '! ='. هذا الخطأ المطبعي سوف يرمي NullPointerException عند استدعاء الأسلوب createNewFile . الشرط بعد تصحيح الخطأ المطبعي يبدو كما يلي:

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

"تم العثور على الخطأ ، تم تصحيحه. يمكنك الاسترخاء ، "سوف تفكر. ولكن لا يهم كيف!

بعد تصحيح خطأ واحد ، وجدنا خطأ آخر. الآن ، يمكن أن يحدث NullPointerException عند استدعاء dataTmpFile.exists () . الآن ، للتخلص من الاستثناء ، من الضروري استبدال عامل التشغيل "||" في الحالة على '&&'. الشرط الذي تختفي به جميع الأخطاء سيكون كما يلي:

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

المركز السادس: خطأ منطقي غريب جدا


المصدر: PVS-Studio لجافا

V6007 [CWE-570] التعبير '"0". Equals (text)' غير صحيح دائمًا. ConvertIntegerToDecimalPredicate.java 46

 public boolean satisfiedBy(@NotNull PsiElement element) { .... @NonNls final String text = expression.getText().replaceAll("_", ""); if (text == null || text.length() < 2) { return false; } if ("0".equals(text) || "0L".equals(text) || "0l".equals(text)) {// <= return false; } return text.charAt(0) == '0'; } 

هذه الطريقة مثيرة للاهتمام لأنها تحتوي على خطأ منطقي واضح. إذا لم تُرجع طريقة satisfBy قيمة بعد الأولى ، فيُعرف أن السلسلة النصية تتكون من حرفين على الأقل. وبسبب هذا ، فإن الاختيار الأول "0". المساواة (النص) في التالي إذا كان لا معنى له. ما يعنيه المطور حقا لا يزال لغزا.

المركز الخامس: هذا منعطف!


المصدر: PVS-Studio يزور أباتشي هايف

V6034 التحول من قيمة 'bitShiftsInWord - 1' قد لا يتعارض مع حجم النوع: 'bitShiftsInWord - 1' = [-1 ... 30]. UnsignedInt128.java (1791)

 private void shiftRightDestructive(int wordShifts, int bitShiftsInWord, boolean roundUp) { if (wordShifts == 0 && bitShiftsInWord == 0) { return; } assert (wordShifts >= 0); assert (bitShiftsInWord >= 0); assert (bitShiftsInWord < 32); if (wordShifts >= 4) { zeroClear(); return; } final int shiftRestore = 32 - bitShiftsInWord; // check this because "123 << 32" will be 123. final boolean noRestore = bitShiftsInWord == 0; final int roundCarryNoRestoreMask = 1 << 31; final int roundCarryMask = (1 << (bitShiftsInWord - 1)); // <= .... } 

باستخدام وسيطات الإدخال wordShifts = 3 و bitShiftsInWord = 0 ، سيصبح متغير roundCarryMask ، الذي يخزن نتيجة تحول البت (1 << (bitShiftsInWord - 1)) ، رقمًا سالبًا. ربما لم يتوقع المطور هذا السلوك.

المركز الرابع: هل ستأتي الاستثناءات للنزهة؟


المصدر: PVS-Studio يزور أباتشي هايف

V6051 يمكن أن يؤدي استخدام عبارة "المرتجعات" في المربع "أخيرًا" إلى فقد الاستثناءات غير المعالجة. ObjectStore.java (9080)

 private List<MPartitionColumnStatistics> getMPartitionColumnStatistics(....) throws NoSuchObjectException, MetaException { boolean committed = false; try { .... /*some actions*/ committed = commitTransaction(); return result; } catch (Exception ex) { LOG.error("Error retrieving statistics via jdo", ex); if (ex instanceof MetaException) { throw (MetaException) ex; } throw new MetaException(ex.getMessage()); } finally { if (!committed) { rollbackTransaction(); return Lists.newArrayList(); } } } 

إن إعلان أسلوب getMPartitionColumnStatistics يكذب علينا ، قائلاً إنه يمكن أن يلقي استثناءً. عند حدوث أي استثناء في المحاولة ، يظل المتغير الملتزم خاطئًا ، وبالتالي ، في العبارة الأخيرة ، تُرجع عبارة الإرجاع القيمة من الطريقة ، وتُفقد جميع الاستثناءات التي يتم طرحها ولا يمكن معالجتها خارج الطريقة. وبالتالي ، فإن أي استثناء يثير في هذه الطريقة لن يكون قادرًا على الخروج منه.

المركز الثالث: أنا أتطور ، أقلب ، أريد الحصول على قناع جديد


المصدر: PVS-Studio يزور أباتشي هايف

V6034 التحول من قيمة 'j' يمكن أن يكون غير متوافق مع حجم النوع: 'j' = [0 ... 63]. IoTrace.java (272)

 public void logSargResult(int stripeIx, boolean[] rgsToRead) { .... for (int i = 0, valOffset = 0; i < elements; ++i, valOffset += 64) { long val = 0; for (int j = 0; j < 64; ++j) { int ix = valOffset + j; if (rgsToRead.length == ix) break; if (!rgsToRead[ix]) continue; val = val | (1 << j); // <= } .... } .... } 

خطأ آخر يتعلق بالتحول bitwise ، ولكن هذه المرة ليس فقط كان متورطا في القضية. في الجزء الداخلي للحلقة ، يتم استخدام المتغير j [0 ... 63] كعداد العداد. ويشارك هذا العداد في تحول قليلا من 1 << ي . ومع ذلك ، لا يوجد شيء ينذر بحدوث مشكلة ، ومع ذلك ، فإن الرقم الصحيح الحرفي '1' من النوع int (قيمة 32 بت) يتم تشغيله هنا. يترتب على ذلك أن نتائج تحول البت ستبدأ في التكرار بعد أن تكون j أكبر من 31. إذا كان السلوك الموصوف غير مرغوب فيه ، فيجب تمثيل الوحدة طويلة ، على سبيل المثال ، 1L << j أو (طويل) 1 << j .

المركز الثاني: ترتيب التهيئة


المصدر: Huawei Cloud: إنه غائم في PVS-Studio اليوم

دورة تهيئة الفئة V6050 موجودة. تظهر تهيئة "INSTANCE" قبل تهيئة "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); // <= } } } 

يعد الترتيب الذي يتم فيه الإعلان عن الحقول في الفصل مهمًا لأنه تتم تهيئة الحقول بالترتيب الذي تم إعلانه به. ومع ذلك ، عندما ينسى الأمر ، تحدث أخطاء خفية ، مثل هذا الخطأ.

أشار المحلل إلى أنه يتم إلغاء تحديد حقل السجل الثابت في المُنشئ عندما يتم تهيئته إلى null ، مما يؤدي إلى سلسلة استثناء NullPointerException -> ExceptionInInitializerError .

"لماذا ، في وقت استدعاء المنشئ ، يكون حقل السجل الثابت باطلاً ؟"

استثناء ExceptionInInitializerError هو تلميح. الحقيقة هي أن هذا المُنشئ يُستخدم لتهيئة حقل ثابت INSTANCE الذي تم الإعلان عنه في الفئة قبل حقل السجل . لذلك ، في وقت استدعاء المنشئ ، لا يزال حقل السجل غير مهيأ. لكي يعمل الرمز بشكل صحيح ، من الضروري تهيئة حقل السجل قبل استدعاء المنشئ.

المركز الأول: البرمجة الموجهة نحو النسخ واللصق


المصدر: كود اباتشي Hadoop الجودة: إنتاج اختبار VS

V6072 تم العثور على شظايا كود مماثلة. ربما ، هذا خطأ مطبعي ويجب استخدام متغير "localFiles" بدلاً من "localArchives". LocalDistributedCacheManager.java (183) ، LocalDistributedCacheManager.java (178) ، LocalDistributedCacheManager.java (176) ، LocalDistributedCacheManager.java (181)

 public synchronized void setup(JobConf conf, JobID jobId) throws IOException { .... // Update the configuration object with localized data. if (!localArchives.isEmpty()) { conf.set(MRJobConfig.CACHE_LOCALARCHIVES, StringUtils .arrayToString(localArchives.toArray(new String[localArchives // <= .size()]))); } if (!localFiles.isEmpty()) { conf.set(MRJobConfig.CACHE_LOCALFILES, StringUtils .arrayToString(localFiles.toArray(new String[localArchives // <= .size()]))); } .... } 

والموضع الأول يتم عن طريق لصق النسخ ، أو بالأحرى خطأ نشأ بسبب إهمال الشخص الذي ارتكب هذا الشيء الخاطئ. من المحتمل جدًا أن يكون الثاني إذا تم إنشاؤه بواسطة نسخ ولصق الأول باستبدال المتغيرات:

  • localArchives on localFiles ؛
  • MRJobConfig.CACHE_LOCALARCHIVES في MRJobConfig.CACHE_LOCALFILES .

ومع ذلك ، حتى مع هذه العملية البسيطة ، حدث خطأ ، حيث أن متغير LocalArchives كان لا يزال يستخدم في السطر الثاني في السطر الثاني في المحلل الثاني ، على الرغم من أن استخدام ملف localFiles كان مرجحًا على الأرجح.

استنتاج


يتطلب تصحيح الأخطاء التي تم العثور عليها في المراحل اللاحقة من التطوير أو بعد إصدار المشروع موارد كبيرة. يعمل محلل ثابت PVS-Studio على تبسيط عملية اكتشاف الأخطاء عند كتابة التعليمات البرمجية ، مما يقلل بشكل كبير من مقدار الموارد التي تنفق على إصلاحها. الاستخدام المستمر للمحلل قد أبسط بالفعل حياة المطورين من العديد من الشركات . إذا كنت ترغب في البرمجة بسرور كبير ، فجرّب المحلل الخاص بنا.

فريقنا لن يتوقف عند هذا الحد وسيواصل تحسين المحلل وتحسينه. توقع تشخيصات ومقالات جديدة مع أخطاء أكثر إثارة للاهتمام العام المقبل.

أشاهد أنت تحب المغامرة! أولاً ، فازت الأخطاء العشرة الأولى في مشاريع C # لعام 2019 ، والآن تمكنت Java من التغلب عليها! مرحبًا بك في المستوى التالي في المقالة حول أفضل أخطاء 2019 في مشاريع C ++ .





إذا كنت ترغب في مشاركة هذه المقالة مع جمهور يتحدث الإنجليزية ، فالرجاء استخدام الرابط الخاص بالترجمة: Valery Komarov. تم العثور على أفضل 10 أخطاء في مشاريع Java في عام 2019 .

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


All Articles