Geekly articles weekly

2019年在Java项目中发现的十大bug


2019年即将结束，PVS-Studio团队正在回顾今年的成就。 在2019年初，我们通过添加Java支持增强了分析仪的诊断能力，这也使我们能够检查和审查Java项目。 在今年，我们发现了很多错误，这是在Java项目中发现的十大错误。


不行 10：签名字节


资料来源： PVS-Studio静态代码分析器对Apache Dubbo RPC框架的分析

V6007表达式'endKey [i] <0xff'始终为true。 OptionUtil.java（32）

public static final ByteSequence prefixEndOf(ByteSequence prefix) { byte[] endKey = prefix.getBytes().clone(); for (int i = endKey.length - 1; i >= 0; i--) { if (endKey[i] < 0xff) { // <= endKey[i] = (byte) (endKey[i] + 1); return ByteSequence.from(Arrays.copyOf(endKey, i + 1)); } } return ByteSequence.from(NO_PREFIX_END); }

许多程序员认为字节类型是无符号的。 在许多编程语言中确实确实如此。 例如，对于C＃，这是正确的。 但这在Java中并非如此。

在endKey [i] <0xff条件下， 字节类型的变量与以十六进制形式表示的数字255（0xff）进行比较。 开发人员可能忘记了Java 字节类型的范围是[-128，127]。 此条件将始终为true，并且for循环将始终仅处理endKey数组的最后一个元素。

不行 9：二合一


资料来源： PVS-Studio for Java上路了。 下一站是Elasticsearch

V6007表达式'（int）x <0'始终为false。 BCrypt.java（429）

V6025可能索引'（int）x'超出范围。 BCrypt.java（431）

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

打折！ 一种方法-两个bug！ 第一个与char类型有关：由于在Java中为unsigned，因此（int）x <0条件始终为false。 第二个错误是当（int）x == index_64.length时，普通索引超出了index_64数组的范围。 发生这种情况是由于条件（int）x> index_64.length 。 可以通过将'>'运算符更改为'> ='来解决该错误： （int）x> = index_64.length 。

不行 8：解决方案及其含义


资料来源： 使用PVS-Studio分析CUBA平台的代码

V6007表达式'previousMenuItemFlatIndex> = 0'始终为true。 CubaSideMenuWidget.java（328）

 protected MenuItemWidget findNextMenuItem(MenuItemWidget currentItem) { List<MenuTreeNode> menuTree = buildVisibleTree(this); List<MenuItemWidget> menuItemWidgets = menuTreeToList(menuTree); int menuItemFlatIndex = menuItemWidgets.indexOf(currentItem); int previousMenuItemFlatIndex = menuItemFlatIndex + 1; if (previousMenuItemFlatIndex >= 0) { // <= return menuItemWidgets.get(previousMenuItemFlatIndex); } return null; }

当menuItemWidgets列表不包含currentItem时， findNextMenuItem方法的作者希望摆脱indexOf方法返回的值-1。 为此，程序员将indexOf的结果加1（ menuItemFlatIndex变量），并将结果值写入变量previousMenuItemFlatIndex ，然后在方法中使用它。 加上-1被证明是一个不好的解决方案，因为它会立即导致多个错误：

  • 返回null语句将永远不会执行，因为previousMenuItemFlatIndex> = 0表达式将始终为true； 因此， findNextMenuItem方法将始终从if语句内返回；
  • 当menuItemWidgets列表变为空时，将引发IndexOutOfBoundsException，因为程序将尝试访问空列表的第一个元素；
  • 当currentItem参数已成为menuItemWidget列表的最后一个元素时，将引发IndexOutOfBoundsException 。

不行 7：一无所获


资料来源： 华为云：如今PVS-Studio多云

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

编写putToCache方法时，程序员在条件dataTmpFile == null ||中输入了错误。 ！dataTmpFile.exists（）在创建新文件dataTmpFile.createNewFile（）之前 ：他们编写了'=='运算符，而不是'！='。 键入createNewFile方法时，此错字将导致引发NullPointerException 。 这是固定错字的情况：

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

您可能会想：“好吧，我们现在可以放松。”还没有！

现在，已修复一个错误，另一个错误出现了。 调用dataTmpFile.exists（）方法时，可能会引发NullPointerException 。 要解决此问题，我们需要替换“ ||” 运算符“ &&”。 在所有修复之后，这是该条件的最终版本：

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

不行 6：一个非常奇怪的逻辑错误


资料来源： PVS-Studio for Java

V6007 [CWE-570]表达式'“ 0” .equals（text）'始终为false。 ConvertIntegerToDecimalPredicate.java 46

 public boolean satisfiedBy(@NotNull PsiElement element) { .... @NonNls final String text = expression.getText().replaceAll("_", ""); if (text == null || text.length() < 2) { return false; } if ("0".equals(text) || "0L".equals(text) || "0l".equals(text)) {// <= return false; } return text.charAt(0) == '0'; }

关于此方法的有趣之处在于它包含一个明显的逻辑错误。 如果满意的方法在第一个if语句之后没有返回，则表示文本字符串至少有两个字符长。 这也意味着下一个if语句中的第一个校验“ 0” .equals（文本）是没有意义的。 程序员实际上的含义仍然是个谜。

不行 5：多么曲折！


资料来源： PVS-Studio访问Apache Hive

V6034按'bitShiftsInWord-1'的值进行移位可能与以下类型的大小不一致：'bitShiftsInWord-1'= [-1 ... 30]。 UnsignedInt128.java（1791）

 private void shiftRightDestructive(int wordShifts, int bitShiftsInWord, boolean roundUp) { if (wordShifts == 0 && bitShiftsInWord == 0) { return; } assert (wordShifts >= 0); assert (bitShiftsInWord >= 0); assert (bitShiftsInWord < 32); if (wordShifts >= 4) { zeroClear(); return; } final int shiftRestore = 32 - bitShiftsInWord; // check this because "123 << 32" will be 123. final boolean noRestore = bitShiftsInWord == 0; final int roundCarryNoRestoreMask = 1 << 31; final int roundCarryMask = (1 << (bitShiftsInWord - 1)); // <= .... }

使用输入参数wordShifts = 3和bitShiftsInWord = 0时 ，存储按位移位结果（1 <<（（bitShiftsInWord-1））的roundCarryMask变量将变为负数。 开发人员可能没想到这一点。

不行 4：我们能看到例外吗？


资料来源： PVS-Studio访问Apache Hive

V6051在“ finally”块中使用“ return”语句可能会导致丢失未处理的异常。 ObjectStore.java（9080）

 private List<MPartitionColumnStatistics> getMPartitionColumnStatistics(....) throws NoSuchObjectException, MetaException { boolean committed = false; try { .... /*some actions*/ committed = commitTransaction(); return result; } catch (Exception ex) { LOG.error("Error retrieving statistics via jdo", ex); if (ex instanceof MetaException) { throw (MetaException) ex; } throw new MetaException(ex.getMessage()); } finally { if (!committed) { rollbackTransaction(); return Lists.newArrayList(); } } }

getMPartitionColumnStatistics方法的声明具有误导性，因为该声明可能会引发异常。 实际上，无论try块中生成了什么异常， 提交的变量将始终为false ，因此finally块中的return语句将返回一个值，而所有引发的异常将丢失，无法在方法外进行处理。 因此，在此方法中引发的任何异常都不能离开它。

不行 3：看病，或尝试换个新面具


资料来源： PVS-Studio访问Apache Hive

V6034移位'j'的值可能与类型的大小不一致：'j'= [0 ... 63]。 IoTrace.java（272）

 public void logSargResult(int stripeIx, boolean[] rgsToRead) { .... for (int i = 0, valOffset = 0; i < elements; ++i, valOffset += 64) { long val = 0; for (int j = 0; j < 64; ++j) { int ix = valOffset + j; if (rgsToRead.length == ix) break; if (!rgsToRead[ix]) continue; val = val | (1 << j); // <= } .... } .... }

这个错误也与按位转换有关，但不仅限于此。 j变量在内部for循环中的[0 ... 63]范围内用作计数器。 该计数器参与按位移位1 << j 。 一切似乎都不错，但是这是int类型（32位值）的整数文字“ 1”起作用的地方。 因此，当j变量的值超过31时，按位移位将开始返回先前返回的值。如果此行为不是程序员想要的，则值1必须表示为long ： 1L << j或（long） 1 << j 。

不行 2：初始化顺序


资料来源： 华为云：如今PVS-Studio多云

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); // <= } } }

类中声明字段的顺序有所不同，因为它们的初始化顺序与声明的顺序相同。 忘记这一事实会导致难以捉摸的错误，如上面的错误。

分析器指出，静态字段LOG在初始化为空值时会在构造函数中取消引用，这将导致引发一系列异常NullPointerException- > ExceptionInInitializerError 。

但是，为什么在调用构造函数时静态字段LOG的值为null ？

提示是ExceptionInInitializerError 。 构造函数正在初始化静态字段INSTANCE ，该字段在LOG字段之前声明。 这就是为什么在调用构造函数时尚未初始化LOG字段的原因。 为了使此代码正常工作，必须在调用之前初始化LOG字段。

第一：面向复制粘贴的编程


来源： Apache Hadoop代码质量：生产VS测试

V6072找到两个相似的代码片段。 也许这是一个错字，应该使用“ localFiles”变量而不是“ localArchives”。 LocalDistributedCacheManager.java（183），LocalDistributedCacheManager.java（178），LocalDistributedCacheManager.java（176），LocalDistributedCacheManager.java（181）

 public synchronized void setup(JobConf conf, JobID jobId) throws IOException { .... // Update the configuration object with localized data. if (!localArchives.isEmpty()) { conf.set(MRJobConfig.CACHE_LOCALARCHIVES, StringUtils .arrayToString(localArchives.toArray(new String[localArchives // <= .size()]))); } if (!localFiles.isEmpty()) { conf.set(MRJobConfig.CACHE_LOCALFILES, StringUtils .arrayToString(localFiles.toArray(new String[localArchives // <= .size()]))); } .... }

我们在前10名列表中的第一名是复制粘贴，或者是由于不小心使用此技术而导致的错误。 第二个if语句看起来很像第一个if语句的副本，但其中一些变量已修改：

  • localArchives已更改为localFiles ;
  • MRJobConfig.CACHE_LOCALARCHIVES已更改为MRJobConfig.CACHE_LOCALFILES 。

但是程序员即使在此简单操作中也设法犯了一个错误：分析器指出的行中的if语句仍在使用localArchives变量，而不是显然打算使用的localFiles变量。

结论


修复在后期开发阶段或发行后发现的错误需要大量资源。 静态分析仪PVS-Studio允许您在编码阶段检测错误，从而使其更容易，更便宜。 许多公司已经开始定期使用分析仪，从而使开发人员的生活更轻松。 如果您想真正享受工作，请尝试PVS-Studio 。

我们不会止步于此，并计划继续改进和增强我们的工具。 敬请关注明年的新诊断和文章，以及更多有趣的错误。

我看到你喜欢冒险！ 首先，您在2019年击败了C＃项目中的前10个bug ，现在您也应对Java！ 欢迎来到下一个级别-有关2019年C ++项目中最佳错误的文章。

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

More articles:


All Articles