Temporary revert of 7913a74b0e22d99c3425dcea22e68783919a9667

The original CL removed support for single-quoted file names in
Content-Disposition headers, to be consistent with spec and other
browsers. This CL temporarily reverts it, to give developers more
time to update their sites accordingly.

This is not an exact revert.  It resolves merge conflicts with
https://chromium-review.googlesource.com/c/1297251, uses git cl
format to simplify the diff, and removes some extraneous changes
(Fixes other code not to rely on the single-quote behavior,
obsoleted histogram).

original CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1286733

Bug: 927366, 896233, 179825
Change-Id: Ic02e1b59ba52fda5923ce1d19207e3176b7b6065
Reviewed-on: https://chromium-review.googlesource.com/c/1465067
Reviewed-by: Asanka Herath <asanka@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#631281}
diff --git a/components/download/internal/common/download_stats.cc b/components/download/internal/common/download_stats.cc
index a60592d..fe44b54 100644
--- a/components/download/internal/common/download_stats.cc
+++ b/components/download/internal/common/download_stats.cc
@@ -58,8 +58,6 @@
 
   CONTENT_DISPOSITION_HAS_NAME_ONLY,  // Obsolete; kept for UMA compatiblity.
 
-  CONTENT_DISPOSITION_HAS_SINGLE_QUOTED_FILENAME,
-
   CONTENT_DISPOSITION_LAST_ENTRY
 };
 
@@ -980,9 +978,6 @@
   RecordContentDispositionCountFlag(
       CONTENT_DISPOSITION_HAS_RFC2047_ENCODED_STRINGS, result,
       net::HttpContentDisposition::HAS_RFC2047_ENCODED_STRINGS);
-  RecordContentDispositionCountFlag(
-      CONTENT_DISPOSITION_HAS_SINGLE_QUOTED_FILENAME, result,
-      net::HttpContentDisposition::HAS_SINGLE_QUOTED_FILENAME);
 }
 
 void RecordOpen(const base::Time& end) {
diff --git a/net/http/http_content_disposition.cc b/net/http/http_content_disposition.cc
index bdf8ab44..aba9e10 100644
--- a/net/http/http_content_disposition.cc
+++ b/net/http/http_content_disposition.cc
@@ -407,11 +407,8 @@
             "filename")) {
       DecodeFilenameValue(iter.value(), referrer_charset, &filename,
                           &parse_result_flags_);
-      if (!filename.empty()) {
+      if (!filename.empty())
         parse_result_flags_ |= HAS_FILENAME;
-        if (filename[0] == '\'')
-          parse_result_flags_ |= HAS_SINGLE_QUOTED_FILENAME;
-      }
     } else if (ext_filename.empty() &&
                base::LowerCaseEqualsASCII(
                    base::StringPiece(iter.name_begin(), iter.name_end()),
@@ -426,9 +423,6 @@
     filename_ = ext_filename;
   else
     filename_ = filename;
-
-  if (!filename.empty() && filename[0] == '\'')
-    parse_result_flags_ |= HAS_SINGLE_QUOTED_FILENAME;
 }
 
 }  // namespace net
diff --git a/net/http/http_content_disposition.h b/net/http/http_content_disposition.h
index 7ae1bee..3ec7669 100644
--- a/net/http/http_content_disposition.h
+++ b/net/http/http_content_disposition.h
@@ -46,10 +46,7 @@
     HAS_PERCENT_ENCODED_STRINGS = 1 << 5,
 
     // Quoted-string contains RFC 2047 encoded words.
-    HAS_RFC2047_ENCODED_STRINGS = 1 << 6,
-
-    // Has a filename that starts with a single quote.
-    HAS_SINGLE_QUOTED_FILENAME = 1 << 7,
+    HAS_RFC2047_ENCODED_STRINGS = 1 << 6
   };
 
   HttpContentDisposition(const std::string& header,
diff --git a/net/http/http_content_disposition_unittest.cc b/net/http/http_content_disposition_unittest.cc
index 36e3994..6759d90 100644
--- a/net/http/http_content_disposition_unittest.cc
+++ b/net/http/http_content_disposition_unittest.cc
@@ -274,8 +274,10 @@
       {"attachment; filename=foo bar.html", HttpContentDisposition::ATTACHMENT,
        L"foo bar.html"},
       // http://greenbytes.de/tech/tc2231/#attwithfntokensq
-      {"attachment; filename='foo.bar'", HttpContentDisposition::ATTACHMENT,
-       L"'foo.bar'"},
+      {
+          "attachment; filename='foo.bar'", HttpContentDisposition::ATTACHMENT,
+          L"foo.bar"  // Should be L"'foo.bar'"
+      },
 #ifdef ICU_SHOULD_FAIL_CONVERSION_ON_INVALID_CHARACTER
       // http://greenbytes.de/tech/tc2231/#attwithisofnplain
       {
@@ -444,10 +446,6 @@
            HttpContentDisposition::HAS_FILENAME},
       {"attachment; filename=x", HttpContentDisposition::HAS_DISPOSITION_TYPE |
                                      HttpContentDisposition::HAS_FILENAME},
-      {"attachment; filename='x'",
-       HttpContentDisposition::HAS_DISPOSITION_TYPE |
-           HttpContentDisposition::HAS_FILENAME |
-           HttpContentDisposition::HAS_SINGLE_QUOTED_FILENAME},
       {"attachment; filename=x; name=y",
        HttpContentDisposition::HAS_DISPOSITION_TYPE |
            HttpContentDisposition::HAS_FILENAME},
diff --git a/net/http/http_util.cc b/net/http/http_util.cc
index 23b71d0..fd29ff4 100644
--- a/net/http/http_util.cc
+++ b/net/http/http_util.cc
@@ -542,9 +542,10 @@
 }
 
 namespace {
-
 bool IsQuote(char c) {
-  return c == '"';
+  // Single quote mark isn't actually part of quoted-text production,
+  // but apparently some servers rely on this.
+  return c == '"' || c == '\'';
 }
 
 bool UnquoteImpl(std::string::const_iterator begin,
@@ -559,10 +560,16 @@
   if (!IsQuote(*begin))
     return false;
 
+  // Anything other than double quotes in strict mode.
+  if (strict_quotes && *begin != '"')
+    return false;
+
   // No terminal quote mark.
   if (end - begin < 2 || *begin != *(end - 1))
     return false;
 
+  char quote = *begin;
+
   // Strip quotemarks
   ++begin;
   --end;
@@ -576,7 +583,7 @@
       prev_escape = true;
       continue;
     }
-    if (strict_quotes && !prev_escape && IsQuote(c))
+    if (strict_quotes && !prev_escape && c == quote)
       return false;
     prev_escape = false;
     unescaped.push_back(c);
@@ -589,7 +596,6 @@
   *out = std::move(unescaped);
   return true;
 }
-
 }  // anonymous namespace
 
 std::string HttpUtil::Unquote(std::string::const_iterator begin,
@@ -1019,7 +1025,7 @@
     bool ignore_empty_values)
     : values_(values_begin, values_end, std::string(1, delimiter)),
       ignore_empty_values_(ignore_empty_values) {
-  values_.set_quote_chars("\"");
+  values_.set_quote_chars("\'\"");
   // Could set this unconditionally, since code below has to check for empty
   // values after trimming, anyways, but may provide a minor performance
   // improvement.
@@ -1057,7 +1063,10 @@
       value_end_(end),
       value_is_quoted_(false),
       values_optional_(optional_values == Values::NOT_REQUIRED),
-      strict_quotes_(strict_quotes == Quotes::STRICT_QUOTES) {}
+      strict_quotes_(strict_quotes == Quotes::STRICT_QUOTES) {
+  if (strict_quotes_)
+    props_.set_quote_chars("\"");
+}
 
 HttpUtil::NameValuePairsIterator::NameValuePairsIterator(
     std::string::const_iterator begin,
@@ -1149,6 +1158,15 @@
   return true;
 }
 
+bool HttpUtil::NameValuePairsIterator::IsQuote(char c) const {
+  if (strict_quotes_)
+    return c == '"';
+
+  // The call to the file-scoped IsQuote must be qualified to avoid re-entrantly
+  // calling NameValuePairsIterator::IsQuote again.
+  return net::IsQuote(c);
+}
+
 bool HttpUtil::ParseAcceptEncoding(const std::string& accept_encoding,
                                    std::set<std::string>* allowed_encodings) {
   DCHECK(allowed_encodings);
diff --git a/net/http/http_util.h b/net/http/http_util.h
index cf007043..b2acaa2 100644
--- a/net/http/http_util.h
+++ b/net/http/http_util.h
@@ -150,6 +150,8 @@
   // unescaped actually is a valid quoted string. Returns false for an empty
   // string, a string without quotes, a string with mismatched quotes, and
   // a string with unescaped embeded quotes.
+  // In accordance with RFC 2616 this method only allows double quotes to
+  // enclose the string.
   static bool StrictUnquote(std::string::const_iterator begin,
                             std::string::const_iterator end,
                             std::string* out) WARN_UNUSED_RESULT;
@@ -342,6 +344,12 @@
     ValuesIterator(const ValuesIterator& other);
     ~ValuesIterator();
 
+    // Set the characters to regard as quotes.  By default, this includes both
+    // single and double quotes.
+    void set_quote_chars(const char* quotes) {
+      values_.set_quote_chars(quotes);
+    }
+
     // Advances the iterator to the next value, if any.  Returns true if there
     // is a next value.  Use value* methods to access the resultant value.
     bool GetNext();
@@ -432,6 +440,8 @@
                                                        value_end_); }
 
    private:
+    bool IsQuote(char c) const;
+
     HttpUtil::ValuesIterator props_;
     bool valid_;
 
diff --git a/net/http/http_util_unittest.cc b/net/http/http_util_unittest.cc
index 7883b3e..58561dc 100644
--- a/net/http/http_util_unittest.cc
+++ b/net/http/http_util_unittest.cc
@@ -262,6 +262,10 @@
   EXPECT_STREQ("X", HttpUtil::Unquote("X").c_str());
   EXPECT_STREQ("\"", HttpUtil::Unquote("\"").c_str());
 
+  // Allow single quotes to act as quote marks.
+  // Not part of RFC 2616.
+  EXPECT_STREQ("x\"", HttpUtil::Unquote("'x\"'").c_str());
+
   // Allow quotes in the middle of the input.
   EXPECT_STREQ("foo\"bar", HttpUtil::Unquote("\"foo\"bar\"").c_str());
 
@@ -1192,26 +1196,25 @@
 }  // namespace
 
 TEST(HttpUtilTest, NameValuePairsIteratorCopyAndAssign) {
-  std::string data =
-      "alpha=\"\\\"a\\\"\"; beta=\" b \"; cappa=\"c;\"; delta=\"d\"";
+  std::string data = "alpha='\\'a\\''; beta=\" b \"; cappa='c;'; delta=\"d\"";
   HttpUtil::NameValuePairsIterator parser_a(data.begin(), data.end(), ';');
 
   EXPECT_TRUE(parser_a.valid());
   ASSERT_NO_FATAL_FAILURE(
-      CheckNextNameValuePair(&parser_a, true, true, "alpha", "\"a\""));
+      CheckNextNameValuePair(&parser_a, true, true, "alpha", "'a'"));
 
   HttpUtil::NameValuePairsIterator parser_b(parser_a);
   // a and b now point to same location
   ASSERT_NO_FATAL_FAILURE(
-      CheckCurrentNameValuePair(&parser_b, true, "alpha", "\"a\""));
+      CheckCurrentNameValuePair(&parser_b, true, "alpha", "'a'"));
   ASSERT_NO_FATAL_FAILURE(
-      CheckCurrentNameValuePair(&parser_a, true, "alpha", "\"a\""));
+      CheckCurrentNameValuePair(&parser_a, true, "alpha", "'a'"));
 
   // advance a, no effect on b
   ASSERT_NO_FATAL_FAILURE(
       CheckNextNameValuePair(&parser_a, true, true, "beta", " b "));
   ASSERT_NO_FATAL_FAILURE(
-      CheckCurrentNameValuePair(&parser_b, true, "alpha", "\"a\""));
+      CheckCurrentNameValuePair(&parser_b, true, "alpha", "'a'"));
 
   // assign b the current state of a, no effect on a
   parser_b = parser_a;
@@ -1238,12 +1241,10 @@
 
 TEST(HttpUtilTest, NameValuePairsIterator) {
   std::string data =
-      "alpha=1; beta= 2 ;"
-      "cappa =' 3; foo=';"
-      "cappa =\" 3; foo=\";"
+      "alpha=1; beta= 2 ;cappa =' 3; ';"
       "delta= \" \\\"4\\\" \"; e= \" '5'\"; e=6;"
-      "f=\"\\\"\\h\\e\\l\\l\\o\\ \\w\\o\\r\\l\\d\\\"\";"
-      "g=\"\"; h=\"hello\"";
+      "f='\\'\\h\\e\\l\\l\\o\\ \\w\\o\\r\\l\\d\\'';"
+      "g=''; h='hello'";
   HttpUtil::NameValuePairsIterator parser(data.begin(), data.end(), ';');
   EXPECT_TRUE(parser.valid());
 
@@ -1251,17 +1252,8 @@
       CheckNextNameValuePair(&parser, true, true, "alpha", "1"));
   ASSERT_NO_FATAL_FAILURE(
       CheckNextNameValuePair(&parser, true, true, "beta", "2"));
-
-  // Single quotes shouldn't be treated as quotes.
   ASSERT_NO_FATAL_FAILURE(
-      CheckNextNameValuePair(&parser, true, true, "cappa", "' 3"));
-  ASSERT_NO_FATAL_FAILURE(
-      CheckNextNameValuePair(&parser, true, true, "foo", "'"));
-
-  // But double quotes should be, and can contain semi-colons and equal signs.
-  ASSERT_NO_FATAL_FAILURE(
-      CheckNextNameValuePair(&parser, true, true, "cappa", " 3; foo="));
-
+      CheckNextNameValuePair(&parser, true, true, "cappa", " 3; "));
   ASSERT_NO_FATAL_FAILURE(
       CheckNextNameValuePair(&parser, true, true, "delta", " \"4\" "));
   ASSERT_NO_FATAL_FAILURE(
@@ -1269,7 +1261,7 @@
   ASSERT_NO_FATAL_FAILURE(
       CheckNextNameValuePair(&parser, true, true, "e", "6"));
   ASSERT_NO_FATAL_FAILURE(
-      CheckNextNameValuePair(&parser, true, true, "f", "\"hello world\""));
+      CheckNextNameValuePair(&parser, true, true, "f", "'hello world'"));
   ASSERT_NO_FATAL_FAILURE(
       CheckNextNameValuePair(&parser, true, true, "g", std::string()));
   ASSERT_NO_FATAL_FAILURE(
@@ -1326,9 +1318,8 @@
   ASSERT_NO_FATAL_FAILURE(CheckInvalidNameValuePair("alpha=1", "; beta"));
   ASSERT_NO_FATAL_FAILURE(CheckInvalidNameValuePair(std::string(), "beta"));
 
-  ASSERT_NO_FATAL_FAILURE(CheckInvalidNameValuePair("alpha=1", "; \"beta\"=2"));
-  ASSERT_NO_FATAL_FAILURE(
-      CheckInvalidNameValuePair(std::string(), "\"beta\"=2"));
+  ASSERT_NO_FATAL_FAILURE(CheckInvalidNameValuePair("alpha=1", "; 'beta'=2"));
+  ASSERT_NO_FATAL_FAILURE(CheckInvalidNameValuePair(std::string(), "'beta'=2"));
   ASSERT_NO_FATAL_FAILURE(CheckInvalidNameValuePair("alpha=1", ";beta="));
   ASSERT_NO_FATAL_FAILURE(CheckInvalidNameValuePair("alpha=1",
                                                     ";beta=;cappa=2"));
@@ -1359,7 +1350,7 @@
 // See comments on the implementation of NameValuePairsIterator::GetNext
 // regarding this derogation from the spec.
 TEST(HttpUtilTest, NameValuePairsIteratorMissingEndQuote) {
-  std::string data = "name=\"value";
+  std::string data = "name='value";
   HttpUtil::NameValuePairsIterator parser(data.begin(), data.end(), ';');
   EXPECT_TRUE(parser.valid());