التحقق من WildFly - خادم تطبيق JavaEE

WildFly (خادم تطبيق JBoss سابقًا) هو خادم تطبيق JavaEE مفتوح المصدر تم إنشاؤه بواسطة JBoss في فبراير 2008. الهدف الرئيسي لمشروع WildFly هو توفير مجموعة من الأدوات التي تحتاجها تطبيقات Java للمؤسسات عادةً. ونظرًا لاستخدام الخادم لتطوير تطبيقات المؤسسة ، فمن المهم بشكل خاص تقليل عدد الأخطاء ونقاط الضعف المحتملة في التعليمات البرمجية. يتم الآن تطوير WildFly بواسطة شركة Red Hat الكبيرة ، ويتم الحفاظ على جودة رمز المشروع عند مستوى عالٍ إلى حد ما ، لكن المحلل لا يزال قادرًا على العثور على عدد من الأخطاء في المشروع.



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

تحليل المشروع

بالنسبة للتحليل ، استخدمت الكود المصدري لمشروع WildFly المنشور على GitHub . أحصى Cloc 600 ألف سطر من كود Java في المشروع ، باستثناء الفراغات والتعليقات. تم البحث عن الأخطاء في الكود بواسطة PVS-Studio . PVS-Studio هي أداة لاكتشاف الأخطاء ونقاط الضعف المحتملة في الكود المصدري للبرامج المكتوبة بلغة C و C ++ و C # و Java. تم استخدام المكون الإضافي للمحلل للإصدار 7.09 من IntelliJ IDEA.



نتيجة للتحقق من المشروع ، تم استلام 491 مشغل محلل فقط ، مما يشير إلى مستوى جيد من جودة رمز WildFly. من بين هؤلاء ، 113 عالية و 146 متوسطة. في الوقت نفسه ، يقع جزء لائق من الاكتشافات على التشخيص:

• V6002. لا تغطي عبارة التبديل كافة قيم التعداد.
• V6008. مرجع فارغ محتمل.
• V6021. يتم تعيين القيمة إلى المتغير "x" ولكن لم يتم استخدامها.

لم أفكر في بدء تشغيل هذه التشخيصات في المقالة ، لأنه من الصعب فهم ما إذا كانت في الواقع أخطاء. يفهم مؤلفو الكود هذه التحذيرات بشكل أفضل.



بعد ذلك ، سننظر في 10 مشغلات محلل وجدتها أكثر إثارة للاهتمام. اسأل - لماذا 10؟ فقط لأن الرقم يشبهه. :)



لذا ، دعنا نذهب.

تحذير N1 عبارة شرطية عديمة الفائدة

V6004 عبارة "then" تعادل جملة "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;
        }
    }
}

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

تحذير N2 - شروط مكررة

V6007 تعبير "poolStatsSize> 0" يكون دائمًا صحيحًا. PooledConnectionFactoryStatisticsService.java (85)

@Override
public void start(StartContext context) throws StartException {
  ....
  if (poolStatsSize > 0) {
    if (registration != null) {
      if (poolStatsSize > 0) {
        ....
      }
    }
  }
}

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



أمثلة أخرى لهذا التشخيص الذي يتم تشغيله في WildFly:

• V6007 Expression 'ReferalMode == null' هو دائمًا خطأ. DirContext.java (93)
• يكون تعبير V6007 "mBeanServer == فارغ" دائمًا صحيحًا. WildFlyServerPlatform.java (82)
• V6007 Expression 'نتيجة! = Null' دائمًا صحيح. إرجاع مرجعي جديد غير فارغ. JarCheck.java (84)
• يكون تعبير "النتيجة" V6007 صحيحًا دائمًا. MultipleAdminObject2Impl.java (147)

تحذير N3 - مرجع بمرجع فارغ

V6008 لا يوجد مرجع لـ "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());
     }
   }
   ....
}

من الواضح أنهم أفسدوا هنا. أولاً ، نتأكد من أن المرجع فارغ ، ثم نسمي طريقة getName على هذا المرجع الفارغ للغاية. سينتج عن ذلك NullPointerException بدلاً من الاستثناء المتوقع من connectorNotDefined (....).

تحذير N4 - كود غريب جدا

تم اكتشاف رمز لا يمكن الوصول إليه V6019 . من الممكن أن يكون هناك خطأ. EJB3Subsystem12Parser.java (79)



V6037 "رمية" غير مشروطة داخل حلقة. 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);
    }
}

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

if(reader.getAttributeCount() > 0) {
  throw ParseUtils.unexpectedAttribute(reader, 0);
}

ومع ذلك ، يمكنك البحث بشكل أعمق قليلاً وإلقاء نظرة على طريقة needNoNamespaceAttribute (....) :

public static void requireNoNamespaceAttribute
 (XMLExtendedStreamReader reader, int index) 
  throws XMLStreamException {
   if (!isNoNamespaceAttribute(reader, index)) {
        throw unexpectedAttribute(reader, index);
   }
}

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

تحذير N5 - تمرير المعلمات للمنشئ

لا يتم استخدام المعلمة V6022 " mechanName " داخل جسم المنشئ. 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);
}

عادةً لا تكون المتغيرات والمعلمات الوظيفية غير المستخدمة شيئًا بالغ الأهمية: بشكل عام ، تظل بعد إعادة البناء أو تُضاف لتنفيذ وظائف جديدة في المستقبل. ومع ذلك ، بدت لي هذه العملية مشبوهة للغاية:

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) {....}

إذا نظرت إلى المُنشئ الثاني للفئة ، يمكنك أن ترى أن سلسلة mechanizmName تُفترض كمعامل سادس . يأخذ المُنشئ الأول سلسلة تحمل نفس اسم المعلمة الثالثة ويستدعي المُنشئ الثاني. ومع ذلك ، لا يتم استخدام هذه السلسلة ، ويتم تمرير ثابت إلى المنشئ الثاني بدلاً من ذلك. ربما خطط مؤلف الكود هنا لتمرير اسم الآلية إلى المُنشئ بدلاً من ثابت DEFAULT_NAME .

تحذير N6 - خطوط مكررة

V6033 عنصر بنفس المفتاح "org.apache.activemq.artemis.core.remoting.impl.netty.

تمت إضافة TransportConstants.NIO_REMOTING_THREADS_PROPNAME 'بالفعل. 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);
    ....
}

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

تحذير N7 - متغيرات مفقودة

تنسيق V6046 غير صحيح. من المتوقع وجود عدد مختلف من عناصر التنسيق. الوسائط المفقودة: 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);
  }
}

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

تحذير N8 - مقارنة السلسلة بكائن

V6058 تقوم وظيفة "يساوي" بمقارنة العناصر ذات الأنواع غير المتوافقة: 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());        // <=
}

في هذه الحالة ، تتم مقارنة السلسلة بكائن ، وتكون هذه المقارنة خاطئة دائمًا. وهذا يعني أنه إذا تم تمرير قيمة modelNode مساوية لـ attribute.getDefaultValue () إلى الطريقة ، فسيكون سلوك الطريقة غير صحيح ، وسيتم التعرف على القيمة على أنها صالحة للإرسال بالرغم من التعليق.



على الأرجح ، نسوا استدعاء طريقة asString () لتمثيل attribute.getDefaultValue () كسلسلة. قد تبدو النسخة المصححة كما يلي:

return !value.equals(attribute.getDefaultValue().asString());

لدى WildFly مشغل آخر مشابه لتشخيص V6058 :

• V6058 تقوم وظيفة "يساوي" بمقارنة العناصر ذات الأنواع غير المتوافقة: String ، ObjectTypeAttributeDefinition. DataSourceDefinition.java (141)

تحذير N9 - الفحص المتأخر

V6060 تم استخدام مرجع "dataSourceController" قبل أن يتم التحقق منه مقابل قيمة خالية. 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) {....}
  ....
}

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

تحذير N10 - قفل مزدوج التحقق

V6082 قفل فحص مزدوج غير آمن. يمكن استبدال كائن تم تعيينه مسبقًا بكائن آخر. 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;
}

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



لاحظ مؤشر الترابط أ أن القيمة لم تتم تهيئتها ، لذلك تحصل على القفل وتبدأ في تهيئة القيمة. في هذه الحالة ، يدير مؤشر الترابط كتابة الكائن في الحقل حتى قبل تهيئته حتى النهاية. يكتشف مؤشر الترابط B أن الكائن قد تم إنشاؤه ويعيده ، على الرغم من أن الخيط A لم يتح له الوقت بعد للقيام بكل الأعمال مع المصنع .



نتيجة لذلك ، قد يتم إرجاع كائن من الطريقة التي لم يتم تنفيذ جميع الإجراءات المخططة عليها.

الاستنتاجات

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



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

إذا كنت ترغب في مشاركة هذه المقالة مع جمهور يتحدث الإنجليزية ، فيرجى استخدام رابط الترجمة: ديمتري شيرباكوف. التحقق من WildFly ، خادم تطبيق JavaEE .