Bug 1733981: Allow single range header values without preflight. r=twisniewski,necko-reviewers,kershaw

The Range header was added as a safe-listed header as long as the value
is in a particular format. Update IsCORSSafelistedRequestHeader
implementations to account for this.

Differential Revision: https://phabricator.services.mozilla.com/D182861

dom/base/nsContentUtils.cpp

@@ -212,6 +212,7 @@
 #include "mozilla/ipc/ProtocolUtils.h"
 #include "mozilla/ipc/SharedMemory.h"
 #include "mozilla/net/UrlClassifierCommon.h"
+#include "mozilla/Tokenizer.h"
 #include "mozilla/widget/IMEData.h"
 #include "nsAboutProtocolUtils.h"
 #include "nsAlgorithm.h"
@@ -7510,6 +7511,77 @@ bool nsContentUtils::ContainsForbiddenMethod(const nsACString& headerValue) {
   return hasInsecureMethod;
 }
 
+Maybe<nsContentUtils::ParsedRange> nsContentUtils::ParseSingleRangeRequest(
+    const nsACString& aHeaderValue, bool aAllowWhitespace) {
+  // See https://fetch.spec.whatwg.org/#simple-range-header-value
+  mozilla::Tokenizer p(aHeaderValue);
+  Maybe<uint32_t> rangeStart;
+  Maybe<uint32_t> rangeEnd;
+
+  // Step 2 and 3
+  if (!p.CheckWord("bytes")) {
+    return Nothing();
+  }
+
+  // Step 4
+  if (aAllowWhitespace) {
+    p.SkipWhites();
+  }
+
+  // Step 5 and 6
+  if (!p.CheckChar('=')) {
+    return Nothing();
+  }
+
+  // Step 7
+  if (aAllowWhitespace) {
+    p.SkipWhites();
+  }
+
+  // Step 8 and 9
+  int32_t res;
+  if (p.ReadInteger(&res)) {
+    rangeStart = Some(res);
+  }
+
+  // Step 10
+  if (aAllowWhitespace) {
+    p.SkipWhites();
+  }
+
+  // Step 11
+  if (!p.CheckChar('-')) {
+    return Nothing();
+  }
+
+  // Step 13
+  if (aAllowWhitespace) {
+    p.SkipWhites();
+  }
+
+  // Step 14 and 15
+  if (p.ReadInteger(&res)) {
+    rangeEnd = Some(res);
+  }
+
+  // Step 16
+  if (!p.CheckEOF()) {
+    return Nothing();
+  }
+
+  // Step 17
+  if (!rangeStart && !rangeEnd) {
+    return Nothing();
+  }
+
+  // Step 18
+  if (rangeStart && rangeEnd && *rangeStart > *rangeEnd) {
+    return Nothing();
+  }
+
+  return Some(ParsedRange(rangeStart, rangeEnd));
+}
+
 // static
 bool nsContentUtils::IsCorsUnsafeRequestHeaderValue(
     const nsACString& aHeaderValue) {
@@ -7579,6 +7651,19 @@ bool nsContentUtils::IsAllowedNonCorsLanguage(const nsACString& aHeaderValue) {
   return true;
 }
 
+bool nsContentUtils::IsAllowedNonCorsRange(const nsACString& aHeaderValue) {
+  Maybe<ParsedRange> parsedRange = ParseSingleRangeRequest(aHeaderValue, false);
+  if (!parsedRange) {
+    return false;
+  }
+
+  if (!parsedRange->Start()) {
+    return false;
+  }
+
+  return true;
+}
+
 // static
 bool nsContentUtils::IsCORSSafelistedRequestHeader(const nsACString& aName,
                                                    const nsACString& aValue) {
@@ -7593,7 +7678,9 @@ bool nsContentUtils::IsCORSSafelistedRequestHeader(const nsACString& aName,
          (aName.LowerCaseEqualsLiteral("content-language") &&
           nsContentUtils::IsAllowedNonCorsLanguage(aValue)) ||
          (aName.LowerCaseEqualsLiteral("content-type") &&
-          nsContentUtils::IsAllowedNonCorsContentType(aValue));
+          nsContentUtils::IsAllowedNonCorsContentType(aValue)) ||
+         (aName.LowerCaseEqualsLiteral("range") &&
+          nsContentUtils::IsAllowedNonCorsRange(aValue));
 }
 
 mozilla::LogModule* nsContentUtils::ResistFingerprintingLog() {

dom/base/nsContentUtils.h

@@ -2769,6 +2769,34 @@ class nsContentUtils {
    * Checks whether the  header value contains any forbidden method
    */
   static bool ContainsForbiddenMethod(const nsACString& headerValue);
+
+  class ParsedRange {
+   public:
+    explicit ParsedRange(mozilla::Maybe<uint32_t> aStart,
+                         mozilla::Maybe<uint32_t> aEnd)
+        : mStart(aStart), mEnd(aEnd) {}
+
+    mozilla::Maybe<uint32_t> Start() const { return mStart; }
+    mozilla::Maybe<uint32_t> End() const { return mEnd; }
+
+    bool operator==(const ParsedRange& aOther) const {
+      return Start() == aOther.Start() && End() == aOther.End();
+    }
+
+   private:
+    mozilla::Maybe<uint32_t> mStart;
+    mozilla::Maybe<uint32_t> mEnd;
+  };
+
+  /**
+   * Parse a single range request and return a pair containing the resulting
+   * start and end of the range.
+   *
+   * See https://fetch.spec.whatwg.org/#simple-range-header-value
+   */
+  static mozilla::Maybe<ParsedRange> ParseSingleRangeRequest(
+      const nsACString& aHeaderValue, bool aAllowWhitespace);
+
   /**
    * Returns whether a given header has characters that aren't permitted
    */
@@ -2792,6 +2820,12 @@ class nsContentUtils {
    */
   static bool IsAllowedNonCorsLanguage(const nsACString& aHeaderValue);
 
+  /**
+   * Returns whether a given Range header value is allowed for a non-CORS XHR or
+   * fetch request.
+   */
+  static bool IsAllowedNonCorsRange(const nsACString& aHeaderValue);
+
   /**
    * Returns whether a given header and value is a CORS-safelisted request
    * header per https://fetch.spec.whatwg.org/#cors-safelisted-request-header

dom/base/test/gtest/TestContentUtils.cpp

@@ -21,6 +21,21 @@ struct IsURIInListMatch {
   bool firstMatch, secondMatch;
 };
 
+std::ostream& operator<<(std::ostream& aStream,
+                         const nsContentUtils::ParsedRange& aParsedRange) {
+  if (aParsedRange.Start()) {
+    aStream << *aParsedRange.Start();
+  }
+
+  aStream << "-";
+
+  if (aParsedRange.End()) {
+    aStream << *aParsedRange.End();
+  }
+
+  return aStream;
+}
+
 TEST(DOM_Base_ContentUtils, IsURIInList)
 {
   nsCOMPtr<nsIURI> uri, subURI;
@@ -144,3 +159,63 @@ TEST(DOM_Base_ContentUtils, StringifyJSON_Object_UndefinedIsVoidString)
 
   ASSERT_TRUE(serializedValue.EqualsLiteral("{\"key1\":\"Hello World!\"}"));
 }
+
+TEST(DOM_Base_ContentUtils, ParseSingleRangeHeader)
+{
+  // Parsing a simple range should succeed
+  EXPECT_EQ(nsContentUtils::ParseSingleRangeRequest("bytes=0-42"_ns, false),
+            mozilla::Some(nsContentUtils::ParsedRange(mozilla::Some(0),
+                                                      mozilla::Some(42))));
+
+  // Range containing a invalid rangeStart should fail
+  EXPECT_EQ(nsContentUtils::ParseSingleRangeRequest("bytes= t-200"_ns, true),
+            mozilla::Nothing());
+
+  // Range containing whitespace, with allowWhitespace=false should fail.
+  EXPECT_EQ(nsContentUtils::ParseSingleRangeRequest("bytes= 2-200"_ns, false),
+            mozilla::Nothing());
+
+  // Range containing whitespace, with allowWhitespace=true should succeed
+  EXPECT_EQ(
+      nsContentUtils::ParseSingleRangeRequest("bytes \t= 2 - 200"_ns, true),
+      mozilla::Some(
+          nsContentUtils::ParsedRange(mozilla::Some(2), mozilla::Some(200))));
+
+  // Range containing invalid whitespace should fail
+  EXPECT_EQ(
+      nsContentUtils::ParseSingleRangeRequest("bytes \r= 2 - 200"_ns, true),
+      mozilla::Nothing());
+
+  // Range without a rangeStart should succeed
+  EXPECT_EQ(nsContentUtils::ParseSingleRangeRequest("bytes\t=\t-200"_ns, true),
+            mozilla::Some(nsContentUtils::ParsedRange(mozilla::Nothing(),
+                                                      mozilla::Some(200))));
+
+  // Range without a rangeEnd should succeed
+  EXPECT_EQ(nsContentUtils::ParseSingleRangeRequest("bytes=55-"_ns, true),
+            mozilla::Some(nsContentUtils::ParsedRange(mozilla::Some(55),
+                                                      mozilla::Nothing())));
+
+  // Range without a rangeStart or rangeEnd should fail
+  EXPECT_EQ(nsContentUtils::ParseSingleRangeRequest("bytes\t=\t-"_ns, true),
+            mozilla::Nothing());
+
+  // Range with extra characters should fail
+  EXPECT_EQ(nsContentUtils::ParseSingleRangeRequest("bytes=0-42 "_ns, true),
+            mozilla::Nothing());
+
+  // Range with rangeStart > rangeEnd should fail
+  EXPECT_EQ(nsContentUtils::ParseSingleRangeRequest("bytes=42-0 "_ns, true),
+            mozilla::Nothing());
+}
+
+TEST(DOM_Base_ContentUtils, IsAllowedNonCorsRange)
+{
+  EXPECT_EQ(nsContentUtils::IsAllowedNonCorsRange("bytes=-200"_ns), false);
+  EXPECT_EQ(nsContentUtils::IsAllowedNonCorsRange("bytes= 200-"_ns), false);
+  EXPECT_EQ(nsContentUtils::IsAllowedNonCorsRange("bytes=201-200"_ns), false);
+  EXPECT_EQ(nsContentUtils::IsAllowedNonCorsRange("bytes=200-201 "_ns), false);
+  EXPECT_EQ(nsContentUtils::IsAllowedNonCorsRange("bytes=200-"_ns), true);
+  EXPECT_EQ(nsContentUtils::IsAllowedNonCorsRange("bytes=200-201"_ns), true);
+  EXPECT_EQ(nsContentUtils::IsAllowedNonCorsRange("bytes=-200 "_ns), false);
+}

dom/fetch/InternalHeaders.cpp

@@ -148,6 +148,7 @@ bool InternalHeaders::DeleteInternal(const nsCString& aLowerName,
 }
 
 void InternalHeaders::Delete(const nsACString& aName, ErrorResult& aRv) {
+  // See https://fetch.spec.whatwg.org/#dom-headers-delete
   nsAutoCString lowerName;
   ToLowerCase(aName, lowerName);
 
@@ -156,36 +157,33 @@ void InternalHeaders::Delete(const nsACString& aName, ErrorResult& aRv) {
     return;
   }
 
-  // Step 2
   if (IsImmutable(aRv)) {
     return;
   }
 
-  // Step 3
   nsAutoCString value;
   GetInternal(lowerName, value, aRv);
   if (IsForbiddenRequestHeader(lowerName, value)) {
     return;
   }
 
-  // Step 4
+  // Step 2
   if (mGuard == HeadersGuardEnum::Request_no_cors &&
       !IsNoCorsSafelistedRequestHeaderName(lowerName) &&
       !IsPrivilegedNoCorsRequestHeaderName(lowerName)) {
     return;
   }
 
-  // Step 5
   if (IsForbiddenResponseHeader(lowerName)) {
     return;
   }
 
-  // Steps 6 and 7
+  // Steps 3, 4, and 5
   if (!DeleteInternal(lowerName, aRv)) {
     return;
   }
 
-  // Step 8
+  // Step 6
   if (mGuard == HeadersGuardEnum::Request_no_cors) {
     RemovePrivilegedNoCorsRequestHeaders();
   }
@@ -352,7 +350,9 @@ bool InternalHeaders::IsSimpleHeader(const nsCString& aName,
          (aName.EqualsIgnoreCase("content-language") &&
           nsContentUtils::IsAllowedNonCorsLanguage(aValue)) ||
          (aName.EqualsIgnoreCase("content-type") &&
-          nsContentUtils::IsAllowedNonCorsContentType(aValue));
+          nsContentUtils::IsAllowedNonCorsContentType(aValue)) ||
+         (aName.EqualsIgnoreCase("range") &&
+          nsContentUtils::IsAllowedNonCorsRange(aValue));
 }
 
 // static

testing/web-platform/meta/cors/cors-safelisted-request-header.any.js.ini

@@ -1,18 +0,0 @@
-[cors-safelisted-request-header.any.worker.html]
-  expected:
-    if (os == "android") and fission: [OK, TIMEOUT]
-  [No preflight for {"range":"bytes=100-200"}]
-    expected: FAIL
-
-  [No preflight for {"range":"bytes=200-"}]
-    expected: FAIL
-
-
-[cors-safelisted-request-header.any.html]
-  expected:
-    if (os == "android") and fission: [OK, TIMEOUT]
-  [No preflight for {"range":"bytes=100-200"}]
-    expected: FAIL
-
-  [No preflight for {"range":"bytes=200-"}]
-    expected: FAIL

testing/web-platform/meta/fetch/range/general.any.js.ini

@@ -1,24 +1,3 @@
-[general.any.worker.html]
-  expected:
-    if (os == "android") and fission: [OK, TIMEOUT]
-  [Cross Origin Fetch with safe range header]
-    expected: FAIL
-
-
-[general.any.html]
-  expected:
-    if (os == "android") and fission: [OK, TIMEOUT]
-  [Cross Origin Fetch with safe range header]
-    expected: FAIL
-
-
-[general.any.sharedworker.html]
-  expected:
-    if (os == "android") and fission: [OK, TIMEOUT]
-  [Cross Origin Fetch with safe range header]
-    expected: FAIL
-
-
 [general.any.serviceworker.html]
   expected:
     if (os == "android") and fission: [OK, TIMEOUT]