Should issue warning for SecureRandom object created and used only once

[Mary904]

I found a false negative when spotbugs-maven-plugin checked the following line in eclipse/jetty.project.

BigInteger serial = BigInteger.valueOf(new SecureRandom().nextLong());

According to the description of DMI: Random object created and used only once (DMI_RANDOM_USED_ONLY_ONCE):

You should strongly consider using a java.security.SecureRandom instead (and avoid allocating a new SecureRandom for each random number needed).

Spotbugs should generate a warning because the code line BigInteger serial = BigInteger.valueOf(new SecureRandom().nextLong()); created and used SecureRandom object only once. As noted in the bug description, it should "avoid allocating a new SecureRandom for each random number". However, in the demo, this is allowed by Spotbugs without giving any warning so I think that this is a false negative.

The plugin version used is as follow:

            <plugin>
                <groupId>com.github.spotbugs</groupId>
                <artifactId>spotbugs-maven-plugin</artifactId>
                <version>4.1.4</version>
                <configuration>
                    <effort>Default</effort>
                    <threshold>Low</threshold>
                </configuration>
                <dependencies>
                    <dependency>
                        <groupId>com.github.spotbugs</groupId>
                        <artifactId>spotbugs</artifactId>
                        <version>4.1.4</version>
                    </dependency>
                </dependencies>
            </plugin>

Demo

Here is a small demo that could be used as test case. See #1475

import java.security.SecureRandom;
import java.util.Random;
import edu.umd.cs.findbugs.annotations.ExpectWarning;

public class DMI_RANDOM_USED_ONLY_ONCE {
    @ExpectWarning("DMI_RANDOM_USED_ONLY_ONCE")
    long m1(){
        return new SecureRandom().nextLong();  // This test case will fail now
    }

    @ExpectWarning("DMI_RANDOM_USED_ONLY_ONCE")
    long m2(){
        return new Random().nextLong();  // This will pass
    }
}

[welcome]

Thanks for opening your first issue here! 😃
Please check our contributing guideline. Especially when you report a problem, make sure you share a Minimal, Complete, and Verifiable example to reproduce it in this issue.

[robinvandenbogaard]

This is generating a lot warnings for valid usages. I've added those to the test case and fixed the condition by adding parenthesis.

[hinrik]

With 4.2.3 I'm now seeing false positives because of this change. E.g. this will trigger:

var r = new Random();
var tokenBuilder = new StringBuilder();
for (int i = 0; i < length; i++) {
    var position = r.nextInt(alphabet.length());
    tokenBuilder.append(alphabet.charAt(position));
}

Same for another class where I have a static Random object that gets used in an instance method.