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

一些可能的空指针引用和正则表达式错误 #48

Open
mmyjona opened this issue Nov 16, 2018 · 19 comments
Open

一些可能的空指针引用和正则表达式错误 #48

mmyjona opened this issue Nov 16, 2018 · 19 comments

Comments

@mmyjona
Copy link

mmyjona commented Nov 16, 2018

环境信息

问题描述

我们用我们的源伞Pinopint扫描器扫描出一些有趣的报告,可以参考看看是否有修复价值。

错误信息

APIJSON项目 - 2018年11月16日10-25-54批次 - 已标注缺陷html报告 - 2018年11月16日10-34-30.pdf

@TommyLemon
Copy link
Collaborator

感谢,大概看了下,部分问题已确认确实存在,后续将持续改进。

@anonymoustoken
Copy link

@mmyjona 您这个 是商业工具吗? 我在网上找了一下 怎么 没有找到呢?

@TommyLemon
Copy link
Collaborator

@anonymoustoken 同问,我也没找到哈哈

@TommyLemon
Copy link
Collaborator

TommyLemon commented Dec 2, 2018

总结这个报告:
总共 10617 行代码,16 个「可能」的 bug, 24 个改进建议,
平均每行代码 bug 率低至 0.15%,也就是 (1 - bugs/lines) 高达 99.85% 。
可见 APIJSON 代码非常严谨可靠。

@anonymoustoken
Copy link

下一个版本说明时候 发布 @TommyLemon

@TommyLemon
Copy link
Collaborator

@mmyjona
Copy link
Author

mmyjona commented Apr 25, 2019

@mmyjona 您这个 是商业工具吗? 我在网上找了一下 怎么 没有找到呢?

https://www.sourcebrella.com/pinpoint/
是商业工具哈。

@TommyLemon
Copy link
Collaborator

TommyLemon commented Jun 1, 2019

确实是误判为 bug 的报告用例 1

src/main/java/zuo/biao/apijson/server/AbstractSQLConfig.java: 897 (可信度: 65%)
zuo.biao.apijson.server.AbstractSQLConfig.putWhere(String, Object, boolean)中
AbstractSQLConfig.combine 可能的空指针解引用

对应实际文件
https://github.com/APIJSON/APIJSON/blob/d8cfc9e6be87bd6116ecf8a125a52021f0154761/APIJSON-Java-Server/APIJSONLibrary/src/main/java/zuo/biao/apijson/server/AbstractSQLConfig.java
内方法

	@Override
	public AbstractSQLConfig putWhere(String key, Object value, boolean prior) {
		if (key != null) {
			if (where == null) {
				where = new LinkedHashMap<String, Object>();	
			}
			where.put(key, value);

			combine = getCombine();
			List<String> andList = combine == null ? null : combine.get("&");
			if (value == null) {
				andList.remove(key);
			}
			else if (andList == null || andList.contains(key) == false) {
				int i = 0;
				if (andList == null) {
					andList = new ArrayList<>();
				}
				else if (prior && andList.isEmpty() == false) {
					if (andList.contains(KEY_ID)) {
						i ++;
					}
					if (andList.contains(KEY_ID_IN)) {
						i ++;
					}
					if (andList.contains(KEY_USER_ID)) {
						i ++;
					}
					if (andList.contains(KEY_USER_ID_IN)) {
						i ++;
					}
				}

				if (prior) {
					andList.add(i, key); //userId的优先级不能比id高  0, key);
				} else {
					andList.add(key); //AbstractSQLExecutor.onPutColumn里getSQL,要保证缓存的SQL和查询的SQL里 where 的 key:value 顺序一致
				}
			}
			combine.put("&", andList);
		}
		return this;
	}

实际上在前面已经用
combine = getCombine();
做了处理,return 的值不会是 null

	@NotNull
	@Override
	public Map<String, List<String>> getCombine() {
		List<String> andList = combine == null ? null : combine.get("&");
		if (andList == null) {
			andList = where == null ? new ArrayList<String>() : new ArrayList<String>(where.keySet());
			if (combine == null) {
				combine = new HashMap<>();
			}
			combine.put("&", andList);
		}
		return combine;
	}


确实是误判为 bug 的报告用例 2

src/main/java/zuo/biao/apijson/JSONObject.java: 411 (可信度: 95%)
1 这里比较了 clazz 和 null,说明 clazz 可能为空指针
2 调用 clazz 的 java.lang.Class.getName 方法(使用可疑的空指针)

对应实际文件
https://github.com/APIJSON/APIJSON/blob/d8cfc9e6be87bd6116ecf8a125a52021f0154761/APIJSON-Java-Server/APIJSONLibrary/src/main/java/zuo/biao/apijson/JSONObject.java
内方法

	/**put and return value
	 * @param key  StringUtil.isEmpty(key, true) ? key = value.getClass().getSimpleName();
	 * @param value
	 * @return value
	 */
	@Override
	public Object put(String key, Object value) {
		if (value == null) {
			Log.e(TAG, "put  value == null >> return null;");
			return null;
		}
		if (StringUtil.isEmpty(key, true)) {
			Class<?> clazz = value.getClass();
			if (clazz == null || clazz.getAnnotation(MethodAccess.class) == null) {
				throw new IllegalArgumentException("puts  StringUtil.isNotEmpty(key, true) == false" +
						" && clazz == null || clazz.getAnnotation(MethodAccess.class) == null" +
						" \n key为空时仅支持 类型被@MethodAccess注解 的value !!!" +
						" \n 如果一定要这么用,请对 " + clazz.getName() + " 注解!" +
						" \n 如果是类似 key[]:{} 结构的请求,建议用 putsAll(...) !");
			}
			key = value.getClass().getSimpleName();
		}
		return super.put(key, value);
	}

406 行
Class<?> clazz = value.getClass();
Class 是对象声明后就有的,到执行这行前 value 已经判断过,又不会为 null,
所以实际上 clazz 不会为 null,407 行
if (clazz == null || clazz.getAnnotation(MethodAccess.class) == null) {
中 clazz == null 是一个冗余的判断,后面删掉了,实际上 411 行
clazz.getName()
永远不会因为 clazz == null 而导致 throw NullPointerException,
所以这也是一个误判为 bug 的用例。



确实是误判为 bug 的报告用例 3

严重 空指针解引用 src/main/java/zuo/biao/apijson/server/AbstractSQLConfig.java:844 (可信度 50%)

对应实际文件
https://github.com/Tencent/APIJSON/blob/d8cfc9e6be87bd6116ecf8a125a52021f0154761/APIJSON-Java-Server/APIJSONLibrary/src/main/java/zuo/biao/apijson/server/AbstractSQLConfig.java
内方法

	/**
	 * @param key
	 * @param exactMatch
	 * @return
	 */
	@JSONField(serialize = false)
	@Override
	public Object getWhere(String key, boolean exactMatch) {
		if (exactMatch) {
			return where == null ? null : where.get(key);
		}

		Set<String> set = key == null || where == null ? null : where.keySet();
		if (set != null) {
			synchronized (where) {
				if (where != null) {
					int index;
					for (String k : set) {
						index = k.indexOf(key);
						if (index >= 0 && StringUtil.isName(k.substring(index)) == false) {
							return where.get(k);
						}
					}
				}
			}
		}
		return null;
	}

843 行已经用 set != null 来判断,而根据 842 行代码,where 为 null 时 set 必定为 null,
永远不会因为 where == null 而导致 throw NullPointerException,
所以这也是一个误判为 bug 的用例。

@TommyLemon
Copy link
Collaborator

TommyLemon commented Jun 1, 2019

确实是验证为 bug 的报告用例 1

缺陷位置:
src/main/java/zuo/biao/apijson/server/AbstractParser.java: 1077
标注: 确认
时间: 2018-11-16 10:25:54
可信度: 98%
缺陷ID: 551386e3a9a5cb4d8d4f43e32105174e

确实是验证为 bug 的报告用例 2

image
已在新版通过 Log.DEBUG 判断,false 时不返回数据库对应的 SQLException 真实信息来解决
https://github.com/Tencent/APIJSON/blob/bbcf05e01c724894a5ba0dcddd49a14bbfeb5346/APIJSONORM/src/main/java/apijson/orm/AbstractParser.java
image

@TommyLemon
Copy link
Collaborator

确实是验证为 bug 的报告用例 1
缺陷位置:
src/main/java/zuo/biao/apijson/server/AbstractParser.java: 1077
标注: 确认
时间: 2018-11-16 10:25:54
可信度: 98%
缺陷ID: 551386e3a9a5cb4d8d4f43e32105174e

已解决
bbcf05e

@TommyLemon
Copy link
Collaborator

TommyLemon commented Jan 23, 2021

这两个摘要有点奇怪
image

对应的代码行只是简单的一个 setter 方法给成员变量赋值,并没有加锁,这里也不需要加锁
https://github.com/Tencent/APIJSON/blob/d8cfc9e6be87bd6116ecf8a125a52021f0154761/APIJSON-Java-Server/APIJSONLibrary/src/main/java/zuo/biao/apijson/server/AbstractParser.java
image

@TommyLemon
Copy link
Collaborator

@TommyLemon
Copy link
Collaborator

@Rkyzzy
Copy link
Contributor

Rkyzzy commented Apr 22, 2021

你好!请问这两个剩余的TODO修复了吗?我能尝试修复吗,谢谢!

@TommyLemon
Copy link
Collaborator

你好!请问这两个剩余的TODO修复了吗?我能尝试修复吗,谢谢!

还没有哦,可以的,非常感谢👍

Rkyzzy added a commit to Rkyzzy/APIJSON that referenced this issue Apr 24, 2021
Rkyzzy added a commit to Rkyzzy/APIJSON that referenced this issue Apr 24, 2021
TommyLemon added a commit that referenced this issue Apr 25, 2021
使用entrySet迭代器替代keySet迭代器提高效率 #48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants