Use net's response header parser for parsing multipart headers.

content/child/multipart_response_delegate.cc

Index: content/child/multipart_response_delegate.cc
diff --git a/content/child/multipart_response_delegate.cc b/content/child/multipart_response_delegate.cc
index 21d2d51e02a20bcd08bb487cb202fe916c271a46..fade2d3728d114db7f02327f771a1793beb415e4 100644
--- a/content/child/multipart_response_delegate.cc
+++ b/content/child/multipart_response_delegate.cc
@@ -8,6 +8,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "net/base/net_util.h"
+#include "net/http/http_response_headers.h"
 #include "net/http/http_util.h"
 #include "third_party/WebKit/public/platform/WebHTTPHeaderVisitor.h"
 #include "third_party/WebKit/public/platform/WebString.h"
@@ -209,57 +210,55 @@ int MultipartResponseDelegate::PushOverLine(const std::string& data,
 }
 
 bool MultipartResponseDelegate::ParseHeaders() {
-  int line_feed_increment = 1;
-
-  // Grab the headers being liberal about line endings.
-  size_t line_start_pos = 0;
-  size_t line_end_pos = data_.find('\n');
-  while (line_end_pos != std::string::npos) {
-    // Handle CRLF
-    if (line_end_pos > line_start_pos && data_[line_end_pos - 1] == '\r') {
-      line_feed_increment = 2;
-      --line_end_pos;
-    } else {
-      line_feed_increment = 1;
-    }
-    if (line_start_pos == line_end_pos) {
-      // A blank line, end of headers
-      line_end_pos += line_feed_increment;
-      break;
-    }
-    // Find the next header line.
-    line_start_pos = line_end_pos + line_feed_increment;
-    line_end_pos = data_.find('\n', line_start_pos);
-  }
-  // Truncated in the middle of a header, stop parsing.
-  if (line_end_pos == std::string::npos)
+  int headers_end_pos = -1;
+
+  size_t i = 0;
+  while ((data_[i] == '\r' || data_[i] == '\n') && i < data_.size())
+    i++;

[mmenke, 2015/06/05 16:01:42]

This isn't correct (If we have a bunch of LFs or CRLFs at the start, we'll skip
over them all).

My suggestion:  Add a method to net::HttpUtil to handle this case. 
LocateEndOfAdditionalHeaders?  Internally, it and LocateEndOfHeaders use the
same function from an anonymous namespace - difference is an additional argument
which sets was_lf to true (And perhaps rename it?  can_be_last_line?).  We'll
need an additional unit test or two for the method, but they should be trivial.

[haavardm, 2015/06/09 15:12:35]

agh, of course..
Done. 

+
+  if (i > 0)
+    headers_end_pos = i;
+  else
+    headers_end_pos =
+        net::HttpUtil::LocateEndOfHeaders(data_.c_str(), data_.size(), 0);
+
+  if (headers_end_pos < 0)
     return false;
 
-  // Eat headers
-  std::string headers("\n");
-  headers.append(data_, 0, line_end_pos);
-  data_ = data_.substr(line_end_pos);
+  // Eat headers and prepend a status line as is required by
+  // HttpResponseHeaders.
+  std::string headers("HTTP/1.1 200 OK\r\n");
+  headers.append(data_, 0, headers_end_pos);
+  data_ = data_.substr(headers_end_pos);
+
+  scoped_refptr<net::HttpResponseHeaders> response_headers =

[mmenke, 2015/06/05 16:01:42]

nit:  include base/ref_counted.h

[haavardm, 2015/06/09 15:12:34]

Done.

+      new net::HttpResponseHeaders(
+          net::HttpUtil::AssembleRawHeaders(headers.c_str(), headers.size()));

[mmenke, 2015/06/05 16:01:42]

Again, suggest a method for HttpUtil that does this, too.  Note that this method
may also be useful if we add support for trailers.

[haavardm, 2015/06/09 15:12:34]

I'm a bit unsure about what you suggest to add a method for. The header copying
below is very specific for multiparts, where only some headers are replaced and
some are not copied.  I believe that would be different for header trailers,
since those headers would be additional headers only.

[mmenke, 2015/06/09 20:23:37]

I was thinking of making a method that does this:

scoped_refptr<net::HttpResponseHeaders> Blarg(char* data, int len) {
  std::string headers("HTTP/1.1 200 OK\r\n");
  headers.append(data, 0, len);
  return scoped_refptr(new net::HttpResponseHeaders(
      net::HttpUtil::AssembleRawHeaders(data, len));
}

Anyhow, doesn't save us a whole lot, and if you prefer things as-is, that's
fine.

 
   // Create a WebURLResponse based on the original set of headers + the
-  // replacement headers.  We only replace the same few headers that gecko
-  // does.  See netwerk/streamconv/converters/nsMultiMixedConv.cpp.
-  std::string content_type = net::GetSpecificHeader(headers, "content-type");
+  // replacement headers. We only replace the same few headers that gecko
+  // does. See netwerk/streamconv/converters/nsMultiMixedConv.cpp.
+  std::string content_type;
+  WebURLResponse response(original_response_.url());
+
   std::string mime_type;
+  if (response_headers->GetMimeType(&mime_type)) {
+    response.setMIMEType(WebString::fromUTF8(mime_type));
+  }
   std::string charset;
-  bool has_charset = false;
-  net::HttpUtil::ParseContentType(content_type, &mime_type, &charset,
-                                  &has_charset, NULL);
-  WebURLResponse response(original_response_.url());
-  response.setMIMEType(WebString::fromUTF8(mime_type));
-  response.setTextEncodingName(WebString::fromUTF8(charset));
+  if (response_headers->GetCharset(&charset)) {
+    response.setTextEncodingName(WebString::fromUTF8(charset));
+  }

[mmenke, 2015/06/05 16:01:42]

I don't feel like I can adequately review the implications of switching from
setting to an empty string to not setting these when we don't have the
corresponding headers.

[haavardm, 2015/06/09 15:12:35]

I think it should handle not setting the mime or text encoding if they are not
present, since there are other such methods that are not called. However, I've
changed it to setting empty strings to preserve the current behavior.

 
+  // Copy the response headers from the original response.
   HeaderCopier copier(&response);
   original_response_.visitHTTPHeaderFields(&copier);
 
+  // Replace original headers with multipart headers listed in kReplaceHeaders.
   for (size_t i = 0; i < arraysize(kReplaceHeaders); ++i) {
     std::string name(kReplaceHeaders[i]);
-    std::string value = net::GetSpecificHeader(headers, name);
-    if (!value.empty()) {
+    std::string value;
+    if (response_headers->EnumerateHeader(NULL, name, &value)) {
       response.setHTTPHeaderField(WebString::fromUTF8(name),
                                   WebString::fromUTF8(value));
     }