Allow empty header value in the blink HTTP header parser.

The HTTP header parser in Blink forbids empty header values which should
be allowed. This CL fix the issue.

BUG=380075
R=tyoshino@chromium.org

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

git-svn-id: svn://svn.chromium.org/blink/trunk@175532 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/platform/network/HTTPParsers.cpp

@@ -607,74 +607,97 @@ size_t parseHTTPRequestLine(const char* data, size_t length, String& failureReas
     return end - data;
 }
 
-size_t parseHTTPHeader(const char* start, size_t length, String& failureReason, AtomicString& nameStr, AtomicString& valueStr)
+static bool parseHTTPHeaderName(const char* s, size_t start, size_t size, String& failureReason, size_t* position, AtomicString* name)
 {
-    const char* p = start;
-    const char* end = start + length;
-
-    Vector<char> name;
-    Vector<char> value;
-    nameStr = nullAtom;
-    valueStr = nullAtom;
-
-    for (; p < end; p++) {
-        switch (*p) {
+    size_t nameBegin = start;
+    for (size_t i = start; i < size; ++i) {
+        switch (s[i]) {
         case '\r':
-            if (name.isEmpty()) {
-                if (p + 1 < end && *(p + 1) == '\n')
-                    return (p + 2) - start;
-                failureReason = "CR doesn't follow LF at " + trimInputSample(p, end - p);
-                return 0;
-            }
-            failureReason = "Unexpected CR in name at " + trimInputSample(name.data(), name.size());
-            return 0;
+            failureReason = "Unexpected CR in name at " + trimInputSample(&s[nameBegin], i - nameBegin);
+            return false;
         case '\n':
-            failureReason = "Unexpected LF in name at " + trimInputSample(name.data(), name.size());
-            return 0;
+            failureReason = "Unexpected LF in name at " + trimInputSample(&s[nameBegin], i - nameBegin);
+            return false;
         case ':':
-            break;
+            if (i == nameBegin) {
+                failureReason = "Header name is missing";
+                return false;
+            }
+            *name = AtomicString::fromUTF8(&s[nameBegin], i - nameBegin);
+            if (name->isNull()) {
+                failureReason = "Invalid UTF-8 sequence in header name";
+                return false;
+            }
+            *position = i;
+            return true;
         default:
-            name.append(*p);
-            continue;
-        }
-        if (*p == ':') {
-            ++p;
             break;
         }
     }
+    failureReason = "Unterminated header name";
+    return false;
+}
 
-    for (; p < end && *p == 0x20; p++) { }
+static bool parseHTTPHeaderValue(const char* s, size_t start, size_t size, String& failureReason, size_t* position, AtomicString* value)
+{
+    size_t i = start;
+    for (; i < size && s[i] == ' '; ++i) {
+    }
+    size_t valueBegin = i;
 
-    for (; p < end; p++) {
-        switch (*p) {
-        case '\r':
-            break;
-        case '\n':
-            failureReason = "Unexpected LF in value at " + trimInputSample(value.data(), value.size());
-            return 0;
-        default:
-            value.append(*p);
-        }
-        if (*p == '\r') {
-            ++p;
-            break;
+    for (; i < size && s[i] != '\r'; ++i) {
+        if (s[i] == '\n') {
+            failureReason = "Unexpected LF in value at " + trimInputSample(&s[valueBegin], i - valueBegin);
+            return false;
         }
     }
-    if (p >= end || *p != '\n') {
-        failureReason = "CR doesn't follow LF after value at " + trimInputSample(p, end - p);
+    if (i == size) {
+        failureReason = "Unterminated header value";
+        return false;
+    }
+
+    ASSERT(i < size && s[i] == '\r');
+    if (i + 1 >= size || s[i + 1] != '\n') {
+        failureReason = "LF doesn't follow CR after value at " + trimInputSample(&s[i + 1], size - i - 1);
+        return false;
+    }
+
+    *value = AtomicString::fromUTF8(&s[valueBegin], i - valueBegin);
+    if (i != valueBegin && value->isNull()) {
+        failureReason = "Invalid UTF-8 sequence in header value";
+        return false;
+    }
+
+    *position = i + 1;
+    return true;
+}
+
+// Note that the header is already parsed and re-formatted in chromium side.
+// We assume that the input is more restricted than RFC2616.
+size_t parseHTTPHeader(const char* s, size_t size, String& failureReason, AtomicString& name, AtomicString& value)
+{
+    name = nullAtom;
+    value = nullAtom;
+    if (size >= 1 && s[0] == '\r') {
+        if (size >= 2 && s[1] == '\n') {
+            // Skip an empty line.
+            return 2;
+        }
+        failureReason = "LF doesn't follow CR at " + trimInputSample(0, size);
         return 0;
     }
-    nameStr = AtomicString::fromUTF8(name.data(), name.size());
-    valueStr = AtomicString::fromUTF8(value.data(), value.size());
-    if (nameStr.isNull()) {
-        failureReason = "Invalid UTF-8 sequence in header name";
+    size_t current = 0;
+    if (!parseHTTPHeaderName(s, current, size, failureReason, &current, &name)) {
         return 0;
     }
-    if (valueStr.isNull()) {
-        failureReason = "Invalid UTF-8 sequence in header value";
+    ASSERT(s[current] == ':');
+    ++current;
+
+    if (!parseHTTPHeaderValue(s, current, size, failureReason, &current, &value)) {
         return 0;
     }
-    return p - start;
+
+    return current;
 }
 
 size_t parseHTTPRequestBody(const char* data, size_t length, Vector<unsigned char>& body)

third_party/WebKit/Source/platform/network/HTTPParsersTest.cpp

@@ -6,12 +6,22 @@
 #include "HTTPParsers.h"
 
 #include "wtf/MathExtras.h"
+#include "wtf/testing/WTFTestHelpers.h"
 #include "wtf/text/AtomicString.h"
 
 #include <gtest/gtest.h>
 
 namespace WebCore {
 
+namespace {
+
+size_t parseHTTPHeader(const char* data, String& failureReason, AtomicString& nameStr, AtomicString& valueStr)
+{
+    return WebCore::parseHTTPHeader(data, strlen(data), failureReason, nameStr, valueStr);
+}
+
+} // namespace
+
 TEST(HTTPParsersTest, ParseCacheControl)
 {
     CacheControlHeader header;
@@ -94,5 +104,124 @@ TEST(HTTPParsersTest, ParseCacheControl)
     EXPECT_TRUE(std::isnan(header.maxAge));
 }
 
+TEST(HTTPParsersTest, parseHTTPHeaderSimple)
+{
+    String failureReason;
+    AtomicString name, value;
+    EXPECT_EQ(11u, parseHTTPHeader("foo:   bar\r\notherdata", failureReason, name, value));
+    EXPECT_TRUE(failureReason.isEmpty());
+    EXPECT_EQ("foo", name.string());
+    EXPECT_EQ("bar", value.string());
+}
+
+TEST(HTTPParsersTest, parseHTTPHeaderEmptyName)
+{
+    String failureReason;
+    AtomicString name, value;
+    EXPECT_EQ(0u, parseHTTPHeader(": bar\r\notherdata", failureReason, name, value));
+    EXPECT_EQ("Header name is missing", failureReason);
+}
+
+TEST(HTTPParsersTest, parseHTTPHeaderEmptyValue)
+{
+    String failureReason;
+    AtomicString name, value;
+    EXPECT_EQ(6u, parseHTTPHeader("foo: \r\notherdata", failureReason, name, value));
+    EXPECT_TRUE(failureReason.isEmpty());
+    EXPECT_EQ("foo", name.string());
+    EXPECT_TRUE(value.isEmpty());
+}
+
+TEST(HTTPParsersTest, parseHTTPHeaderInvalidName)
+{
+    String failureReason;
+    AtomicString name, value;
+    EXPECT_EQ(0u, parseHTTPHeader("\xfa: \r\notherdata", failureReason, name, value));
+    EXPECT_EQ("Invalid UTF-8 sequence in header name", failureReason);
+}
+
+TEST(HTTPParsersTest, parseHTTPHeaderInvalidValue)
+{
+    String failureReason;
+    AtomicString name, value;
+    EXPECT_EQ(0u, parseHTTPHeader("foo: \xfa\r\notherdata", failureReason, name, value));
+    EXPECT_EQ("Invalid UTF-8 sequence in header value", failureReason);
+}
+
+TEST(HTTPParsersTest, parseHTTPHeaderEmpty)
+{
+    String failureReason;
+    AtomicString name, value;
+    EXPECT_EQ(0u, parseHTTPHeader("", failureReason, name, value));
+    EXPECT_EQ("Unterminated header name", failureReason);
+}
+
+TEST(HTTPParsersTest, parseHTTPHeaderEmptyLine)
+{
+    String failureReason;
+    AtomicString name, value;
+    EXPECT_EQ(2u, parseHTTPHeader("\r\notherdata", failureReason, name, value));
+    EXPECT_TRUE(failureReason.isEmpty());
+    EXPECT_TRUE(name.isNull());
+    EXPECT_TRUE(value.isNull());
+}
+
+TEST(HTTPParsersTest, parseHTTPHeaderUnexpectedCRinName)
+{
+    String failureReason;
+    AtomicString name, value;
+    EXPECT_EQ(0u, parseHTTPHeader("foo\rotherdata\n", failureReason, name, value));
+    EXPECT_EQ("Unexpected CR in name at foo", failureReason);
+}
+
+TEST(HTTPParsersTest, parseHTTPHeaderUnexpectedLFinName)
+{
+    String failureReason;
+    AtomicString name, value;
+    EXPECT_EQ(0u, parseHTTPHeader("foo\notherdata\n", failureReason, name, value));
+    EXPECT_EQ("Unexpected LF in name at foo", failureReason);
+}
+
+TEST(HTTPParsersTest, parseHTTPHeaderUnexpectedLFinValue)
+{
+    String failureReason;
+    AtomicString name, value;
+    EXPECT_EQ(0u, parseHTTPHeader("foo: bar\notherdata\n", failureReason, name, value));
+    EXPECT_EQ("Unexpected LF in value at bar", failureReason);
+}
+
+TEST(HTTPParsersTest, parseHTTPHeaderNoLFAtEndOfLine)
+{
+    String failureReason;
+    AtomicString name, value;
+    EXPECT_EQ(0u, parseHTTPHeader("foo: bar\r", failureReason, name, value));
+    EXPECT_EQ("LF doesn't follow CR after value at ", failureReason);
+}
+
+TEST(HTTPParsersTest, parseHTTPHeaderNoLF)
+{
+    String failureReason;
+    AtomicString name, value;
+    EXPECT_EQ(0u, parseHTTPHeader("foo: bar\rhoge\r\n", failureReason, name, value));
+    EXPECT_EQ("LF doesn't follow CR after value at hoge\r\n", failureReason);
+}
+
+TEST(HTTPParsersTest, parseHTTPHeaderTwoLines)
+{
+    const char data[] = "foo: bar\r\nhoge: fuga\r\nxxx";
+    String failureReason;
+    AtomicString name, value;
+
+    EXPECT_EQ(9u, parseHTTPHeader(data, failureReason, name, value));
+    EXPECT_TRUE(failureReason.isEmpty());
+    EXPECT_EQ("foo", name.string());
+    EXPECT_EQ("bar", value.string());
+
+    EXPECT_EQ(11u, parseHTTPHeader(data + 10, failureReason, name, value));
+    EXPECT_TRUE(failureReason.isEmpty());
+    EXPECT_EQ("hoge", name.string());
+    EXPECT_EQ("fuga", value.string());
+}
+
 } // namespace WebCore