Share link header parsing code between blink and content.

net/http/http_util_unittest.cc

Index: net/http/http_util_unittest.cc
diff --git a/net/http/http_util_unittest.cc b/net/http/http_util_unittest.cc
index 2b456605d703ae7c5c7bb178d7b64ce4165e7e31..a9593603dd9e9118adbeca4086b2527f93066e22 100644
--- a/net/http/http_util_unittest.cc
+++ b/net/http/http_util_unittest.cc
@@ -249,6 +249,40 @@ TEST(HttpUtilTest, Unquote) {
   EXPECT_STREQ("x\"", HttpUtil::Unquote("'x\"'").c_str());
 }
 
+TEST(HttpUtilTest, StrictUnquote) {
+  std::string out;
+
+  // Replace <backslash> " with ".
+  EXPECT_TRUE(HttpUtil::StrictUnquote("\"xyz\\\"abc\"", &out));
+  EXPECT_STREQ("xyz\"abc", out.c_str());
+
+  // Replace <backslash> <backslash> with <backslash>
+  EXPECT_TRUE(HttpUtil::StrictUnquote("\"xyz\\\\abc\"", &out));
+  EXPECT_STREQ("xyz\\abc", out.c_str());
+  EXPECT_TRUE(HttpUtil::StrictUnquote("\"xyz\\\\\\\\\\\\abc\"", &out));
+  EXPECT_STREQ("xyz\\\\\\abc", out.c_str());
+
+  // Replace <backslash> X with X
+  EXPECT_TRUE(HttpUtil::StrictUnquote("\"xyz\\Xabc\"", &out));
+  EXPECT_STREQ("xyzXabc", out.c_str());
+
+  // Return false on unquoted inputs.
+  EXPECT_FALSE(HttpUtil::StrictUnquote("X", &out));
+
+  // Return false on mismatched quotes.
+  EXPECT_FALSE(HttpUtil::StrictUnquote("\"", &out));
+  EXPECT_FALSE(HttpUtil::StrictUnquote("\"xyz", &out));
+  EXPECT_FALSE(HttpUtil::StrictUnquote("\"abc'", &out));
+
+  // Return false on escaped terminal quote.
+  EXPECT_FALSE(HttpUtil::StrictUnquote("\"abc\\\"", &out));
+
+  // Allow single quotes to act as quote marks.
+  // Not part of RFC 2616.
+  EXPECT_TRUE(HttpUtil::StrictUnquote("'x\"'", &out));

[mmenke, 2016/04/26 18:05:58]

Suggest duplicating this test,but with single and double quotes switched.

+  EXPECT_STREQ("x\"", out.c_str());
+}

[mmenke, 2016/04/26 18:05:58]

Some more other suggested tests:

Minimal quoted string with escaped final quote ("\").  Know you have a test for
this case, just want a smaller string to make sure there are no bounds check
issues (There aren't, but I'm paranoid).

Empty string.

Quoted empty string.

+
 TEST(HttpUtilTest, Quote) {
   EXPECT_STREQ("\"xyz\\\"abc\"", HttpUtil::Quote("xyz\"abc").c_str());
 
@@ -1102,7 +1136,8 @@ TEST(HttpUtilTest, NameValuePairsIteratorOptionalValues) {
 
   HttpUtil::NameValuePairsIterator values_required_parser(
       data.begin(), data.end(), ';',
-      HttpUtil::NameValuePairsIterator::VALUES_NOT_OPTIONAL);
+      HttpUtil::NameValuePairsIterator::Values::NOT_OPTIONAL,
+      HttpUtil::NameValuePairsIterator::Quotes::NOT_STRICT);
   EXPECT_TRUE(values_required_parser.valid());
   ASSERT_NO_FATAL_FAILURE(CheckNextNameValuePair(&values_required_parser, true,
                                                  true, "alpha", "1"));
@@ -1111,7 +1146,8 @@ TEST(HttpUtilTest, NameValuePairsIteratorOptionalValues) {
 
   HttpUtil::NameValuePairsIterator parser(
       data.begin(), data.end(), ';',
-      HttpUtil::NameValuePairsIterator::VALUES_OPTIONAL);
+      HttpUtil::NameValuePairsIterator::Values::OPTIONAL,
+      HttpUtil::NameValuePairsIterator::Quotes::NOT_STRICT);
   EXPECT_TRUE(parser.valid());
 
   ASSERT_NO_FATAL_FAILURE(
@@ -1177,6 +1213,20 @@ TEST(HttpUtilTest, NameValuePairsIteratorMissingEndQuote) {
       &parser, false, true, std::string(), std::string()));
 }
 
+TEST(HttpUtilTest, NameValuePairsIteratorStrictQuotes) {
+  std::string data = "foo=\"bar\"; name='value";

[mmenke, 2016/04/26 18:05:58]

Also, shouldn't use mismatched quotes in this test - HttpUtil::Unquote also
doesn't allow mismatched quotes, so I think this would fail in non-string mode,
too, right?  So do something that only fails in strict mode (\ before final
quote, or an extra quote in the middle of the value).

[mmenke, 2016/04/26 18:05:58]

Could you also test a case with a missing end quote (Codepaths are basically the
same now, but were different in an earlier iteration of this CL, so seems worth
still checking), and an unquoted string?

[Marijn Kruisselbrink, 2016/04/27 01:49:50]

This does in fact only fail in strict mode (the existing test right above it
verifies that non-strict mode doesn't fail with missing end quotes).


On 2016/04/26 at 18:05:58, mmenke wrote:
So the test case here was exactly testing for that missing end quote case. Now
added a few other test cases for other situations as well to better cover the
code.

[mmenke, 2016/04/27 04:02:40]

Oops, sorry, misread the test - was thinking the final quote was part of the
quoted string.

+  HttpUtil::NameValuePairsIterator parser(
+      data.begin(), data.end(), ';',
+      HttpUtil::NameValuePairsIterator::Values::NOT_OPTIONAL,
+      HttpUtil::NameValuePairsIterator::Quotes::STRICT);
+  EXPECT_TRUE(parser.valid());
+
+  ASSERT_NO_FATAL_FAILURE(
+      CheckNextNameValuePair(&parser, true, true, "foo", "bar"));
+  ASSERT_NO_FATAL_FAILURE(CheckNextNameValuePair(&parser, false, false,
+                                                 std::string(), std::string()));
+}
+
 TEST(HttpUtilTest, IsValidHeaderValueRFC7230) {
   EXPECT_TRUE(HttpUtil::IsValidHeaderValueRFC7230(""));