From 6625e76e41c22794711b0a7dc5f56afe8ef6125c Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 11 Oct 2024 12:45:46 -0700 Subject: [PATCH] Upper case all http methods in fetch API with compat flag (#2913) --- src/workerd/api/BUILD.bazel | 6 ++++++ src/workerd/api/http.c++ | 19 +++++++++++-------- src/workerd/api/tests/fetch-test.js | 24 ++++++++++++++++++++++++ src/workerd/api/tests/fetch-test.wd-test | 15 +++++++++++++++ src/workerd/io/compatibility-date.capnp | 12 ++++++++++++ src/workerd/util/strings.c++ | 4 ++++ src/workerd/util/strings.h | 1 + 7 files changed, 73 insertions(+), 8 deletions(-) create mode 100644 src/workerd/api/tests/fetch-test.js create mode 100644 src/workerd/api/tests/fetch-test.wd-test diff --git a/src/workerd/api/BUILD.bazel b/src/workerd/api/BUILD.bazel index b954db0bbbf..e15d29244d7 100644 --- a/src/workerd/api/BUILD.bazel +++ b/src/workerd/api/BUILD.bazel @@ -556,3 +556,9 @@ wd_test( args = ["--experimental"], data = ["tests/no-to-string-tag-test.js"], ) + +wd_test( + src = "tests/fetch-test.wd-test", + args = ["--experimental"], + data = ["tests/fetch-test.js"], +) diff --git a/src/workerd/api/http.c++ b/src/workerd/api/http.c++ index a8e0fb1708d..06c0771e6a9 100644 --- a/src/workerd/api/http.c++ +++ b/src/workerd/api/http.c++ @@ -1002,12 +1002,16 @@ jsg::Ref Request::constructor( } KJ_IF_SOME(m, initDict.method) { - auto originalMethod = kj::str(m); KJ_IF_SOME(code, tryParseHttpMethod(m)) { method = code; - } else { - KJ_IF_SOME(code, kj::tryParseHttpMethod(toUpper(kj::mv(m)))) { - method = code; + } else KJ_IF_SOME(code, kj::tryParseHttpMethod(toUpper(m))) { + method = code; + if (!FeatureFlags::get(js).getUpperCaseAllHttpMethods()) { + // This is actually the spec defined behavior. We're expected to only + // upper case get, post, put, delete, head, and options per the spec. + // Other methods, even if they would be recognized if they were uppercased, + // are supposed to be rejected. + // Refs: https://fetch.spec.whatwg.org/#methods switch (method) { case kj::HttpMethod::GET: case kj::HttpMethod::POST: @@ -1017,12 +1021,11 @@ jsg::Ref Request::constructor( case kj::HttpMethod::OPTIONS: break; default: - JSG_FAIL_REQUIRE( - TypeError, kj::str("Invalid HTTP method string: ", originalMethod)); + JSG_FAIL_REQUIRE(TypeError, kj::str("Invalid HTTP method string: ", m)); } - } else { - JSG_FAIL_REQUIRE(TypeError, kj::str("Invalid HTTP method string: ", originalMethod)); } + } else { + JSG_FAIL_REQUIRE(TypeError, kj::str("Invalid HTTP method string: ", m)); } } diff --git a/src/workerd/api/tests/fetch-test.js b/src/workerd/api/tests/fetch-test.js new file mode 100644 index 00000000000..ce49110d5a2 --- /dev/null +++ b/src/workerd/api/tests/fetch-test.js @@ -0,0 +1,24 @@ +import { strictEqual, throws } from 'assert'; + +// Test depends on the setting of the upper_case_all_http_methods compatibility flag. +strictEqual( + globalThis.Cloudflare.compatibilityFlags['upper_case_all_http_methods'], + true +); + +export const test = { + test() { + // Verify that lower-cased method names are converted to upper-case. + // even though the Fetch API doesn't do this in general for all methods. + // Note that the upper_case_all_http_methods compat flag is intentionally + // diverging from the Fetch API here. + const req = new Request('https://example.com', { method: 'patch' }); + strictEqual(req.method, 'PATCH'); + + // Unrecognized methods error as expected, with the error message + // showing the original-cased method name. + throws(() => new Request('http://example.org', { method: 'patchy' }), { + message: /^Invalid HTTP method string: patchy$/, + }); + }, +}; diff --git a/src/workerd/api/tests/fetch-test.wd-test b/src/workerd/api/tests/fetch-test.wd-test new file mode 100644 index 00000000000..ed658c5f2b1 --- /dev/null +++ b/src/workerd/api/tests/fetch-test.wd-test @@ -0,0 +1,15 @@ +using Workerd = import "/workerd/workerd.capnp"; + +const unitTests :Workerd.Config = ( + services = [ + ( name = "fetch-test", + worker = ( + modules = [ + (name = "worker", esModule = embed "fetch-test.js") + ], + compatibilityDate = "2024-10-01", + compatibilityFlags = ["nodejs_compat", "upper_case_all_http_methods"], + ) + ), + ], +); diff --git a/src/workerd/io/compatibility-date.capnp b/src/workerd/io/compatibility-date.capnp index 7ad5169beb5..253fbe49d25 100644 --- a/src/workerd/io/compatibility-date.capnp +++ b/src/workerd/io/compatibility-date.capnp @@ -639,4 +639,16 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef { # fix several spec compliance bugs. Unfortunately it turns out that was more breaking # than expected. This flag restores the original behavior for compat dates before # 2024-09-26 + + upperCaseAllHttpMethods @65 :Bool + $compatEnableFlag("upper_case_all_http_methods") + $compatDisableFlag("no_upper_case_all_http_methods") + $compatEnableDate("2024-10-14"); + # HTTP methods are expected to be upper-cased. Per the fetch spec, if the methods + # is specified as `get`, `post`, `put`, `delete`, `head`, or `options`, implementations + # are expected to uppercase the method. All other method names would generally be + # expected to throw as unrecognized (e.g. `patch` would be an error while `PATCH` is + # accepted). This is a bit restrictive, even if it is in the spec. This flag modifies + # the behavior to uppercase all methods prior to parsing to that the method is always + # recognized if it is a known method. } diff --git a/src/workerd/util/strings.c++ b/src/workerd/util/strings.c++ index 568b56b19e4..6042e48d47f 100644 --- a/src/workerd/util/strings.c++ +++ b/src/workerd/util/strings.c++ @@ -63,6 +63,10 @@ kj::String toLower(kj::ArrayPtr ptr) { return toLower(kj::str(ptr)); } +kj::String toUpper(kj::ArrayPtr ptr) { + return toUpper(kj::str(ptr)); +} + kj::ArrayPtr trimLeadingAndTrailingWhitespace(kj::ArrayPtr ptr) { size_t start = 0; auto end = ptr.size(); diff --git a/src/workerd/util/strings.h b/src/workerd/util/strings.h index d632295abf0..7b43f7a09c1 100644 --- a/src/workerd/util/strings.h +++ b/src/workerd/util/strings.h @@ -84,6 +84,7 @@ kj::String toUpper(kj::String&& str); // Copy the input and convert ASCII alpha characters in the given string to lowercase. kj::String toLower(kj::ArrayPtr ptr); +kj::String toUpper(kj::ArrayPtr ptr); kj::ArrayPtr trimLeadingAndTrailingWhitespace(kj::ArrayPtr ptr); kj::ArrayPtr trimTailingWhitespace(kj::ArrayPtr ptr);