Don't interpret embeded NULLs in a response header line as a line terminator.

net/http/http_util.cc

Index: net/http/http_util.cc
===================================================================
--- net/http/http_util.cc	(revision 100022)
+++ net/http/http_util.cc	(working copy)
@@ -547,7 +547,7 @@
       raw_headers.append(FindFirstNonLWS(line_begin, line_end), line_end);
     } else {
       // Terminate the previous line.
-      raw_headers.push_back('\0');
+      raw_headers.push_back('\n');
 
       // Copy the raw data to output.
       raw_headers.append(line_begin, line_end);
@@ -557,7 +557,15 @@
     }
   }
 
-  raw_headers.append("\0\0", 2);
+  raw_headers.append("\n\n", 2);
+
+  // Use '\0' as the canonical line terminator. If the input already contained
+  // any embeded NULLs we will strip them first to avoid interpreting them as

[wtc, 2011/09/12 18:48:09]

Nit: NULLs => '\0' characters

for consistenty.

[eroman, 2011/09/12 21:19:24]

Done.

+  // line breaks.
+  raw_headers.erase(std::remove(raw_headers.begin(), raw_headers.end(), '\0'),
+                    raw_headers.end());
+  std::replace(raw_headers.begin(), raw_headers.end(), '\n', '\0');

[wtc, 2011/09/12 18:48:09]

This seems a high cost (two extra linear scans of raw_headers)
to pay for an uncommon scenario.

Is there a better solution?

[eroman, 2011/09/12 21:19:24]

I did some performance tests for moderate sized headers and found that on
average (across 1000 runs) the extra scans add ~1 microsecond to the time spent
in AssembleRawHeaders. This is less than 5% of the total cost of
AssembleRawHeaders. Across the total loading time this difference is
un-measurable (as it is well beyond the variance). Hence I don't expect this to
have any sort of performance impact in Chrome.

That said, I could do a more invasive change to avoid some scanning, but it
would come at the cost of code simplicity.

Rather than doing that, I think the bigger performance win here would be to (1)
consolidate the header line parsing directly into HttpResponseHeaders (rather
than building this string externally) since this would save us some memory
allocations and string copies, and (2) implement my TODO to use \n as the
canonical line terminator rather than \0. I can file a separate bug for
refactoring these components, but I would classify it as low-priority.

+
   return raw_headers;
 }