select *的规则检查有bug

[index3389]

非常感谢博主的速度，这么快就增加了select *的规则检查。

今天体验了一下，发现有bug。

select order_id,user_id,order_amount * discount as product_amount from orders where user_id=#{user_id} and status=1 and is_del=false

这段SQL被插件扫描出来了，里面有*，但是并不是查询所有的字段。

[index3389]

当然了，如果从另外一个角度看，SQL里面不能有任何业务逻辑（业务逻辑应该放在程序里面，以便系统的扩展）。扫描出来，也是没有问题的。

算不算一个问题，看博主的权衡了

[index3389]

select order_id,user_id,order_amount from orders where user_id=#{user_id} and status=1 and is_del=false and available_amount > order_amount*0.05

这种也会扫描出来

[donhui]

当然了，如果从另外一个角度看，SQL里面不能有任何业务逻辑（业务逻辑应该放在程序里面，以便系统的扩展）。扫描出来，也是没有问题的。

算不算一个问题，看博主的权衡了

这里的 * 是乘法的意思了

[donhui]

这个情况 还没想好怎么规避

[snoopyhzy]

自己其实也一直想写个不依赖于SONAR的MYBATIS或IBATIS SQL分析器，包括去支持SQL注入风险检测（主要是$替换在where中使用，in ('') 和 函数出现这种场景多一点），主要是个人不太会写XML读取一直没有找到一个好点的反向MYBATIS XML为SQLID对照关系的开源代码。

这个其实感觉和博主没引入任何一个SQL解析器通过生成SQL的AST语法树有关系。
个人觉得可以尝试使用druid的sqlparser进行这个事情，但这个可能也没有行号，博主通过内容去匹配行号，在小的规范的项目里可能还没什么，我们的项目一个XML里有几千个语句可能就不适合了。

如果通过AST去处理这个事情，那参数里可能就要加数据库类型，比如MYSQL还是ORACLE还是PG之类的了。

当然引入AST后可以基于SQL语法做更多的判定，包括性能判定。
比如博主认为1=1是个风险点，我们内部也这样查过，但不能用1等于1，需要判定恒等，否则很大程度上开发会通过2=2来规避这个规则。并且1=1是一个拼接条件表达式的必要内容，用这个判定风险，误报的概率可能会非常大，比较合适的做法是通过AST语法树，判定WHERE 节点中，非可选的是否只有一个恒等表达式。

就性能上，一些做法的参考，包括
1.纯基于语法，比如去判定，是否存在笛卡尔积。
2.通过JDBC关联一下数据库，获取一些METADATA，包括索引/索引字段信息，去判定OTLP语句有没有正确使用索引等等。

但感觉这些就要对这个工具做比较大的改造了，毕竟基于AST和基于纯文本的扫描规则，差异还是很大的。

[donhui]

@snoopyhzy
非常赞同你的这些想法
1、匹配行号在 Mapper 文件比较大时，会出现性能问题
2、AST 我们也曾内部讨论过，目前也还停留在理论层面，我们还没有进行相关实践
3、关于连接数据库，连接不同环境的数据库（数据量不一样），可能会有不同的结果
4、相对对这个工具做大的改造，我更倾向与写一个通用的工具，而不是 SonarQube 插件

再次谢谢你～～

[snoopyhzy]

@donghui AST的话，相关的内容我已经内部实践过了，也自己查过mybatis的了，但mybatis有一些语法要完全从XML还原为SQL还是很麻烦的，比如trim标签/if标签等等。
因为公司的一些要求的原因不能开源。
用的druid的ast，写法在官方都有说明 https://github.com/snoopyhzy/druid-sql-ast-gui

是直接用visit访问，每个检查规则都继承了vistor很方便的 https://github.com/alibaba/druid/wiki/SQL_Parser_Demo_visitor
然后用java spi或者其他方法来注册规则，以实现规则的动态配置。

我自己也写了个小工具来快速的查看AST语法树的结构，小工具是我非工作时间开发的，所以可以开源，https://github.com/snoopyhzy/druid-sql-ast-gui，找到语法树对应的节点，override一下visit方法，把具体的结果加到结果报告里就好了。

[donhui]

@snoopyhzy 谢谢你的反馈，我学习下～～

[snoopyhzy]

@snoopyhzy 谢谢你的反馈，我学习下～～

其实关注到您这个项目主要是想找个MYBATIS的解析插件把行号和对应的SQL解析出来。

fock了一下，我来试试改，加AST，做点共享，最近写的多了有感觉，刚刚开发把内部的检查工具转SONAR插件了，被仓库KEY冲突坑了好久。- -邮件您咨询也没啥反应哈哈。。

[donhui]

@snoopyhzy
前几天比较忙把邮件提醒给关掉了，刚打开，哈哈
一种插件只能支持一种语言的
它是根据文件后缀判断是哪种语言的
一个文件貌似也只能被一种语言的插件解析

[snoopyhzy]

@snoopyhzy
前几天比较忙把邮件提醒给关掉了，刚打开，哈哈
一种插件只能支持一种语言的
它是根据文件后缀判断是哪种语言的
一个文件貌似也只能被一种语言的插件解析

不是，一个插件可以多语言的，我已经成了，我的问题是仓库的KEY重了。一开始没注意到web.log，只看了sonar.log。

[donhui]

@snoopyhzy
前几天比较忙把邮件提醒给关掉了，刚打开，哈哈
一种插件只能支持一种语言的
它是根据文件后缀判断是哪种语言的
一个文件貌似也只能被一种语言的插件解析

不是，一个插件可以多语言的，我已经成了，我的问题是仓库的KEY重了。一开始没注意到web.log，只看了sonar.log。

嗯，仓库的 KEY 这个我这边是 git 仓库的 namespace+project_name ，这样就不会重复了

[snoopyhzy]

@donhui 就这个ISSUE做了个DEMO供您参考，SQL语言没指定就是DRUID的通用解释器，具体应该配到SONAR中应该配变量，入口类参考Checker实际上那个SQL嵌入到您那个扫描器的matchRuleAndSaveIssue方法里就可用了。

snoopyhzy@671df82

因为做的最简单的，结果是
[Result{obj=*, rule='NoUseSelectAllColumnsRule'}]

Process finished with exit code 0

[LinWanCen]

https://github.com/gretard/sonar-sql-plugin

这个插件里实现了，而且他的描述比较美观，可以参考

[donhui]

@LinWanCen 感谢，有空我研究下～～

[snoopyhzy]

https://github.com/gretard/sonar-sql-plugin

这个插件里实现了，而且他的描述比较美观，可以参考

哇，自从你离开sdc以后，很久没看到你在wiki更新了，没想到还能在github看到