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


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


لا. 10: توقيع البايت


المصدر: تحليل إطار عمل Apache Dubbo RPC من خلال محلل الشفرات الثابتة 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 #. ولكن هذا ليس كذلك في جافا.

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

لا. 9: اثنان في واحد


المصدر: PVS-Studio لجافا يضرب الطريق. المحطة التالية هي 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) × <0 سيكون دائمًا خاطئًا. الخطأ الثاني هو الفهرسة العادية وراء حدود الصفيف index_64 عندما يكون (int) x == index_64.length . يحدث هذا بسبب الشرط (int) x> index_64.length . يمكن إصلاح الخلل عن طريق تغيير ">" عامل التشغيل إلى '> =': (int) x> = index_64.length .

لا. 8: الحل وآثاره


المصدر: تحليل كود منصة 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 . للقيام بذلك ، يقوم المبرمج بإضافة 1 إلى نتيجة indexOf (المتغير menuItemFlatIndex ) وكتابة القيمة الناتجة إلى المتغير السابق MenuItemFlatIndex ، والتي يتم استخدامها بعد ذلك في الطريقة. تثبت إضافة -1 حلاً سيئًا لأنه يؤدي إلى العديد من الأخطاء في وقت واحد:

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

لا. 7: صنع ملف من لا شيء


المصدر: 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()) 

لا. 6: خطأ منطق غريب جدا


المصدر: 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 بعد أول عبارة if ، فهذا يعني أن السلسلة النصية لا تقل عن حرفين. هذا يعني أيضًا أن الاختيار الأول "0". المساواة (النص) في التالي إذا كان العبارة لا معنى له. ما الذي يعنيه هذا المبرمج في الواقع لا يزال لغزا.

لا. 5: يا له من تطور!


المصدر: 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 ، الذي يخزن نتيجة التحول bitwise (1 << (bitShiftsInWord - 1)) ، رقمًا سالبًا. ربما لم يتوقع المطور ذلك.

لا. 4: هل يمكن أن نرى استثناءات من فضلك؟


المصدر: 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 مضلل لأنه يقول إنه قد يلقي استثناء. في الواقع ، أيا كان الاستثناء الذي يتم إنشاؤه في كتلة المحاولة ، فإن المتغير الملتزم سيكون خاطئًا دائمًا ، وبالتالي فإن بيان المرتجعات في المربع الأخير سيُرجع قيمة ، في حين ستُفقد جميع الاستثناءات التي تم إلقاؤها ، ولا يمكن معالجتها خارج الطريقة. لذلك ، لن تتمكن أي من الاستثناءات التي تم طرحها في هذه الطريقة من تركها.

لا. 3: التركيز ، أو محاولة الحصول على قناع جديد


المصدر: 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] في الجزء الداخلي للحلقة. يشارك هذا العداد في تحول bitwise 1 << j . يبدو كل شيء على ما يرام ، ولكن هنا يأتي دور الحرف الصحيح "1" من النوع int (قيمة 32 بت). وبسبب ذلك ، سيبدأ التحول bitwise في إرجاع القيم التي تم إرجاعها سابقًا عندما تتجاوز قيمة المتغير j 31. إذا لم يكن هذا السلوك هو ما يريده المبرمج ، فيجب تمثيل القيمة 1 طالما : 1L << j أو (طويل) 1 <<

لا. 2: ترتيب التهيئة


المصدر: 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); // <= } } } 

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

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

ولكن لماذا يحتوي الحقل الثابت LOG على قيمة فارغة في لحظة استدعاء المنشئ؟

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

يتم منح المركز الأول في قائمة أفضل 10 قوائم لدينا لنسخ اللصق ، أو بالأحرى خلل ناجم عن الاستخدام المتهور لهذه التقنية. تشبه العبارة if إلى حد كبير نسخة من الأولى ، مع تعديل بعض المتغيرات:

  • تم تغيير localArchives إلى localFiles ؛
  • تم تغيير MRJobConfig.CACHE_LOCALARCHIVES إلى MRJobConfig.CACHE_LOCALFILES .

لكن المبرمج تمكن من ارتكاب خطأ حتى في هذه العملية البسيطة: عبارة if في السطر الذي أشار إليه المحلل لا تزال تستخدم متغير localArchives بدلاً من متغير localFiles المقصود على ما يبدو.

استنتاج


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

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

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

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


All Articles