Geekly articles weekly

华为云：今天PVS-Studio多云

图片2

如今，每个人都知道云服务。 许多公司已经打入这个市场领域，并创建了自己的各种用途的云服务。 最近，我们的团队也对将PVS-Studio代码分析器集成到其中的这些服务感兴趣。 通常，我们的普通读者已经猜到了这次我们将检查哪种类型的项目。 选择取决于华为云服务的代码。

引言


如果您关注PVS-Studio团队的帖子，您可能已经注意到我们最近一直在深入研究云技术。 我们已经发表了几篇有关该主题的文章：


就在我为即将到来的文章寻找不寻常的项目时，我收到了一封包含华为工作机会的电子邮件。 在收集了有关该公司的一些信息之后，事实证明他们拥有自己的云服务，但主要的是这些服务的源代码可在GitHub上获得。 这是本文选择该公司的主要原因。 正如一位中国圣人所说：“事故并非偶然。”

让我给您一些有关我们分析仪的细节。 PVS-Studio是一种静态分析器，用于用C，C ++，C＃和Java编写的程序源代码中的错误检测。 该分析仪可在Windows，Linux和macOS上运行。 除了用于经典开发环境（例如Visual Studio或IntelliJ IDEA）的插件之外，该分析仪还可以集成到SonarQube和Jenkins中：


项目分析


在对本文进行一些研究时，我发现华为拥有一个开发人员中心，其中提供了可用的信息，手册和云服务的来源。 各种各样的编程语言被用来创建这些服务，但是诸如Go，Java和Python之类的语言最为流行。

由于我专门研究Java，因此选择了符合我的知识和技能的项目。 您可以在GitHub存储库huaweicloud中的文章中分析项目源。

要分析项目，我只需要做几件事：

  • 从资源库中获取项目；
  • 使用Java启动分析器说明，并在每个项目上运行分析。

在分析了项目之后，我们仅选择了三个我们要注意的项目。 这是因为事实证明，其余Java项目的大小过小。

项目分析结果（警告数量和文件数量）：


很少有警告可以告诉我们有关代码质量的信息，更重要的是，因为并非所有警告都指向实际错误。 这是由于这样的事实，即分析器有时缺乏将正确代码与错误代码区分开的信息。 因此，我们会根据用户的信息，每天对分析仪的诊断进行调整。 欢迎您阅读以下文章：“ 静态分析器与误报作斗争的方式，以及为什么要这样做 。”

在分析项目时，我只选择了最热门的警告，本文将对此进行讨论。

字段初始化顺序


V6050存在类初始化周期。 在初始化“ LOG”之前出现“ INSTANCE”的初始化。 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类构造函数中有任何异常，则使用LOG记录器将有关此异常的信息记录在catch块中。 但是，由于静态字段的初始化顺序，在初始化INSTANCE字段时，尚未初始化LOG 。 因此，如果您在构造函数中记录有关异常的信息，则将导致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字段，但是方法的名称之间用一个's'符号表示区别。 程序员根据方法的名称来命名这些方法的参数。 结果，在分析器指向的行中，向其自身分配了metricSchema字段，并且未使用metricsSchema方法的参数。

V6005变量“挂起”已分配给自身。 SuspendTransferTaskRequest.java（77）

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

这是一个与粗心有关的琐碎错误，因此会将suspend参数分配给它自己。 结果，将不会像隐式那样为suspend字段分配获得的参数的值。 正确的版本：

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

条件预先确定


经常发生的是， V6007规则在警告数量方面居于领先地位。

V6007表达式“ firewallPolicyId == null”始终为false。 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％确保传递给此方法的参数不等于null 。 由于removeFirewallRuleFromPolicy方法的两个参数均由checkNotNull方法检查，因此进一步检查是否为null毫无意义。 但是，将重新检查firewallPolicyId和firewallRuleId参数为null的表达式作为第一个参数传递给checkState方法。

也为firewallRuleId发出类似的警告：

  • V6007表达式“ firewallRuleId == null”始终为false。 FirewallPolicyServiceImpl.java（125）

V6007表达式'filteringParams！= Null'始终为true。 NetworkPolicyServiceImpl.java（60）

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

在此方法中，如果filteringParams参数为null ，则该方法返回一个值。 这就是分析仪指向的检查始终为真的原因，这反过来意味着该检查没有意义。

还有13个类似的类：

  • V6007表达式'filteringParams！= Null'始终为true。 PolicyRuleServiceImpl.java（58）
  • V6007表达式'filteringParams！= Null'始终为true。 GroupServiceImpl.java（58）
  • V6007表达式'filteringParams！= Null'始终为true。 ExternalSegmentServiceImpl.java（57）
  • V6007表达式'filteringParams！= Null'始终为true。 L3policyServiceImpl.java（57）
  • V6007表达式'filteringParams！= Null'始终为true。 PolicyRuleSetServiceImpl.java（58）
  • 等等...

空参考


V6008可能会取消引用'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; }

在此方法中，如果blockDevice参数为null，则不会初始化m.blockDeviceMapping参考字段。 仅在此方法中初始化此字段，因此从m.blockDeviceMapping字段调用add方法时，将发生NullPointerException 。

V6008函数'<init>'中'FileId.get（path）'的潜在空引用。 TrackedFile.java（140），TrackedFile.java（115）

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

TrackedFile类的构造函数接收静态FileId.get（路径）方法的结果作为第三个参数。 但是此方法可以返回null ：

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

在通过this调用的构造函数中， id参数直到第一次使用才改变：

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

如我们所见，如果将null作为第三个参数传递给该方法，则会发生异常。

这是另一种类似的情况：

  • V6008可能会取消引用“缓冲区”。 PublishingQueue.java（518）

V6008可能会取消引用“ 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 ，因为substring方法的参数必须为非负数。 为了使方法正确运行，必须添加输入参数验证或检查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变量作为第二个参数而不是functionUrn传递给uri方法，因为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方法时，默认情况下将返回null的原因 。 但最有可能的是，程序员打算获取该对象，该对象先前已传递给构造函数。

相同的功能


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对象。 最有可能的是， disable方法是通过复制enable方法的代码创建的，但是方法的主体保持不变。

忘了检查主要的东西


V6060在对null进行验证之前，已使用“ 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)); }

在这种方法中，作者决定检查Map类型的结构的内容是否为null 。 为此，从params参数调用get方法两次。 get方法的结果传递给checkNotNull方法。 一切似乎合乎逻辑，但事实并非如此！ 在if中检查params参数是否为null 。 此后，预期输入参数可能为null ，但在此检查之前，已经从params中调用了get方法两次。 如果将null作为参数传递给此方法，则首次调用get方法时，将引发异常。

在其他三个地方也发生类似的情况：

  • V6060在对null进行验证之前，已使用“ params”参考。 DomainService.java（389），DomainService.java（387）
  • V6060在对null进行验证之前，已使用“ params”参考。 DomainService.java（372），DomainService.java（369）
  • V6060在对null进行验证之前，已使用“ params”参考。 DomainService.java（353），DomainService.java（350）

结论


当今的大公司离不开使用云服务。 大量的人使用这些服务。 按照这种观点，即使是很小的服务错误也可能导致许多人面临问题，并导致公司蒙受更多损失，以弥补该错误的不利后果。 始终应考虑人为缺陷，尤其是因为本文所述每个人迟早都会犯错。 这个事实证实了所有可能工具的使用，以提高代码质量。

PVS-Studio一定会通知华为公司有关其云服务的检查结果，以便华为开发人员可以继续使用它们，因为本文（ 1，2 ）所介绍的一次性使用静态代码分析无法完全演示所有的优点。 您可以在此处下载PVS-Studio分析仪。

Source: https://habr.com/ru/post/zh-CN477558/

More articles:


All Articles