spotbugs / discuss

SpotBugs mailing list
6 stars 1 forks source link

Clarification requested regarding DMI: Random object created and used only once #116

Closed soddencarpenter closed 3 years ago

soddencarpenter commented 3 years ago

Hi,

We have some code somewhat similar to this simple case:

public class ExampleRndClass
{
  private static final Random RND = new SecureRandom();
  private static final ExampleRndClass INST = new ExampleRndClass();

  private ExampleRndClass() {}

  public ExampleRndClass getInstance()
  {
    return INST;
  }

  public String getStr()
  {
    return String.format("Hi There-%d",
        RND.nextInt());
  }

SpotBugs is marking the line RND.nextInt() with

Bug: Random object created and used only once in ExampleRndClass.getStr()

This code creates a java.util.Random object, uses it to generate one random number, and then discards the Random object. This produces mediocre quality random numbers and is inefficient. If possible, rewrite the code so that the Random object is created once and saved, and each time a new random number is required invoke a method on the existing Random object to obtain it.
If it is important that the generated Random numbers not be guessable, you must not create a new Random for each random number; the values are too easily guessable. You should strongly consider using a java.security.SecureRandom instead (and avoid allocating a new SecureRandom for each random number needed).

Rank: Troubling (14), confidence: High

Pattern: DMI_RANDOM_USED_ONLY_ONCE

Type: DMI, Category: BAD_PRACTICE (Bad practice)


The Random object is not created in the method; it is a static final for the class.

Consequently, it is unclear what the error is indicating, and what the mitigation should be.

This appears to have been flagged with 4.3.0.

What should be the approach here? Thanks!

soddencarpenter commented 3 years ago

In tracing another question, it appears there is a desire to use the Discussion area, so I moved this question to: https://github.com/spotbugs/spotbugs/discussions/1628.