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

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

مقدمة

إذا كنت تتابع فريق PVS-Studio ، فربما لاحظت أننا مهتمون مؤخرًا بتقنيات السحابة. لقد نشرنا بالفعل العديد من المقالات المتعلقة بهذا الموضوع:

• يذهب PVS-Studio إلى السحب: Azure DevOps
• يذهب PVS-Studio إلى السحب: Travis CI
• يذهب PVS-Studio إلى السحب: CircleCI
• يذهب PVS-Studio إلى السحب: GitLab CI / CD

وهكذا ، عندما كنت أختار مشروعًا آخر مثيرًا للاهتمام للمقال القادم ، تلقيت عرض عمل من Huawei في البريد. عند جمع معلومات حول هذه الشركة ، تبين أن لديهم خدمات سحابية خاصة بهم ، ولكن الشيء الرئيسي هو أن الكود المصدري لهذه الخدمات متاح مجانًا على GitHub. كان هذا هو السبب الرئيسي لاختيار هذه الشركة لهذه المقالة. كما قال حكيم صيني: "الحوادث ليست عرضية".

الآن قليلا عن محلل لدينا. PVS-Studio هو محلل ثابت للكود يحدد الأخطاء ونقاط الضعف في الكود المصدري للبرامج المكتوبة بلغات C و C ++ و C # و Java. يعمل المحلل على أنظمة التشغيل Windows و Linux و macOS. بالإضافة إلى المكونات الإضافية لبيئات التطوير الكلاسيكية مثل Visual Studio أو IntelliJ IDEA ، يتمتع المحلل بالقدرة على الاندماج مع SonarQube و Jenkins:

• دمج نتائج تحليل PVS-Studio في SonarQube
• إطلاق PVS-Studio في جنكينز

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

في عملية البحث عن معلومات لهذه المقالة ، اكتشفت أن لدى Huawei مركز مطور حيث يمكنك الحصول على المعلومات والأدلة والكود المصدر لخدمات السحابة الخاصة بهم. تم استخدام مجموعة متنوعة من لغات البرمجة لإنشاء هذه الخدمات ، ولكن تبين أن اللغات مثل Go و Java و Python هي الأكثر شعبية.

منذ أن تخصصت في Java ، تم اختيار المشاريع وفقًا لذلك. يمكن الحصول على مصادر المشاريع التي تم تحليلها في المقال في مستودع جيثب هوايكلود .

لتحليل المشاريع ، كنت بحاجة إلى القيام ببعض الإجراءات فقط:

• الحصول على مشاريع من المستودع ؛
• استخدم الإرشادات لبدء محلل Java وبدء التحليل في كل مشروع.

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

نتائج تحليل المشروع (عدد التحذيرات وعدد الملفات):

• huaweicloud-sdk-java : 31 - مرتفع ، 2 - متوسط ​​و 16 - منخفض ، 2700+ ملف.
• huaweicloud-dis-agent : 7 - مرتفع ، 6 - متوسط ​​و 6 - منخفض ، 100+ ملف.
• huaweicloud-sdk-java-dis : 15 - مرتفع ، 6 - متوسط ​​و 16 - منخفض ، 270+ ملف.

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

في عملية تحليل المشاريع ، اخترت التحذيرات الأكثر إثارة للاهتمام ، والتي سأناقشها في هذا المقال.

ترتيب تهيئة الحقل

دورة تهيئة الفئة 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); // <= } } } 

في حالة حدوث أي استثناء في مُنشئ فئة UntrustedSSL ، يتم تسجيل المعلومات حول هذا الاستثناء في كتلة catch باستخدام سجل LOG . ومع ذلك ، نظرًا لترتيب تهيئة الحقول الثابتة ، لم يتم تهيئة السجل في وقت تهيئة حقل INSTANCE بعد. لذلك ، عند تسجيل معلومات الاستثناء في المُنشئ ، سيتم طرح NullPointerException . هذا الاستثناء هو سبب استثناء ExceptionInInitializerError آخر يتم طرحه في حالة حدوث استثناء أثناء تهيئة الحقل الثابت. كل ما هو مطلوب لحل المشكلة الموضحة هو وضع تهيئة السجل قبل تهيئة INSTANCE .

الخطأ المطبعي غير مرئية

V6005 يتم تعيين المتغير 'this.metricSchema' لنفسه. OpenTSDBSchema.java (72)

 public class OpenTSDBSchema { @JsonProperty("metric") private List<SchemaField> metricSchema; .... public void setMetricsSchema(List<SchemaField> metricsSchema) { this.metricSchema = metricSchema; // <= } public void setMetricSchema(List<SchemaField> metricSchema) { this.metricSchema = metricSchema; } .... } 

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

V6005 يتم تعيين المتغير "تعليق مؤقت " لنفسه. SuspendTransferTaskRequest.java (77)

 public class SuspendTransferTaskRequest { .... private boolean suspend; .... public void setSuspend(boolean suspend) { suspend = suspend; } .... } 

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

 public void setSuspend(boolean suspend) { this.suspend = suspend; } 

شروط محددة سلفا

كما يحدث غالبًا ، تتقدم قاعدة V6007 من حيث عدد التحذيرات.

تعبير V6007 'firewallPolicyId == null' غير صحيح دائمًا. FirewallPolicyServiceImpl.java (125)

 public FirewallPolicy removeFirewallRuleFromPolicy(String firewallPolicyId, String firewallRuleId) { checkNotNull(firewallPolicyId); checkNotNull(firewallRuleId); checkState(!(firewallPolicyId == null && firewallRuleId == null), "Either a Firewall Policy or Firewall Rule identifier must be set"); .... } 

في هذه الطريقة ، يتم التحقق من الوسيطات فارغة من خلال طريقة checkNotNull :

 @CanIgnoreReturnValue public static <T> T checkNotNull(T reference) { if (reference == null) { throw new NullPointerException(); } else { return reference; } } 

بعد التحقق من الوسيطة باستخدام الأسلوب checkNotNull ، يمكنك التأكد بنسبة 100٪ من أن الوسيطة التي تم تمريرها إلى هذه الطريقة ليست خالية . نظرًا لأن كلا الوسيطتين إلى طريقة removeFirewallRuleFromPolicy يتم فحصهما بواسطة طريقة checkNotNull ، فليس من المنطقي التحقق من الوسيطات لمعرفة القيم الخالية مرة أخرى. ومع ذلك ، يتم تمرير تعبير إلى الأسلوب checkState كالوسيطة الأولى ، حيث يتم إعادة التحقق من الوسيطتين firewallPolicyId و firewallRuleId لإلغاء null .

يتم إصدار تحذير مماثل لـ firewallRuleId :

• تعبير V6007 'firewallRuleId == null' غير صحيح دائمًا. FirewallPolicyServiceImpl.java (125)

تعبير V6007 'filteringParams! = Null' صحيح دائمًا. NetworkPolicyServiceImpl.java (60)

 private Invocation<NetworkServicePolicies> buildInvocation(Map<String, String> filteringParams) { .... if (filteringParams == null) { return servicePoliciesInvocation; } if (filteringParams != null) { // <= .... } return servicePoliciesInvocation; } 

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

يحدث مماثل في 13 فصول:

• تعبير V6007 'filteringParams! = Null' صحيح دائمًا. بوليسيور سيرفيسيمبلجافا (58)
• تعبير V6007 'filteringParams! = Null' صحيح دائمًا. GroupServiceImpl.java (58)
• تعبير V6007 'filteringParams! = Null' صحيح دائمًا. ExternalSegmentServiceImpl.java (57)
• تعبير V6007 'filteringParams! = Null' صحيح دائمًا. L3policyServiceImpl.java (57)
• تعبير V6007 'filteringParams! = Null' صحيح دائمًا. بوليسيول سيرست سيرفيسيمبل.جافا (58)
• وهلم جرا ...

مرجع فارغ

V6008 dereference خالية من "m.blockDeviceMapping". NovaServerCreate.java (390)

 @Override public ServerCreateBuilder blockDevice(BlockDeviceMappingCreate blockDevice) { if (blockDevice != null && m.blockDeviceMapping == null) { m.blockDeviceMapping = Lists.newArrayList(); } m.blockDeviceMapping.add(blockDevice); // <= return this; } 

في هذه الطريقة ، لن تحدث تهيئة الحقل المرجعي m.blockDeviceMapping إذا كانت الوسيطة blockDevice خالية . تتم تهيئة هذا الحقل فقط في هذه الطريقة ، لذلك عندما يتم استدعاء طريقة الإضافة ، سيرمي حقل m.blockDeviceMapping NullPointerException .

V6008 dereference خالية محتملة من 'FileId.get (مسار)' في الوظيفة '<init>'. TrackedFile.java (140) ، TrackedFile.java (115)

 public TrackedFile(FileFlow<?> flow, Path path) throws IOException { this(flow, path, FileId.get(path), ....); } 

يتم تمرير نتيجة الأسلوب الثابت FileId.get (path) إلى مُنشئ فئة TrackedFile كوسيطة ثالثة. ولكن هذه الطريقة يمكن أن تعود خالية :

 public static FileId get(Path file) throws IOException { if (!Files.exists(file)) { return null; } .... } 

في المنشئ الذي تم استدعاؤه من خلال هذا ، لا تتغير الوسيطة id حتى يتم استخدامها لأول مرة:

 public TrackedFile(...., ...., FileId id, ....) throws IOException { .... FileId newId = FileId.get(path); if (!id.equals(newId)) { .... } } 

لذلك ، إذا تم تمرير قيمة فارغة إلى الطريقة كوسيطة ثالثة ، فسيحدث استثناء.

يحدث موقف مشابه في حالة أخرى:

• V6008 dereference خالية من "العازلة". PublishingQueue.java (518)

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 ; } } .... } 

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

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

ركائز وأرقام سالبة

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

 @Override public String apply(String url) { String urlRmovePojectId = url.substring(0, url.lastIndexOf("/")); return urlRmovePojectId.substring(0, urlRmovePojectId.lastIndexOf("/")); } 

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

إليك بعض الأماكن الأخرى التي يحدث فيها هذا:

• V6009 قد تتلقى الدالة "سلسلة فرعية" القيمة "-1" بينما من المتوقع أن تكون القيمة غير السالبة. فحص الوسيطة: 2. RemoveProjectIdFromURL.java (37)
• V6009 قد تتلقى الدالة "سلسلة فرعية" القيمة "-1" بينما من المتوقع أن تكون القيمة غير السالبة. فحص الوسيطة: 2. RemoveVersionProjectIdFromURL.java (38)

النتيجة المنسية

V6010 يجب استخدام قيمة الإرجاع للدالة 'concat'. AKSK.java (278)

 public static String buildCanonicalHost(URL url) { String host = url.getHost(); int port = url.getPort(); if (port > -1) { host.concat(":" + Integer.toString(port)); } return host; } 

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

 if (port > -1) { host = host.concat(":" + Integer.toString(port)); } 

المتغيرات غير المستخدمة

V6021 لا يتم استخدام "url" المتغير. TriggerV2Service.java (95)

 public ActionResponse deleteAllTriggersForFunction(String functionUrn) { checkArgument(!Strings.isNullOrEmpty(functionUrn), ....); String url = ClientConstants.FGS_TRIGGERS_V2 + ClientConstants.URI_SEP + functionUrn; return deleteWithResponse(uri(triggersUrlFmt, functionUrn)).execute(); } 

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

حجة لا تستخدم في البناء

لا يتم استخدام المعلمة V6022 "returnType" داخل هيكل المنشئ. HttpRequest.java (68)

 public class HttpReQuest<R> { .... Class<R> returnType; .... public HttpRequest(...., Class<R> returnType) // <= { this.endpoint = endpoint; this.path = path; this.method = method; this.entity = entity; } .... public Class<R> getReturnType() { return returnType; } .... } 

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

نفس الوظيفة

V6032 من الغريب أن يكون نص الأسلوب "تمكين" مكافئًا تمامًا لجسم طريقة أخرى "تعطيل". ServiceAction.java (32) ، ServiceAction.java (36)

 public class ServiceAction implements ModelEntity { private String binary; private String host; private ServiceAction(String binary, String host) { this.binary = binary; this.host = host; } public static ServiceAction enable(String binary, String host) { // <= return new ServiceAction(binary, host); } public static ServiceAction disable(String binary, String host) { // <= return new ServiceAction(binary, host); } .... } 

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

نسيت أن تحقق الشيء الرئيسي

V6060 تم استخدام مرجع "params" قبل أن يتم التحقق منه ضد قيمة خالية. DomainService.java (49) ، DomainService.java (46)

 public Domains list(Map<String, String> params) { Preconditions.checkNotNull(params.get("page_size"), ....); Preconditions.checkNotNull(params.get("page_number"), ....); Invocation<Domains> domainInvocation = get(Domains.class, uri("/domains")); if (params != null) { // <= .... } return domainInvocation.execute(this.buildExecutionOptions(Domains.class)); } 

في هذه الطريقة ، قررنا التحقق من محتويات بنية نوع الخريطة لعدم المساواة الفارغة . للقيام بذلك ، تقوم الوسيطة params باستدعاء أسلوب get مرتين ، ويتم تمرير النتيجة إلى أسلوب checkNotNull . كل شيء يبدو منطقيا ، ولكن بغض النظر عن كيف! في حالة ، يتم التحقق من وسيطة params لإلغاء . بعد ذلك ، يمكن افتراض أن وسيطة الإدخال قد تكون خالية ، ولكن قبل هذا الاختيار ، تم استدعاء طريقة get على params مرتين بالفعل. إذا تم تمرير null كوسيطة لهذه الطريقة ، فحين يتم استدعاء الأسلوب get للمرة الأولى ، سيتم طرح استثناء.

يحدث موقف مماثل في ثلاثة أماكن أخرى:

• V6060 تم استخدام مرجع "params" قبل أن يتم التحقق منه ضد قيمة خالية. DomainService.java (389) ، DomainService.java (387)
• V6060 تم استخدام مرجع "params" قبل أن يتم التحقق منه ضد قيمة خالية. DomainService.java (372) ، DomainService.java (369)
• V6060 تم استخدام مرجع "params" قبل أن يتم التحقق منه ضد قيمة خالية. DomainService.java (353) ، DomainService.java (350)

استنتاج

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

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

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

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