Share code to determine a forbidden method
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Oct 2017 22:40:35 +0000 (22:40 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Oct 2017 22:40:35 +0000 (22:40 +0000)
https://bugs.webkit.org/show_bug.cgi?id=177833

Reviewed by Andy Estes.

Currently we duplicate code in XMLHttpRequest and FetchRequest to determine if a method is
forbidden. We should add a common helper function and update both classes to make use of it.

No functionality changed. So, no new tests.

* Modules/fetch/FetchRequest.cpp:
(WebCore::setMethod): Modified to use WebCore::isForbiddenMethod().
* platform/network/HTTPParsers.cpp:
(WebCore::isForbiddenMethod): Added.
* platform/network/HTTPParsers.h:
* xml/XMLHttpRequest.cpp:
(WebCore::XMLHttpRequest::open): Modified to use WebCore::isForbiddenMethod().
(WebCore::XMLHttpRequest::isAllowedHTTPMethod): Deleted.
* xml/XMLHttpRequest.h:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@222815 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/Modules/fetch/FetchRequest.cpp
Source/WebCore/platform/network/HTTPParsers.cpp
Source/WebCore/platform/network/HTTPParsers.h
Source/WebCore/xml/XMLHttpRequest.cpp
Source/WebCore/xml/XMLHttpRequest.h

index be6ec0a..3452d2e 100644 (file)
@@ -1,3 +1,25 @@
+2017-10-03  Daniel Bates  <dabates@apple.com>
+
+        Share code to determine a forbidden method
+        https://bugs.webkit.org/show_bug.cgi?id=177833
+
+        Reviewed by Andy Estes.
+
+        Currently we duplicate code in XMLHttpRequest and FetchRequest to determine if a method is
+        forbidden. We should add a common helper function and update both classes to make use of it.
+
+        No functionality changed. So, no new tests.
+
+        * Modules/fetch/FetchRequest.cpp:
+        (WebCore::setMethod): Modified to use WebCore::isForbiddenMethod().
+        * platform/network/HTTPParsers.cpp:
+        (WebCore::isForbiddenMethod): Added.
+        * platform/network/HTTPParsers.h:
+        * xml/XMLHttpRequest.cpp:
+        (WebCore::XMLHttpRequest::open): Modified to use WebCore::isForbiddenMethod().
+        (WebCore::XMLHttpRequest::isAllowedHTTPMethod): Deleted.
+        * xml/XMLHttpRequest.h:
+
 2017-10-03  Zalan Bujtas  <zalan@apple.com>
 
         RenderMenuList should not hold raw pointers
index 1c3389b..4084b1f 100644 (file)
@@ -39,13 +39,10 @@ static std::optional<Exception> setMethod(ResourceRequest& request, const String
 {
     if (!isValidHTTPToken(initMethod))
         return Exception { TypeError, ASCIILiteral("Method is not a valid HTTP token.") };
-
-    String method = initMethod.convertToASCIIUppercase();
-    if (method == "CONNECT" || method == "TRACE" || method == "TRACK")
+    if (isForbiddenMethod(initMethod))
         return Exception { TypeError, ASCIILiteral("Method is forbidden.") };
-
+    String method = initMethod.convertToASCIIUppercase();
     request.setHTTPMethod((method == "DELETE" || method == "GET" || method == "HEAD" || method == "OPTIONS" || method == "POST" || method == "PUT") ? method : initMethod);
-
     return std::nullopt;
 }
 
index 95b5e86..48acc3c 100644 (file)
@@ -778,7 +778,7 @@ void parseAccessControlExposeHeadersAllowList(const String& headerValue, HTTPHea
     }
 }
 
-// Implementation of https://fetch.spec.whatwg.org/#cors-safelisted-response-header-name
+// Implements <https://fetch.spec.whatwg.org/#forbidden-header-name>.
 bool isForbiddenHeaderName(const String& name)
 {
     HTTPHeaderName headerName;
@@ -812,11 +812,18 @@ bool isForbiddenHeaderName(const String& name)
     return startsWithLettersIgnoringASCIICase(name, "sec-") || startsWithLettersIgnoringASCIICase(name, "proxy-");
 }
 
+// Implements <https://fetch.spec.whatwg.org/#forbidden-response-header-name>.
 bool isForbiddenResponseHeaderName(const String& name)
 {
     return equalLettersIgnoringASCIICase(name, "set-cookie") || equalLettersIgnoringASCIICase(name, "set-cookie2");
 }
 
+// Implements <https://fetch.spec.whatwg.org/#forbidden-method>.
+bool isForbiddenMethod(const String& name)
+{
+    return equalLettersIgnoringASCIICase(name, "connect") || equalLettersIgnoringASCIICase(name, "trace") || equalLettersIgnoringASCIICase(name, "track");
+}
+
 bool isSimpleHeader(const String& name, const String& value)
 {
     HTTPHeaderName headerName;
index 6a82ad2..4981d80 100644 (file)
@@ -95,6 +95,7 @@ void parseAccessControlExposeHeadersAllowList(const String& headerValue, HTTPHea
 // HTTP Header routine as per https://fetch.spec.whatwg.org/#terminology-headers
 bool isForbiddenHeaderName(const String&);
 bool isForbiddenResponseHeaderName(const String&);
+bool isForbiddenMethod(const String&);
 bool isSimpleHeader(const String& name, const String& value);
 bool isCrossOriginSafeHeader(HTTPHeaderName, const HTTPHeaderSet&);
 bool isCrossOriginSafeHeader(const String&, const HTTPHeaderSet&);
index 575e6fd..e9902db 100644 (file)
@@ -327,13 +327,6 @@ ExceptionOr<void> XMLHttpRequest::setWithCredentials(bool value)
     return { };
 }
 
-bool XMLHttpRequest::isAllowedHTTPMethod(const String& method)
-{
-    return !equalLettersIgnoringASCIICase(method, "trace")
-        && !equalLettersIgnoringASCIICase(method, "track")
-        && !equalLettersIgnoringASCIICase(method, "connect");
-}
-
 String XMLHttpRequest::uppercaseKnownHTTPMethod(const String& method)
 {
     const char* const methods[] = { "DELETE", "GET", "HEAD", "OPTIONS", "POST", "PUT" };
@@ -375,7 +368,7 @@ ExceptionOr<void> XMLHttpRequest::open(const String& method, const URL& url, boo
     if (!isValidHTTPToken(method))
         return Exception { SyntaxError };
 
-    if (!isAllowedHTTPMethod(method))
+    if (isForbiddenMethod(method))
         return Exception { SecurityError };
 
     if (!async && scriptExecutionContext()->isDocument()) {
index 27ff879..dbdc03d 100644 (file)
@@ -104,7 +104,6 @@ public:
     void didCacheResponse();
 
     // Expose HTTP validation methods for other untrusted requests.
-    static bool isAllowedHTTPMethod(const String&);
     static String uppercaseKnownHTTPMethod(const String&);
 
     enum class ResponseType { EmptyString, Arraybuffer, Blob, Document, Json, Text };