Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upper case all http methods in fetch API with compat flag #2913

Merged
merged 2 commits into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
jasnell marked this conversation as resolved.
Show resolved Hide resolved
// 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.
jasnell marked this conversation as resolved.
Show resolved Hide resolved
// 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));
jasnell marked this conversation as resolved.
Show resolved Hide resolved
}
} 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