Make std::basic_string<uint8_t> the underlying storage for NtlmBufferWriter.

BUG=chromium:22532

Change-Id: If7b0f874fd75de0827df5b0d4edd24d4abca6834
Reviewed-on: https://chromium-review.googlesource.com/577635
Commit-Queue: Zentaro Kavanagh <zentaro@google.com>
Reviewed-by: Asanka Herath <asanka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491530}

net/http/http_auth_handler_ntlm_portable_unittest.cc

@@ -48,13 +48,20 @@ class HttpAuthHandlerNtlmPortableTest : public PlatformTest {
         &auth_handler_);
   }
 
-  std::string CreateNtlmAuthHeader(base::StringPiece message) {
+  std::string CreateNtlmAuthHeader(ntlm::Buffer message) {
     std::string output;
-    base::Base64Encode(message, &output);
+    base::Base64Encode(
+        base::StringPiece(reinterpret_cast<const char*>(message.data()),
+                          message.size()),
+        &output);
 
     return "NTLM " + output;
   }
 
+  std::string CreateNtlmAuthHeader(const uint8_t* buffer, size_t length) {
+    return CreateNtlmAuthHeader(ntlm::Buffer(buffer, length));
+  }
+
   HttpAuth::AuthorizationResult HandleAnotherChallenge(
       const std::string& challenge) {
     HttpAuthChallengeTokenizer tokenizer(challenge.begin(), challenge.end());
@@ -202,9 +209,8 @@ TEST_F(HttpAuthHandlerNtlmPortableTest, MinimalStructurallyValidType2) {
   ASSERT_EQ(OK, CreateHandler());
   ASSERT_EQ(OK, GetGenerateAuthTokenResult());
   ASSERT_EQ(HttpAuth::AUTHORIZATION_RESULT_ACCEPT,
-            HandleAnotherChallenge(CreateNtlmAuthHeader(base::StringPiece(
-                reinterpret_cast<const char*>(ntlm::test::kMinChallengeMessage),
-                ntlm::kChallengeHeaderLen))));
+            HandleAnotherChallenge(CreateNtlmAuthHeader(
+                ntlm::test::kMinChallengeMessage, ntlm::kChallengeHeaderLen)));
   ASSERT_EQ(OK, GetGenerateAuthTokenResult());
 }
 
@@ -212,13 +218,12 @@ TEST_F(HttpAuthHandlerNtlmPortableTest, Type2MessageTooShort) {
   ASSERT_EQ(OK, CreateHandler());
   ASSERT_EQ(OK, GetGenerateAuthTokenResult());
 
-  char raw[31];
+  uint8_t raw[31];
   memcpy(raw, ntlm::test::kMinChallengeMessage, 31);
 
   // Fail because the minimum size valid message is 32 bytes.
   ASSERT_EQ(HttpAuth::AUTHORIZATION_RESULT_ACCEPT,
-            HandleAnotherChallenge(
-                CreateNtlmAuthHeader(base::StringPiece(raw, sizeof(raw)))));
+            HandleAnotherChallenge(CreateNtlmAuthHeader(raw, arraysize(raw))));
   ASSERT_EQ(ERR_UNEXPECTED, GetGenerateAuthTokenResult());
 }
 
@@ -226,7 +231,7 @@ TEST_F(HttpAuthHandlerNtlmPortableTest, Type2MessageWrongSignature) {
   ASSERT_EQ(OK, CreateHandler());
   ASSERT_EQ(OK, GetGenerateAuthTokenResult());
 
-  char raw[32];
+  uint8_t raw[32];
   memcpy(raw, ntlm::test::kMinChallengeMessage, 32);
   // Modify the default valid message to overwrite the last byte of the
   // signature.
@@ -234,8 +239,7 @@ TEST_F(HttpAuthHandlerNtlmPortableTest, Type2MessageWrongSignature) {
 
   // Fail because the first 8 bytes don't match "NTLMSSP\0"
   ASSERT_EQ(HttpAuth::AUTHORIZATION_RESULT_ACCEPT,
-            HandleAnotherChallenge(
-                CreateNtlmAuthHeader(base::StringPiece(raw, sizeof(raw)))));
+            HandleAnotherChallenge(CreateNtlmAuthHeader(raw, arraysize(raw))));
   ASSERT_EQ(ERR_UNEXPECTED, GetGenerateAuthTokenResult());
 }
 
@@ -243,7 +247,7 @@ TEST_F(HttpAuthHandlerNtlmPortableTest, Type2WrongMessageType) {
   ASSERT_EQ(OK, CreateHandler());
   ASSERT_EQ(OK, GetGenerateAuthTokenResult());
 
-  char raw[32];
+  uint8_t raw[32];
   memcpy(raw, ntlm::test::kMinChallengeMessage, 32);
   // Modify the message type so it is not 0x00000002
   raw[8] = 0x03;
@@ -251,7 +255,7 @@ TEST_F(HttpAuthHandlerNtlmPortableTest, Type2WrongMessageType) {
   // Fail because the message type should be MessageType::kChallenge
   // (0x00000002)
   ASSERT_EQ(HttpAuth::AUTHORIZATION_RESULT_ACCEPT,
-            HandleAnotherChallenge(CreateNtlmAuthHeader(raw)));
+            HandleAnotherChallenge(CreateNtlmAuthHeader(raw, arraysize(raw))));
   ASSERT_EQ(ERR_UNEXPECTED, GetGenerateAuthTokenResult());
 }
 
@@ -264,14 +268,13 @@ TEST_F(HttpAuthHandlerNtlmPortableTest, Type2MessageWithNoTargetName) {
   // expected response from a compliant server when no target name is sent.
   // In reality the offset should always be ignored if the length is zero.
   // Also implementations often just write zeros.
-  char raw[32];
+  uint8_t raw[32];
   memcpy(raw, ntlm::test::kMinChallengeMessage, 32);
   // Modify the default valid message to overwrite the offset to zero.
   raw[16] = 0x00;
 
   ASSERT_EQ(HttpAuth::AUTHORIZATION_RESULT_ACCEPT,
-            HandleAnotherChallenge(
-                CreateNtlmAuthHeader(base::StringPiece(raw, sizeof(raw)))));
+            HandleAnotherChallenge(CreateNtlmAuthHeader(raw, arraysize(raw))));
   ASSERT_EQ(OK, GetGenerateAuthTokenResult());
 }
 
@@ -280,7 +283,7 @@ TEST_F(HttpAuthHandlerNtlmPortableTest, Type2MessageWithTargetName) {
   ASSERT_EQ(OK, GetGenerateAuthTokenResult());
 
   // One extra byte is provided for target name.
-  char raw[33];
+  uint8_t raw[33];
   memcpy(raw, ntlm::test::kMinChallengeMessage, 32);
   // Modify the default valid message to indicate 1 byte is present in the
   // target name payload.
@@ -290,8 +293,7 @@ TEST_F(HttpAuthHandlerNtlmPortableTest, Type2MessageWithTargetName) {
   raw[32] = 'Z';
 
   ASSERT_EQ(HttpAuth::AUTHORIZATION_RESULT_ACCEPT,
-            HandleAnotherChallenge(
-                CreateNtlmAuthHeader(base::StringPiece(raw, sizeof(raw)))));
+            HandleAnotherChallenge(CreateNtlmAuthHeader(raw, arraysize(raw))));
   ASSERT_EQ(OK, GetGenerateAuthTokenResult());
 }
 
@@ -299,7 +301,7 @@ TEST_F(HttpAuthHandlerNtlmPortableTest, NoTargetNameOverflowFromOffset) {
   ASSERT_EQ(OK, CreateHandler());
   ASSERT_EQ(OK, GetGenerateAuthTokenResult());
 
-  char raw[32];
+  uint8_t raw[32];
   memcpy(raw, ntlm::test::kMinChallengeMessage, 32);
   // Modify the default valid message to claim that the target name field is 1
   // byte long overrunning the end of the message message.
@@ -310,8 +312,7 @@ TEST_F(HttpAuthHandlerNtlmPortableTest, NoTargetNameOverflowFromOffset) {
   // the message buffer because the offset is past the end of the message.
   // Verify it gets rejected.
   ASSERT_EQ(HttpAuth::AUTHORIZATION_RESULT_ACCEPT,
-            HandleAnotherChallenge(
-                CreateNtlmAuthHeader(base::StringPiece(raw, sizeof(raw)))));
+            HandleAnotherChallenge(CreateNtlmAuthHeader(raw, arraysize(raw))));
   ASSERT_EQ(ERR_UNEXPECTED, GetGenerateAuthTokenResult());
 }
 
@@ -321,7 +322,7 @@ TEST_F(HttpAuthHandlerNtlmPortableTest, NoTargetNameOverflowFromLength) {
 
   // Message has 1 extra byte of space after the header for the target name.
   // One extra byte is provided for target name.
-  char raw[33];
+  uint8_t raw[33];
   memcpy(raw, ntlm::test::kMinChallengeMessage, 32);
   // Modify the default valid message to indicate 2 bytes are present in the
   // target name payload (however there is only space for 1).
@@ -334,8 +335,7 @@ TEST_F(HttpAuthHandlerNtlmPortableTest, NoTargetNameOverflowFromLength) {
   // the message buffer because the length is longer than available space.
   // Verify it gets rejected.
   ASSERT_EQ(HttpAuth::AUTHORIZATION_RESULT_ACCEPT,
-            HandleAnotherChallenge(
-                CreateNtlmAuthHeader(base::StringPiece(raw, sizeof(raw)))));
+            HandleAnotherChallenge(CreateNtlmAuthHeader(raw, arraysize(raw))));
   ASSERT_EQ(ERR_UNEXPECTED, GetGenerateAuthTokenResult());
 }
 

net/ntlm/ntlm_buffer_reader.cc

@@ -11,14 +11,17 @@
 namespace net {
 namespace ntlm {
 
-NtlmBufferReader::NtlmBufferReader(base::StringPiece buffer)
+NtlmBufferReader::NtlmBufferReader(const Buffer& buffer)
     : buffer_(buffer), cursor_(0) {
   DCHECK(buffer.data());
 }
 
+NtlmBufferReader::NtlmBufferReader(base::StringPiece str)
+    : NtlmBufferReader(reinterpret_cast<const uint8_t*>(str.data()),
+                       str.size()) {}
+
 NtlmBufferReader::NtlmBufferReader(const uint8_t* ptr, size_t len)
-    : NtlmBufferReader(
-          base::StringPiece(reinterpret_cast<const char*>(ptr), len)) {}
+    : NtlmBufferReader(Buffer(ptr, len)) {}
 
 NtlmBufferReader::~NtlmBufferReader() {}
 

net/ntlm/ntlm_buffer_reader.h

@@ -46,6 +46,7 @@ namespace ntlm {
 // [2] http://davenport.sourceforge.net/ntlm.html
 class NET_EXPORT_PRIVATE NtlmBufferReader {
  public:
+  explicit NtlmBufferReader(const Buffer& buffer);
   explicit NtlmBufferReader(base::StringPiece buffer);
 
   // This class does not take ownership of |ptr|, so the caller must ensure
@@ -165,9 +166,7 @@ class NET_EXPORT_PRIVATE NtlmBufferReader {
   void AdvanceCursor(size_t count) { SetCursor(GetCursor() + count); }
 
   // Returns a constant pointer to the start of the buffer.
-  const uint8_t* GetBufferPtr() const {
-    return reinterpret_cast<const uint8_t*>(buffer_.data());
-  }
+  const uint8_t* GetBufferPtr() const { return buffer_.data(); }
 
   // Returns a pointer to the underlying buffer at the current cursor
   // position.
@@ -179,7 +178,7 @@ class NET_EXPORT_PRIVATE NtlmBufferReader {
     return *(GetBufferAtCursor());
   }
 
-  const base::StringPiece buffer_;
+  const Buffer buffer_;
   size_t cursor_;
 
   DISALLOW_COPY_AND_ASSIGN(NtlmBufferReader);

net/ntlm/ntlm_buffer_writer.cc

@@ -10,13 +10,13 @@
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 
+template class std::basic_string<uint8_t>;
+
 namespace net {
 namespace ntlm {
 
 NtlmBufferWriter::NtlmBufferWriter(size_t buffer_len)
-    : buffer_len_(buffer_len), cursor_(0) {
-  buffer_.reset(new uint8_t[buffer_len]());
-}
+    : buffer_(buffer_len, 0), cursor_(0) {}
 
 NtlmBufferWriter::~NtlmBufferWriter() {}
 

net/ntlm/ntlm_buffer_writer.h

@@ -46,15 +46,11 @@ class NET_EXPORT_PRIVATE NtlmBufferWriter {
   explicit NtlmBufferWriter(size_t buffer_len);
   ~NtlmBufferWriter();
 
-  size_t GetLength() const { return buffer_len_; }
+  size_t GetLength() const { return buffer_.size(); }
   size_t GetCursor() const { return cursor_; }
   bool IsEndOfBuffer() const { return cursor_ >= GetLength(); }
-
-  // Gets a base::StringPiece view over the entire buffer.
-  base::StringPiece GetBuffer() const {
-    return base::StringPiece(reinterpret_cast<const char*>(buffer_.get()),
-                             buffer_len_);
-  }
+  const Buffer& GetBuffer() const { return buffer_; }
+  Buffer Pass() const { return std::move(buffer_); }
 
   // Returns true if there are |len| more bytes between the current cursor
   // position and the end of the buffer.
@@ -159,13 +155,16 @@ class NET_EXPORT_PRIVATE NtlmBufferWriter {
   void AdvanceCursor(size_t count) { SetCursor(GetCursor() + count); }
 
   // Returns a pointer to the start of the buffer.
-  uint8_t* GetBufferPtr() const { return buffer_.get(); }
+  const uint8_t* GetBufferPtr() const { return &buffer_[0]; }
+  uint8_t* GetBufferPtr() { return &buffer_[0]; }
 
   // Returns pointer into the buffer at the current cursor location.
-  uint8_t* GetBufferPtrAtCursor() const { return buffer_.get() + GetCursor(); }
+  const uint8_t* GetBufferPtrAtCursor() const {
+    return GetBufferPtr() + GetCursor();
+  }
+  uint8_t* GetBufferPtrAtCursor() { return GetBufferPtr() + GetCursor(); }
 
-  std::unique_ptr<uint8_t[]> buffer_;
-  size_t buffer_len_;
+  Buffer buffer_;
   size_t cursor_;
 
   DISALLOW_COPY_AND_ASSIGN(NtlmBufferWriter);

net/ntlm/ntlm_buffer_writer_unittest.cc

@@ -13,16 +13,15 @@ namespace ntlm {
 
 namespace {
 
-// Helper method to hide all the ugly casting.
+// Helper method to get a raw pointer to the buffer.
 const uint8_t* GetBufferPtr(const NtlmBufferWriter& writer) {
-  return reinterpret_cast<const uint8_t*>(writer.GetBuffer().data());
+  return writer.GetBuffer().data();
 }
 
 // Helper method to get a byte at a specific index in the buffer.
 uint8_t GetByteFromBuffer(const NtlmBufferWriter& writer, size_t index) {
-  base::StringPiece piece(writer.GetBuffer());
-  EXPECT_TRUE(index < piece.length());
-  return static_cast<uint8_t>(piece.data()[index]);
+  EXPECT_TRUE(index < writer.GetLength());
+  return writer.GetBuffer()[index];
 }
 
 }  // namespace

net/ntlm/ntlm_constants.h

@@ -7,6 +7,7 @@
 
 #include <stddef.h>
 #include <stdint.h>
+#include <string>
 #include <type_traits>
 
 #include "base/macros.h"
@@ -15,6 +16,8 @@
 namespace net {
 namespace ntlm {
 
+using Buffer = std::basic_string<uint8_t>;
+
 // A security buffer is a structure within an NTLM message that indicates
 // the offset from the beginning of the message and the length of a payload
 // that occurs later in the message. Within the raw message there is also