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

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

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

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

doOtherTasks();

上述代码存在下述问题：

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

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

如果觉得我的文章对您有用，请随意打赏。您的支持将鼓励我继续创作！

打赏支持

共收到 17 条回复时间点赞

simonpatrick #1 · 2018年08月10日

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

onesbyones #2 · 2018年08月10日

为啥要判断：

obj.isSuccess()==true

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

simple #3 · 2018年08月10日

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

Jerry li #4 · 2018年08月10日

对 java 代码不太熟悉，不一定对：

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

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

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

对

simonpatrick 回复

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

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

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

对

onesbyones 回复

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

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

对

Jerry li 回复

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

simonpatrick #8 · 2018年08月10日

对

乾行回复

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

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

对

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();
}

槽神 #10 · 2018年08月10日

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

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

doOtherTasks();

槽神 #11 · 2018年08月10日

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