Share link header parsing code between blink and content.

net/http/http_util.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // The rules for parsing content-types were borrowed from Firefox:
 // http://lxr.mozilla.org/mozilla/source/netwerk/base/src/nsURLHelper.cpp#834
 
 #include "net/http/http_util.h"
 
 #include <algorithm>
@@ skipping 483 matching lines @@
   }
   return unescaped;
 }
 
 // static
 std::string HttpUtil::Unquote(const std::string& str) {
   return Unquote(str.begin(), str.end());
 }
 
 // static
+bool HttpUtil::StrictUnquote(std::string::const_iterator begin,

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

Hrm...Should this and Unquote share an implementation?  The only differences are
the c == quote check, and the terminal escape characted check, right?

I'd suggest a shared implementation in an anonymous namespace in this file, and
just have these two methods call it.  It makes the differences between them
clearer, and gets us a bit better test coverage of the code.  Also makes
accidental future divergence of the two methods less likely.

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

Good idea. Done.

+                             std::string::const_iterator end,
+                             std::string* out) {
+  // Empty string
+  if (begin == end)
+    return false;
+
+  // Nothing to unquote.
+  if (!IsQuote(*begin))
+    return false;
+
+  // No terminal quote mark.
+  if (end - begin < 2 || *begin != *(end - 1))
+    return false;
+
+  char quote = *begin;
+
+  // Strip quotemarks
+  ++begin;
+  --end;
+
+  // Terminal quote is escaped.
+  if (begin != end && *(end - 1) == '\\')

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

BUG:  "\\" is valid.  Instead, should have a "prev_unescaped == false" check on
the end.  Add a test for this.

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

Yeah, just caught that myself too. Added a test and fixed the code.

+    return false;
+
+  // Unescape quoted-pair (defined in RFC 2616 section 2.2)
+  bool prev_escape = false;
+  std::string unescaped;
+  for (; begin != end; ++begin) {
+    char c = *begin;
+    if (c == '\\' && !prev_escape) {
+      prev_escape = true;
+      continue;
+    }
+    if (!prev_escape && c == quote)
+      return false;
+    prev_escape = false;
+    unescaped.push_back(c);
+  }
+  *out = std::move(unescaped);
+  return true;
+}
+
+// static
+bool HttpUtil::StrictUnquote(const std::string& str, std::string* out) {
+  return StrictUnquote(str.begin(), str.end(), out);
+}
+
+// static
 std::string HttpUtil::Quote(const std::string& str) {
   std::string escaped;
   escaped.reserve(2 + str.size());
 
   std::string::const_iterator begin = str.begin();
   std::string::const_iterator end = str.end();
 
   // Esape any backslashes or quotemarks within the string, and
   // then surround with quotes.
   escaped.push_back('"');
@@ skipping 389 matching lines @@
     if (value_begin_ != value_end_)
       return true;
   }
   return false;
 }
 
 HttpUtil::NameValuePairsIterator::NameValuePairsIterator(
     std::string::const_iterator begin,
     std::string::const_iterator end,
     char delimiter,
-    OptionalValues optional_values)
+    Values optional_values,
+    Quotes strict_quotes)
     : props_(begin, end, delimiter),
       valid_(true),
       name_begin_(end),
       name_end_(end),
       value_begin_(end),
       value_end_(end),
       value_is_quoted_(false),
-      values_optional_(optional_values == VALUES_OPTIONAL) {}
+      values_optional_(optional_values == Values::OPTIONAL),
+      strict_quotes_(strict_quotes == Quotes::STRICT) {}
 
 HttpUtil::NameValuePairsIterator::NameValuePairsIterator(
     std::string::const_iterator begin,
     std::string::const_iterator end,
     char delimiter)
-    : NameValuePairsIterator(begin, end, delimiter, VALUES_NOT_OPTIONAL) {}
+    : NameValuePairsIterator(begin,
+                             end,
+                             delimiter,
+                             Values::NOT_OPTIONAL,
+                             Quotes::NOT_STRICT) {}
 
 HttpUtil::NameValuePairsIterator::NameValuePairsIterator(
     const NameValuePairsIterator& other) = default;
 
 HttpUtil::NameValuePairsIterator::~NameValuePairsIterator() {}
 
 // We expect properties to be formatted as one of:
 //   name="value"
 //   name='value'
 //   name='\'value\''
@@ skipping 34 matching lines @@
   TrimLWS(&name_begin_, &name_end_);
   TrimLWS(&value_begin_, &value_end_);
   value_is_quoted_ = false;
   unquoted_value_.clear();
 
   if (equals != value_end_ && value_begin_ == value_end_) {
     // Malformed; value is empty
     return valid_ = false;
   }
 
   if (value_begin_ != value_end_ && HttpUtil::IsQuote(*value_begin_)) {

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

So strict mode fails on foo='bar but succeeds on foo=bar'?  IsToken even returns
true on the latter (Though if it were ", it would return false).  Is this
expected behavior?

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

Argh, excellent question... And not really something I have an answer to. Maybe
it makes most sense for strict-mode to disallow single quotes entirely. Also
because treating single quotes as quoted strings is actually possibly
spec-incompliant (at least in the Link header spec foo='bar is perfectly legal,
assigning "'bar" to the foo extended parameter. Similarly foo='a\b' should
result in "'a\b'" as value, without any unescaping/unquoting).

Yoav: any particular reason you wrote LinkHeader.cpp to (incorrectly) unquote
strings in single quotes?

Of course as you point out the situation is also weird for foo=bar". At least
code consuming NameValuePairsIterator could detect that via
(!it.value_is_quoted() && !HttpUtil::IsToken(it.value_begin(), it.value_end())).
I don't think it makes sense to make NameValuePairsIterator even more strict by
implying this IsToken check as part of strict quotes (for example the Link
header spec actually uses a |ptoken| rule as valid unquoted values, which allows
for more characters than a plain |token|...)

+    value_is_quoted_ = true;
+
+    if (strict_quotes_) {
+      if (!HttpUtil::StrictUnquote(value_begin_, value_end_, &unquoted_value_))
+        return valid_ = false;
+      return true;
+    }
+
     // Trim surrounding quotemarks off the value
     if (*value_begin_ != *(value_end_ - 1) || value_begin_ + 1 == value_end_) {
       // NOTE: This is not as graceful as it sounds:
       // * quoted-pairs will no longer be unquoted
       //   (["\"hello] should give ["hello]).
       // * Does not detect when the final quote is escaped
       //   (["value\"] should give [value"])
+      value_is_quoted_ = false;
       ++value_begin_;  // Gracefully recover from mismatching quotes.
     } else {
-      value_is_quoted_ = true;
       // Do not store iterators into this. See declaration of unquoted_value_.
       unquoted_value_ = HttpUtil::Unquote(value_begin_, value_end_);
     }
   }
 
   return true;
 }
 
 }  // namespace net