白盒测试 一个简单的 if 判断可能导致潜在的业务不安全

乾行 · 2018年08月09日 · 最后由 SinDynasty 回复于 2018年08月11日 · 1873 次阅读

下面是一个简单的 if 构成的程序片段:

obj = rpc.getObj();
if(obj != null && obj.isSuccess()==true){
    doTask1();
}

doOtherTasks();

上述代码存在下述问题:

  1. obj==null 时,没有任何代码处理; 2.obj!=null && obj.isSuccess()==false 时,没有代码处理;

如果在代码中看到类似的代码,请停下来思考一下代码逻辑是否影响到业务的安全性。举例说明潜在的风险:
rpc.getObj() 是判断一个用户是否是黑名单用户,由于 rpc 方法在外部接口异常时返回 null,可能导致在黑名单里面的用户做了其他事情(即调用了 doOtherTasks() 方法)。

如果觉得我的文章对您有用,请随意打赏。您的支持将鼓励我继续创作!
共收到 17 条回复 时间 点赞

那也是 doothertask 的问题吧 和 if 没什么关系吧

为啥要判断:

obj.isSuccess()==true

obj.isSuccess() 这个方法,本身返回的就是布尔类型吧

白盒分析的 topic 很少,楼主加油,继续分享

对 java 代码不太熟悉, 不一定对:

按我的理解上面的代码等效于:
obj = rpc.getObj();
if(obj != null && obj.isSuccess()==true){
doTask1();
return;
}
else{
doOtherTasks();
}

如果这种情况, null 和 success 是 false 的情况不是都在 doOtherTasks 里处理了吗?

simonpatrick 回复

doothertask 与 if 语句没有关系,但是由于没有考虑 if 语句没有对应的 else 处理其他异常,导致 doothertask 成功执行了。

部分接口从业务上来说应该是强依赖关系,一旦返回非预期结果应该中断流程或者抛出异常。

onesbyones 回复

是这样的,写成这样是为了强调仅判断了返回结果 success==true 的情况

乾行 #10 · 2018年08月10日 Author
Jerry li 回复

描述有点问题,if 语句中的 return 去掉。doTask1() 与 doothertask() 是顺序执行的。

乾行 回复

坦白讲我觉得还是 else 里面要处理什么响应的验证也是在 doothertask 这个方法里面做 本质应该我觉得是 doothertask 这个业务校验不严格 以后如果其他地方调用可能会出现一样的问题

simonpatrick 回复

看到的代码类似这样:

obj1 = rpc1.getObj();
if(obj != null && obj.isSuccess()==true){
    doTask1();
}

obj2= rpc2.getObj();
if(obj2 != null && obj2.isSuccess()==true){
    doTask2();
}

obj3= rp3.getObj();
if(obj3!= null && obj3.isSuccess()==true){
    doTask3();
}

这样就了了……这是习惯不好,跟 IF 没啥关系
另外,非空判断尽量把 null 写左边吧,少一点 npe 的报错~这是写 java 的经验,其他语言就不清楚了~

obj = rpc.getObj();
if(null != obj && obj.isSuccess()){
    doTask1();
    return;
}

doOtherTasks();

我合作过的 coder,最常犯的、最愚蠢的而且能反复错的问题:
1、循环外定义变量,内部不做初始化
2、不做 NPE 预处理,出了事就一脸懵逼反问测试会不会操作,换个数据给他立刻吃惊:还有这种骚操作?
3、catch exception 之后不在 finally 里面做事务完整性保护习惯性在 catch 里面做
……

乾行 #12 · 2018年08月10日 Author
槽神 回复

赞,代码气味很好

乾行 回复

原来是被你的 return 给带歪了

乾行 #14 · 2018年08月10日 Author
Jerry li 回复

抱歉

一定要加 return...

乾行 #16 · 2018年08月10日 Author

是常见的任务串行处理流程中常见的问题,if 语句内部可能处理的正常逻辑

抛开业务讲代码安不安全?你们连需求都不知道,连 doTask1 和 doOtherTasks 的内容都不知道,谈代码安不安全有啥意义

需要 登录 后方可回复, 如果你还没有账号请点击这里 注册