Add RandomString() and its test, and use it everywhere it makes sense

This unifies several things that used a 16-character random string, and
a few other users of random identifiers where it also made sense to use
a 16-character random string.

TEST=crashpad_util_test RandomString.RandomString
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1451793002 .

test/mac/mach_multiprocess.cc

@@ -23,7 +23,6 @@
 #include "base/logging.h"
 #include "base/mac/scoped_mach_port.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/rand_util.h"
 #include "gtest/gtest.h"
 #include "test/errors.h"
 #include "test/mac/mach_errors.h"
@@ -31,6 +30,7 @@
 #include "util/mach/mach_extensions.h"
 #include "util/mach/mach_message.h"
 #include "util/misc/implicit_cast.h"
+#include "util/misc/random_string.h"
 #include "util/misc/scoped_forbid_return.h"
 
 namespace {
@@ -93,9 +93,7 @@ void MachMultiprocess::PreFork() {
   // forking, so that it’s guaranteed to be there when the child attempts to
   // look it up.
   info_->service_name = "org.chromium.crashpad.test.mach_multiprocess.";
-  for (int index = 0; index < 16; ++index) {
-    info_->service_name.append(1, base::RandInt('A', 'Z'));
-  }
+  info_->service_name.append(RandomString());
 
   info_->local_port = BootstrapCheckIn(info_->service_name);
   ASSERT_TRUE(info_->local_port.is_valid());

test/scoped_temp_dir_win.cc

@@ -17,10 +17,10 @@
 #include <windows.h>
 
 #include "base/logging.h"
-#include "base/rand_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "util/misc/random_string.h"
 #include "gtest/gtest.h"
 
 namespace crashpad {
@@ -34,7 +34,7 @@ base::FilePath GenerateCandidateName() {
   PCHECK(path_len != 0) << "GetTempPath";
   base::FilePath system_temp_dir(temp_path);
   base::string16 new_dir_name = base::UTF8ToUTF16(base::StringPrintf(
-        "crashpad.test.%d.%I64x", GetCurrentProcessId(), base::RandUint64()));
+        "crashpad.test.%d.%s", GetCurrentProcessId(), RandomString().c_str()));
   return system_temp_dir.Append(new_dir_name);
 }
 

util/mac/service_management_test.mm

@@ -24,9 +24,9 @@
 #include "base/mac/scoped_cftyperef.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/sys_string_conversions.h"
-#include "base/rand_util.h"
 #include "gtest/gtest.h"
 #include "util/misc/clock.h"
+#include "util/misc/random_string.h"
 #include "util/posix/process_info.h"
 #include "util/stdlib/objc.h"
 
@@ -108,10 +108,7 @@ void ExpectProcessIsNotRunning(pid_t pid, std::string& last_arg) {
 
 TEST(ServiceManagement, SubmitRemoveJob) {
   @autoreleasepool {
-    std::string cookie;
-    for (int index = 0; index < 16; ++index) {
-      cookie.append(1, base::RandInt('A', 'Z'));
-    }
+    const std::string cookie = RandomString();
 
     std::string shell_script =
         base::StringPrintf("sleep 10; echo %s", cookie.c_str());

util/mach/child_port_handshake.cc

@@ -37,6 +37,7 @@
 #include "util/mach/mach_message.h"
 #include "util/mach/mach_message_server.h"
 #include "util/misc/implicit_cast.h"
+#include "util/misc/random_string.h"
 
 namespace crashpad {
 namespace {
@@ -97,10 +98,10 @@ mach_port_t ChildPortHandshakeServer::RunServer(
   errno = pthread_threadid_np(pthread_self(), &thread_id);
   PCHECK(errno == 0) << "pthread_threadid_np";
   std::string service_name = base::StringPrintf(
-      "org.chromium.crashpad.child_port_handshake.%d.%llu.%016llx",
+      "org.chromium.crashpad.child_port_handshake.%d.%llu.%s",
       getpid(),
       thread_id,
-      base::RandUint64());
+      RandomString().c_str());
 
   // Check the new service in with the bootstrap server, obtaining a receive
   // right for it.

util/mach/mach_extensions_test.cc

@@ -15,10 +15,10 @@
 #include "util/mach/mach_extensions.h"
 
 #include "base/mac/scoped_mach_port.h"
-#include "base/rand_util.h"
 #include "gtest/gtest.h"
 #include "test/mac/mach_errors.h"
 #include "util/mac/mac_util.h"
+#include "util/misc/random_string.h"
 
 namespace crashpad {
 namespace test {
@@ -139,9 +139,7 @@ TEST(MachExtensions, BootstrapCheckInAndLookUp) {
   EXPECT_NE(report_crash, kMachPortNull);
 
   std::string service_name = "org.chromium.crashpad.test.bootstrap_check_in.";
-  for (int index = 0; index < 16; ++index) {
-    service_name.append(1, base::RandInt('A', 'Z'));
-  }
+  service_name.append(RandomString());
 
   {
     // The new service hasn’t checked in yet, so this should fail.

util/misc/random_string.cc

@@ -0,0 +1,29 @@
+// Copyright 2015 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "util/misc/random_string.h"
+
+#include "base/rand_util.h"
+
+namespace crashpad {
+
+std::string RandomString() {
+  std::string random_string;
+  for (int index = 0; index < 16; ++index) {
+    random_string.append(1, static_cast<char>(base::RandInt('A', 'Z')));
+  }
+  return random_string;
+}
+
+}  // namespace crashpad

util/misc/random_string.h

@@ -0,0 +1,31 @@
+// Copyright 2015 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef CRASHPAD_UTIL_MISC_RANDOM_STRING_H_
+#define CRASHPAD_UTIL_MISC_RANDOM_STRING_H_
+
+#include <string>
+
+namespace crashpad {
+
+//! \brief Returns a random string.
+//!
+//! The string consists of 16 uppercase characters chosen at random. The
+//! returned string has over 75 bits of randomness (26<sup>16</sup> &gt;
+//! 2<sup>75</sup>).
+std::string RandomString();
+
+}  // namespace crashpad
+
+#endif  // CRASHPAD_UTIL_MISC_RANDOM_STRING_H_

util/misc/random_string_test.cc

@@ -0,0 +1,71 @@
+// Copyright 2015 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "util/misc/random_string.h"
+
+#include <set>
+
+#include "base/basictypes.h"
+#include "gtest/gtest.h"
+
+namespace crashpad {
+namespace test {
+namespace {
+
+TEST(RandomString, RandomString) {
+  // Explicitly list the allowed characters, rather than relying on a range.
+  // This prevents the test from having any dependency on the character set, so
+  // that the implementation is free to assume all uppercase letters are
+  // contiguous as in ASCII.
+  const std::string allowed_characters("ABCDEFGHIJKLMNOPQRSTUVWXYZ");
+
+  size_t character_counts[26] = {};
+  ASSERT_EQ(arraysize(character_counts), allowed_characters.size());
+
+  std::set<std::string> strings;
+
+  for (size_t i = 0; i < 256; ++i) {
+    const std::string random_string = RandomString();
+    EXPECT_EQ(16u, random_string.size());
+
+    // Make sure that the string is unique. It is possible, but extremely
+    // unlikely, for there to be collisions.
+    auto result = strings.insert(random_string);
+    EXPECT_TRUE(result.second) << random_string;
+
+    for (char c : random_string) {
+      size_t character_index = allowed_characters.find(c);
+
+      // Make sure that no unexpected characters appear.
+      EXPECT_NE(character_index, std::string::npos) << c;
+
+      if (character_index != std::string::npos) {
+        ++character_counts[character_index];
+      }
+    }
+  }
+
+  // Make sure every character appears at least once. It is possible, but
+  // extremely unlikely, for a character to not appear at all.
+  for (size_t character_index = 0;
+       character_index < arraysize(character_counts);
+       ++character_index) {
+    EXPECT_GT(character_counts[character_index], 0u)
+        << allowed_characters[character_index];
+  }
+}
+
+}  // namespace
+}  // namespace test
+}  // namespace crashpad

util/net/http_transport_test.cc

@@ -24,7 +24,6 @@
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/rand_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
@@ -32,6 +31,7 @@
 #include "test/multiprocess_exec.h"
 #include "test/paths.h"
 #include "util/file/file_io.h"
+#include "util/misc/random_string.h"
 #include "util/net/http_body.h"
 #include "util/net/http_headers.h"
 #include "util/net/http_multipart_builder.h"
@@ -89,14 +89,11 @@ class HTTPTransportTestFixture : public MultiprocessExec {
     // The parent will also tell the web server what response body to send back.
     // The web server will only send the response body if the response code is
     // 200.
-    std::string expect_response_body;
-    for (size_t index = 0; index < 8; ++index) {
-      expect_response_body += static_cast<char>(base::RandInt(' ', '~'));
-    }
+    const std::string random_string = RandomString();
 
     ASSERT_TRUE(LoggingWriteFile(WritePipeHandle(),
-                                 expect_response_body.c_str(),
-                                 expect_response_body.size()));
+                                 random_string.c_str(),
+                                 random_string.size()));
 
     // Now execute the HTTP request.
     scoped_ptr<HTTPTransport> transport(HTTPTransport::Create());
@@ -111,7 +108,7 @@ class HTTPTransportTestFixture : public MultiprocessExec {
     bool success = transport->ExecuteSynchronously(&response_body);
     if (response_code_ == 200) {
       EXPECT_TRUE(success);
-      expect_response_body += "\r\n";
+      std::string expect_response_body = random_string + "\r\n";
       EXPECT_EQ(expect_response_body, response_body);
     } else {
       EXPECT_FALSE(success);

util/net/http_transport_test_server.py

@@ -20,7 +20,7 @@
 When invoked, this server will write a short integer to stdout, indiciating on
 which port the server is listening. It will then read one integer from stdin,
 indiciating the response code to be sent in response to a request. It also reads
-8 characters from stdin, which, after having "\r\n" appended, will form the
+16 characters from stdin, which, after having "\r\n" appended, will form the
 response body in a successful response (one with code 200). The server will
 process one HTTP request, deliver the prearranged response to the client, and
 write the entire request to stdout. It will then terminate.
@@ -144,9 +144,9 @@ def Main():
   sys.stdout.flush()
 
   # Read the desired test response code as an unsigned short and the desired
-  # response body as an 8-byte string from the parent process.
+  # response body as a 16-byte string from the parent process.
   RequestHandler.response_code, RequestHandler.response_body = \
-      struct.unpack('=H8s', sys.stdin.read(struct.calcsize('=H8s')))
+      struct.unpack('=H16s', sys.stdin.read(struct.calcsize('=H16s')))
 
   # Handle the request.
   server.handle_request()

util/util.gyp

@@ -94,6 +94,8 @@
         'misc/initialization_state_dcheck.h',
         'misc/pdb_structures.cc',
         'misc/pdb_structures.h',
+        'misc/random_string.cc',
+        'misc/random_string.h',
         'misc/scoped_forbid_return.cc',
         'misc/scoped_forbid_return.h',
         'misc/symbolic_constants_common.h',

util/util_test.gyp

@@ -59,6 +59,7 @@
         'misc/initialization_state_dcheck_test.cc',
         'misc/initialization_state_test.cc',
         'misc/scoped_forbid_return_test.cc',
+        'misc/random_string_test.cc',
         'misc/uuid_test.cc',
         'net/http_body_test.cc',
         'net/http_body_test_util.cc',

util/win/exception_handler_server.cc

@@ -26,6 +26,7 @@
 #include "snapshot/crashpad_info_client_options.h"
 #include "snapshot/win/process_snapshot_win.h"
 #include "util/file/file_writer.h"
+#include "util/misc/random_string.h"
 #include "util/misc/tri_state.h"
 #include "util/misc/uuid.h"
 #include "util/win/get_function.h"
@@ -318,10 +319,7 @@ std::wstring ExceptionHandlerServer::CreatePipe() {
       base::StringPrintf("\\\\.\\pipe\\crashpad_%d_", GetCurrentProcessId());
   std::wstring pipe_name;
   do {
-    pipe_name = base::UTF8ToUTF16(pipe_name_base);
-    for (int index = 0; index < 16; ++index) {
-      pipe_name.append(1, static_cast<wchar_t>(base::RandInt('A', 'Z')));
-    }
+    pipe_name = base::UTF8ToUTF16(pipe_name_base + RandomString());
 
     first_pipe_instance_.reset(CreateNamedPipeInstance(pipe_name, true));
 

util/win/process_info_test.cc

@@ -20,7 +20,6 @@
 
 #include "base/files/file_path.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/rand_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
@@ -30,6 +29,7 @@
 #include "test/paths.h"
 #include "test/win/child_launcher.h"
 #include "util/file/file_io.h"
+#include "util/misc/random_string.h"
 #include "util/misc/uuid.h"
 #include "util/win/command_line.h"
 #include "util/win/get_function.h"
@@ -559,9 +559,9 @@ TEST(ProcessInfo, Handles) {
   ASSERT_TRUE(scoped_key.is_valid());
 
   std::wstring mapping_name =
-      base::UTF8ToUTF16(base::StringPrintf("Local\\test_mapping_%d_%I64x",
+      base::UTF8ToUTF16(base::StringPrintf("Local\\test_mapping_%d_%s",
                                            GetCurrentProcessId(),
-                                           base::RandUint64()));
+                                           RandomString().c_str()));
   ScopedKernelHANDLE mapping(CreateFileMapping(INVALID_HANDLE_VALUE,
                                                nullptr,
                                                PAGE_READWRITE,