Skip to content

Commit

Permalink
Upper case all http methods in fetch API with compat flag (#2913)
Browse files Browse the repository at this point in the history
  • Loading branch information
jasnell authored Oct 11, 2024
1 parent caeb4e0 commit 6625e76
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 8 deletions.
6 changes: 6 additions & 0 deletions src/workerd/api/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
)
19 changes: 11 additions & 8 deletions src/workerd/api/http.c++
Original file line number Diff line number Diff line change
Expand Up @@ -1002,12 +1002,16 @@ jsg::Ref<Request> 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:
Expand All @@ -1017,12 +1021,11 @@ jsg::Ref<Request> 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));
}
}

Expand Down
24 changes: 24 additions & 0 deletions src/workerd/api/tests/fetch-test.js
Original file line number Diff line number Diff line change
@@ -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$/,
});
},
};
15 changes: 15 additions & 0 deletions src/workerd/api/tests/fetch-test.wd-test
Original file line number Diff line number Diff line change
@@ -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"],
)
),
],
);
12 changes: 12 additions & 0 deletions src/workerd/io/compatibility-date.capnp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}
4 changes: 4 additions & 0 deletions src/workerd/util/strings.c++
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ kj::String toLower(kj::ArrayPtr<const char> ptr) {
return toLower(kj::str(ptr));
}

kj::String toUpper(kj::ArrayPtr<const char> ptr) {
return toUpper(kj::str(ptr));
}

kj::ArrayPtr<const char> trimLeadingAndTrailingWhitespace(kj::ArrayPtr<const char> ptr) {
size_t start = 0;
auto end = ptr.size();
Expand Down
1 change: 1 addition & 0 deletions src/workerd/util/strings.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<const char> ptr);
kj::String toUpper(kj::ArrayPtr<const char> ptr);

kj::ArrayPtr<const char> trimLeadingAndTrailingWhitespace(kj::ArrayPtr<const char> ptr);
kj::ArrayPtr<const char> trimTailingWhitespace(kj::ArrayPtr<const char> ptr);
Expand Down

0 comments on commit 6625e76

Please sign in to comment.