ake string_util::WriteInto() DCHECK() that the supplied |length_with_null| > 1, meaning that the without-'\0' string is non-empty. This replaces the conditional code added recently that makes this case return NULL. It's easier to understand if it's simply an error to call WriteInto() in this case at all.

Add DCHECK()s or conditionals as appropriate to callers in order to ensure this assertion holds.

BUG=none
TEST=none
Review URL: http://codereview.chromium.org/8418034

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@112005 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 59 insertions, 59 deletions

diff --git a/net/base/net_util.cc b/net/base/net_util.cc
index 204e768..7ae4363d 100644
--- a/net/base/net_util.cc
+++ b/net/base/net_util.cc
@@ -162,13 +162,9 @@ static const int kAllowedFtpPorts[] = {
 std::string::size_type CountTrailingChars(
     const std::string input,
     const std::string::value_type trailing_chars[]) {
-  const std::string::size_type last_good_char =
-      input.find_last_not_of(trailing_chars);
-
-  if (last_good_char == std::string::npos)
-    return input.length();
-  else
-    return input.length() - last_good_char - 1;
+  const size_t last_good_char = input.find_last_not_of(trailing_chars);
+  return (last_good_char == std::string::npos) ?
+      input.length() : (input.length() - last_good_char - 1);
 }
 
 // Similar to Base64Decode. Decodes a Q-encoded string to a sequence
@@ -176,71 +172,68 @@ std::string::size_type CountTrailingChars(
 bool QPDecode(const std::string& input, std::string* output) {
   std::string temp;
   temp.reserve(input.size());
-  std::string::const_iterator it = input.begin();
-  while (it != input.end()) {
+  for (std::string::const_iterator it = input.begin(); it != input.end();
+       ++it) {
     if (*it == '_') {
       temp.push_back(' ');
     } else if (*it == '=') {
-      if (input.end() - it < 3) {
-        return false;
-      }
-      if (IsHexDigit(static_cast<unsigned char>(*(it + 1))) &&
-          IsHexDigit(static_cast<unsigned char>(*(it + 2)))) {
-        unsigned char ch = HexDigitToInt(*(it + 1)) * 16 +
-                           HexDigitToInt(*(it + 2));
-        temp.push_back(static_cast<char>(ch));
-        ++it;
-        ++it;
-      } else {
+      if ((input.end() - it < 3) ||
+          !IsHexDigit(static_cast<unsigned char>(*(it + 1))) ||
+          !IsHexDigit(static_cast<unsigned char>(*(it + 2))))
         return false;
-      }
+      unsigned char ch = HexDigitToInt(*(it + 1)) * 16 +
+                         HexDigitToInt(*(it + 2));
+      temp.push_back(static_cast<char>(ch));
+      ++it;
+      ++it;
     } else if (0x20 < *it && *it < 0x7F) {
       // In a Q-encoded word, only printable ASCII characters
       // represent themselves. Besides, space, '=', '_' and '?' are
       // not allowed, but they're already filtered out.
-      DCHECK(*it != 0x3D && *it != 0x5F && *it != 0x3F);
+      DCHECK_NE('=', *it);
+      DCHECK_NE('?', *it);
+      DCHECK_NE('_', *it);
       temp.push_back(*it);
     } else {
       return false;
     }
-    ++it;
   }
   output->swap(temp);
   return true;
 }
 
 enum RFC2047EncodingType {Q_ENCODING, B_ENCODING};
-bool DecodeBQEncoding(const std::string& part, RFC2047EncodingType enc_type,
-                       const std::string& charset, std::string* output) {
+bool DecodeBQEncoding(const std::string& part,
+                      RFC2047EncodingType enc_type,
+                      const std::string& charset,
+                      std::string* output) {
   std::string decoded;
-  if (enc_type == B_ENCODING) {
-    if (!base::Base64Decode(part, &decoded)) {
-      return false;
-    }
-  } else {
-    if (!QPDecode(part, &decoded)) {
-      return false;
-    }
+  if (!((enc_type == B_ENCODING) ?
+      base::Base64Decode(part, &decoded) : QPDecode(part, &decoded)))
+    return false;
+
+  if (decoded.empty()) {
+    output->clear();
+    return true;
   }
 
   UErrorCode err = U_ZERO_ERROR;
   UConverter* converter(ucnv_open(charset.c_str(), &err));
-  if (U_FAILURE(err)) {
+  if (U_FAILURE(err))
     return false;
-  }
 
   // A single byte in a legacy encoding can be expanded to 3 bytes in UTF-8.
   // A 'two-byte character' in a legacy encoding can be expanded to 4 bytes
-  // in UTF-8. Therefore, the expansion ratio is 3 at most.
-  int length = static_cast<int>(decoded.length());
-  char* buf = WriteInto(output, length * 3);
-  length = ucnv_toAlgorithmic(UCNV_UTF8, converter, buf, length * 3,
-      decoded.data(), length, &err);
+  // in UTF-8. Therefore, the expansion ratio is 3 at most. Add one for a
+  // trailing '\0'.
+  size_t output_length = decoded.length() * 3 + 1;
+  char* buf = WriteInto(output, output_length);
+  output_length = ucnv_toAlgorithmic(UCNV_UTF8, converter, buf, output_length,
+                                     decoded.data(), decoded.length(), &err);
   ucnv_close(converter);
-  if (U_FAILURE(err)) {
+  if (U_FAILURE(err))
     return false;
-  }
-  output->resize(length);
+  output->resize(output_length);
   return true;
 }
 
diff --git a/net/base/x509_certificate_win.cc b/net/base/x509_certificate_win.cc
index 7309021..12acdaf 100644
--- a/net/base/x509_certificate_win.cc
+++ b/net/base/x509_certificate_win.cc
@@ -557,18 +557,22 @@ void X509Certificate::Initialize() {
                             &cert_handle_->pCertInfo->Subject,
                             CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG,
                             NULL, 0);
-  name_size = CertNameToStr(cert_handle_->dwCertEncodingType,
-                            &cert_handle_->pCertInfo->Subject,
-                            CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG,
-                            WriteInto(&subject_info, name_size), name_size);
+  if (name_size > 1) {
+    CertNameToStr(cert_handle_->dwCertEncodingType,
+                  &cert_handle_->pCertInfo->Subject,
+                  CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG,
+                  WriteInto(&subject_info, name_size), name_size);
+  }
   name_size = CertNameToStr(cert_handle_->dwCertEncodingType,
                             &cert_handle_->pCertInfo->Issuer,
                             CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG,
                             NULL, 0);
-  name_size = CertNameToStr(cert_handle_->dwCertEncodingType,
-                            &cert_handle_->pCertInfo->Issuer,
-                            CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG,
-                            WriteInto(&issuer_info, name_size), name_size);
+  if (name_size > 1) {
+    CertNameToStr(cert_handle_->dwCertEncodingType,
+                  &cert_handle_->pCertInfo->Issuer,
+                  CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG,
+                  WriteInto(&issuer_info, name_size), name_size);
+  }
   ParsePrincipal(WideToUTF8(subject_info), &subject_);
   ParsePrincipal(WideToUTF8(issuer_info), &issuer_);
 
diff --git a/net/disk_cache/entry_impl.cc b/net/disk_cache/entry_impl.cc
index 976e16f..0a6a2d2 100644
--- a/net/disk_cache/entry_impl.cc
+++ b/net/disk_cache/entry_impl.cc
@@ -773,11 +773,11 @@ std::string EntryImpl::GetKey() const {
   File* key_file = const_cast<EntryImpl*>(this)->GetBackingFile(address,
                                                                 kKeyFileIndex);
 
-  if (!offset && key_file->GetLength() != static_cast<size_t>(key_len + 1))
+  ++key_len;  // We store a trailing \0 on disk that we read back below.
+  if (!offset && key_file->GetLength() != static_cast<size_t>(key_len))
     return std::string();
 
-  if (!key_file ||
-      !key_file->Read(WriteInto(&key_, key_len + 1), key_len + 1, offset))
+  if (!key_file || !key_file->Read(WriteInto(&key_, key_len), key_len, offset))
     key_.clear();
   return key_;
 }
diff --git a/net/http/http_mac_signature.cc b/net/http/http_mac_signature.cc
index 93498ef..e445165 100644
--- a/net/http/http_mac_signature.cc
+++ b/net/http/http_mac_signature.cc
@@ -158,9 +158,12 @@ bool HttpMacSignature::GenerateMAC(const std::string& age,
 
   std::string signature;
   size_t length = hmac.DigestLength();
-  char* buffer = WriteInto(&signature, length);
-  if (!hmac.Sign(request, reinterpret_cast<unsigned char*>(buffer),
-                 length)) {
+  DCHECK_GT(length, 0u);
+  if (!hmac.Sign(request,
+          // We need the + 1 here not because the call will write a trailing \0,
+          // but so that signature.length() is correctly set to |length|.
+          reinterpret_cast<unsigned char*>(WriteInto(&signature, length + 1)),
+          length)) {
     NOTREACHED();
     return false;
   }
diff --git a/net/http/http_mac_signature_unittest.cc b/net/http/http_mac_signature_unittest.cc
index 74064dd..130ce39 100644
--- a/net/http/http_mac_signature_unittest.cc
+++ b/net/http/http_mac_signature_unittest.cc
@@ -59,7 +59,7 @@ TEST(HttpMacSignatureTest, GenerateHeaderString) {
   EXPECT_TRUE(signature.GenerateHeaderString(age, nonce, &header_string));
   EXPECT_EQ("MAC id=\"dfoi30j0qnf\", "
             "nonce=\"239034:mn4302j0n+32r2/f3r=\", "
-            "mac=\"GrkHtPKzB1m1dCHfa7OCWOw6EQ==\"",
+            "mac=\"GrkHtPKzB1m1dCHfa7OCWOw6Ecw=\"",
             header_string);
 }
 
@@ -104,6 +104,6 @@ TEST(HttpMacSignatureTest, GenerateMAC) {
 
   std::string mac;
   EXPECT_TRUE(signature.GenerateMAC(age, nonce, &mac));
-  EXPECT_EQ("GrkHtPKzB1m1dCHfa7OCWOw6EQ==", mac);
+  EXPECT_EQ("GrkHtPKzB1m1dCHfa7OCWOw6Ecw=", mac);
 }
 }