9
\$\begingroup\$

I have an Android app that uses randomly generated tokens to log in. They're 5 characters long, are sent to a user's email address, and have a short lifespan, so I don't think I need anything that's extremely secure.

I'm getting odd result that suggest I'm not really generating random numbers. That, or I'm seeing patterns that don't really exist.

For example, the last few tokens generated were:

  • 0pqrH
  • UMOPQ
  • d789r
  • jmnde

These have patterns:

  • pqr
  • 789
  • all caps
  • all lowercase

There are some tokens generated with scattered characters:

  • f80!#
  • NLog#
  • Z6kpJ

It just seems that there are too many patterns to be coincidence.

static public String getToken(int chars) {
    String CharSet = "abcdefghijkmnopqrstuvwxyzABCDEFGHJKLMNOPQRSTUVWXYZ234567890!@#$";
    String Token = "";
    for (int a = 1; a <= chars; a++) {
        Token += CharSet.charAt(new Random().nextInt(CharSet.length()));
    }
    return Token;
}

Am I generating random numbers correctly? Should I be picking a seed? Is this in my imagination?

| improve this question | | | | |
\$\endgroup\$
12
\$\begingroup\$

This is not entirely only in your imagination

Random objects are meant to be reused. Recreating a Random object each time may cause a specific pattern in the result. This depends a bit on the Java version. There is no need to pick a seed, Java picks a seed for you based on a varying number, and the current system time (see Java 6 source). Newer versions of Java uses a better seedUniquifier method. I am not sure about what implementation the Android platform uses.

Additionally, Java variable names should start with lowercase letters by the coding conventions.

I would also use a StringBuilder for better efficiency and lower memory usage.

To avoid confusion with the Charset class, I would name your String variable chars and your int variable length.

private static final Random random = new Random();
private static final String CHARS = "abcdefghijkmnopqrstuvwxyzABCDEFGHJKLMNOPQRSTUVWXYZ234567890!@#$";

public static String getToken(int length) {
    StringBuilder token = new StringBuilder(length);
    for (int i = 0; i < length; i++) {
        token.append(CHARS.charAt(random.nextInt(CHARS.length())));
    }
    return token.toString();
}
| improve this answer | | | | |
\$\endgroup\$
  • 2
    \$\begingroup\$ Thanks - I had been under the impression that the best way to create a diverse set of numbers was to recreate Random... This explains why 5 characters all generated within a millisecond or two from each other were similar, but a different token generated a minute later was completely different. I'll implement your suggestions. Thanks! \$\endgroup\$ – Scott Mar 18 '15 at 0:04
  • \$\begingroup\$ Re: your edits - what's the benefit of moving the random and chars variables outside the method? I would think that once I don't need them anymore they're better off disappearing from memory when the method ends. \$\endgroup\$ – Scott Mar 18 '15 at 0:10
  • \$\begingroup\$ @Scott I guess it depends on how often you call the getToken method. I assumed it was called more than once in the application's lifespan. Keeping the String inside or outside the method is essentially no different, but this is a topic on it's own. The possible memory overhead for storing them outside the method is minimal, and the generation results are randomized better. Constants are normally declared outside methods. Don't worry about the memory footprint on them. \$\endgroup\$ – Simon Forsberg Mar 18 '15 at 0:16
  • 1
    \$\begingroup\$ It may be important to mention that all random number generators are not truly random. They generate normal distributions of sets over time. Since the tokens here have a short lifespan, it may not be pertinent, however this should be considered. \$\endgroup\$ – Evan Bechtol Mar 18 '15 at 1:37
  • \$\begingroup\$ @EvanBechtol There's also always room for another answer :) \$\endgroup\$ – Simon Forsberg Mar 18 '15 at 10:46
0
\$\begingroup\$

It may be important to mention that all random number generators are not truly random. They generate normal distributions of sets over time.

Since the tokens here have a short lifespan, it may not be pertinent, however this should be considered. If you are going for security, overtime someone will be able to figure out the algorithm to crack it, thus be able to generate their own keys.

| improve this answer | | | | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.