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

الصورة 2

يعد Apache Dubbo أحد أكثر مشاريع Java شيوعًا على GitHub. هذا ليس مستغربا. تم إنشاؤه منذ 8 سنوات ويتم تطبيقه على نطاق واسع كبيئة RPC عالية الأداء. بالطبع ، تم إصلاح معظم الأخطاء في الكود الخاص بها لفترة طويلة ، ويتم الحفاظ على جودة الكود على مستوى عالٍ. ومع ذلك ، لا يوجد سبب لإلغاء الاشتراك في التحقق من هذا المشروع المثير للإعجاب باستخدام محلل الكود الثابت PVS-Studio. دعونا نرى كيف اتضح.

حول PVS-Studio


إن محلل الشفرات الثابتة PVS-Studio موجود منذ أكثر من 10 سنوات في سوق تكنولوجيا المعلومات وهو حل برمجي متعدد الوظائف وسهل تقديمه. في الوقت الحالي ، يدعم المحلل C و C ++ و C # و Java ويعمل على Windows و Linux و macOS.

PVS-Studio هو حل B2B ويستخدمه عدد كبير من الفرق في مختلف الشركات. إذا كنت ترغب في تقدير إمكانات المحلل ، فنحن نرحب بك لتنزيل التوزيعة وطلب مفتاح تجريبي هنا .

هناك أيضًا خيارات لكيفية استخدام PVS-Studio مجانًا في مشاريع مفتوحة المصدر أو إذا كنت طالبًا.

أباتشي دوبو: ما هو والميزات الرئيسية


في الوقت الحاضر ، يتم توزيع تقريبا جميع أنظمة البرمجيات الكبيرة. إذا كان هناك نظام تفاعلي في النظام الموزع بين المكونات البعيدة ذات الكمون المنخفض والقليل من البيانات التي يتم تمريرها ، فهذا سبب قوي لاستخدام بيئة RPC (استدعاء الإجراء عن بُعد).

Apache Dubbo هي بيئة RPC (استدعاء إجراء عن بُعد) عالية الأداء مع شفرة مفتوحة المصدر تعتمد على Java. مثل العديد من أنظمة RPC الأخرى ، يعتمد dubbo على فكرة إنشاء خدمة تحدد بعض الطرق التي يمكن استدعاؤها عن بعد بمعلماتها وأنواع قيم الإرجاع الخاصة بها. على جانب الخادم ، يتم تنفيذ واجهة ويتم تشغيل خادم dubbo للتعامل مع استفسارات العملاء. من جانب العميل ، هناك كعب روتين يوفر نفس الأساليب التي يوفرها الخادم. يقترح Dubbo ثلاث وظائف رئيسية ، بما في ذلك الاتصال عن بُعد بالواجهة والتسامح مع الفشل وموازنة التحميل ، بالإضافة إلى التسجيل التلقائي واكتشاف الخدمة.

حول التحليل


تسلسل الإجراءات للتحليل بسيط للغاية ولم يتطلب الكثير من الوقت في حالتي:

  • تم تحميل Apache Dubbo من GitHub ؛
  • استخدام تعليمات بدء محلل Java وتشغيل التحليل ؛
  • حصلت على تقرير محلل ، استعرضته وأبرز حالات مثيرة للاهتمام.

نتائج التحليل: تم إصدار 73 تحذيرا من مستويات اليقين العالية والمتوسطة (46 و 27 ، على التوالي) لأكثر من 4000 ملف ، وهو مؤشر جيد لجودة الشفرة.

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

لم أفكر في 9 تحذيرات (7 مرتفعة و 2 متوسطة) ، تتعلق بالملفات مع الاختبارات.

نتيجة لذلك ، تلقيت عددًا قليلًا من التحذيرات ، والتي تضمنت أيضًا إيجابيات كاذبة ، لأنني لم أقم بتكوين المحلل. الخوض في جميع التحذيرات الـ 73 مع مزيد من الوصف في المقالة طويل ومثير للسخرية وممل ، لذا اخترت أكثر التحذيرات.

تسجيل بايت في جافا


تعبير 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); } 

تتم مقارنة قيمة نوع البايت (القيمة من -128 إلى 127) بالقيمة 0xff (255). في هذه الحالة ، لا يؤخذ في الاعتبار أن نوع البايت في Java قد تم توقيعه ، وبالتالي فإن الشرط سيكون دائمًا صحيحًا ، مما يعني أنه لا معنى له.

عودة نفس القيم


تعبير V6007 'isPreferIPV6Address ()' غير صحيح دائمًا. NetUtils.java (236)

 private static Optional<InetAddress> toValidAddress(InetAddress address) { if (address instanceof Inet6Address) { Inet6Address v6Address = (Inet6Address) address; if (isPreferIPV6Address()) { // <= return Optional.ofNullable(normalizeV6Address(v6Address)); } } if (isValidV4Address(address)) { return Optional.of(address); } return Optional.empty(); } 

الأسلوب هو PreferIPV6Address .

 static boolean isPreferIPV6Address() { boolean preferIpv6 = Boolean.getBoolean("java.net.preferIPv6Addresses"); if (!preferIpv6) { return false; // <= } return false; // <= } 

إرجاع الأسلوب isPreferIPV6Address false في كلتا الحالتين. على الأرجح ، أراد أحد المطورين عودة حالة واحدة إلى الحالة الحقيقية ، وإلا فإن الطريقة لا معنى لها.

الشيكات غير المجدية


تعبير V6007 '! قناع [i]. المساواة (ipAddress [i])' صحيح دائمًا. NetUtils.java (476)

 public static boolean matchIpRange(....) throws UnknownHostException { .... for (int i = 0; i < mask.length; i++) { if ("*".equals(mask[i]) || mask[i].equals(ipAddress[i])) { continue; } else if (mask[i].contains("-")) { .... } else if (....) { continue; } else if (!mask[i].equals(ipAddress[i])) { // <= return false; } } return true; } 

في حالة if-else-if ، تحقق من "*". يساوي (قناع [i]) || يتم تنفيذ قناع [i]. المساواة (ipAddress [i]) . إذا لم يتم استيفاء الشرط ، فسوف يتحقق تسجيل الدخول التالي في حالة حدوث شيء آخر ، مما يوضح لنا أن القناع [i] و ipAddress [i] ليسا متساوين. ولكن في أحد عمليات التدقيق التالية ، إذا كان الأمر كذلك ، إذا تم التحقق من أن القناع [i] و ipAddress [i] ليسا متساوين. نظرًا لأن القناع [i] و ipAddress [i ] لم يتم تعيينهما لأي قيم ، فإن الفحص الثاني لا معنى له.

تعبير V6007 'message.length> 0' صحيح دائمًا. DeprecatedTelnetCodec.java (302)

تعبير V6007 'message! = Null' صحيح دائمًا. DeprecatedTelnetCodec.java (302)

 protected Object decode(.... , byte[] message) throws IOException { .... if (message == null || message.length == 0) { return NEED_MORE_INPUT; } .... // Here the variable <i>message </i> doesn't change! .... if (....) { String value = history.get(index); if (value != null) { byte[] b1 = value.getBytes(); if (message != null && message.length > 0) { // <= byte[] b2 = new byte[b1.length + message.length]; System.arraycopy(b1, 0, b2, 0, b1.length); System.arraycopy(message, 0, b2, b1.length, message.length); message = b2; } else { message = b1; } } } .... } 

رسالة التحقق ! = خالية && message.length> 0 في السطر 302 لا لزوم لها. قبل تسجيل الدخول في السطر 302 ، يتم إجراء الفحص التالي:

 if (message == null || message.length == 0) { return NEED_MORE_INPUT; } 

إذا لم تستوف شرط التحقق ، فسنعلم أن الرسالة ليست خالية وأن طولها لا يساوي 0. ويترتب على ذلك أن طولها يزيد عن 0 (نظرًا لأن طول السلسلة لا يمكن أن يكون رقم سالب). لم يتم تعيين رسالة المتغير المحلي أي قيمة قبل السطر 302 ، مما يعني في السطر 302 قيمة متغير الرسالة هي نفسها كما في الكود أعلاه. قد يتم استنتاج أن تعبير الرسالة! = Null && message.length> 0 سيكون دائمًا صحيحًا ، ولكن لن يتم تنفيذ التعليمات البرمجية في الكتلة الأخرى.

تحديد قيمة حقل مرجع غير مهيأ


التعبير V6007 '! ShouldExport ()' غير صحيح دائمًا. ServiceConfig.java (371)

 public synchronized void export() { checkAndUpdateSubConfigs(); if (!shouldExport()) { // <= return; } if (shouldDelay()) { .... } else { doExport(); } 

يستدعي الأسلوب shouldExport للفئة ServiceConfig أسلوب getExport ، المعرف في نفس الفئة.

 private boolean shouldExport() { Boolean export = getExport(); // default value is true return export == null ? true : export; } .... @Override public Boolean getExport() { return (export == null && provider != null) ? provider.getExport() : export; } 

يستدعي الأسلوب getExport طريقة getExport لفئة AbstractServiceConfig المجردة ، والتي تُرجع قيمة حقل التصدير من النوع Boolean . هناك أيضًا طريقة setExport لتعيين قيمة الحقل.

 protected Boolean export; .... public Boolean getExport() { return export; } .... public void setExport(Boolean export) { this.export = export; } 

يتم تعيين حقل التصدير في التعليمات البرمجية فقط بواسطة طريقة setExport . يتم استدعاء أسلوب setExport فقط في طريقة الإنشاء لفئة AbstractServiceBuilder المجردة (توسيع AbstractServiceConfig ) فقط في حالة إذا كان الحقل غير فارغ.

 @Override public void build(T instance) { .... if (export != null) { instance.setExport(export); } .... } 

نظرًا لحقيقة أن جميع الحقول المرجعية بشكل افتراضي تتم تهيئتها بواسطة null وأن حقل التصدير لم يتم تعيين قيمة له ، فلن يتم استدعاء طريقة setExport أبدًا.

كنتيجة لذلك ، فإن طريقة getExport لفئة ServiceConfig ، وتوسيع فئة AbstractServiceConfig ستظل دائمًا خالية . يتم استخدام قيمة الإرجاع في الأسلوب shouldExport لفئة ServiceConfig ، وبالتالي فإن الأسلوب shouldExport سيعود دائمًا صواب . بسبب إرجاع true ، ستكون قيمة التعبير ! ShouldExport () دومًا خاطئة. اتضح أنه لن يتم إرجاع حقل التصدير للفئة ServiceConfig أبدًا حتى يتم تنفيذ التعليمات البرمجية التالية:

 if (shouldDelay()) { DELAY_EXPORT_EXECUTOR.schedule(this::doExport, getDelay(), ....); } else { doExport(); } 

قيمة المعلمة غير السلبية


V6009 قد تتلقى الدالة "سلسلة فرعية" القيمة "-1" بينما من المتوقع أن تكون القيمة غير السالبة. تفقد الوسيطة: 2. AbstractEtcdClient.java (169)

 protected void createParentIfAbsent(String fixedPath) { int i = fixedPath.lastIndexOf('/'); if (i > 0) { String parentPath = fixedPath.substring(0, i); if (categories.stream().anyMatch(c -> fixedPath.endsWith(c))) { if (!checkExists(parentPath)) { this.doCreatePersistent(parentPath); } } else if (categories.stream().anyMatch(c -> parentPath.endsWith(c))) { String grandfather = parentPath .substring(0, parentPath.lastIndexOf('/')); // <= if (!checkExists(grandfather)) { this.doCreatePersistent(grandfather); } } } } 

يتم تمرير نتيجة دالة lastIndexOf كمعلمة ثانية إلى وظيفة السلسلة الفرعية ، التي يجب ألا تكون معلمتها الثانية رقماً سالباً ، على الرغم من أن lastIndexOf يمكن أن تُرجع -1 إذا لم تعثر على القيمة المطلوبة في السلسلة. إذا كانت المعلمة الثانية لأسلوب السلسلة الفرعية أقل من الأولى (-1 <0) ، فسيتم طرح الاستثناء StringIndexOutOfBoundsException . لإصلاح الخطأ ، نحتاج إلى التحقق من النتيجة ، التي يتم إرجاعها بواسطة الدالة lastIndexOf . إذا كان هذا صحيحًا (على الأقل وليس سالبًا) ، فقم بتمريره إلى وظيفة السلسلة الفرعية .

عداد حلقة غير المستخدمة


V6016 وصول مشبوه إلى عنصر كائن "أنواع" بفهرس ثابت داخل حلقة. RpcUtils.java (153)

 public static Class<?>[] getParameterTypes(Invocation invocation) { if ($INVOKE.equals(invocation.getMethodName()) && invocation.getArguments() != null && invocation.getArguments().length > 1 && invocation.getArguments()[1] instanceof String[]) { String[] types = (String[]) invocation.getArguments()[1]; if (types == null) { return new Class<?>[0]; } Class<?>[] parameterTypes = new Class<?>[types.length]; for (int i = 0; i < types.length; i++) { parameterTypes[i] = ReflectUtils.forName(types[0]); // <= } return parameterTypes; } return invocation.getParameterTypes(); } 

في حلقة for ، يتم استخدام فهرس ثابت 0 للوصول إلى عنصر أنواع الصفيف. قد يكون الغرض منه استخدام المتغير i كدليل للوصول إلى عناصر الصفيف. لكن المؤلفين تركوا ذلك.

لا طائل منه القيام به


V6019 تم اكتشاف كود غير قابل للوصول. من الممكن وجود خطأ. GrizzlyCodecAdapter.java (136)

 @Override public NextAction handleRead(FilterChainContext context) throws IOException { .... do { savedReadIndex = frame.readerIndex(); try { msg = codec.decode(channel, frame); } catch (Exception e) { previousData = ChannelBuffers.EMPTY_BUFFER; throw new IOException(e.getMessage(), e); } if (msg == Codec2.DecodeResult.NEED_MORE_INPUT) { frame.readerIndex(savedReadIndex); return context.getStopAction(); } else { if (savedReadIndex == frame.readerIndex()) { previousData = ChannelBuffers.EMPTY_BUFFER; throw new IOException("Decode without read data."); } if (msg != null) { context.setMessage(msg); return context.getInvokeAction(); } else { return context.getInvokeAction(); } } } while (frame.readable()); // <= .... } 

التعبير في حالة الحلقة do - بينما (frame.readable ()) هو رمز غير قابل للوصول ، حيث يتم إنهاء الطريقة أثناء التكرار الأول للحلقة. يتم إجراء العديد من الاختبارات لمتغير msg في نص الحلقة باستخدام if-else. في القيام بذلك ، سواء في أو في حالة إرجاع القيم من الطريقة أو يتم طرح استثناءات. هذا هو السبب في أن جسم الحلقة ينفذ مرة واحدة فقط ، لذلك فإن استخدام هذه الحلقة ليس له أية فائدة.

نسخ لصق في التبديل


V6067 يقوم اثنان أو أكثر من فروع الحالة بتنفيذ نفس الإجراءات. JVMUtil.java (67) ، JVMUtil.java (71)

 private static String getThreadDumpString(ThreadInfo threadInfo) { .... if (i == 0 && threadInfo.getLockInfo() != null) { Thread.State ts = threadInfo.getThreadState(); switch (ts) { case BLOCKED: sb.append("\t- blocked on " + threadInfo.getLockInfo()); sb.append('\n'); break; case WAITING: // <= sb.append("\t- waiting on " + threadInfo.getLockInfo()); // <= sb.append('\n'); // <= break; // <= case TIMED_WAITING: // <= sb.append("\t- waiting on " + threadInfo.getLockInfo()); // <= sb.append('\n'); // <= break; // <= default: } } .... } 

يحتوي رمز التبديل لحالات الانتظار و TIMED_WAITING على رمز لصق النسخ. إذا كان يجب تنفيذ نفس الإجراءات ، فيمكن تبسيط الرمز عن طريق إزالة محتوى كتلة حالة الانتظار . نتيجةً لذلك ، سيتم تنفيذ نفس الرمز لـ WAITING و TIMED_WAITING.

استنتاج


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

المذكرة. هذه الفحوصات لمرة واحدة توضح قدرات محلل الكود الثابت ، ولكنها تمثل طريقة خاطئة تمامًا لاستخدامه. ويرد المزيد من التفاصيل عن هذه الفكرة هنا وهنا . استخدم التحليل بانتظام!

بفضل مطوري Apache Dubbo على هذه الأداة الرائعة. آمل أن تساعدك هذه المقالة على تحسين الكود. لا تصف المقالة جميع الأجزاء المشبوهة من التعليمات البرمجية ، لذلك من الأفضل للمطورين التحقق من المشروع بأنفسهم وتقييم النتائج.

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


All Articles