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

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

حول التحليل

تسلسل خطوات التحليل بسيط للغاية ولا يتطلب الكثير من الوقت:

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

إرجاع نفس القيمة

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

طريقة IsPreferIPV6Address .

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

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

الشيكات بلا معنى

تعبير 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-if-if ، يحدث تحقق "*". يساوي (قناع [i]) || mask [i] .equals (ipAddress [i]) . إذا لم يتم استيفاء هذا الشرط ، فستنتقل التعليمة البرمجية إلى الاختيار التالي في if-else-if ، ونحن ندرك أن القناع [i] و ipAddress [i] غير متكافئين. ولكن أحد عمليات التدقيق التالية في if-else-if فقط يتحقق من أن القناع [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; } .... //   message  ! .... 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. ويترتب على هذه المعلومات أن طولها أكبر من الصفر (لأن طول السلسلة لا يمكن أن يكون رقمًا سالبًا). لم يتم تعيين أي من قيم الرسالة المتغيرة المحلية قبل السطر 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); } .... } 

نظرًا لحقيقة أن جميع الحقول المرجعية تتم تهيئتها للإلغاء بشكل افتراضي وأن حقل التصدير لم يتم تعيينه لأي قيمة في أي مكان في التعليمات البرمجية ، فلن يتم استدعاء طريقة 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()); // <= .... } 

التعبير في حالة الحلقة - - (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: } } .... } 

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

استنتاج

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

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

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

إذا كنت ترغب في مشاركة هذه المقالة مع جمهور يتحدث الإنجليزية ، فالرجاء استخدام الرابط الخاص بالترجمة: Valery Komarov. تحليل إطار عمل Apache Dubbo RPC بواسطة محلل الكود الثابت في PVS-Studio .