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字段。 正确地：

 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方法检查 ，因此再次检查参数是否为空 值是没有意义的。 但是，将表达式作为第一个参数传递到checkState方法，其中将重新检查firewallPolicyId和firewallRuleId参数是否为null 。

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

在此方法中，如果参数filterParams为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参考字段的初始化。 仅在此方法中初始化此字段，因此，在调用add方法时， m.blockDeviceMapping字段将引发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), ....); }

静态方法FileId.get（路径）的结果作为第三个参数传递给TrackedFile类的构造函数。 但是此方法可以返回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对象为null以便进一步取消引用。 我认为这里有两种错别字，检查实际上应该是这样的：

 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类型结构的内容是否存在空不等式。 为此， params参数调用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一定会通知华为其云服务检查结果，以便该公司的开发人员可以更详细地研究它们。

本文中演示的一次性使用静态代码分析不能显示其所有优点。 有关如何正确使用静态分析的更多详细信息，请参见此处和此处 。 您可以在此处下载PVS-Studio分析仪。



如果您想与说英语的读者分享这篇文章，请使用以下链接：Valery Komarov。 华为云：如今PVS-Studio多云 。

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

More articles:


All Articles