Index: chrome/browser/autofill/password_generator.cc |
diff --git a/chrome/browser/autofill/password_generator.cc b/chrome/browser/autofill/password_generator.cc |
index 3361fb47d1dc4df19cc243c45dcd5fe97c3f1b2d..c5b8fe6bc518390447f4bfa8603781027fd04dc4 100644 |
--- a/chrome/browser/autofill/password_generator.cc |
+++ b/chrome/browser/autofill/password_generator.cc |
@@ -4,22 +4,98 @@ |
#include "chrome/browser/autofill/password_generator.h" |
+#include "base/logging.h" |
#include "base/rand_util.h" |
const int kMinChar = 33; // First printable character '!' |
const int kMaxChar = 126; // Last printable character '~' |
-const int kPasswordLength = 12; |
+const int kMinUpper = 65; // First upper case letter 'A' |
+const int kMaxUpper = 90; // Last upper case letter 'Z' |
+const int kMinLower = 97; // First lower case letter 'a' |
+const int kMaxLower = 122; // Last lower case letter 'z' |
+const int kMinDigit = 48; // First digit '0' |
+const int kMaxDigit = 57; // Last digit '9' |
+// Copy of the other printable symbols from the ascii table since they |
Ilya Sherman
2012/06/01 01:09:55
nit: "ascii" -> "ASCII"
zysxqn
2012/06/01 19:01:32
Done.
|
+// are disjointed. |
+const char kOtherSymbols[32] = |
Ilya Sherman
2012/06/01 01:09:55
nit: No need to specify "32", and it would be inco
zysxqn
2012/06/01 19:01:32
Done.
|
+ {'!', '\"', '#', '$', '%', '&', '\'', '(', |
+ ')', '*', '+', ',', '-', '.', '/', ':', |
+ ';', '<', '=', '>', '?', '@', '[', '\\', |
+ ']', '^', '_', '`', '{', '|', '}', '~'}; |
+const int kMinPasswordLength = 4; |
+const int kMaxPasswordLength = 20; |
+ |
+namespace { |
+ |
+// Classic algorithm to randomly select M elements out of N. |
+// One description can be found at: "http://stackoverflow.com/questions/48087/select-a-random-n-elements-from-listt-in-c-sharp/48089#48089" |
+int* SelectRandomMFromN(int M, int N) { |
Ilya Sherman
2012/06/01 01:09:55
This should be a void function that takes a std::v
Ilya Sherman
2012/06/01 01:09:55
nit: M and N are completely obfuscated names. Ple
zysxqn
2012/06/01 19:01:32
Done.
zysxqn
2012/06/01 19:01:32
Done.
|
+ DCHECK_GE(N, M); |
+ int* result = new int[M]; |
+ int number_left = N; |
+ int number_needed = M; |
+ for (int i = 0; i < N; ++i) { |
+ if (number_needed == 0) |
Ilya Sherman
2012/06/01 01:09:55
nit: You can just add this to the loop condition.
zysxqn
2012/06/01 19:01:32
Done.
|
+ break; |
+ // we have probability = number_needed / number_left to select |
+ // this position. |
+ int probability = base::RandInt(0, number_left - 1); |
+ if (probability < number_needed) { |
+ result[4 - number_needed] = i; |
Ilya Sherman
2012/06/01 01:09:55
The hardcoded "4" does not make sense now that thi
zysxqn
2012/06/01 19:01:32
Done.
|
+ --number_needed; |
+ } |
+ --number_left; |
+ } |
+ DCHECK_EQ(0, number_needed); |
+ return result; |
+} |
+ |
+} // namespace |
namespace autofill { |
-PasswordGenerator::PasswordGenerator() {} |
+PasswordGenerator::PasswordGenerator() |
+ : password_length_(kDefaultPasswordLength) {} |
Ilya Sherman
2012/06/01 01:09:55
nit: If we need to keep this default constructor f
zysxqn
2012/06/01 19:01:32
Prefer to keep this in case some application just
|
+PasswordGenerator::PasswordGenerator(int max_length) |
+ : password_length_( |
+ max_length >= kMinPasswordLength && max_length <= kMaxPasswordLength ? |
+ max_length : kDefaultPasswordLength) {} |
Ilya Sherman
2012/06/01 01:09:55
nit: Please decompose this logic into a tiny helpe
zysxqn
2012/06/01 19:01:32
Done.
|
PasswordGenerator::~PasswordGenerator() {} |
std::string PasswordGenerator::Generate() { |
std::string ret; |
- for (int i = 0; i < kPasswordLength; i++) { |
- ret.push_back(static_cast<char>(base::RandInt(kMinChar, kMaxChar))); |
+ // First, randomly select 4 positions to hold one upper case letter, |
+ // one lower case letter, one digit, and one other symbol respectively, |
+ // to make sure at least one of each category of characters will be |
+ // included in the password. |
+ int* positions = SelectRandomMFromN(4, password_length_); |
+ |
+ // To enhance the strenght of the password, the upper case letter will be |
Ilya Sherman
2012/06/01 01:09:55
nit: "strenght" -> "strength"
zysxqn
2012/06/01 19:01:32
Done.
|
+ // put at "start"th position, lower case letter will be put at |
+ // "(start+1)%4"th position, digit will be put at "(start+2)%4"th position, |
+ // and other symbol will be put at "(start+3)%4"th position. |
+ int start = base::RandInt(0,3); |
Ilya Sherman
2012/06/01 01:09:55
Please use std::random_shuffle [1] instead.
[1] ht
zysxqn
2012/06/01 19:01:32
Good to know! Done.
|
+ |
+ // Next, generate each character of the password. |
+ for (int i = 0; i < password_length_; ++i) { |
+ if (i == positions[start]) { |
+ // Generate random upper case letter. |
+ ret.push_back(static_cast<char>(base::RandInt(kMinUpper, kMaxUpper))); |
+ } else if (i == positions[(start + 1) % 4]) { |
+ // Generate random lower case letter. |
+ ret.push_back(static_cast<char>(base::RandInt(kMinLower, kMaxLower))); |
+ } else if (i == positions[(start + 2) % 4]) { |
+ // Generate random digit. |
+ ret.push_back(static_cast<char>(base::RandInt(kMinDigit, kMaxDigit))); |
+ } else if (i == positions[(start + 3) % 4]) { |
+ // Generate random other symbol. |
+ ret.push_back(kOtherSymbols[base::RandInt(0, 31)]); |
Ilya Sherman
2012/06/01 01:09:55
nit: Please use the arraysize macro [1] rather tha
zysxqn
2012/06/01 19:01:32
Done.
|
+ } else { |
+ // Generate random character from all categories. |
+ ret.push_back(static_cast<char>(base::RandInt(kMinChar, kMaxChar))); |
+ } |
} |
+ delete positions; |
Ilya Sherman
2012/06/01 01:09:55
Chromium code should almost never need explicit "d
zysxqn
2012/06/01 19:01:32
Done.
|
return ret; |
} |