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

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

مقدمة

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

• PVS-Studio in the Clouds: Azure DevOps
• PVS-Studio in the Clouds: Travis CI
• PVS-Studio in the Clouds: CircleCI
• PVS-Studio in the Clouds: GitLab CI / CD

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

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

• دمج نتائج تحليل PVS-Studio في SonarQube
• PVS- استوديو يعمل في جنكينز

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

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

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

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

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

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

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

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

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

فيما يتعلق بتحليل المشروع ، التقطت فقط أكثر تحذيرات hotshot ، والتي سأتحدث عنها في هذا المقال.

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

دورة تهيئة الفئة 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 ، والذي يتم طرحه إذا كان هناك استثناء عند تهيئة الحقل الثابت. ما تحتاجه لحل هذه المشكلة هو وضع تهيئة LOG قبل التهيئة 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 ، فإن التحقق من وجودهما لا معنى له. ومع ذلك ، يتم تمرير التعبير ، حيث يتم إعادة التحقق من الوسيطتين firewallPolicyId و firewallRuleId لإلغاء null ، كوسيطة أولى لطريقة checkState .

يتم إصدار تحذير مماثل لـ 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), ....); } 

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

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

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

هنا حالة أخرى مماثلة:

• 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 مرة أخرى. يسمح عدد من عمليات التحقق في عامل التشغيل الشرطي لبيانات كائن الصفر TmpFile لمزيد من التراجع . أعتقد أن هناك نوعان من الأخطاء المطبعية هنا والشيك يجب أن يبدو بالفعل كما يلي:

 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 لن يغير سلسلة المضيف بسبب ثبات كائنات نوع السلسلة . لتشغيل الطريقة الصحيحة ، يجب تعيين نتيجة الأسلوب 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)); } 

في هذه الطريقة ، قرر المؤلف التحقق من محتويات بنية نوع الخريطة لإلغاء تحديدها. للقيام بذلك ، يتم استدعاء أسلوب get مرتين من وسيطة 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 بنتائج فحص خدمات السحابة الخاصة بهم حتى يتمكن مطورو Huawei من التركيز عليها ، لأن الاستخدام لمرة واحدة لتحليل الشفرة الثابتة المشمولة بهذه المقالات ( 1 ، 2 ) لا يمكن إثباته بالكامل كل مزاياه. يمكنك تنزيل محلل PVS-Studio من هنا .