PVS-Studio for Java上路了。 下一站是Elasticsearch

PVS-Studio团队多年来一直通过同名静态代码分析器保留有关开源项目检查的博客。 迄今为止，已经检查了300多个项目，错误基础包含12000多个案例。 最初，分析器用于检查C和C ++代码，后来添加了对C＃的支持。 因此，在所有检查的项目中，大多数（> 80％）占C和C ++。 最近，Java已被添加到受支持的语言列表中，这意味着PVS-Studio现在有了一个全新的开放世界，因此现在该用Java项目中的错误来补充基础了。

Java世界是千差万别的，因此，在选择测试新分析器的项目时，甚至都不知道首先要看哪里。 最终，选择取决于全文搜索和分析引擎Elasticsearch。 这是一个非常成功的项目，在重大项目中发现错误甚至特别令人愉悦。 那么，PVS-Studio for Java设法检测到哪些缺陷？ 有关检查结果的进一步讨论将是正确的。

简要介绍Elasticsearch

Elasticsearch是具有开放源代码的分布式RESTful搜索和分析引擎，能够解决越来越多的用例。 它使您能够存储大量数据，进行快速搜索和分析（几乎是实时模式）。 通常，它用作基础机制/技术，为应用程序提供复杂的功能和搜索要求。

在使用Elasticsearch的主要站点中，有Wikimedia，StumbleUpon，Quora，Foursquare，SoundCloud，GitHub，Netflix，Amazon，IBM，Qbox。

很好，足够介绍。

关于事情如何的整个故事

支票本身没有问题。 动作序列非常简单，不需要花费很多时间：

• 从GitHub下载Elasticsearch;
• 遵循说明如何运行Java分析器并运行分析；
• 收到了分析器的报告，对其进行了深入研究并指出了一些有趣的案例。

现在，让我们继续要讲的重点。

当心！ 可能的NullPointerException

V6008空取消引用“行”。 GoogleCloudStorageFixture.java（451）

private static PathTrie<RequestHandler> defaultHandlers(....) { .... handlers.insert("POST /batch/storage/v1", (request) -> { .... // Reads the body line = reader.readLine(); byte[] batchedBody = new byte[0]; if ((line != null) || (line.startsWith("--" + boundary) == false)) // <= { batchedBody = line.getBytes(StandardCharsets.UTF_8); } .... }); .... } 

此代码段中的错误是，如果未读取缓冲区中的字符串，则在if语句条件下对startsWith方法的调用将导致抛出NullPointerException异常。 这很可能是一个错字，在编写条件时，开发人员意味着&&运算符而不是||。 。

V6008'followIndexMetadata '的潜在空引用。 TransportResumeFollowAction.java（171），TransportResumeFollowAction.java（170），TransportResumeFollowAction.java（194）

 void start( ResumeFollowAction.Request request, String clusterNameAlias, IndexMetaData leaderIndexMetadata, IndexMetaData followIndexMetadata, ....) throws IOException { MapperService mapperService = followIndexMetadata != null // <= ? .... : null; validate(request, leaderIndexMetadata, followIndexMetadata, // <= leaderIndexHistoryUUIDs, mapperService); .... } 

V6008诊断的另一个警告。 对象followIndexMetadata激发了我的兴趣。 start方法接受几个参数作为输入，我们的猜想是正确的。 之后，基于检查我们的对象是否为空，将创建一个新对象，该对象将包含在其他方法逻辑中。 检查是否为空表明我们followIndexMetadata仍然可以作为空对象从外部发出。 好吧，让我们进一步看。

然后将多个参数推送到validate方法（同样，其中有我们考虑的对象）。 如果我们看一下验证方法的实现，那么一切都就位了。 我们潜在的空对象作为第三个参数传递给validate方法，在此无条件地将其取消引用。 结果可能是NullPointerException 。

 static void validate( final ResumeFollowAction.Request request, final IndexMetaData leaderIndex, final IndexMetaData followIndex, // <= ....) { .... Map<String, String> ccrIndexMetadata = followIndex.getCustomData(....); // <= if (ccrIndexMetadata == null) { throw new IllegalArgumentException(....); } .... }} 

我们不确定用什么参数来调用start方法。 很有可能在调用该方法之前在某处检查了所有参数，并且不会发生空对象取消引用。 无论如何，我们应该承认这样的代码实现仍然看起来不可靠并且值得关注。

V6060在验证是否为空之前，已使用“节点”引用。 RestTasksAction.java（152），RestTasksAction.java（151）

 private void buildRow(Table table, boolean fullId, boolean detailed, DiscoveryNodes discoveryNodes, TaskInfo taskInfo) { .... DiscoveryNode node = discoveryNodes.get(nodeId); .... // Node information. Note that the node may be null because it has // left the cluster between when we got this response and now. table.addCell(fullId ? nodeId : Strings.substring(nodeId, 0, 4)); table.addCell(node == null ? "-" : node.getHostAddress()); table.addCell(node.getAddress().address().getPort()); table.addCell(node == null ? "-" : node.getName()); table.addCell(node == null ? "-" : node.getVersion().toString()); .... } 

另一个具有相同问题的诊断规则在此处触发。 NullPointerException 。 该规则对开发人员大声疾呼：“伙计们，您在做什么？ 你怎么能这样 哦，太可怕了！ 为什么首先使用对象并在下一行中检查是否为null ？ 这是空对象解除引用的过程。 las，即使是开发人员的评论也无济于事。

V6060在验证是否为null之前，已使用“原因”参考。 StartupException.java（76），StartupException.java（73）

 private void printStackTrace(Consumer<String> consumer) { Throwable originalCause = getCause(); Throwable cause = originalCause; if (cause instanceof CreationException) { cause = getFirstGuiceCause((CreationException)cause); } String message = cause.toString(); // <= consumer.accept(message); if (cause != null) { // <= // walk to the root cause while (cause.getCause() != null) { cause = cause.getCause(); } .... } .... } 

在这种情况下，我们应该考虑Throwable类的getCause方法可能返回null 。 上面的问题再重复一遍，因此无需赘述。

无意义的条件

V6007表达式's.charAt（i）！='\ T''始终为true。 Cron.java（1223）

 private static int findNextWhiteSpace(int i, String s) { for (; i < s.length() && (s.charAt(i) != ' ' || s.charAt(i) != '\t'); i++) { // intentionally empty } return i; } 

所考虑的函数从i索引开始返回第一个空格字符的索引。 怎么了 分析器警告说s.charAt（i）！='\ T'始终为true，这表示表达式（s.charAt（i）！=''|| s.charAt（i）！='\ T '）也将永远为真。 这是真的吗 我认为，您可以通过替换任何字符来轻松地确保这一点。

结果，此方法将始终返回等于s.length（）的索引，这是错误的。 我敢建议上述方法应归咎于此：

 private static int skipWhiteSpace(int i, String s) { for (; i < s.length() && (s.charAt(i) == ' ' || s.charAt(i) == '\t'); i++) { // intentionally empty } return i; } 

开发人员实现了该方法，然后进行了一些编辑，然后复制并得到了错误的方法findNextWhiteSpace 。 他们一直在固定和固定方法，但尚未固定。 要获得此权利，应使用&&运算符而不是||。 。

V6007表达式'remaining == 0'始终为false。 PemUtils.java（439）

 private static byte[] generateOpenSslKey(char[] password, byte[] salt, int keyLength) { .... int copied = 0; int remaining; while (copied < keyLength) { remaining = keyLength - copied; .... copied += bytesToCopy; if (remaining == 0) { // <= break; } .... } .... } 

根据复制的<keyLength循环的条件，我们可以注意到， 复制的内容始终小于keyLength 。 因此，将剩余的变量与0进行比较是毫无意义的，并且它始终为false，此时循环不会因条件而退出。 删除代码还是重新考虑行为逻辑？ 我认为，只有开发人员才能将所有问题都放在i上。

V6007表达式' healthCheckDn.indexOf （'='）> 0'始终为false。 ActiveDirectorySessionFactory.java（73）

 ActiveDirectorySessionFactory(RealmConfig config, SSLService sslService, ThreadPool threadPool) throws LDAPException { super(...., () -> { if (....) { final String healthCheckDn = ....; if (healthCheckDn.isEmpty() && healthCheckDn.indexOf('=') > 0) { return healthCheckDn; } } return ....; }, ....); .... } 

再次毫无意义的表达。 根据条件， healthCheckDn字符串必须既为空，又不包含字符'='，但lambda表达式将返回字符串变量healthCheckDn 。 ew，仅此而已！ 正如您可能理解的那样，这是不可能的。 我们不会深入研究代码，让开发人员自行决定。

我仅列举了一些错误的示例，但除此之外，还有很多V6007诊断触发，应该一一考虑，并得出相关结论。

小方法可以走很长一段路

 private static byte char64(char x) { if ((int)x < 0 || (int)x > index_64.length) return -1; return index_64[(int)x]; } 

因此，在这里，我们有几行的小方法。 但是，有bug在注意！ 对这种方法的分析得出以下结果：

1. V6007表达式'（int）x <0'始终为false。 BCrypt.java（429）
2. V6025可能索引'（int）x'超出范围。 BCrypt.java（431）

问题N1。 表达式（int）x <0始终为false（是，再次为V6007 ）。 x变量不能为负数，因为它是char类型。 字符类型是无符号整数。 它不能被称为真正的错误，但是尽管如此，检查是多余的，可以删除。

问题N2。 可能的数组索引超出范围，导致ArrayIndexOutOfBoundsException异常。 然后它提出了一个表面上的问题：“等等，索引检查怎么样？”

因此，我们有一个包含128个元素的固定大小的数组：

 private static final byte index_64[] = { -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 0, 1, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, -1, -1, -1, -1, -1, -1, -1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, -1, -1, -1, -1, -1, -1, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, -1, -1, -1, -1, -1 }; 

当char64方法接收x变量时，将检查索引有效性。 缺陷在哪里？ 为什么仍然可能超出数组索引？

检查（int）x> index_64.length不太正确。 如果char64方法接收到值为128的x，则该检查将无法防止ArrayIndexOutOfBoundsException。 也许这永远不会发生。 但是，该支票的书写方式不正确，必须将“大于”运算符（>）更改为“大于或等于（> =）”。

比较，尽力而为

V6013数字“ displaySize”和“ that.displaySize”通过引用进行比较。 可能希望进行平等比较。 ColumnInfo.java（122）

 .... private final String table; private final String name; private final String esType; private final Integer displaySize; .... @Override public boolean equals(Object o) { if (this == o) { return true; } if (o == null || getClass() != o.getClass()) { return false; } ColumnInfo that = (ColumnInfo) o; return displaySize == that.displaySize && // <= Objects.equals(table, that.table) && Objects.equals(name, that.name) && Objects.equals(esType, that.esType); } 

这里不正确的是使用==运算符（即通过引用）比较了Integer类型的displaySize对象。 很有可能将对displayInfo字段具有不同引用但内容相同的ColumnInfo对象进行比较。 在这种情况下，当我们期望实现时，比较会给我们带来负面结果。

我敢冒险猜测这样的比较可能是重构失败的结果，并且最初的displaySize字段是int类型的。

V6058 “等于”功能比较不兼容类型的对象：整数，时间值。 DatafeedUpdate.java（375）

 .... private final TimeValue queryDelay; private final TimeValue frequency; .... private final Integer scrollSize; .... boolean isNoop(DatafeedConfig datafeed) { return (frequency == null || Objects.equals(frequency, datafeed.getFrequency())) && (queryDelay == null || Objects.equals(queryDelay, datafeed.getQueryDelay())) && (scrollSize == null || Objects.equals(scrollSize, datafeed.getQueryDelay())) // <= && ....) } 

再次进行不正确的对象比较。 比较具有不兼容类型的时间对象（ Integer和TimeValue ）。 这种比较的结果是显而易见的，并且总是错误的。 您可以看到类字段相互比较，只需要更改字段名称即可。 这就是重点-开发人员决定通过使用复制粘贴来加快流程，并讨价还价中的一个bug。 该类为scrollSize字段实现了一个吸气剂，因此要解决该错误，应使用datafeed .getScrollSize（）方法。

让我们看几个错误的例子，不做任何解释。 问题很明显。

V6001在'=='运算符的左侧和右侧有相同的子表达式'tookInMillis'。 TermVectorsResponse.java（152）

 @Override public boolean equals(Object obj) { .... return index.equals(other.index) && type.equals(other.type) && Objects.equals(id, other.id) && docVersion == other.docVersion && found == other.found && tookInMillis == tookInMillis // <= && Objects.equals(termVectorList, other.termVectorList); } 

V6009函数“等于”收到一个奇数参数。 对象“ shardId.getIndexName（）”用作其自身方法的参数。 SnapshotShardFailure.java（208）

 @Override public boolean equals(Object o) { .... return shardId.id() == that.shardId.id() && shardId.getIndexName().equals(shardId.getIndexName()) && // <= Objects.equals(reason, that.reason) && Objects.equals(nodeId, that.nodeId) && status.getStatus() == that.status.getStatus(); } 

杂项

V6006已创建对象， 但未使用该对象。 'throw'关键字可能丢失。 JdbcConnection.java（88）

 @Override public void setAutoCommit(boolean autoCommit) throws SQLException { checkOpen(); if (!autoCommit) { new SQLFeatureNotSupportedException(....); } } 

该错误是显而易见的，不需要解释。 开发人员创建了一个异常，但没有将其抛出其他任何地方。 这样的匿名异常创建成功，并且最重要的是，将无缝删除它。 原因是缺少throw运算符。

V6003检测到使用'if（A）{....} else if（A）{....}'模式。 存在逻辑错误的可能性。 MockScriptEngine.java（94），MockScriptEngine.java（105）

 @Override public <T> T compile(....) { .... if (context.instanceClazz.equals(FieldScript.class)) { .... } else if (context.instanceClazz.equals(FieldScript.class)) { .... } else if(context.instanceClazz.equals(TermsSetQueryScript.class)) { .... } else if (context.instanceClazz.equals(NumberSortScript.class)) .... } 

在多个情况下，其中一个条件重复两次，因此需要对代码进行适当的审查。

V6039有两个带有相同条件表达式的'if'语句。 第一个'if'语句包含方法return。 这意味着第二个“ if”语句是毫无意义的。 SearchAfterBuilder.java（94），SearchAfterBuilder.java（93）

 public SearchAfterBuilder setSortValues(Object[] values) { .... for (int i = 0; i < values.length; i++) { if (values[i] == null) continue; if (values[i] instanceof String) continue; if (values[i] instanceof Text) continue; if (values[i] instanceof Long) continue; if (values[i] instanceof Integer) continue; if (values[i] instanceof Short) continue; if (values[i] instanceof Byte) continue; if (values[i] instanceof Double) continue; if (values[i] instanceof Float) continue; if (values[i] instanceof Boolean) continue; // <= if (values[i] instanceof Boolean) continue; // <= throw new IllegalArgumentException(....); } .... } 

同一条件连续使用两次。 第二个条件是多余的还是应该使用其他类型代替Boolean ？

V6009函数“子字符串”接收奇数个参数。 'queryStringIndex + 1'参数不应大于'queryStringLength'。 LoggingAuditTrail.java（660）

 LogEntryBuilder withRestUriAndMethod(RestRequest request) { final int queryStringIndex = request.uri().indexOf('?'); int queryStringLength = request.uri().indexOf('#'); if (queryStringLength < 0) { queryStringLength = request.uri().length(); } if (queryStringIndex < 0) { logEntry.with(....); } else { logEntry.with(....); } if (queryStringIndex > -1) { logEntry.with(...., request.uri().substring(queryStringIndex + 1,// <= queryStringLength)); // <= } .... } 

让我们在这里考虑可能导致StringIndexOutOfBoundsException异常的错误情况。 当request.uri（）返回在“？”之前包含字符“＃”的字符串时，将发生异常。 该方法没有检查，因此万一发生，麻烦就在酝酿中。 也许，由于方法外对对象的各种检查，这种情况永远不会发生，但是对此抱有希望不是最好的主意。

结论

多年来，PVS-Studio一直在帮助发现商业和免费开源项目代码中的缺陷。 最近，Java加入了支持的语言列表以进行分析。 Elasticsearch成为我们新来者的首批测试之一。 我们希望这项检查对项目有用，对读者也有帮助。

PVS-Studio for Java需要新的挑战，新的用户，有效的反馈和客户，以便快速适应新的世界：）。 因此，我邀请您立即下载并在工作项目中试用我们的分析仪！