SonarQube扫描bugs&漏洞处理汇总
工作中遇到和参考其他资料汇总--仅供自我学习
目录
Bugs
Use an "instanceof" comparison instead.
Cast one of the operands of this integer division to a "double"
Remove this throw statement from this finally block.
Remove this return statement from this finally block
A "NullPointerException" could be thrown; "pkList" is nullable here.
Use try-with-resources or close this "ResultSet" in a "finally" clause.
Use "Arrays.toString(array)" instead.
Save and re-use this “Random”.
Either re-interrupt this method or rethrow the "InterruptedException".
Synchronize on a new "Object" instead.
Replace the call to "Thread.sleep(...)" with a call to "wait(...)"
Use "BigDecimal.valueOf" instead
Call "Optional#isPresent()" before accessing the value.
Use try-with-resources or close this "PreparedStatement" in a "finally" clause.
漏洞
Make this "public static producer" field final
Use a logger to log this exception
Lower the visibility of this setter or remove it altogether.
Do something with the "boolean" value returned by "delete".
Make this "public static redisTemplate" field final
Implement Iterator rather than Enumeration.
Reduce the total number of break and continue statements in this loop to use at most one.
Use classes from the Java API instead of Sun classes.
Remove this use of "encode"; it is deprecated.
异味
Reorder the modifiers to comply with the Java Language Specification.
Put single-quotes around '?' to use the faster "indexOf(char)" method.
Use a StringBuilder instead.
Replace "Collections.EMPTY_LIST" by "Collections.emptyList()"
Use isEmpty() to check whether the collection is empty or not.
Return an empty collection instead of null.
Put single-quotes around '/' to use the faster "indexOf(char)" method.
Combine this catch with the one at line 200,which has the same body
Add a nested comment explaining why this method is empty, throw an UnsupportedOperationException or complete the implementation.
Make this "code" field final.
Replace this lambda with a method reference.
Bugs
- Use an "instanceof" comparison instead.
修改为:
- Cast one of the operands of this integer division to a "double"
修改为:
- Remove this throw statement from this finally block.
说明:在finally块中使用return、break、throw等可以抑制try或catch块中抛出的任何未处理的Throwable的传播,修改为:
- Remove this return statement from this finally block
说明:因为finally里面写了return语句的时候,就会覆盖掉try代码块里面的return。因为finally是肯定会执行的。例子如下:
上述代码修改为:
- A "NullPointerException" could be thrown; "pkList" is nullable here.
增加空值判断,如下所示:
- Use try-with-resources or close this "ResultSet" in a "finally" clause.
修改为:
或者参考如下:
- Use "Arrays.toString(array)" instead.
参考如下:
说明:这种提示是随机数应该需要重用,然后他给出的参考是这样的
- Either re-interrupt this method or rethrow the "InterruptedException".
修改为:
- Synchronize on a new "Object" instead.
修改为如下:
- Replace the call to "Thread.sleep(...)" with a call to "wait(...)"
说明:如果在当前线程持有锁时调用Thread.sleep(…),则可能导致性能和可伸缩性问题,甚至更糟,因为持有锁的线程的执行被冻结。最好对monitor对象调用wait(…)来暂时释放锁并允许其他线程运行。修改为如下:
- Use "BigDecimal.valueOf" instead
说明:由于浮点不精确,您不太可能从BigDecimal(double)构造函数中获得预期的值。修改为如下:
- Call "Optional#isPresent()" before accessing the value.
说明:Optional value可以保存值,也可以不保存。可选方法中的值可以使用get()方法访问,但它会抛出一个
如果不存在值,则NoSuchElementException。为了避免异常,应该总是在调用get()之前调用isPresent()方法。
另外,请注意其他方法,如orElse(…)、orElseGet(…)或orElseThrow(…),可用于指定如何处理空的可选对象。
修改为如下:
- Use try-with-resources or close this "PreparedStatement" in a "finally" clause.
修改为如下所示:使用try-with-resources语法
漏洞
- Make this "public static producer" field final
修改为如下:
- Use a logger to log this exception
修改为如下:
- Lower the visibility of this setter or remove it altogether.
解决方法:去掉枚举中的set方法
- Do something with the "boolean" value returned by "delete".
修改为如下:
- Make this "public static redisTemplate" field final
修改为如下:
- Implement Iterator rather than Enumeration.
修改为如下:
- Reduce the total number of break and continue statements in this loop to use at most one.
修改为如下:
- Use classes from the Java API instead of Sun classes.
修改为如下:
原方法
BASE64Encoder encoder = new BASE64Encoder();
String imagestr = encoder.encode(captcha);
BASE64Decoder decoder = new BASE64Decoder();
byte[] bytes = decoder.decodeBuffer(imagestr);
现方法
import java.util.Base64.Encoder import java.util.Base64.Decoder
Encoder encoder = Base64.getEncoder();
String result = encoder.encodeToString(byteArray);
Decoder decoder = Base64.getDecoder();
byte[] result = decoder.decode(str);
- Remove this use of "encode"; it is deprecated.
修改为如下:
- Use a logger to log this exception
这种提示就是异常应该用日志打印出来。
- ‘password’ detected in this expression, review this potentially hard-coded credential
提示密码不能直接这样传递,不安全。但是也没有提供参考的案例。所以我是这样的改的,也能消除漏洞。如下图:
- Make areaList a static final constant or non-public and provide accessors if needed
- Secure this “Transformer” by either disabling external DTDs or enabling secure processing.
提示应该保护XML变换器。创建javax.xml.transform.Transformer但未启用“安全处理”或创建一个而不禁用外部DTD时,可能会发生XML外部实体或XSLT外部实体(XXE)漏洞。 如果该外部实体被攻击者劫持,则可能导致机密数据泄露,拒绝服务,服务器端请求伪造,从解析器所在机器的角度进行端口扫描,以及其他系统影响。 进行修改如下可以消除漏洞:
- Do something with the “boolean” value returned by “delete”.
提示当包含操作状态代码时,不应忽略返回值。也就是说不应该忽略文件删除操作的结果。 所以进行如下修改,但是如下修改虽然修复了漏洞,但是新增了异味。
异味提示"java.nio.Files#delete" should be preferred (squid:S4042)。应该使用Files.delete()方法,而不能之间文件delete.所以最后修改成:
- Make this “public static userInfoUrl” field final
这种“public static”字段应该成员变量应该是不变的,所以需要加上final修饰,如下:
- 还有几种漏洞不好修复,暂时没有思路
- Change this method so it throws exceptions
这种提示,TrustManagers不应盲目接受任何证书。通常会创建X509TrustManager接口的空实现,以允许连接到未由根证书颁发机构签名的主机。 这样的实现将接受任何证书,这使得应用程序容易受到中间人攻击。 正确的解决方案是提供适当的信任存储。
- Use the recommended AES (Advanced Encryption Standard) instead.
这种原来用DES加密的提示不应使用DES(数据加密标准)和DESede(3DES)。它推荐的使用AES.但是将DES加密改成AES加密虽然程序异味消除了,但是程序肯定不对吧,加密方式换了肯定会出问题的吧。
异味
- Reorder the modifiers to comply with the Java Language Specification.
说明:java语言规范建议按照以下顺序列出修饰符:
1. Annotations
2. public
3. protected
4. private
5. abstract
6. static
7. final
8. transient
9. volatile
10. synchronized
11. native
12. strictfp
修改如下:
- Put single-quotes around '?' to use the faster "indexOf(char)" method.
说明:带有单个字母字符串的indexOf或lastIndexOf调用可以通过切换到带有char参数的调用来提高性能。
修改如下:
- Use a StringBuilder instead.
说明:字符串是不可变的对象,所以连接不是简单地将新字符串添加到现有字符串的末尾。相反,在每个循环迭代中,第一个字符串被转换为中间对象类型,第二个字符串被追加,然后中间对象被转换回字符串。而且,这些中间操作的性能会随着字符串变长而下降。因此,最好使用StringBuilder。
修改为如下:
- Replace "Collections.EMPTY_LIST" by "Collections.emptyList()"
说明:由于在Java 5中引入了泛型,所以建议使用泛型类型(如List<String>),而不是使用原始类型(如List)。将原始类型分配给泛型类型是不安全的,并将生成警告。老EMPTY_……Collections类的字段返回原始类型,而较新的empty…()方法返回泛型类型。
修改如下:
- Use isEmpty() to check whether the collection is empty or not.
修改如下:
- Return an empty collection instead of null.
说明:应该返回空数组和集合,而不是null
修改为:
- Put single-quotes around '/' to use the faster "indexOf(char)" method.
修改为:
- Combine this catch with the one at line 200,which has the same body
修改为:
- Add a nested comment explaining why this method is empty, throw an UnsupportedOperationException or complete the implementation.
修改为:
- Make this "code" field final.
修改为:
- Replace this lambda with a method reference.
修改为:
- Replace the type specification in this constructor call with the diamond operator ("<>").
Java 7引入了菱形运算符(<>)来减少泛型代码的冗长。 例如,您现在可以使用<>简化构造函数声明,而不必在其声明及其构造函数中声明List的类型,编译器将推断该类型。如下:
- Add a default case to this switch
swatch 中没有default,也没有break;虽然上面的代码不要break也不会有什么问题。但是万一哪天变了,不是return 就很容易出错了吧。
- Remove this empty statement.
两个分号,代码中有很多地方有这种情况感觉,删掉多余的。
- Remove this useless assignment to local variable “XXX”.
上图,定义的变量但是没有使用,就会抛出这种异味,解决这种异味,是需要看看这个变量有什么作用,没有作用的可以删除掉,如果不改随意改动,可以在他们下面增加一条日志打印他们,这样也能消除这个异味。
- Directly append the argument of String.valueOf().
String.valueOf()不应附加到String。这里我的理解是,result是string类型,arerType是int类型,在拼接的时候会自动的将int类型转换成string,不用多此一举。
- Define a constant instead of duplicating this literal “XXXX” 4 times.
类似这种,当一个不变的字符串在一个文件中多次出现,就应该给这些字符串提取成常量,这样方便以后修改和维护。但是说实话提取常量这个异味真的很枯燥,并且代码中有大量的这种情况,感觉每个项目或者每个模块都应该提取一个常量类,这样这个模块用到这些不变字符串,就直接从这个类中获取,但是这个工作量有点大哈哈,我就简单的尝试了一下,把自定义报表那部分提取常量,但是进行了一部分就没有进行下去了,枯燥且工作量大
但是我觉得这个工作还是做比较好,这样便于以后的维护,常量值改变只用改一个地方就好了,不用所有地方都修改。
- Use a StringBuilder instead.
当字符串需要频繁变化时,用stringBuilder代替string .这里还有一种异味是代码中有的地方用的是StringBuffer,也需要转成StringBuilder.因为Stringbuffer是确保线程安全的,stringBuilder 不用,所以stringBuilder效率更高。那会不会引发线程安全问题呢,不会,因为这个是在方法内部定义的变量,所以对这个方法而言是线程封闭的,不会引发线程安全问题。
- Reorder the modifiers to comply with the Java Language Specification.
提示修饰符的顺序应该符合java语言规范,它给出的参考如下:
所以把static修饰符 放到final就好了。
- Declare “XXX” on a separate line.
定义变量的时候,一个变量一行,便于查看
- Return an empty collection instead of null.
最好不要直接返回null,应返回空数组和集合.如下:
- Use isEmpty() to check whether the collection is empty or not.
判断集合时候为空是,不要使用size(),建议使用isEmpty()方法。如下:
- This block of commented-out lines of code should be removed
这种可以直接删除掉,或者不想删除的可以用/***/注释,对于单行的可以把后面的分号去掉就不会报错异味了。
- Remove the literal “false” boolean value
布尔文字不应该是多余的。用true或者false在if中判断是不好的写好,直接可以通过本身进行判断,如下:
- Add a private constructor to hide the implicit public one.
如果一个类的里面的方法都是static修饰的静态方法,那么需要给这个类定义一个非公共构造函数(添加私有构造函数以隐藏隐式公共构造函数)如下:
- Refactor this method to reduce its Cognitive Complexity from 55 to the 15 allowed.
关于代码圈复杂度大于15的异味,以及代码过长的异味,说实话也没有什么好的方法,只能读代码,然后抽离函数出来,当然抽离函数不可能一次就能做到代码简洁之道要求的那样,一个函数只做一件事,单一层次原则。但是我们也不能因为做不到这样就就什么都不做,还是要迈出这一步,先抽离函数,虽然没有做到单一层次原则,但是消除了异味。下面代码是对上面的进行简单的函数抽离,消除异味
- 关于代码中很多的stitch分支问题。
这样代码上看去显得非常的臃肿且感觉特别low,但是这却是我们最喜欢写的包括我自己,因为简单进本不用思考。并且如下图,18个case都没有报异味,所以SonarQube上也没有检查出来,所以大家都将就先改其他,后来被数据端同事看到了下面这个代码,说代码简洁之道不是说不能用这么多的swatch么?我竟然一时不知道怎么回答,只能说修改成本太大,不好改。但是事后自己仔细想想自己接受这段代码看的也感觉特别low,那自己为什么不能对自己的要求高一点呢,所以痛定思痛打算改这段代码。
我修改这部分代码采用的是枚举类型,先创建一个枚举,并将所有的case换成对应的枚举值,然后创建两个成员变量和一个带两个参数的枚举的构造方法。然后实现这两个成员变量的get方法,使得其他类可以访问。如下图:
然后在原来的swatch的代码中,删除这些分支,创建这个枚举,并根据分支创建对应的枚举值,如下:
- 还有一些其他的异味消除。直接贴图
两个分支一模一样的,需要删掉其中一个。然后像这种有很多if-else if的getsql()方法的圈复杂度肯定是超过了,这里比较好的方法我也不知道怎么做,但是我是将整个分成多个一样的if-else if的方法。但是这样只能消除异味,代码的可读性变差。
条件语句如果是单行,需要使用缩进。