Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some potential NPE bugs #1403

Closed
JulyChen728 opened this issue Apr 14, 2020 · 6 comments · Fixed by #1589
Closed

Some potential NPE bugs #1403

JulyChen728 opened this issue Apr 14, 2020 · 6 comments · Fixed by #1589
Labels
good first issue Good for newcomers kind/enhancement Category issues or prs related to enhancement.

Comments

@JulyChen728
Copy link

Hi all, our bug scanner has reported some NPE bugs.

1.The first one is at FileInJarReadableDataSource.java#L99 since the variable inputStream may be null.

2.The second one is at SimpleHttpClient.java#L168 cause the variable charset may be null. Since the variable charset is checked for null at SimpleHttpClient.java#L100, a NPE bug may take place when the call chain is SimpleHttpClient.java#L111 => SimpleHttpClient.java#L168.

3.The third one is at EagleEye.java#L101 since the variable tmpPath may be null. A possible call chain is getSystemProperty() => EagleEye.java#L99 => EagleEye.java#L101.The same bug may take place at EagleEye.java#L113. Some familiar situationa are at EagleEye.java#L64 since the variable charsetName may be null, EagleEye.java#L89 since the variable userHome may be null.

4.The forth is at StatisticSlot.java#L147. A possible call chain is getCurNode() =>StatisticSlot.java#L136 =>StatisticSlot.java#L147. Since the method getCurNode() may return null, the variable node at StatisticSlot.java#L147 may be null and thus a NPE bug may take place.

5.The fifth is at SimpleHttpResponse.java#L50. A possible call chain is getHeader() =>SimpleHttpResponse.java#L49 =>SimpleHttpResponse.java#L50.Since the method getHeader() may return null, the variable contentType at SimpleHttpResponse.java#L50 may be null and thus a NPE bug may take place.

Thanks.

@sczyh30 sczyh30 added good first issue Good for newcomers kind/enhancement Category issues or prs related to enhancement. labels Apr 20, 2020
@HermioneSW
Copy link

@sczyh30 Thanks for your feedback. We are trying to get some results of our dogfooding. Could you please confirm which ones are indeed bugs? Thanks a million!

@JiangZian
Copy link
Contributor

1.The first one is at FileInJarReadableDataSource.java#L99 since the variable inputStream may be null.

  • 1.jarEntry对象为空时 java.util.zip.ZipFile#getInputStream#L346 会显示抛出NPE
    • 此异常已在上层捕获,赋值流对象无真实IO,危险系数较低
  • 2.内存分配失败时,native 方法返回值为0时返回null值 FileInJarReadableDataSource.java#L100会抛出NPE
    • 此异常已在上层捕获,赋值流对象无真实IO,危险系数较低

2.The second one is at SimpleHttpClient.java#L168 cause the variable charset may be null. Since the variable charset is checked for null at SimpleHttpClient.java#L100, a NPE bug may take place when the call chain is SimpleHttpClient.java#L111 => SimpleHttpClient.java#L168.

  • 此问题真实存在,需要增加请求charset非空判断,避免charset.name()空指针异常

@JiangZian
Copy link
Contributor

下图对应的是1.2内存分配失败代码逻辑
image
下图对应的是1.1实例为空
image

@JiangZian
Copy link
Contributor

3.The third one is at EagleEye.java#L101 since the variable tmpPath may be null. A possible call chain is getSystemProperty() => EagleEye.java#L99 => EagleEye.java#L101.The same bug may take place at EagleEye.java#L113. Some familiar situationa are at EagleEye.java#L64 since the variable charsetName may be null, EagleEye.java#L89 since the variable userHome may be null.

  • 这个问题不存在,已有非空判断

String charsetName = EagleEyeCoreUtils.getSystemProperty("EAGLEEYE.CHARSET"); if (EagleEyeCoreUtils.isNotBlank(charsetName)) { charsetName = charsetName.trim();

public static boolean isNotBlank(String str) { return !isBlank(str); }

public static boolean isBlank(String str) { int strLen; if (str == null || (strLen = str.length()) == 0) { return true; }

@JiangZian
Copy link
Contributor

4.The forth is at StatisticSlot.java#L147. A possible call chain is getCurNode() =>StatisticSlot.java#L136 =>StatisticSlot.java#L147. Since the method getCurNode() may return null, the variable node at StatisticSlot.java#L147 may be null and thus a NPE bug may take place.

  • context,上层已经判断了
  • sentinel 的context node,是根据上下文名称在enter时生成(上下文初始化时ContextUtil.trueEnter),忽略空值问题

5.The fifth is at SimpleHttpResponse.java#L50. A possible call chain is getHeader() =>SimpleHttpResponse.java#L49 =>SimpleHttpResponse.java#L50.Since the method getHeader() may return null, the variable contentType at SimpleHttpResponse.java#L50 may be null and thus a NPE bug may take place.

  • 我发现确实有空指针问题,并提交了代码

@JiangZian
Copy link
Contributor

@sczyh30 这个后续的改动您看看,是否有必要合并

CST11021 pushed a commit to CST11021/Sentinel that referenced this issue Nov 3, 2021
[ISSUE alibaba#1400]do disk space detection in another thread
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers kind/enhancement Category issues or prs related to enhancement.
Projects
None yet
4 participants