From 35f9a265fb9b849dde2e340e99a4ec11e76e181e Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Mon, 29 Jul 2024 12:33:24 -0700 Subject: [PATCH 01/12] Add CMake option to explicitly skip wasm-opt and use `wasm-tools strip` instead. (#89) As per #8, it seems the main purpose of using wasm-opt was to reduce module size by stripping debug sections. The comment in the CMake file also indicates an intent to optimize the binary. However, `wasm-opt -O3` is extremely expensive on large programs; on my 18-core, 36-thread workstation, it took several minutes to complete. This is very painful when part of a debug loop that requires release builds (e.g. for performance features). This PR adds a `USE_WASM_OPT` CMake option, on by default to preserve existing behavior, that allows one to do `cmake -DCMAKE_BUILD_TYPE=Release -DUSE_WASM_OPT=OFF` to get a release build without `wasm-opt`. In its place, this PR uses `wasm-tools strip -a`, which strips debug sections (and all other custom sections) without rewriting code. --- CMakeLists.txt | 36 ++++++++++++++++++++++++------------ cmake/wasm-tools.cmake | 1 + 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 98c3e9d8..611f1f9c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -58,24 +58,36 @@ add_executable(starling.wasm runtime/script_loader.cpp ) +option(USE_WASM_OPT "use wasm-opt to optimize the StarlingMonkey binary" ON) + # For release builds, use wasm-opt to optimize the generated wasm file. -if (CMAKE_BUILD_TYPE STREQUAL "Release") - add_custom_command( - TARGET starling.wasm - POST_BUILD - COMMAND ${WASM_OPT} --strip-debug -O3 -o starling.wasm starling.wasm - WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} - ) -elseif (CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo") - add_custom_command( +if (USE_WASM_OPT) + if (CMAKE_BUILD_TYPE STREQUAL "Release") + add_custom_command( + TARGET starling.wasm + POST_BUILD + COMMAND ${WASM_OPT} --strip-debug -O3 -o starling.wasm starling.wasm + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} + ) + elseif (CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo") + add_custom_command( + TARGET starling.wasm + POST_BUILD + COMMAND ${WASM_OPT} -O3 -o starling.wasm starling.wasm + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} + ) + endif() +else() + if (CMAKE_BUILD_TYPE STREQUAL "Release") + add_custom_command( TARGET starling.wasm POST_BUILD - COMMAND ${WASM_OPT} -O3 -o starling.wasm starling.wasm + COMMAND ${WASM_TOOLS_BIN} strip -a -o starling.wasm starling.wasm WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} - ) + ) + endif() endif() - target_link_libraries(starling.wasm PRIVATE host_api extension_api builtins spidermonkey rust-url) set(RUNTIME_FILE "starling.wasm") diff --git a/cmake/wasm-tools.cmake b/cmake/wasm-tools.cmake index 191d5c2e..ebfbd86e 100644 --- a/cmake/wasm-tools.cmake +++ b/cmake/wasm-tools.cmake @@ -3,3 +3,4 @@ set(WASM_TOOLS_VERSION 1.0.54) set(WASM_TOOLS_URL https://github.com/bytecodealliance/wasm-tools/releases/download/wasm-tools-${WASM_TOOLS_VERSION}/wasm-tools-${WASM_TOOLS_VERSION}-${HOST_ARCH}-${HOST_OS}.tar.gz) CPMAddPackage(NAME wasm-tools URL ${WASM_TOOLS_URL} DOWNLOAD_ONLY TRUE) set(WASM_TOOLS_DIR ${CPM_PACKAGE_wasm-tools_SOURCE_DIR}) +set(WASM_TOOLS_BIN ${WASM_TOOLS_DIR}/wasm-tools) From a163c55bf4ee54f7ab87a88eb11500e68ff0e15b Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Mon, 29 Jul 2024 13:45:23 -0700 Subject: [PATCH 02/12] SpiderMonkey CMake package: use alternate artifacts if specified. (#90) SpiderMonkey CMake package: use alternate artifacts if specified. When developing StarlingMonkey using an existing SpiderMonkey build with no SM changes, it is very convenient to download and use pre-built artifacts for SM from the CI on `spidermonkey-wasi-embedding`, and we do not want to lose this convenience or approachability. However, when modifying SpiderMonkey itself, it is nearly impossible to do development with the current setup: the debug loop requires merging changes to `gecko-dev` with one PR, updating the hash in `spidermonkey-wasi-embedding` with another PR, getting both merged and waiting for the build in CI before StarlingMonkey can use the new engine. Rather than require a local hack for such cases, this modifies the SpiderMonkey CMake module to recognize a `SPIDERMONKEY_BINARIES` environment variable, and use artifacts there instead. This allows the developer to point this to a local clone of `spidermonkey-wasi-embedding` with a modified `gecko-dev`, rebuilt with `rebuild.sh` as needed. The `README` is updated with details on how to use this. --- README.md | 52 ++++++++++++++++++++++++++++++++++++++++ cmake/spidermonkey.cmake | 21 ++++++++++++---- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 5929d292..a6f509df 100644 --- a/README.md +++ b/README.md @@ -160,3 +160,55 @@ name of one of the directories. Currently, only an implementation in terms of [w (host-apis/wasi-0.2.0) is provided, and used by default. To provide a custom host API implementation, you can set `HOST_API` to the (absolute) path of a directory containing that implementation. + +## Developing Changes to SpiderMonkey + +StarlingMonkey uses SpiderMonkey as its underlying JS engine, and by default, +downloads build artifacts from [a wrapper +repository](https://github.com/bytecodealliance/spidermonkey-wasi-embedding) +around [our local SpiderMonkey +tree](https://github.com/bytecodealliance/gecko-dev). That wrapper repository +contains a SpiderMonkey commit-hash in a file, and its CI jobs build the +artifacts that StarlingMonkey downloads during its build. + +This flow is optimized for ease of development of StarlingMonkey, and avoiding +the need to build SpiderMonkey locally, which requires some additional tools +and is resource-intensive. However, sometimes it is necessary or desirable to +make modifications to SpiderMonkey directly, whether to make fixes or optimize +performance. + +In order to do so, first clone the above two repositories, with `gecko-dev` +(SpiderMonkey itself) as a subdirectory to `spidermonkey-wasi-embedding`: + +```shell +git clone https://github.com/bytecodealliance/spidermonkey-wasi-embedding +cd spidermonkey-wasi-embedding/ +git clone https://github.com/bytecodealliance/gecko-dev +``` + +and switch to the commit that we are currently using: + +```shell +git checkout `cat ../gecko-revision` +# now edit the source +``` + +Then make changes as necessary, eventually rebuilding from the +`spidermonkey-wasi-embedding` root: + +```shell +cd ../ # back to spidermonkey-wasi-embedding +./rebuild.sh release +``` + +This will produce a `release/` directory with artifacts of the same form +normally downloaded by StarlingMonkey. So, finally, from within StarlingMonkey, +set an environment variable `SPIDERMONKEY_BINARIES`: + +```shell +export SPIDERMONKEY_BINARIES=/path/to/spidermonkey-wasi-embedding/release +cmake -S . -B cmake-build-release -DCMAKE_BUILD_TYPE=Release +cmake --build cmake-build-release --parallel 8 +``` + +and use/test as above. diff --git a/cmake/spidermonkey.cmake b/cmake/spidermonkey.cmake index 475a358a..5a5dee84 100644 --- a/cmake/spidermonkey.cmake +++ b/cmake/spidermonkey.cmake @@ -6,12 +6,23 @@ else() set(SM_BUILD_TYPE release) endif() -CPMAddPackage(NAME spidermonkey-${SM_BUILD_TYPE} - URL https://github.com/bytecodealliance/spidermonkey-wasi-embedding/releases/download/rev_${SM_REV}/spidermonkey-wasm-static-lib_${SM_BUILD_TYPE}.tar.gz - DOWNLOAD_ONLY YES -) +# If the developer has specified an alternate local set of SpiderMonkey +# artifacts, use them. This allows for local/in-tree development without +# requiring a roundtrip through GitHub CI. +# +# This can be set, for example, to the output directly (`release/` or `debug/`) +# under a local clone of the `spidermonkey-wasi-embedding` repo. +if (DEFINED ENV{SPIDERMONKEY_BINARIES}) + set(SM_SOURCE_DIR $ENV{SPIDERMONKEY_BINARIES}) +else() + CPMAddPackage(NAME spidermonkey-${SM_BUILD_TYPE} + URL https://github.com/bytecodealliance/spidermonkey-wasi-embedding/releases/download/rev_${SM_REV}/spidermonkey-wasm-static-lib_${SM_BUILD_TYPE}.tar.gz + DOWNLOAD_ONLY YES + ) + + set(SM_SOURCE_DIR ${CPM_PACKAGE_spidermonkey-${SM_BUILD_TYPE}_SOURCE_DIR} CACHE STRING "Path to spidermonkey ${SM_BUILD_TYPE} build" FORCE) +endif() -set(SM_SOURCE_DIR ${CPM_PACKAGE_spidermonkey-${SM_BUILD_TYPE}_SOURCE_DIR} CACHE STRING "Path to spidermonkey ${SM_BUILD_TYPE} build" FORCE) set(SM_INCLUDE_DIR ${SM_SOURCE_DIR}/include) file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/null.cpp "") From e5fe814a2ee517bef3b75ca271039b17a45a5ac2 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Mon, 29 Jul 2024 13:49:15 -0700 Subject: [PATCH 03/12] CMake build: do not strip component-related custom sections in non-wasm-opt config. (#99) In #89, I had added an alternative (under `USE_WASM_OPT=OFF`) to avoid expensive wasm-opt runs for local development. However I had replaced this with `wasm-tools strip`, which strips all custom sections, including those important for component-related linkage. Getting further along my integration path I discovered the world info for the module was missing. This PR instead uses `wasm-opt -O0 --strip-debug`, which does what I want (using the same tool as the ordinary build path) without the expensive opt passes. --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 611f1f9c..4509545d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -82,7 +82,7 @@ else() add_custom_command( TARGET starling.wasm POST_BUILD - COMMAND ${WASM_TOOLS_BIN} strip -a -o starling.wasm starling.wasm + COMMAND ${WASM_OPT} --strip-debug -O0 -o starling.wasm starling.wasm WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} ) endif() From ea1ad370cba4f70786eb6d72be56f68db79575ef Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 29 Jul 2024 23:11:20 +0200 Subject: [PATCH 04/12] fix: providing a custom WPT_ROOT env var (#100) --- tests/wpt-harness/wpt.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/wpt-harness/wpt.cmake b/tests/wpt-harness/wpt.cmake index 077b451d..96a5279f 100644 --- a/tests/wpt-harness/wpt.cmake +++ b/tests/wpt-harness/wpt.cmake @@ -2,8 +2,8 @@ enable_testing() include("wasmtime") -if(DEFINED $ENV{WPT_ROOT}) - set(WPT_ROOT $ENV{WPT_ROOT}) +if(DEFINED ENV{WPT_ROOT}) + set(WPT_ROOT ENV{WPT_ROOT}) else() CPMAddPackage( NAME wpt-suite From 1a1dc3de5157fed787c704059f51e8666c6bc553 Mon Sep 17 00:00:00 2001 From: Till Schneidereit Date: Tue, 30 Jul 2024 19:51:24 +0200 Subject: [PATCH 05/12] Implement `Request.clone` (#92) * Cleanup: make `Request::method` infallible It already is infallible internally, so might as well reflect that in the signature and clean up the callsites. * Bugfix: always reify input headers in the `Request` initializer I stumbled upon this while implementing `Request.clone`, and while this change doesn't cause any additional WPT tests to pass, I'm certain that it's required: otherwise, `new Request(incomingRequest)` will not apply `incomingRequest`'s header entries to the new request if `.headers` has never been read on `incomingRequest`. * Implement `Request.clone` Fixes #84 * add simple clone test * fix headers clone --------- Co-authored-by: Guy Bedford --- builtins/web/fetch/fetch-api.cpp | 6 +- builtins/web/fetch/fetch-errors.h | 1 + builtins/web/fetch/request-response.cpp | 218 +++++++----------- builtins/web/fetch/request-response.h | 2 +- tests/integration/fetch/fetch.js | 56 +++++ tests/integration/handlers.js | 2 +- tests/integration/headers/headers.js | 9 - tests/tests.cmake | 2 +- .../api/request/request-structure.any.js.json | 4 +- tests/wpt-harness/post-harness.js | 1 + 10 files changed, 152 insertions(+), 149 deletions(-) create mode 100644 tests/integration/fetch/fetch.js delete mode 100644 tests/integration/headers/headers.js diff --git a/builtins/web/fetch/fetch-api.cpp b/builtins/web/fetch/fetch-api.cpp index 67748c1c..85bf1b0d 100644 --- a/builtins/web/fetch/fetch-api.cpp +++ b/builtins/web/fetch/fetch-api.cpp @@ -35,11 +35,7 @@ bool fetch(JSContext *cx, unsigned argc, Value *vp) { return ReturnPromiseRejectedWithPendingError(cx, args); } - RootedString method_str(cx, Request::method(cx, request_obj)); - if (!method_str) { - return ReturnPromiseRejectedWithPendingError(cx, args); - } - + RootedString method_str(cx, Request::method(request_obj)); host_api::HostString method = core::encode(cx, method_str); if (!method.ptr) { return ReturnPromiseRejectedWithPendingError(cx, args); diff --git a/builtins/web/fetch/fetch-errors.h b/builtins/web/fetch/fetch-errors.h index b74403a0..8e0d9087 100644 --- a/builtins/web/fetch/fetch-errors.h +++ b/builtins/web/fetch/fetch-errors.h @@ -12,6 +12,7 @@ DEF_ERR(InvalidInitArg, JSEXN_TYPEERR, "{0}: |init| parameter can't be converted DEF_ERR(NonBodyRequestWithBody, JSEXN_TYPEERR, "Request constructor: HEAD or GET Request cannot have a body", 0) DEF_ERR(NonBodyResponseWithBody, JSEXN_TYPEERR, "Response constructor: response with status {0} cannot have a body", 1) DEF_ERR(BodyStreamUnusable, JSEXN_TYPEERR, "Can't use a ReadableStream that's locked or has ever been read from or canceled", 0) +DEF_ERR(BodyStreamTeeingFailed, JSEXN_ERR, "Cloning body stream failed", 0) DEF_ERR(InvalidStatus, JSEXN_RANGEERR, "{0}: invalid status {1}", 2) DEF_ERR(InvalidStreamChunk, JSEXN_TYPEERR, "ReadableStream used as a Request or Response body must produce Uint8Array values", 0) DEF_ERR(EmptyHeaderName, JSEXN_TYPEERR, "{0}: Header name can't be empty", 1) diff --git a/builtins/web/fetch/request-response.cpp b/builtins/web/fetch/request-response.cpp index 578b0b6a..caf094fb 100644 --- a/builtins/web/fetch/request-response.cpp +++ b/builtins/web/fetch/request-response.cpp @@ -1074,8 +1074,8 @@ bool RequestOrResponse::maybe_stream_body(JSContext *cx, JS::HandleObject body_o } JSObject *RequestOrResponse::create_body_stream(JSContext *cx, JS::HandleObject owner) { - MOZ_ASSERT(is_instance(owner)); MOZ_ASSERT(!body_stream(owner)); + MOZ_ASSERT(has_body(owner)); JS::RootedObject source(cx, streams::NativeStreamSource::create( cx, owner, JS::UndefinedHandleValue, body_source_pull_algorithm, body_source_cancel_algorithm)); @@ -1144,7 +1144,7 @@ JSObject *Request::response_promise(JSObject *obj) { .toObject(); } -JSString *Request::method(JSContext *cx, JS::HandleObject obj) { +JSString *Request::method(HandleObject obj) { MOZ_ASSERT(is_instance(obj)); return JS::GetReservedSlot(obj, static_cast(Slots::Method)).toString(); } @@ -1152,11 +1152,7 @@ JSString *Request::method(JSContext *cx, JS::HandleObject obj) { bool Request::method_get(JSContext *cx, unsigned argc, JS::Value *vp) { METHOD_HEADER(0) - JSString *method = Request::method(cx, self); - if (!method) - return false; - - args.rval().setString(method); + args.rval().setString(Request::method(self)); return true; } @@ -1197,128 +1193,86 @@ bool Request::bodyUsed_get(JSContext *cx, unsigned argc, JS::Value *vp) { JSString *GET_atom; -// bool Request::clone(JSContext *cx, unsigned argc, JS::Value *vp) { -// METHOD_HEADER(0); - -// auto hasBody = RequestOrResponse::has_body(self); - -// if (hasBody) { -// if (RequestOrResponse::body_unusable(self)) { -// return api::throw_error(cx, FetchErrors::BodyStreamUnusable); -// } - -// // Here we get the current request's body stream and call ReadableStream.prototype.tee to -// return -// // two versions of the stream. Once we get the two streams, we create a new request handle -// and -// // attach one of the streams to the new handle and the other stream is attached to the -// request -// // handle that `clone()` was called upon. -// JS::RootedObject body_stream(cx, RequestOrResponse::body_stream(self)); -// if (!body_stream) { -// body_stream = RequestOrResponse::create_body_stream(cx, self); -// if (!body_stream) { -// return false; -// } -// } -// JS::RootedValue tee_val(cx); -// if (!JS_GetProperty(cx, body_stream, "tee", &tee_val)) { -// return false; -// } -// JS::Rooted tee(cx, JS_GetObjectFunction(&tee_val.toObject())); -// if (!tee) { -// return false; -// } -// JS::RootedVector argv(cx); -// JS::RootedValue rval(cx); -// if (!JS::Call(cx, body_stream, tee, argv, &rval)) { -// return false; -// } -// JS::RootedObject rval_array(cx, &rval.toObject()); -// JS::RootedValue body1_val(cx); -// if (!JS_GetProperty(cx, rval_array, "0", &body1_val)) { -// return false; -// } -// JS::RootedValue body2_val(cx); -// if (!JS_GetProperty(cx, rval_array, "1", &body2_val)) { -// return false; -// } - -// auto res = host_api::HttpBody::make(request_handle); -// if (auto *err = res.to_err()) { -// HANDLE_ERROR(cx, *err); -// return false; -// } - -// auto body_handle = res.unwrap(); -// if (!JS::IsReadableStream(&body1_val.toObject())) { -// return false; -// } -// body_stream.set(&body1_val.toObject()); -// if (RequestOrResponse::body_unusable(cx, body_stream)) { -// return api::throw_error(cx, FetchErrors::BodyStreamUnusable); -// } - -// JS::SetReservedSlot(requestInstance, static_cast(Slots::Body), -// JS::Int32Value(body_handle.handle)); -// JS::SetReservedSlot(requestInstance, static_cast(Slots::BodyStream), body1_val); - -// JS::SetReservedSlot(self, static_cast(Slots::BodyStream), body2_val); -// JS::SetReservedSlot(self, static_cast(Slots::BodyUsed), JS::FalseValue()); -// JS::SetReservedSlot(self, static_cast(Slots::HasBody), JS::BooleanValue(true)); -// } - -// JS::RootedObject headers(cx); -// JS::RootedObject headers_obj( -// cx, RequestOrResponse::headers(cx, self)); -// if (!headers_obj) { -// return false; -// } -// JS::RootedObject headersInstance( -// cx, JS_NewObjectWithGivenProto(cx, &Headers::class_, Headers::proto_obj)); -// if (!headersInstance) -// return false; - -// headers = Headers::create(cx, headersInstance, Headers::Mode::ProxyToRequest, -// requestInstance, headers_obj); - -// if (!headers) { -// return false; -// } - -// JS::SetReservedSlot(requestInstance, static_cast(Slots::Headers), -// JS::ObjectValue(*headers)); +/// https://fetch.spec.whatwg.org/#dom-request-clone +bool Request::clone(JSContext *cx, unsigned argc, JS::Value *vp) { + METHOD_HEADER(0); + + // clone operation step 1. + // Let newRequest be a copy of request, except for its body. + // Note that the spec doesn't say what it means to copy a request, exactly. + // Since a request only has the fields "method", "url", and "headers", and the "Body" mixin, + // we copy those three fields in this step. + RootedObject new_request(cx, create(cx)); + if (!new_request) { + return false; + } + init_slots(new_request); -// JSString *method = Request::method(cx, self); -// if (!method) { -// return false; -// } + RootedValue cloned_headers_val(cx, JS::NullValue()); + RootedObject headers(cx, RequestOrResponse::maybe_headers(self)); + if (headers) { + RootedValue headers_val(cx, ObjectValue(*headers)); + JSObject *cloned_headers = + Headers::create(cx, headers_val, host_api::HttpHeadersGuard::Request); + if (!cloned_headers) { + return false; + } + cloned_headers_val.set(ObjectValue(*cloned_headers)); + } else if (RequestOrResponse::handle(self)) { + auto handle = + RequestOrResponse::headers_handle_clone(cx, self, host_api::HttpHeadersGuard::Request); + JSObject *cloned_headers = + Headers::create(cx, handle.release(), host_api::HttpHeadersGuard::Request); + if (!cloned_headers) { + return false; + } + cloned_headers_val.set(ObjectValue(*cloned_headers)); + } + + SetReservedSlot(new_request, static_cast(Slots::Headers), cloned_headers_val); + Value url_val = GetReservedSlot(self, static_cast(Slots::URL)); + SetReservedSlot(new_request, static_cast(Slots::URL), url_val); + Value method_val = JS::StringValue(method(self)); + ENGINE->dump_value(method_val, stderr); + SetReservedSlot(new_request, static_cast(Slots::Method), method_val); + + // clone operation step 2. + // If request’s body is non-null, set newRequest’s body to the result of cloning request’s body. + RootedObject new_body(cx); + auto has_body = RequestOrResponse::has_body(self); + if (!has_body) { + args.rval().setObject(*new_request); + return true; + } -// JS::SetReservedSlot(requestInstance, static_cast(Slots::Method), -// JS::StringValue(method)); + // Here we get the current request's body stream and call ReadableStream.prototype.tee to + // get two streams for the same content. + // One of these is then used to replace the current request's body, the other is used as + // the body of the clone. + JS::RootedObject body_stream(cx, RequestOrResponse::body_stream(self)); + if (!body_stream) { + body_stream = RequestOrResponse::create_body_stream(cx, self); + if (!body_stream) { + return false; + } + } -// auto *request_handle_res = new host_api::HttpOutgoingRequest() -// if (auto *err = request_handle_res.to_err()) { -// HANDLE_ERROR(cx, *err); -// return false; -// } + if (RequestOrResponse::body_unusable(cx, body_stream)) { + return api::throw_error(cx, FetchErrors::BodyStreamUnusable); + } -// auto request_handle = request_handle_res.unwrap(); + RootedObject self_body(cx); + if (!ReadableStreamTee(cx, body_stream, &self_body, &new_body)) { + return false; + } -// JS::RootedObject requestInstance(cx, Request::create_instance(cx)); -// JS::SetReservedSlot(requestInstance, static_cast(Slots::Request), -// JS::Int32Value(request_handle.handle)); -// JS::SetReservedSlot(requestInstance, static_cast(Slots::BodyUsed), -// JS::FalseValue()); -// JS::SetReservedSlot( -// requestInstance, static_cast(Slots::URL), -// JS::GetReservedSlot(self, static_cast(Slots::URL))); -// JS::SetReservedSlot(requestInstance, static_cast(Slots::HasBody), -// JS::BooleanValue(hasBody)); + SetReservedSlot(self, static_cast(Slots::BodyStream), ObjectValue(*self_body)); + SetReservedSlot(new_request, static_cast(Slots::BodyStream), ObjectValue(*new_body)); + SetReservedSlot(new_request, static_cast(Slots::HasBody), JS::BooleanValue(true)); -// args.rval().setObject(*requestInstance); -// return true; -// } + args.rval().setObject(*new_request); + return true; +} const JSFunctionSpec Request::static_methods[] = { JS_FS_END, @@ -1333,7 +1287,7 @@ const JSFunctionSpec Request::methods[] = { JSPROP_ENUMERATE), JS_FN("json", Request::bodyAll, 0, JSPROP_ENUMERATE), JS_FN("text", Request::bodyAll, 0, JSPROP_ENUMERATE), - // JS_FN("clone", Request::clone, 0, JSPROP_ENUMERATE), + JS_FN("clone", Request::clone, 0, JSPROP_ENUMERATE), JS_FS_END, }; @@ -1372,7 +1326,7 @@ void Request::init_slots(JSObject *requestInstance) { * Create a new Request object, roughly according to * https://fetch.spec.whatwg.org/#dom-request * - * "Roughly" because not all aspects of Request handling make sense in C@E. + * "Roughly" because not all aspects of Request handling make sense in StarlingMonkey. * The places where we deviate from the spec are called out inline. */ bool Request::initialize(JSContext *cx, JS::HandleObject request, JS::HandleValue input, @@ -1416,10 +1370,7 @@ bool Request::initialize(JSContext *cx, JS::HandleObject request, JS::HandleValu url_str = RequestOrResponse::url(input_request).toString(); // method: `request`’s method. - method_str = Request::method(cx, input_request); - if (!method_str) { - return false; - } + method_str = Request::method(input_request); // referrer: `request`’s referrer. // TODO: evaluate whether we want to implement support for setting the @@ -1430,7 +1381,14 @@ bool Request::initialize(JSContext *cx, JS::HandleObject request, JS::HandleValu // header list: A copy of `request`’s header list. // Note: copying the headers is postponed, see step 32 below. - input_headers = RequestOrResponse::maybe_headers(input_request); + // Note: we're forcing reification of the input request's headers here. That is suboptimal, + // because we might end up not using them. Additionally, if the headers are represented + // internally as a handle (e.g. because the input is an incoming request), we would in + // principle not need to ever reify it just to get a clone. + // Applying these optimizations is somewhat complex though, so for now we're not doing so. + if (!(input_headers = RequestOrResponse::headers(cx, input_request))) { + return false; + } // The following properties aren't applicable: // unsafe-request flag: Set. diff --git a/builtins/web/fetch/request-response.h b/builtins/web/fetch/request-response.h index c98fa5fa..a3d5bfaa 100644 --- a/builtins/web/fetch/request-response.h +++ b/builtins/web/fetch/request-response.h @@ -142,7 +142,7 @@ class Request final : public BuiltinImpl { }; static JSObject *response_promise(JSObject *obj); - static JSString *method(JSContext *cx, JS::HandleObject obj); + static JSString *method(JS::HandleObject obj); static host_api::HttpRequest *request_handle(JSObject *obj); static host_api::HttpOutgoingRequest *outgoing_handle(JSObject *obj); static host_api::HttpIncomingRequest *incoming_handle(JSObject *obj); diff --git a/tests/integration/fetch/fetch.js b/tests/integration/fetch/fetch.js new file mode 100644 index 00000000..56c6b58e --- /dev/null +++ b/tests/integration/fetch/fetch.js @@ -0,0 +1,56 @@ +import { serveTest } from '../test-server.js'; +import { strictEqual, deepStrictEqual, throws } from '../assert.js'; + +export const handler = serveTest(async (t) => { + await t.test('headers-non-ascii-latin1-field-value', async () => { + const response = await fetch("https://http-me.glitch.me/meow?header=cat:é"); + strictEqual(response.headers.get('cat'), "é"); + }); + + t.test('request-clone-bad-calls', () => { + throws(() => new Request.prototype.clone(), TypeError); + throws(() => new Request.prototype.clone.call(undefined), TypeError); + }); + + await t.test('request-clone-valid', async () => { + { + const request = new Request('https://www.fastly.com', { + headers: { + hello: 'world' + }, + body: 'te', + method: 'post' + }); + const newRequest = request.clone(); + strictEqual(newRequest instanceof Request, true, 'newRequest instanceof Request'); + strictEqual(newRequest.method, request.method, 'newRequest.method'); + strictEqual(newRequest.url, request.url, 'newRequest.url'); + deepStrictEqual([...newRequest.headers], [...request.headers], 'newRequest.headers'); + strictEqual(request.bodyUsed, false, 'request.bodyUsed'); + strictEqual(newRequest.bodyUsed, false, 'newRequest.bodyUsed'); + strictEqual(newRequest.body instanceof ReadableStream, true, 'newRequest.body instanceof ReadableStream'); + } + + { + const request = new Request('https://www.fastly.com', { + method: 'get' + }) + const newRequest = request.clone(); + + strictEqual(newRequest.bodyUsed, false, 'newRequest.bodyUsed'); + strictEqual(newRequest.body, null, 'newRequest.body'); + } + }); + + await t.test('request-clone-invalid', async () => { + const request = new Request('https://www.fastly.com', { + headers: { + hello: 'world' + }, + body: 'te', + method: 'post' + }); + await request.text(); + throws(() => request.clone()); + }); +}); diff --git a/tests/integration/handlers.js b/tests/integration/handlers.js index fbc0bfdf..f920320e 100644 --- a/tests/integration/handlers.js +++ b/tests/integration/handlers.js @@ -2,4 +2,4 @@ export { handler as btoa } from './btoa/btoa.js'; export { handler as performance } from './performance/performance.js'; export { handler as crypto } from './crypto/crypto.js'; export { handler as timers } from './timers/timers.js'; -export { handler as headers } from './headers/headers.js'; +export { handler as fetch } from './fetch/fetch.js'; diff --git a/tests/integration/headers/headers.js b/tests/integration/headers/headers.js deleted file mode 100644 index bccc8293..00000000 --- a/tests/integration/headers/headers.js +++ /dev/null @@ -1,9 +0,0 @@ -import { serveTest } from '../test-server.js'; -import { strictEqual } from '../assert.js'; - -export const handler = serveTest(async (t) => { - await t.test('non-ascii-latin1-field-value', async () => { - const response = await fetch("https://http-me.glitch.me/meow?header=cat:é"); - strictEqual(response.headers.get('cat'), "é"); - }); -}); diff --git a/tests/tests.cmake b/tests/tests.cmake index 477e5070..452dc764 100644 --- a/tests/tests.cmake +++ b/tests/tests.cmake @@ -38,5 +38,5 @@ test_e2e(tla-runtime-resolve) test_integration(btoa) test_integration(performance) test_integration(crypto) -test_integration(headers) +test_integration(fetch) test_integration(timers) diff --git a/tests/wpt-harness/expectations/fetch/api/request/request-structure.any.js.json b/tests/wpt-harness/expectations/fetch/api/request/request-structure.any.js.json index 58602918..be010b84 100644 --- a/tests/wpt-harness/expectations/fetch/api/request/request-structure.any.js.json +++ b/tests/wpt-harness/expectations/fetch/api/request/request-structure.any.js.json @@ -1,6 +1,6 @@ { "Request has clone method": { - "status": "FAIL" + "status": "PASS" }, "Request has arrayBuffer method": { "status": "PASS" @@ -71,4 +71,4 @@ "Request does not expose blocking attribute": { "status": "PASS" } -} \ No newline at end of file +} diff --git a/tests/wpt-harness/post-harness.js b/tests/wpt-harness/post-harness.js index b1dff363..936acf1e 100644 --- a/tests/wpt-harness/post-harness.js +++ b/tests/wpt-harness/post-harness.js @@ -11,6 +11,7 @@ setup({ explicit_done: true }); async function handleRequest(event) { let url = new URL(event.request.url); + console.log(`running test ${url.pathname}`); let input = `http://web-platform.test:8000${url.pathname}${url.search}`; globalThis.baseURL = new URL(input); globalThis.location = baseURL; From f7e2745929abaf09926fee4a0605a1d83bcbbe9a Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Wed, 31 Jul 2024 16:36:01 -0700 Subject: [PATCH 06/12] Support weval-based ahead-of-time compilation of JavaScript. (#91) * Support weval-based ahead-of-time compilation of JavaScript. When the `WEVAL` option is turned on (`cmake -DWEVAL=ON`), this PR adds: - Integration to the CMake machinery to fetch a PBL+weval-ified version of SpiderMonkey artifacts; - Likewise, to fetch weval binaries; - A rule to pre-build a compilation cache of IC bodies specific to the StarlingMonkey build, so weval can use this cache and spend time only on user-provided code on first run; - Integration in `componentize.sh`. When built with: ``` $ mkdir build/; cd build/ $ cmake .. -DCMAKE_BUILD_TYPE=Release -DUSE_WASM_OPT=OFF -DWEVAL=ON $ make ``` We can then do: ``` $ build/componentize.sh file.js --aot -o file.wasm $ wasmtime serve -S cli=y file.wasm ``` Using the Richards Octane benchmark adapted slightly with a `main()` for the HTTP server world [1], I get the following results: ``` % build/componentize.sh richards.js --aot -o weval.wasm Componentizing richards.js into weval.wasm [ verbose weval progress output ] % wasmtime serve -S cli=y weval.wasm Serving HTTP on http://0.0.0.0:8080/ stdout [0] :: Log: Richards: 676 stdout [0] :: Log: ---- stdout [0] :: Log: Score (version 9): 676 % wasmtime serve -S cli=y base.wasm Serving HTTP on http://0.0.0.0:8080/ stdout [0] :: Log: Richards: 189 stdout [0] :: Log: ---- stdout [0] :: Log: Score (version 9): 189 ``` [1]: https://gist.github.com/cfallin/4b18da12413e93f7e88568a92d09e4b7 * support weval testing * add ci workflow for Weval test suite * Update weval, resolving two test failures. This commit updates to weval v0.2.7 which has no output by default, allowing the expected-failure tests checking syntax error messages to pass; we now pass 9/10 integration tests. It also updates the flags on `componentize.sh`: a `--verbose` flag to allow the user to see weval progress messages (perhaps useful if the build is taking a long time, or to see the stats on function specializations); and removal of the `--aot` flag, because there is only one valid setting for a build configuration and it is not baked into the shell script automatically. * Update SpiderMonkey version to include two fixes (bytecodealliance/spidermonkey-wasi-embedding#19). --------- Co-authored-by: Guy Bedford --- .github/workflows/main.yml | 28 ++++++++++++++++++++++++++ CMakeLists.txt | 22 +++++++++++++++++++- cmake/spidermonkey.cmake | 10 ++++++++- cmake/weval.cmake | 6 ++++++ componentize.sh | 27 +++++++++++++++++++++---- tests/test.sh | 3 ++- tests/tests.cmake | 1 + tests/wpt-harness/build-wpt-runtime.sh | 4 +++- tests/wpt-harness/wpt.cmake | 9 ++++++++- 9 files changed, 101 insertions(+), 9 deletions(-) create mode 100644 cmake/weval.cmake diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index f923475e..d883aff5 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -44,6 +44,34 @@ jobs: run: | CTEST_OUTPUT_ON_FAILURE=1 ctest --test-dir cmake-build-debug -j4 + test-weval: + name: Test Weval + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v2 + + - name: Install Rust 1.77.1 + run: | + rustup toolchain install 1.77.1 + rustup target add wasm32-wasi --toolchain 1.77.1 + + - uses: actions/setup-node@v2 + with: + node-version: 'lts/*' + + - name: Build StarlingMonkey + run: | + cmake -S . -B cmake-build-weval -DCMAKE_BUILD_TYPE=Release -DUSE_WASM_OPT=OFF -DWEVAL=ON + cmake --build cmake-build-weval --parallel 4 --target all integration-test-server + + - name: StarlingMonkey E2E & Integration Tests + run: | + CTEST_OUTPUT_ON_FAILURE=1 ctest --test-dir cmake-build-weval -j4 + wpt: name: Web Platform Tests strategy: diff --git a/CMakeLists.txt b/CMakeLists.txt index 4509545d..7df68108 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -28,6 +28,7 @@ include("init-corrosion") include("wasm-tools") include("binaryen") include("wizer") +include("weval") include("wasmtime") include("fmt") @@ -90,9 +91,28 @@ endif() target_link_libraries(starling.wasm PRIVATE host_api extension_api builtins spidermonkey rust-url) +# build a compilation cache of ICs +if(WEVAL) + include("weval") + file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/null.js "function main() {}") + add_custom_target( + starling-ics.wevalcache + ALL + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} + COMMAND rm -f starling-ics.wevalcache + COMMAND echo ./null.js | ${WEVAL_BIN} weval --dir . --show-stats --cache starling-ics.wevalcache -w -i starling.wasm -o /dev/null + DEPENDS starling.wasm + VERBATIM + ) + set(WEVAL_CACHE_FILE "starling-ics.wevalcache") + set(AOT 1) +else() + set(AOT 0) +endif() + set(RUNTIME_FILE "starling.wasm") set(ADAPTER_FILE "preview1-adapter.wasm") -configure_file("componentize.sh" "${CMAKE_CURRENT_BINARY_DIR}/componentize.sh" COPYONLY) +configure_file("componentize.sh" "${CMAKE_CURRENT_BINARY_DIR}/componentize.sh" @ONLY) configure_file(${ADAPTER} "${CMAKE_CURRENT_BINARY_DIR}/${ADAPTER_FILE}" COPYONLY) configure_file(spin.toml spin.toml COPYONLY) diff --git a/cmake/spidermonkey.cmake b/cmake/spidermonkey.cmake index 5a5dee84..d3aceb32 100644 --- a/cmake/spidermonkey.cmake +++ b/cmake/spidermonkey.cmake @@ -1,10 +1,18 @@ -set(SM_REV ffbf1c4641440e74174199def6558c710b3ac323) +set(SM_REV 99d3bca2aa1f477d3f7f3dcbad2ef2218c14f41b) if (CMAKE_BUILD_TYPE STREQUAL "Debug") set(SM_BUILD_TYPE debug) else() set(SM_BUILD_TYPE release) endif() +set(SM_BUILD_TYPE_DASH ${SM_BUILD_TYPE}) + +option(WEVAL "Build with a SpiderMonkey variant that supports weval-based AOT compilation" OFF) + +if (WEVAL) + set(SM_BUILD_TYPE_DASH "${SM_BUILD_TYPE}-weval") + set(SM_BUILD_TYPE "${SM_BUILD_TYPE}_weval") +endif() # If the developer has specified an alternate local set of SpiderMonkey # artifacts, use them. This allows for local/in-tree development without diff --git a/cmake/weval.cmake b/cmake/weval.cmake new file mode 100644 index 00000000..aef6360e --- /dev/null +++ b/cmake/weval.cmake @@ -0,0 +1,6 @@ +set(WEVAL_VERSION v0.2.7) + +set(WEVAL_URL https://github.com/cfallin/weval/releases/download/${WEVAL_VERSION}/weval-${WEVAL_VERSION}-${HOST_ARCH}-${HOST_OS}.tar.xz) +CPMAddPackage(NAME weval URL ${WEVAL_URL} DOWNLOAD_ONLY TRUE) +set(WEVAL_DIR ${CPM_PACKAGE_weval_SOURCE_DIR}) +set(WEVAL_BIN ${WEVAL_DIR}/weval) diff --git a/componentize.sh b/componentize.sh index 0d8419e7..017eb863 100755 --- a/componentize.sh +++ b/componentize.sh @@ -4,9 +4,11 @@ wizer="${WIZER:-wizer}" wasm_tools="${WASM_TOOLS:-wasm-tools}" +weval="${WEVAL:-@WEVAL_BIN@}" +aot=@AOT@ usage() { - echo "Usage: $(basename "$0") [input.js] [-o output.wasm]" + echo "Usage: $(basename "$0") [input.js] [--verbose] [-o output.wasm]" echo " Providing an input file but no output uses the input base name with a .wasm extension" echo " Providing an output file but no input creates a component without running any top-level script" exit 1 @@ -19,6 +21,7 @@ fi IN_FILE="" OUT_FILE="" +VERBOSE=0 while [ $# -gt 0 ] do @@ -27,6 +30,10 @@ do OUT_FILE="$2" shift 2 ;; + --verbose) + VERBOSE=1 + shift + ;; *) if [ -n "$IN_FILE" ] && [ -z "$OUT_FILE" ] && [ $# -eq 1 ] then @@ -62,8 +69,20 @@ else echo "Creating runtime-script component $OUT_FILE" fi +if [[ $aot -ne 0 ]]; then + WEVAL_VERBOSE="" + if [[ $VERBOSE -ne 0 ]]; then + WEVAL_VERBOSE="--verbose --show-stats" + fi -echo "$IN_FILE" | WASMTIME_BACKTRACE_DETAILS=1 $wizer --allow-wasi --wasm-bulk-memory true \ - --inherit-stdio true --inherit-env true $PREOPEN_DIR -o "$OUT_FILE" \ - -- "$(dirname "$0")/starling.wasm" + echo "$IN_FILE" | WASMTIME_BACKTRACE_DETAILS=1 $weval weval -w $PREOPEN_DIR \ + --cache-ro "$(dirname "$0")/starling-ics.wevalcache" \ + $WEVAL_VERBOSE \ + -o "$OUT_FILE" \ + -i "$(dirname "$0")/starling.wasm" +else + echo "$IN_FILE" | WASMTIME_BACKTRACE_DETAILS=1 $wizer --allow-wasi --wasm-bulk-memory true \ + --inherit-stdio true --inherit-env true $PREOPEN_DIR -o "$OUT_FILE" \ + -- "$(dirname "$0")/starling.wasm" +fi $wasm_tools component new -v --adapt "wasi_snapshot_preview1=$(dirname "$0")/preview1-adapter.wasm" --output "$OUT_FILE" "$OUT_FILE" diff --git a/tests/test.sh b/tests/test.sh index 426b6c86..cc8340cd 100755 --- a/tests/test.sh +++ b/tests/test.sh @@ -5,6 +5,7 @@ test_runtime="$1" test_component="${3:-}" test_name="$(basename $test_dir)" test_serve_path="${4:-}" +componentize_flags="${COMPONENTIZE_FLAGS:-}" wasmtime="${WASMTIME:-wasmtime}" @@ -24,7 +25,7 @@ if [ -z "$test_component" ]; then # Run Wizer set +e - "$test_runtime/componentize.sh" "$test_dir/$test_name.js" "$test_component" 1> "$stdout_log" 2> "$stderr_log" + "$test_runtime/componentize.sh" $componentize_flags "$test_dir/$test_name.js" "$test_component" 1> "$stdout_log" 2> "$stderr_log" wizer_result=$? set -e diff --git a/tests/tests.cmake b/tests/tests.cmake index 452dc764..a863c68e 100644 --- a/tests/tests.cmake +++ b/tests/tests.cmake @@ -3,6 +3,7 @@ enable_testing() find_program(BASH_PROGRAM bash) include("wizer") include("wasmtime") +include("weval") function(test_e2e TEST_NAME) get_target_property(RUNTIME_DIR starling.wasm BINARY_DIR) diff --git a/tests/wpt-harness/build-wpt-runtime.sh b/tests/wpt-harness/build-wpt-runtime.sh index 9f147407..b255451f 100755 --- a/tests/wpt-harness/build-wpt-runtime.sh +++ b/tests/wpt-harness/build-wpt-runtime.sh @@ -2,6 +2,8 @@ set -euo pipefail +componentize_flags="${COMPONENTIZE_FLAGS:-}" + if [ -z "${WPT_ROOT:-}" ]; then echo "The WPT_ROOT environment variable is not set" exit 1 @@ -16,4 +18,4 @@ inputs=( ) cat "${inputs[@]}" > wpt-test-runner.js -./componentize.sh wpt-test-runner.js wpt-runtime.wasm +./componentize.sh $componentize_flags wpt-test-runner.js wpt-runtime.wasm diff --git a/tests/wpt-harness/wpt.cmake b/tests/wpt-harness/wpt.cmake index 96a5279f..26a182c1 100644 --- a/tests/wpt-harness/wpt.cmake +++ b/tests/wpt-harness/wpt.cmake @@ -1,6 +1,13 @@ enable_testing() include("wasmtime") +include("weval") + +if(WEVAL) + set(COMPONENTIZE_FLAGS "--aot") +else() + set(COMPONENTIZE_FLAGS "") +endif() if(DEFINED ENV{WPT_ROOT}) set(WPT_ROOT ENV{WPT_ROOT}) @@ -21,7 +28,7 @@ add_builtin(wpt_support add_custom_command( OUTPUT wpt-runtime.wasm WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} - COMMAND ${CMAKE_COMMAND} -E env PATH=${WASM_TOOLS_DIR}:${WIZER_DIR}:$ENV{PATH} WPT_ROOT=${WPT_ROOT} ${CMAKE_CURRENT_SOURCE_DIR}/tests/wpt-harness/build-wpt-runtime.sh + COMMAND ${CMAKE_COMMAND} -E env PATH=${WASM_TOOLS_DIR}:${WIZER_DIR}:$ENV{PATH} env "COMPONENTIZE_FLAGS=${COMPONENTIZE_FLAGS}" WPT_ROOT=${WPT_ROOT} ${CMAKE_CURRENT_SOURCE_DIR}/tests/wpt-harness/build-wpt-runtime.sh DEPENDS starling.wasm componentize.sh tests/wpt-harness/build-wpt-runtime.sh tests/wpt-harness/pre-harness.js tests/wpt-harness/post-harness.js VERBATIM ) From 0bd457ecf0153b1cbd6dd74e97c6e90e651ec226 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Fri, 2 Aug 2024 10:08:10 -0700 Subject: [PATCH 07/12] Update upstream SpiderMonkey for a PBL+weval fix. (#104) Update upstream SpiderMonkey and weval for a PBL+weval fix. Pulls in bytecodealliance/gecko-dev#56. --- cmake/spidermonkey.cmake | 2 +- cmake/weval.cmake | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/spidermonkey.cmake b/cmake/spidermonkey.cmake index d3aceb32..ce69c94a 100644 --- a/cmake/spidermonkey.cmake +++ b/cmake/spidermonkey.cmake @@ -1,4 +1,4 @@ -set(SM_REV 99d3bca2aa1f477d3f7f3dcbad2ef2218c14f41b) +set(SM_REV dcae16d570211715103fe9604899a99f1df8a0e3) if (CMAKE_BUILD_TYPE STREQUAL "Debug") set(SM_BUILD_TYPE debug) diff --git a/cmake/weval.cmake b/cmake/weval.cmake index aef6360e..671d8f3e 100644 --- a/cmake/weval.cmake +++ b/cmake/weval.cmake @@ -1,4 +1,4 @@ -set(WEVAL_VERSION v0.2.7) +set(WEVAL_VERSION v0.2.9) set(WEVAL_URL https://github.com/cfallin/weval/releases/download/${WEVAL_VERSION}/weval-${WEVAL_VERSION}-${HOST_ARCH}-${HOST_OS}.tar.xz) CPMAddPackage(NAME weval URL ${WEVAL_URL} DOWNLOAD_ONLY TRUE) From c83e718d5aa9db921ec8da9be36bb65a0ac655d4 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Tue, 6 Aug 2024 16:30:13 -0700 Subject: [PATCH 08/12] SpiderMonkey: update to pull in PBL+weval fix. (#106) Updates to commit hash of bytecodealliance/spidermonkey-wasi-embedding#21, which pulls in bytecodealliance/gecko-dev#57. --- cmake/spidermonkey.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/spidermonkey.cmake b/cmake/spidermonkey.cmake index ce69c94a..c61911a1 100644 --- a/cmake/spidermonkey.cmake +++ b/cmake/spidermonkey.cmake @@ -1,4 +1,4 @@ -set(SM_REV dcae16d570211715103fe9604899a99f1df8a0e3) +set(SM_REV 4f4347d5e44ec44f87d8d54d85526a3713c9fb11) if (CMAKE_BUILD_TYPE STREQUAL "Debug") set(SM_BUILD_TYPE debug) From cab7ffb32c188c430281726de574a0fcf40b7dbd Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Thu, 8 Aug 2024 04:21:21 +0200 Subject: [PATCH 09/12] ci: fix WPT failures for missing tests (#107) * ci: fix WPT failures for missing tests * fixup * fixup * exclude currently missing tests * fixup backtrace logging --- tests/wpt-harness/run-wpt.mjs | 9 +++++---- tests/wpt-harness/tests.json | 8 ++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/tests/wpt-harness/run-wpt.mjs b/tests/wpt-harness/run-wpt.mjs index 5ae3a3c2..c1908a96 100644 --- a/tests/wpt-harness/run-wpt.mjs +++ b/tests/wpt-harness/run-wpt.mjs @@ -222,8 +222,8 @@ async function run() { if (config.tests.updateExpectations) { console.log(`Expectations updated: ${expectationsUpdated}`); shutdown(); - } else if (stats.unexpectedFail + stats.unexpectedPass != 0 || unexpectedFailure) { - shutdown('Unexpected failures or passes. Verify that these new results match your expectations and run with --update-expectations to update the recorded expectations.'); + } else if (stats.unexpectedFail + stats.unexpectedPass + stats.missing != 0 || unexpectedFailure) { + shutdown('Unexpected failures, passes or missing results. Verify that these new results match your expectations and run with --update-expectations to update the recorded expectations.'); } else { shutdown(); } @@ -331,7 +331,7 @@ async function startWasmtime(runtime, addr, logLevel) { }); let backtrace = ""; - let backtrace_re = /Error \{\s+context: "error while executing at wasm backtrace:(.+?)",\s+source: "(.+?)"/s; + let backtrace_re = /Error \{\s+context: "error while executing at wasm backtrace:(.+?)",\s+source: (.+?)/s; wasmtime.stderr.on("data", data => { if (backtrace.length > 0 || data.includes("error while executing at wasm backtrace:")) { backtrace += data; @@ -489,7 +489,8 @@ async function runTests(testPaths, wasmtime, resultCallback, errorCallback) { await resultCallback(path, results, stats); } catch (e) { if (!results) { - e = new Error(`Parsing test results as JSON failed. Output was:\n ${body}`); + e = new Error(`\nMISSING TEST RESULTS: ${path}\nParsing test results as JSON failed. Output was:\n ${body}`); + totalStats.missing += Math.max(Object.keys(expectations).length, 1); } if (config.logLevel >= LogLevel.Verbose) { console.log(`Error running file ${path}: ${e.message}, stack:\n${e.stack}`); diff --git a/tests/wpt-harness/tests.json b/tests/wpt-harness/tests.json index 5091d74b..0253e388 100644 --- a/tests/wpt-harness/tests.json +++ b/tests/wpt-harness/tests.json @@ -54,7 +54,7 @@ "fetch/api/abort/request.any.js", "fetch/api/basic/accept-header.any.js", "fetch/api/basic/conditional-get.any.js", - "fetch/api/basic/error-after-response.any.js", + "SKIP fetch/api/basic/error-after-response.any.js", "fetch/api/basic/header-value-combining.any.js", "fetch/api/basic/header-value-null-byte.any.js", "fetch/api/basic/historical.any.js", @@ -73,7 +73,7 @@ "SKIP fetch/api/basic/request-private-network-headers.tentative.any.js", "fetch/api/basic/request-referrer.any.js", "fetch/api/basic/request-upload.any.js", - "fetch/api/basic/request-upload.h2.any.js", + "SKIP fetch/api/basic/request-upload.h2.any.js", "fetch/api/basic/response-null-body.any.js", "fetch/api/basic/response-url.sub.any.js", "fetch/api/basic/scheme-about.any.js", @@ -98,7 +98,7 @@ "fetch/api/headers/headers-normalize.any.js", "fetch/api/headers/headers-record.any.js", "fetch/api/headers/headers-structure.any.js", - "fetch/api/idlharness.any.js", + "SKIP fetch/api/idlharness.any.js", "fetch/api/redirect/redirect-back-to-original-origin.any.js", "fetch/api/redirect/redirect-count.any.js", "fetch/api/redirect/redirect-empty-location.any.js", @@ -235,7 +235,7 @@ "streams/writable-streams/start.any.js", "streams/writable-streams/write.any.js", "url/historical.any.js", - "url/idlharness.any.js", + "SKIP url/idlharness.any.js", "SLOW url/url-constructor.any.js", "url/url-origin.any.js", "url/url-searchparams.any.js", From 10e51063783ed6868b5422b4c95f418ff4289705 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 12 Aug 2024 18:24:53 +0200 Subject: [PATCH 10/12] fix: headers WPT conformance (#108) --- builtins/web/fetch/fetch-api.cpp | 12 +- builtins/web/fetch/fetch-errors.h | 1 + builtins/web/fetch/fetch_event.cpp | 30 +- builtins/web/fetch/headers.cpp | 1201 +++++++++++------ builtins/web/fetch/headers.h | 122 +- builtins/web/fetch/request-response.cpp | 104 +- builtins/web/fetch/request-response.h | 9 +- builtins/web/url.cpp | 114 +- builtins/web/url.h | 1 - componentize.sh | 15 +- crates/rust-url/rust-url.h | 2 + host-apis/wasi-0.2.0/host_api.cpp | 190 +-- include/builtin.h | 54 + include/host_api.h | 48 +- runtime/decode.cpp | 8 +- runtime/decode.h | 1 + runtime/encode.cpp | 37 + runtime/encode.h | 4 + runtime/sequence.hpp | 29 +- tests/{integration => }/assert.js | 0 tests/e2e/headers/expect_serve_body.txt | 1 + tests/e2e/headers/expect_serve_headers.txt | 8 + tests/e2e/headers/expect_serve_stderr.txt | 0 tests/e2e/headers/headers.js | 42 + tests/integration/btoa/btoa.js | 2 +- tests/integration/crypto/crypto.js | 2 +- tests/integration/fetch/fetch.js | 2 +- tests/integration/performance/performance.js | 2 +- tests/integration/test-server.js | 2 +- tests/integration/tests/performance.js | 285 ---- tests/integration/timers/timers.js | 2 +- tests/test.sh | 15 +- tests/tests.cmake | 15 +- .../request-headers-nonascii.any.js.json | 2 +- .../api/headers/header-setcookie.any.js.json | 34 +- .../api/headers/headers-basic.any.js.json | 18 +- .../api/headers/headers-combine.any.js.json | 2 +- .../api/headers/headers-errors.any.js.json | 6 +- .../api/headers/headers-record.any.js.json | 14 +- .../api/request/request-headers.any.js.json | 2 +- .../response-headers-guard.any.js.json | 2 +- tests/wpt-harness/tests.json | 2 +- tests/wpt-harness/wpt.cmake | 2 +- 43 files changed, 1280 insertions(+), 1164 deletions(-) rename tests/{integration => }/assert.js (100%) create mode 100644 tests/e2e/headers/expect_serve_body.txt create mode 100644 tests/e2e/headers/expect_serve_headers.txt create mode 100644 tests/e2e/headers/expect_serve_stderr.txt create mode 100644 tests/e2e/headers/headers.js delete mode 100644 tests/integration/tests/performance.js diff --git a/builtins/web/fetch/fetch-api.cpp b/builtins/web/fetch/fetch-api.cpp index 85bf1b0d..fac7c7fa 100644 --- a/builtins/web/fetch/fetch-api.cpp +++ b/builtins/web/fetch/fetch-api.cpp @@ -7,6 +7,8 @@ #include +using builtins::web::fetch::Headers; + namespace builtins::web::fetch { static api::Engine *ENGINE; @@ -31,7 +33,8 @@ bool fetch(JSContext *cx, unsigned argc, Value *vp) { return ReturnPromiseRejectedWithPendingError(cx, args); } - if (!Request::initialize(cx, request_obj, args[0], args.get(1))) { + if (!Request::initialize(cx, request_obj, args[0], args.get(1), + Headers::HeadersGuard::Immutable)) { return ReturnPromiseRejectedWithPendingError(cx, args); } @@ -47,14 +50,13 @@ bool fetch(JSContext *cx, unsigned argc, Value *vp) { return ReturnPromiseRejectedWithPendingError(cx, args); } - unique_ptr headers = RequestOrResponse::headers_handle_clone(cx, - request_obj, host_api::HttpHeadersGuard::Request); + unique_ptr headers = + RequestOrResponse::headers_handle_clone(cx, request_obj); if (!headers) { return ReturnPromiseRejectedWithPendingError(cx, args); } - auto request = host_api::HttpOutgoingRequest::make(method, std::move(url), - std::move(headers)); + auto request = host_api::HttpOutgoingRequest::make(method, std::move(url), std::move(headers)); MOZ_RELEASE_ASSERT(request); JS_SetReservedSlot(request_obj, static_cast(Request::Slots::Request), PrivateValue(request)); diff --git a/builtins/web/fetch/fetch-errors.h b/builtins/web/fetch/fetch-errors.h index 8e0d9087..ba54c1e6 100644 --- a/builtins/web/fetch/fetch-errors.h +++ b/builtins/web/fetch/fetch-errors.h @@ -19,6 +19,7 @@ DEF_ERR(EmptyHeaderName, JSEXN_TYPEERR, "{0}: Header name can't be empty", 1) DEF_ERR(InvalidHeaderName, JSEXN_TYPEERR, "{0}: Invalid header name \"{1}\"", 2) DEF_ERR(InvalidHeaderValue, JSEXN_TYPEERR, "{0}: Invalid header value \"{1}\"", 2) DEF_ERR(HeadersCloningFailed, JSEXN_ERR, "Failed to clone headers", 0) +DEF_ERR(HeadersImmutable, JSEXN_TYPEERR, "{0}: Headers are immutable", 1) }; // namespace FetchErrors #endif // FETCH_ERRORS_H diff --git a/builtins/web/fetch/fetch_event.cpp b/builtins/web/fetch/fetch_event.cpp index 1448f847..e1fc2514 100644 --- a/builtins/web/fetch/fetch_event.cpp +++ b/builtins/web/fetch/fetch_event.cpp @@ -167,8 +167,7 @@ bool send_response(host_api::HttpOutgoingResponse *response, JS::HandleObject se bool start_response(JSContext *cx, JS::HandleObject response_obj) { auto status = Response::status(response_obj); - auto headers = RequestOrResponse::headers_handle_clone(cx, response_obj, - host_api::HttpHeadersGuard::Response); + auto headers = RequestOrResponse::headers_handle_clone(cx, response_obj); if (!headers) { return false; } @@ -252,16 +251,15 @@ bool FetchEvent::respondWith(JSContext *cx, unsigned argc, JS::Value *vp) { // Step 2 if (!is_dispatching(self)) { - return dom_exception::DOMException::raise(cx, - "FetchEvent#respondWith must be called synchronously from within a FetchEvent handler", - "InvalidStateError"); + return dom_exception::DOMException::raise( + cx, "FetchEvent#respondWith must be called synchronously from within a FetchEvent handler", + "InvalidStateError"); } // Step 3 if (state(self) != State::unhandled) { - return dom_exception::DOMException::raise(cx, - "FetchEvent#respondWith can't be called twice on the same event", - "InvalidStateError"); + return dom_exception::DOMException::raise( + cx, "FetchEvent#respondWith can't be called twice on the same event", "InvalidStateError"); } // Step 4 @@ -293,7 +291,7 @@ bool FetchEvent::respondWith(JSContext *cx, unsigned argc, JS::Value *vp) { bool FetchEvent::respondWithError(JSContext *cx, JS::HandleObject self) { MOZ_RELEASE_ASSERT(state(self) == State::unhandled || state(self) == State::waitToRespond); - auto headers = std::make_unique(host_api::HttpHeadersGuard::Response); + auto headers = std::make_unique(); auto *response = host_api::HttpOutgoingResponse::make(500, std::move(headers)); auto body_res = response->body(); @@ -331,9 +329,7 @@ bool FetchEvent::waitUntil(JSContext *cx, unsigned argc, JS::Value *vp) { // Step 2 if (!is_active(self)) { return dom_exception::DOMException::raise( - cx, - "waitUntil called on a FetchEvent that isn't active anymore", - "InvalidStateError"); + cx, "waitUntil called on a FetchEvent that isn't active anymore", "InvalidStateError"); } // Steps 3-4 @@ -345,13 +341,9 @@ bool FetchEvent::waitUntil(JSContext *cx, unsigned argc, JS::Value *vp) { return true; } -void FetchEvent::increase_interest() { - inc_pending_promise_count(INSTANCE); -} +void FetchEvent::increase_interest() { inc_pending_promise_count(INSTANCE); } -void FetchEvent::decrease_interest() { - dec_pending_promise_count(INSTANCE); -} +void FetchEvent::decrease_interest() { dec_pending_promise_count(INSTANCE); } const JSFunctionSpec FetchEvent::static_methods[] = { JS_FS_END, @@ -571,7 +563,7 @@ bool eval_request_body(host_api::HttpIncomingRequest *request) { return true; } -bool handle_incoming_request(host_api::HttpIncomingRequest * request) { +bool handle_incoming_request(host_api::HttpIncomingRequest *request) { #ifdef DEBUG fprintf(stderr, "Warning: Using a DEBUG build. Expect things to be SLOW.\n"); #endif diff --git a/builtins/web/fetch/headers.cpp b/builtins/web/fetch/headers.cpp index 064f82fd..915a48e7 100644 --- a/builtins/web/fetch/headers.cpp +++ b/builtins/web/fetch/headers.cpp @@ -1,21 +1,16 @@ #include "headers.h" -#include "encode.h" +#include "builtin.h" #include "decode.h" +#include "encode.h" #include "fetch-errors.h" #include "sequence.hpp" #include "js/Conversions.h" +#include namespace builtins::web::fetch { namespace { -#define HEADERS_ITERATION_METHOD(argc) \ - METHOD_HEADER(argc) \ - JS::RootedObject entries(cx, get_entries(cx, self)); \ - if (!entries) { \ - return false; \ - } - const char VALID_NAME_CHARS[128] = { 0, 0, 0, 0, 0, 0, 0, 0, // 0 0, 0, 0, 0, 0, 0, 0, 0, // 8 @@ -38,19 +33,7 @@ const char VALID_NAME_CHARS[128] = { 1, 1, 1, 0, 1, 0, 1, 0 // 120 }; -#define NORMALIZE_NAME(name, fun_name) \ - bool name_changed; \ - auto name_chars = normalize_header_name(cx, name, &name_changed, fun_name); \ - if (!name_chars) { \ - return false; \ - } - -#define NORMALIZE_VALUE(value, fun_name) \ - bool value_changed; \ - auto value_chars = normalize_header_value(cx, value, &value_changed, fun_name); \ - if (!value_chars.ptr) { \ - return false; \ - } +host_api::HostString set_cookie_str; host_api::HttpHeadersReadOnly *get_handle(JSObject *self) { MOZ_ASSERT(Headers::is_instance(self)); @@ -59,73 +42,21 @@ host_api::HttpHeadersReadOnly *get_handle(JSObject *self) { return static_cast(handle); } -/** - * Validates and normalizes the given header name, by - * - checking for invalid characters - * - converting to lower-case - * - * See - * https://searchfox.org/mozilla-central/rev/9f76a47f4aa935b49754c5608a1c8e72ee358c46/netwerk/protocol/http/nsHttp.cpp#172-215 - * For details on validation. - */ -host_api::HostString normalize_header_name(JSContext *cx, HandleValue name_val, bool* named_changed, - const char *fun_name) { - *named_changed = !name_val.isString(); - JS::RootedString name_str(cx, JS::ToString(cx, name_val)); - if (!name_str) { - return nullptr; - } - - auto name = core::encode(cx, name_str); - if (!name) { - return nullptr; - } - - if (name.len == 0) { - api::throw_error(cx, FetchErrors::EmptyHeaderName, fun_name); - return nullptr; - } - - char *name_chars = name.begin(); - for (size_t i = 0; i < name.len; i++) { - const unsigned char ch = name_chars[i]; - if (ch > 127 || !VALID_NAME_CHARS[ch]) { - api::throw_error(cx, FetchErrors::InvalidHeaderName, fun_name, name_chars); - return nullptr; - } - - if (ch >= 'A' && ch <= 'Z') { - *named_changed = true; - name_chars[i] = ch - 'A' + 'a'; - } - } - - return name; -} - /** * Validates and normalizes the given header value, by * - stripping leading and trailing whitespace * - checking for interior line breaks and `\0` * + * Trim normalization is performed in-place. + * Returns true if the header value is valid. + * * See * https://searchfox.org/mozilla-central/rev/9f76a47f4aa935b49754c5608a1c8e72ee358c46/netwerk/protocol/http/nsHttp.cpp#247-260 * For details on validation. */ -host_api::HostString normalize_header_value(JSContext *cx, HandleValue value_val, - bool* value_changed, const char *fun_name) { - *value_changed = !value_val.isString(); - JS::RootedString value_str(cx, JS::ToString(cx, value_val)); - if (!value_str) { - return nullptr; - } - - auto value = core::encode(cx, value_str); - if (!value.ptr) { - return nullptr; - } - +bool normalize_header_value(host_api::HostString &value) { auto *value_chars = value.begin(); + size_t start = 0; size_t end = value.len; @@ -147,26 +78,94 @@ host_api::HostString normalize_header_value(JSContext *cx, HandleValue value_val } } - if (start != 0 || end != value.len) { - *value_changed = true; - } - for (size_t i = start; i < end; i++) { unsigned char ch = value_chars[i]; if (ch == '\r' || ch == '\n' || ch == '\0') { - api::throw_error(cx, FetchErrors::InvalidHeaderValue, fun_name, value_chars); - return nullptr; + return false; } } - if (*value_changed) { + if (start != 0 || end != value.len) { memmove(value_chars, value_chars + start, end - start); value.len = end - start; } + return true; +} + +host_api::HostString normalize_and_validate_header_value(JSContext *cx, HandleValue value_val, + const char *fun_name) { + host_api::HostString value = core::encode_byte_string(cx, value_val); + if (!value.ptr) { + return value; + } + bool valid = normalize_header_value(value); + if (!valid) { + api::throw_error(cx, FetchErrors::InvalidHeaderValue, fun_name, value.begin()); + return host_api::HostString{}; + } return value; } +static const std::vector *forbidden_request_headers; +static const std::vector *forbidden_response_headers; + +enum class Ordering { Less, Equal, Greater }; + +inline char header_lowercase(const char c) { return c >= 'A' && c <= 'Z' ? c + ('a' - 'A') : c; } + +inline Ordering header_compare(const std::string_view a, const std::string_view b) { + auto it_a = a.begin(); + auto it_b = b.begin(); + while (it_a != a.end() && it_b != b.end()) { + char ca = header_lowercase(*it_a); + char cb = header_lowercase(*it_b); + if (ca < cb) { + return Ordering::Less; + } else if (ca > cb) { + return Ordering::Greater; + } + ++it_a; + ++it_b; + } + if (it_a == a.end()) { + return it_b == b.end() ? Ordering::Equal : Ordering::Less; + } else { + return Ordering::Greater; + } +} + +struct HeaderCompare { + bool operator()(const std::string_view a, const std::string_view b) { + return header_compare(a, b) == Ordering::Less; + } +}; + +class HeadersSortListCompare { + const Headers::HeadersList *headers_; + +public: + HeadersSortListCompare(const Headers::HeadersList *headers) : headers_(headers) {} + + bool operator()(size_t a, size_t b) { + auto header_a = &std::get<0>(headers_->at(a)); + auto header_b = &std::get<0>(headers_->at(b)); + return header_compare(*header_a, *header_b) == Ordering::Less; + } +}; + +class HeadersSortListLookupCompare { + const Headers::HeadersList *headers_; + +public: + HeadersSortListLookupCompare(const Headers::HeadersList *headers) : headers_(headers) {} + + bool operator()(size_t a, string_view b) { + auto header_a = &std::get<0>(headers_->at(a)); + return header_compare(*header_a, b) == Ordering::Less; + } +}; + JS::PersistentRooted comma; bool retrieve_value_for_header_from_handle(JSContext *cx, JS::HandleObject self, @@ -189,7 +188,7 @@ bool retrieve_value_for_header_from_handle(JSContext *cx, JS::HandleObject self, RootedString res_str(cx); RootedString val_str(cx); for (auto &str : values.value()) { - val_str = JS_NewStringCopyN(cx, reinterpret_cast(str.ptr.get()), str.len); + val_str = core::decode_byte_string(cx, str); if (!val_str) { return false; } @@ -212,71 +211,214 @@ bool retrieve_value_for_header_from_handle(JSContext *cx, JS::HandleObject self, return true; } -std::string_view special_chars = "=,;"; - -std::vector splitCookiesString(std::string_view cookiesString) { - std::vector cookiesStrings; - std::size_t currentPosition = 0; // Current position in the string - std::size_t start; // Start position of the current cookie - std::size_t lastComma; // Position of the last comma found - std::size_t nextStart; // Position of the start of the next cookie - - // Iterate over the string and split it into cookies. - while (currentPosition < cookiesString.length()) { - start = currentPosition; - - // Iterate until we find a comma that might be used as a separator. - while ((currentPosition = cookiesString.find_first_of(',', currentPosition)) != - std::string_view::npos) { - // ',' is a cookie separator only if we later have '=', before having ';' or ',' - lastComma = currentPosition; - nextStart = ++currentPosition; - - // Check if the next sequence of characters is a non-special character followed by an equals - // sign. - currentPosition = cookiesString.find_first_of(special_chars, currentPosition); - - // If the current character is an equals sign, we have found a cookie separator. - if (currentPosition != std::string_view::npos && cookiesString.at(currentPosition) == '=') { - // currentPosition is inside the next cookie, so back up and return it. - currentPosition = nextStart; - cookiesStrings.push_back(cookiesString.substr(start, lastComma - start)); - start = currentPosition; - } else { - // The cookie contains ';' or ',' as part of the value - // so we need to keep accumulating characters - currentPosition = lastComma + 1; - } +// for getSetCookie +bool retrieve_values_for_header_from_handle(JSContext *cx, JS::HandleObject self, + const host_api::HostString &name, + JS::MutableHandleObject out_arr) { + auto handle = get_handle(self); + auto ret = handle->get(name); + + if (auto *err = ret.to_err()) { + HANDLE_ERROR(cx, *err); + return false; + } + + auto &values = ret.unwrap(); + if (!values.has_value()) { + return true; + } + + RootedString val_str(cx); + size_t i = 0; + for (auto &str : values.value()) { + val_str = core::decode_byte_string(cx, str); + if (!val_str) + return false; + if (!JS_SetElement(cx, out_arr, i, val_str)) + return false; + i++; + } + + return true; +} + +// Get the combined comma-separated value for a given header +bool retrieve_value_for_header_from_list(JSContext *cx, JS::HandleObject self, size_t *index, + JS::MutableHandleValue value, bool is_iterator) { + MOZ_ASSERT(Headers::is_instance(self)); + Headers::HeadersList *headers_list = Headers::headers_list(self); + const auto entry = Headers::get_index(cx, self, *index); + const host_api::HostString *key = &std::get<0>(*entry); + const host_api::HostString *val = &std::get<1>(*entry); + // check if we need to join with the next value if it is the same key, comma-separated + RootedString str(cx, core::decode_byte_string(cx, *val)); + if (!str) { + return false; + } + // iterator doesn't join set-cookie, only get + if (is_iterator && header_compare(*key, set_cookie_str) == Ordering::Equal) { + value.setString(str); + return true; + } + size_t len = headers_list->size(); + while (*index + 1 < len) { + const auto entry = Headers::get_index(cx, self, *index + 1); + const host_api::HostString *next_key = &std::get<0>(*entry); + if (header_compare(*next_key, *key) != Ordering::Equal) { + break; } + str = JS_ConcatStrings(cx, str, comma); + if (!str) { + return false; + } + val = &std::get<1>(*entry); + RootedString next_str(cx, core::decode_byte_string(cx, *val)); + if (!next_str) { + return false; + } + str = JS_ConcatStrings(cx, str, next_str); + if (!str) { + return false; + } + *index = *index + 1; + } + value.setString(str); + return true; +} - // If we reach the end of the string without finding a separator, add the last cookie to the - // vector. - if (currentPosition >= cookiesString.length()) { - cookiesStrings.push_back(cookiesString.substr(start, cookiesString.length() - start)); +// Get the array of values for a given header (set-cookie, this is only for set-cookie) +bool retrieve_values_for_header_from_list(JSContext *cx, JS::HandleObject self, size_t index, + JS::MutableHandleObject out_arr) { + MOZ_ASSERT(Headers::is_instance(self)); + Headers::HeadersList *headers_list = Headers::headers_list(self); + const host_api::HostString *key = &std::get<0>(*Headers::get_index(cx, self, index)); + const host_api::HostString *val = &std::get<1>(*Headers::get_index(cx, self, index)); + // check if we need to join with the next value if it is the same key + RootedString str(cx, core::decode_byte_string(cx, *val)); + if (!str) { + return false; + } + size_t i = 0; + size_t len = headers_list->size(); + if (!JS_SetElement(cx, out_arr, i, str)) + return false; + while (++i < len - index) { + const host_api::HostString *next_key = &std::get<0>(*Headers::get_index(cx, self, index + i)); + val = &std::get<1>(*Headers::get_index(cx, self, index + i)); + if (header_compare(*next_key, *key) != Ordering::Equal) { + break; } + str = core::decode_byte_string(cx, *val); + if (!str) { + return false; + } + if (!JS_SetElement(cx, out_arr, i, str)) + return false; } - return cookiesStrings; + return true; } -} // namespace +// Walk through the repeated values for a given header, updating the index +void skip_values_for_header_from_list(JSContext *cx, JS::HandleObject self, size_t *index, + bool is_iterator) { + MOZ_ASSERT(Headers::is_instance(self)); + Headers::HeadersList *headers_list = Headers::headers_list(self); + const host_api::HostString *key = &std::get<0>(*Headers::get_index(cx, self, *index)); + size_t len = headers_list->size(); + while (*index + 1 < len) { + const host_api::HostString *next_key = &std::get<0>(*Headers::get_index(cx, self, *index + 1)); + // iterator doesn't join set-cookie + if (is_iterator && header_compare(*key, set_cookie_str) == Ordering::Equal) { + break; + } + if (header_compare(*next_key, *key) != Ordering::Equal) { + break; + } + *index = *index + 1; + } +} -bool redecode_str_if_changed(JSContext* cx, HandleValue str_val, string_view chars, - bool changed, MutableHandleValue rval) { - if (!changed) { - rval.set(str_val); +bool validate_guard(JSContext *cx, HandleObject self, string_view header_name, const char *fun_name, + bool *is_valid) { + MOZ_ASSERT(Headers::is_instance(self)); + Headers::HeadersGuard guard = Headers::guard(self); + switch (guard) { + case Headers::HeadersGuard::None: + *is_valid = true; return true; + case Headers::HeadersGuard::Immutable: + return api::throw_error(cx, FetchErrors::HeadersImmutable, fun_name); + case Headers::HeadersGuard::Request: + for (auto forbidden_header_name : *forbidden_request_headers) { + if (header_compare(header_name, forbidden_header_name) == Ordering::Equal) { + *is_valid = false; + return true; + } + } + *is_valid = true; + return true; + case Headers::HeadersGuard::Response: + for (auto forbidden_header_name : *forbidden_response_headers) { + if (header_compare(header_name, forbidden_header_name) == Ordering::Equal) { + *is_valid = false; + return true; + } + } + *is_valid = true; + return true; + default: + MOZ_ASSERT_UNREACHABLE(); } +} - RootedString str(cx, core::decode(cx, chars)); - if (!str) { - return false; +// Update the sort list +void ensure_updated_sort_list(const Headers::HeadersList *headers_list, + std::vector *headers_sort_list) { + MOZ_ASSERT(headers_list); + MOZ_ASSERT(headers_sort_list); + // Empty length means we need to recompute. + if (headers_sort_list->size() == 0) { + headers_sort_list->resize(headers_list->size()); + std::iota(headers_sort_list->begin(), headers_sort_list->end(), 0); + std::sort(headers_sort_list->begin(), headers_sort_list->end(), + HeadersSortListCompare(headers_list)); + } + + MOZ_ASSERT(headers_sort_list->size() == headers_list->size()); +} + +// Clear the sort list, marking it as mutated so it will be recomputed on the next lookup. +void mark_for_sort(JS::HandleObject self) { + MOZ_ASSERT(Headers::is_instance(self)); + std::vector *headers_sort_list = Headers::headers_sort_list(self); + headers_sort_list->clear(); +} + +bool append_valid_normalized_header(JSContext *cx, HandleObject self, string_view header_name, + string_view header_val) { + Headers::Mode mode = Headers::mode(self); + if (mode == Headers::Mode::HostOnly) { + auto handle = get_handle(self)->as_writable(); + MOZ_ASSERT(handle); + auto res = handle->append(header_name, header_val); + if (auto *err = res.to_err()) { + HANDLE_ERROR(cx, *err); + return false; + } + } else { + MOZ_ASSERT(mode == Headers::Mode::ContentOnly); + + Headers::HeadersList *list = Headers::headers_list(self); + + list->emplace_back(host_api::HostString(header_name), host_api::HostString(header_val)); + // add the new index to the sort list for sorting + mark_for_sort(self); } - rval.setString(str); return true; } -static bool switch_mode(JSContext* cx, HandleObject self, const Headers::Mode mode) { +static bool switch_mode(JSContext *cx, HandleObject self, const Headers::Mode mode) { auto current_mode = Headers::mode(self); if (mode == current_mode) { return true; @@ -288,65 +430,27 @@ static bool switch_mode(JSContext* cx, HandleObject self, const Headers::Mode mo if (!map) { return false; } - SetReservedSlot(self, static_cast(Headers::Slots::Entries), ObjectValue(*map)); - SetReservedSlot(self, static_cast(Headers::Slots::Mode), JS::Int32Value(static_cast(mode))); + MOZ_ASSERT(static_cast( + JS::GetReservedSlot(self, static_cast(Headers::Slots::HeadersList)) + .toPrivate()) == nullptr); + SetReservedSlot(self, static_cast(Headers::Slots::HeadersList), + PrivateValue(new Headers::HeadersList())); + MOZ_ASSERT(static_cast *>( + JS::GetReservedSlot(self, static_cast(Headers::Slots::HeadersSortList)) + .toPrivate()) == nullptr); + SetReservedSlot(self, static_cast(Headers::Slots::HeadersSortList), + PrivateValue(new std::vector())); + SetReservedSlot(self, static_cast(Headers::Slots::Mode), + JS::Int32Value(static_cast(Headers::Mode::ContentOnly))); return true; } if (current_mode == Headers::Mode::ContentOnly) { MOZ_ASSERT(mode == Headers::Mode::CachedInContent, "Switching from ContentOnly to HostOnly is wasteful and not implemented"); - RootedObject entries(cx, Headers::get_entries(cx, self)); - MOZ_ASSERT(entries); - RootedValue iterable(cx); - if (!MapEntries(cx, entries, &iterable)) { - return false; - } + Headers::HeadersList *list = Headers::headers_list(self); - JS::ForOfIterator it(cx); - if (!it.init(iterable)) { - return false; - } - - using host_api::HostString; - vector> string_entries; - - RootedValue entry_val(cx); - RootedObject entry(cx); - RootedValue name_val(cx); - RootedString name_str(cx); - RootedValue value_val(cx); - RootedString value_str(cx); - while (true) { - bool done; - if (!it.next(&entry_val, &done)) { - return false; - } - - if (done) { - break; - } - - entry = &entry_val.toObject(); - JS_GetElement(cx, entry, 0, &name_val); - JS_GetElement(cx, entry, 1, &value_val); - name_str = name_val.toString(); - value_str = value_val.toString(); - - auto name = core::encode(cx, name_str); - if (!name.ptr) { - return false; - } - - auto value = core::encode(cx, value_str); - if (!value.ptr) { - return false; - } - - string_entries.emplace_back(std::move(name), std::move(value)); - } - - auto handle = host_api::HttpHeaders::FromEntries(Headers::guard(self), string_entries); + auto handle = host_api::HttpHeaders::FromEntries(*list); if (handle.is_err()) { return api::throw_error(cx, FetchErrors::HeadersCloningFailed); } @@ -354,9 +458,20 @@ static bool switch_mode(JSContext* cx, HandleObject self, const Headers::Mode mo PrivateValue(handle.unwrap())); } - // Regardless of whether we're switching to CachedInContent or ContentOnly, - // get all entries into content. if (current_mode == Headers::Mode::HostOnly) { + MOZ_ASSERT(mode == Headers::Mode::CachedInContent); + MOZ_ASSERT(static_cast( + JS::GetReservedSlot(self, static_cast(Headers::Slots::HeadersList)) + .toPrivate()) == nullptr); + Headers::HeadersList *list = new Headers::HeadersList(); + SetReservedSlot(self, static_cast(Headers::Slots::HeadersList), PrivateValue(list)); + MOZ_ASSERT(static_cast *>( + JS::GetReservedSlot(self, static_cast(Headers::Slots::HeadersSortList)) + .toPrivate()) == nullptr); + SetReservedSlot(self, static_cast(Headers::Slots::HeadersSortList), + PrivateValue(new std::vector())); + SetReservedSlot(self, static_cast(Headers::Slots::Mode), + JS::Int32Value(static_cast(Headers::Mode::ContentOnly))); auto handle = get_handle(self); MOZ_ASSERT(handle); auto res = handle->entries(); @@ -365,35 +480,13 @@ static bool switch_mode(JSContext* cx, HandleObject self, const Headers::Mode mo return false; } - RootedObject map(cx, JS::NewMapObject(cx)); - if (!map) { - return false; - } - - RootedString key(cx); - RootedValue key_val(cx); - RootedString value(cx); - RootedValue value_val(cx); for (auto &entry : std::move(res.unwrap())) { - key = core::decode(cx, std::get<0>(entry)); - if (!key) { - return false; - } - value = core::decode(cx, std::get<1>(entry)); - if (!value) { - return false; - } - key_val.setString(key); - value_val.setString(value); - if (!MapSet(cx, map, key_val, value_val)) { - return false; - } + list->emplace_back(std::move(std::get<0>(entry)), std::move(std::get<1>(entry))); } - - SetReservedSlot(self, static_cast(Headers::Slots::Entries), ObjectValue(*map)); } if (mode == Headers::Mode::ContentOnly) { + MOZ_ASSERT(current_mode == Headers::Mode::CachedInContent); auto handle = get_handle(self); delete handle; SetReservedSlot(self, static_cast(Headers::Slots::Handle), PrivateValue(nullptr)); @@ -404,12 +497,12 @@ static bool switch_mode(JSContext* cx, HandleObject self, const Headers::Mode mo return true; } -bool prepare_for_entries_modification(JSContext* cx, JS::HandleObject self) { +bool prepare_for_entries_modification(JSContext *cx, JS::HandleObject self) { auto mode = Headers::mode(self); if (mode == Headers::Mode::HostOnly) { auto handle = get_handle(self); if (!handle->is_writable()) { - auto new_handle = handle->clone(Headers::guard(self)); + auto new_handle = handle->clone(); if (!new_handle) { return api::throw_error(cx, FetchErrors::HeadersCloningFailed); } @@ -422,97 +515,75 @@ bool prepare_for_entries_modification(JSContext* cx, JS::HandleObject self) { return true; } -bool append_single_normalized_header_value(JSContext *cx, HandleObject self, - HandleValue name, string_view name_chars, bool name_changed, - HandleValue value, string_view value_chars, bool value_changed, - const char *fun_name) { - Headers::Mode mode = Headers::mode(self); - if (mode == Headers::Mode::HostOnly) { - auto handle = get_handle(self)->as_writable(); - MOZ_ASSERT(handle); - auto res = handle->append(name_chars, value_chars); - if (auto *err = res.to_err()) { - HANDLE_ERROR(cx, *err); - return false; - } - } else { - MOZ_ASSERT(mode == Headers::Mode::ContentOnly); - auto guard = Headers::guard(self); - if (!host_api::HttpHeaders::check_guard(guard, name_chars)) { - return true; - } - - RootedObject entries(cx, Headers::get_entries(cx, self)); - if (!entries) { - return false; - } - - RootedValue name_val(cx); - if (!redecode_str_if_changed(cx, name, name_chars, name_changed, &name_val)) { - return false; - } - - RootedValue value_val(cx); - if (!redecode_str_if_changed(cx, value, value_chars, value_changed, &value_val)) { - return false; - } +} // namespace - RootedValue entry(cx); - if (!MapGet(cx, entries, name_val, &entry)) { - return false; - } +Headers::HeadersList *Headers::headers_list(JSObject *self) { + Headers::HeadersList *list = static_cast( + JS::GetReservedSlot(self, static_cast(Headers::Slots::HeadersList)).toPrivate()); + MOZ_ASSERT(list); + return list; +} - if (!entry.isUndefined()) { - RootedString entry_str(cx, JS::ToString(cx, entry)); - entry_str = JS_ConcatStrings(cx, entry_str, comma); - if (!entry_str) { - return false; - } - RootedString val_str(cx, value_val.toString()); - entry_str = JS_ConcatStrings(cx, entry_str, val_str); - if (!entry_str) { - return false; - } - value_val.setString(entry_str); - } +Headers::HeadersSortList *Headers::headers_sort_list(JSObject *self) { + Headers::HeadersSortList *list = static_cast( + JS::GetReservedSlot(self, static_cast(Headers::Slots::HeadersSortList)).toPrivate()); + MOZ_ASSERT(list); + return list; +} - if (!MapSet(cx, entries, name_val, value_val)) { - return false; - } +Headers::Mode Headers::mode(JSObject *self) { + MOZ_ASSERT(Headers::is_instance(self)); + Value modeVal = JS::GetReservedSlot(self, static_cast(Headers::Slots::Mode)); + if (modeVal.isUndefined()) { + return Headers::Mode::Uninitialized; } + return static_cast(modeVal.toInt32()); +} - return true; +Headers::HeadersGuard Headers::guard(JSObject *self) { + MOZ_ASSERT(Headers::is_instance(self)); + Value modeVal = JS::GetReservedSlot(self, static_cast(Headers::Slots::Guard)); + return static_cast(modeVal.toInt32()); } -bool Headers::append_header_value(JSContext *cx, JS::HandleObject self, JS::HandleValue name, - JS::HandleValue value, const char *fun_name) { - NORMALIZE_NAME(name, fun_name) - NORMALIZE_VALUE(value, fun_name) +/** + * Validates the given header name, by checking for invalid characters + * + * See + * https://searchfox.org/mozilla-central/rev/9f76a47f4aa935b49754c5608a1c8e72ee358c46/netwerk/protocol/http/nsHttp.cpp#172-215 + * For details on validation. + */ +host_api::HostString Headers::validate_header_name(JSContext *cx, HandleValue name_val, + const char *fun_name) { + JS::RootedString name_str(cx, JS::ToString(cx, name_val)); + if (!name_str) { + return host_api::HostString{}; + } - if (!prepare_for_entries_modification(cx, self)) { - return false; + host_api::HostString name = core::encode(cx, name_str); + if (!name) { + return host_api::HostString{}; } - std::string_view name_str = name_chars; - if (name_str == "set-cookie") { - for (auto value : splitCookiesString(value_chars)) { - if (!append_single_normalized_header_value(cx, self, name, name_chars, name_changed, UndefinedHandleValue, - value, true, fun_name)) { - return false; - } - } - } else { - if (!append_single_normalized_header_value(cx, self, name, name_chars, name_changed, value, - value_chars, value_changed, fun_name)) { - return false; + if (name.len == 0) { + api::throw_error(cx, FetchErrors::EmptyHeaderName, fun_name); + return host_api::HostString{}; + } + + char *name_chars = name.begin(); + for (size_t i = 0; i < name.len; i++) { + const unsigned char ch = name_chars[i]; + if (ch > 127 || !VALID_NAME_CHARS[ch]) { + api::throw_error(cx, FetchErrors::InvalidHeaderName, fun_name, name_chars); + return host_api::HostString{}; } } - return true; + return name; } -JSObject *Headers::create(JSContext *cx, host_api::HttpHeadersGuard guard) { - JSObject* self = JS_NewObjectWithGivenProto(cx, &class_, proto_obj); +JSObject *Headers::create(JSContext *cx, HeadersGuard guard) { + JSObject *self = JS_NewObjectWithGivenProto(cx, &class_, proto_obj); if (!self) { return nullptr; } @@ -521,10 +592,14 @@ JSObject *Headers::create(JSContext *cx, host_api::HttpHeadersGuard guard) { JS::Int32Value(static_cast(guard))); SetReservedSlot(self, static_cast(Slots::Mode), JS::Int32Value(static_cast(Mode::Uninitialized))); + + SetReservedSlot(self, static_cast(Slots::HeadersList), PrivateValue(nullptr)); + SetReservedSlot(self, static_cast(Slots::HeadersSortList), PrivateValue(nullptr)); return self; } -JSObject *Headers::create(JSContext *cx, host_api::HttpHeadersReadOnly *handle, host_api::HttpHeadersGuard guard) { +JSObject *Headers::create(JSContext *cx, host_api::HttpHeadersReadOnly *handle, + HeadersGuard guard) { RootedObject self(cx, create(cx, guard)); if (!self) { return nullptr; @@ -537,7 +612,7 @@ JSObject *Headers::create(JSContext *cx, host_api::HttpHeadersReadOnly *handle, return self; } -JSObject *Headers::create(JSContext *cx, HandleValue init_headers, host_api::HttpHeadersGuard guard) { +JSObject *Headers::create(JSContext *cx, HandleValue init_headers, HeadersGuard guard) { RootedObject self(cx, create(cx, guard)); if (!self) { return nullptr; @@ -545,7 +620,8 @@ JSObject *Headers::create(JSContext *cx, HandleValue init_headers, host_api::Htt if (!init_entries(cx, self, init_headers)) { return nullptr; } - MOZ_ASSERT(mode(self) == Headers::Mode::ContentOnly || mode(self) == Headers::Mode::Uninitialized); + MOZ_ASSERT(mode(self) == Headers::Mode::ContentOnly || + mode(self) == Headers::Mode::Uninitialized); return self; } @@ -553,8 +629,9 @@ bool Headers::init_entries(JSContext *cx, HandleObject self, HandleValue initv) // TODO: check if initv is a Headers instance and clone its handle if so. // TODO: But note: forbidden headers have to be applied correctly. bool consumed = false; - if (!core::maybe_consume_sequence_or_record(cx, initv, self, - &consumed, "Headers")) { + if (!core::maybe_consume_sequence_or_record(cx, initv, self, &consumed, + "Headers")) { return false; } @@ -569,7 +646,10 @@ bool Headers::init_entries(JSContext *cx, HandleObject self, HandleValue initv) bool Headers::get(JSContext *cx, unsigned argc, JS::Value *vp) { METHOD_HEADER(1) - NORMALIZE_NAME(args[0], "Headers.get") + auto name_chars = validate_header_name(cx, args[0], "Headers.get"); + if (!name_chars) { + return false; + } Mode mode = Headers::mode(self); if (mode == Headers::Mode::Uninitialized) { @@ -581,21 +661,36 @@ bool Headers::get(JSContext *cx, unsigned argc, JS::Value *vp) { return retrieve_value_for_header_from_handle(cx, self, name_chars, args.rval()); } - RootedObject entries(cx, get_entries(cx, self)); - if (!entries) { - return false; + auto idx = Headers::lookup(cx, self, name_chars); + if (!idx) { + args.rval().setNull(); + return true; } - RootedValue name_val(cx); - if (!redecode_str_if_changed(cx, args[0], name_chars, name_changed, &name_val)) { - return false; - } - if (!MapGet(cx, entries, name_val, args.rval())) { + if (!retrieve_value_for_header_from_list(cx, self, &idx.value(), args.rval(), false)) { return false; } - if (args.rval().isUndefined()) { - args.rval().setNull(); + return true; +} + +bool Headers::getSetCookie(JSContext *cx, unsigned argc, JS::Value *vp) { + METHOD_HEADER(0) + + JS::RootedObject out_arr(cx, JS::NewArrayObject(cx, 0)); + args.rval().setObject(*out_arr); + + Mode mode = Headers::mode(self); + if (mode == Headers::Mode::Uninitialized) + return true; + + if (mode == Mode::HostOnly) { + if (!retrieve_values_for_header_from_handle(cx, self, set_cookie_str, &out_arr)) + return false; + } else { + auto idx = Headers::lookup(cx, self, set_cookie_str); + if (idx && !retrieve_values_for_header_from_list(cx, self, idx.value(), &out_arr)) + return false; } return true; @@ -604,13 +699,26 @@ bool Headers::get(JSContext *cx, unsigned argc, JS::Value *vp) { bool Headers::set(JSContext *cx, unsigned argc, JS::Value *vp) { METHOD_HEADER(2) - NORMALIZE_NAME(args[0], "Headers.set") - NORMALIZE_VALUE(args[1], "Headers.set") + auto name_chars = validate_header_name(cx, args[0], "Headers.set"); + if (!name_chars) + return false; + + auto value_chars = normalize_and_validate_header_value(cx, args[1], "headers.set"); + if (!value_chars.ptr) + return false; - if (!prepare_for_entries_modification(cx, self)) { + bool is_valid; + if (!validate_guard(cx, self, name_chars, "Headers.append", &is_valid)) return false; + + if (!is_valid) { + args.rval().setUndefined(); + return true; } + if (!prepare_for_entries_modification(cx, self)) + return false; + Mode mode = Headers::mode(self); if (mode == Mode::HostOnly) { auto handle = get_handle(self)->as_writable(); @@ -622,23 +730,42 @@ bool Headers::set(JSContext *cx, unsigned argc, JS::Value *vp) { } } else { MOZ_ASSERT(mode == Mode::ContentOnly); - RootedObject entries(cx, get_entries(cx, self)); - if (!entries) { - return false; - } - RootedValue name_val(cx); - if (!redecode_str_if_changed(cx, args[0], name_chars, name_changed, &name_val)) { - return false; - } - - RootedValue value_val(cx); - if (!redecode_str_if_changed(cx, args[1], value_chars, value_changed, &value_val)) { - return false; - } - - if (!MapSet(cx, entries, name_val, value_val)) { - return false; + auto idx = Headers::lookup(cx, self, name_chars); + if (!idx) { + if (!append_valid_normalized_header(cx, self, std::move(name_chars), std::move(value_chars))) + return false; + } else { + size_t index = idx.value(); + // The lookup above will guarantee that sort_list is up to date. + std::vector *headers_sort_list = Headers::headers_sort_list(self); + HeadersList *headers_list = Headers::headers_list(self); + + // Update the first entry in place to the new value + host_api::HostString *header_val = + &std::get<1>(headers_list->at(headers_sort_list->at(index))); + + // Swap in the new value respecting the disposal semantics + header_val->ptr.swap(value_chars.ptr); + header_val->len = value_chars.len; + + // Delete all subsequent entries for this header excluding the first, + // as a variation of Headers::delete. + size_t len = headers_list->size(); + size_t delete_cnt = 0; + while (index + delete_cnt + 1 < len && + headers_sort_list->at(index + delete_cnt + 1) >= delete_cnt && + (header_compare(std::get<0>(headers_list->at( + headers_sort_list->at(index + delete_cnt + 1) - delete_cnt)), + name_chars) == Ordering::Equal)) { + headers_list->erase(headers_list->begin() + headers_sort_list->at(index + delete_cnt + 1) - + delete_cnt); + delete_cnt++; + } + // Reset the sort list if we performed additional deletions. + if (delete_cnt > 0) { + headers_sort_list->clear(); + } } } @@ -649,7 +776,9 @@ bool Headers::set(JSContext *cx, unsigned argc, JS::Value *vp) { bool Headers::has(JSContext *cx, unsigned argc, JS::Value *vp) { METHOD_HEADER(1) - NORMALIZE_NAME(args[0], "Headers.has") + auto name_chars = validate_header_name(cx, args[0], "Headers.has"); + if (!name_chars) + return false; Mode mode = Headers::mode(self); if (mode == Mode::Uninitialized) { @@ -664,20 +793,7 @@ bool Headers::has(JSContext *cx, unsigned argc, JS::Value *vp) { MOZ_ASSERT(!res.is_err()); args.rval().setBoolean(res.unwrap()); } else { - RootedObject entries(cx, get_entries(cx, self)); - if (!entries) { - return false; - } - - RootedValue name_val(cx); - if (!redecode_str_if_changed(cx, args[0], name_chars, name_changed, &name_val)) { - return false; - } - bool has; - if (!MapHas(cx, entries, name_val, &has)) { - return false; - } - args.rval().setBoolean(has); + args.rval().setBoolean(Headers::lookup(cx, self, name_chars).has_value()); } return true; @@ -686,18 +802,61 @@ bool Headers::has(JSContext *cx, unsigned argc, JS::Value *vp) { bool Headers::append(JSContext *cx, unsigned argc, JS::Value *vp) { METHOD_HEADER(2) - if (!append_header_value(cx, self, args[0], args[1], "Headers.append")) { + auto name_chars = validate_header_name(cx, args[0], "Headers.append"); + if (!name_chars) + return false; + + auto value_chars = normalize_and_validate_header_value(cx, args[1], "Headers.append"); + if (!value_chars) return false; + + bool is_valid; + if (!validate_guard(cx, self, name_chars, "Headers.append", &is_valid)) { + return false; + } + + if (is_valid) { + if (!prepare_for_entries_modification(cx, self)) + return false; + + // name casing must come from existing name match if there is one. + auto idx = Headers::lookup(cx, self, name_chars); + if (idx) { + // set-cookie doesn't combine + if (header_compare(name_chars, set_cookie_str) == Ordering::Equal) { + if (!append_valid_normalized_header(cx, self, + std::get<0>(*Headers::get_index(cx, self, idx.value())), + std::move(value_chars))) + return false; + } else { + // walk to the last name if multiple to do the combining into + size_t index = idx.value(); + skip_values_for_header_from_list(cx, self, &index, false); + host_api::HostString *header_val = &std::get<1>(*Headers::get_index(cx, self, index)); + size_t combined_len = header_val->len + value_chars.len + 2; + char *combined = static_cast(malloc(combined_len)); + memcpy(combined, header_val->ptr.get(), header_val->len); + memcpy(combined + header_val->len, ", ", 2); + memcpy(combined + header_val->len + 2, value_chars.ptr.get(), value_chars.len); + JS::UniqueChars combined_chars(combined); + header_val->ptr.swap(combined_chars); + header_val->len = combined_len; + } + } else { + if (!append_valid_normalized_header(cx, self, std::move(name_chars), std::move(value_chars))) + return false; + } + return true; } args.rval().setUndefined(); return true; } -bool Headers::set_if_undefined(JSContext *cx, HandleObject self, string_view name, string_view value) { - if (!prepare_for_entries_modification(cx, self)) { +bool Headers::set_valid_if_undefined(JSContext *cx, HandleObject self, string_view name, + string_view value) { + if (!prepare_for_entries_modification(cx, self)) return false; - } if (mode(self) == Mode::HostOnly) { auto handle = get_handle(self)->as_writable(); @@ -716,38 +875,32 @@ bool Headers::set_if_undefined(JSContext *cx, HandleObject self, string_view nam } MOZ_ASSERT(mode(self) == Mode::ContentOnly); - RootedObject entries(cx, get_entries(cx, self)); - RootedString name_str(cx, core::decode(cx, name)); - if (!name_str) { - return false; - } - RootedValue name_val(cx, StringValue(name_str)); - - bool has; - if (!MapHas(cx, entries, name_val, &has)) { - return false; - } - if (has) { + if (Headers::lookup(cx, self, name)) { return true; } - RootedString value_str(cx, core::decode(cx, value)); - if (!value_str) { - return false; - } - RootedValue value_val(cx, StringValue(value_str)); - - return JS::MapSet(cx, entries, name_val, value_val); + return append_valid_normalized_header(cx, self, name, value); } bool Headers::delete_(JSContext *cx, unsigned argc, JS::Value *vp) { METHOD_HEADER_WITH_NAME(1, "delete") - if (!prepare_for_entries_modification(cx, self)) { + auto name_chars = validate_header_name(cx, args[0], "Headers.delete"); + if (!name_chars) return false; + + bool is_valid; + if (!validate_guard(cx, self, name_chars, "Headers.delete", &is_valid)) + return false; + + if (!is_valid) { + args.rval().setUndefined(); + return true; } - NORMALIZE_NAME(args[0], "Headers.delete") + if (!prepare_for_entries_modification(cx, self)) + return false; + Mode mode = Headers::mode(self); if (mode == Mode::HostOnly) { auto handle = get_handle(self)->as_writable(); @@ -760,81 +913,60 @@ bool Headers::delete_(JSContext *cx, unsigned argc, JS::Value *vp) { } } else { MOZ_ASSERT(mode == Mode::ContentOnly); - RootedObject entries(cx, get_entries(cx, self)); - if (!entries) { - return false; - } - RootedValue name_val(cx); - if (!redecode_str_if_changed(cx, args[0], name_chars, name_changed, &name_val)) { - return false; + auto idx = Headers::lookup(cx, self, name_chars); + if (idx) { + size_t index = idx.value(); + // The lookup above will guarantee that sort_list is up to date. + std::vector *headers_sort_list = Headers::headers_sort_list(self); + HeadersList *headers_list = Headers::headers_list(self); + + // Delete all case-insensitively equal names. + // The ordering guarantee for sort_list is that equal names will come later in headers_list + // so that we can continue to use sort list during the delete operation, only recomputing it + // after. + size_t delete_cnt = 0; + size_t len = headers_sort_list->size(); + do { + headers_list->erase(headers_list->begin() + headers_sort_list->at(index + delete_cnt) - + delete_cnt); + delete_cnt++; + } while ( + index + delete_cnt < len && headers_sort_list->at(index + delete_cnt) >= delete_cnt && + header_compare( + std::get<0>(headers_list->at(headers_sort_list->at(index + delete_cnt) - delete_cnt)), + name_chars) == Ordering::Equal); + headers_sort_list->clear(); } - bool had; - return MapDelete(cx, entries, name_val, &had); } - args.rval().setUndefined(); return true; } -bool Headers::forEach(JSContext *cx, unsigned argc, JS::Value *vp) { - HEADERS_ITERATION_METHOD(1) - - if (!args[0].isObject() || !JS::IsCallable(&args[0].toObject())) { - return api::throw_error(cx, api::Errors::ForEachCallback, "Headers"); - } - - JS::RootedValueArray<3> newArgs(cx); - newArgs[2].setObject(*self); - - JS::RootedValue rval(cx); - - JS::RootedValue iterable(cx); - if (!JS::MapEntries(cx, entries, &iterable)) +bool Headers::append_valid_header(JSContext *cx, JS::HandleObject self, + host_api::HostString valid_key, JS::HandleValue value, + const char *fun_name) { + auto value_chars = normalize_and_validate_header_value(cx, value, fun_name); + if (!value_chars.ptr) return false; - JS::ForOfIterator it(cx); - if (!it.init(iterable)) + if (!prepare_for_entries_modification(cx, self)) return false; - JS::RootedValue entry_val(cx); - JS::RootedObject entry(cx); - while (true) { - bool done; - if (!it.next(&entry_val, &done)) - return false; - - if (done) - break; - - entry = &entry_val.toObject(); - JS_GetElement(cx, entry, 1, newArgs[0]); - JS_GetElement(cx, entry, 0, newArgs[1]); - - if (!JS::Call(cx, args.thisv(), args[0], newArgs, &rval)) + // name casing must come from existing name match if there is one. + auto idx = Headers::lookup(cx, self, valid_key); + if (idx) { + if (!append_valid_normalized_header( + cx, self, std::get<0>(*Headers::get_index(cx, self, idx.value())), value_chars)) { return false; + } + } else if (!append_valid_normalized_header(cx, self, valid_key, value_chars)) { + return false; } - - args.rval().setUndefined(); return true; } -bool Headers::entries(JSContext *cx, unsigned argc, Value *vp) { - HEADERS_ITERATION_METHOD(0) - return MapEntries(cx, entries, args.rval()); -} - -bool Headers::keys(JSContext *cx, unsigned argc, Value *vp) { - HEADERS_ITERATION_METHOD(0) - return MapKeys(cx, entries, args.rval()); -} - -bool Headers::values(JSContext *cx, unsigned argc, Value *vp) { - HEADERS_ITERATION_METHOD(0) - return MapValues(cx, entries, args.rval()); -} - const JSFunctionSpec Headers::static_methods[] = { JS_FS_END, }; @@ -844,14 +976,15 @@ const JSPropertySpec Headers::static_properties[] = { }; const JSFunctionSpec Headers::methods[] = { - JS_FN("get", Headers::get, 1, JSPROP_ENUMERATE), - JS_FN("has", Headers::has, 1, JSPROP_ENUMERATE), - JS_FN("set", Headers::set, 2, JSPROP_ENUMERATE), JS_FN("append", Headers::append, 2, JSPROP_ENUMERATE), JS_FN("delete", Headers::delete_, 1, JSPROP_ENUMERATE), - JS_FN("forEach", Headers::forEach, 1, JSPROP_ENUMERATE), JS_FN("entries", Headers::entries, 0, JSPROP_ENUMERATE), + JS_FN("forEach", Headers::forEach, 1, JSPROP_ENUMERATE), + JS_FN("get", Headers::get, 1, JSPROP_ENUMERATE), + JS_FN("getSetCookie", Headers::getSetCookie, 0, JSPROP_ENUMERATE), + JS_FN("has", Headers::has, 1, JSPROP_ENUMERATE), JS_FN("keys", Headers::keys, 0, JSPROP_ENUMERATE), + JS_FN("set", Headers::set, 2, JSPROP_ENUMERATE), JS_FN("values", Headers::values, 0, JSPROP_ENUMERATE), // [Symbol.iterator] added in init_class. JS_FS_END, @@ -864,23 +997,42 @@ const JSPropertySpec Headers::properties[] = { bool Headers::constructor(JSContext *cx, unsigned argc, JS::Value *vp) { CTOR_HEADER("Headers", 0); HandleValue headersInit = args.get(0); - RootedObject headersInstance(cx, JS_NewObjectForConstructor(cx, &class_, args)); - if (!headersInstance) { + RootedObject self(cx, JS_NewObjectForConstructor(cx, &class_, args)); + if (!self) { return false; } - SetReservedSlot(headersInstance, static_cast(Slots::Guard), - JS::Int32Value(static_cast(host_api::HttpHeadersGuard::None))); - if (!init_entries(cx, headersInstance, headersInit)) { + SetReservedSlot(self, static_cast(Slots::Guard), + JS::Int32Value(static_cast(HeadersGuard::None))); + SetReservedSlot(self, static_cast(Slots::HeadersList), PrivateValue(nullptr)); + SetReservedSlot(self, static_cast(Slots::HeadersSortList), PrivateValue(nullptr)); + + // walk the headers list writing in the ordered normalized case headers (distinct from the wire) + if (!init_entries(cx, self, headersInit)) { return false; } - args.rval().setObject(*headersInstance); + + args.rval().setObject(*self); return true; } bool Headers::init_class(JSContext *cx, JS::HandleObject global) { - bool ok = init_class_impl(cx, global); - if (!ok) + // get the host forbidden headers for guard checks + forbidden_request_headers = &host_api::HttpHeaders::get_forbidden_request_headers(); + forbidden_response_headers = &host_api::HttpHeaders::get_forbidden_response_headers(); + + // sort the forbidden headers with the lowercase-invariant comparator + MOZ_RELEASE_ASSERT(std::is_sorted(forbidden_request_headers->begin(), + forbidden_request_headers->end(), HeaderCompare()), + "Forbidden request headers must be sorted"); + MOZ_RELEASE_ASSERT(std::is_sorted(forbidden_response_headers->begin(), + forbidden_response_headers->end(), HeaderCompare()), + "Forbidden response headers must be sorted"); + + if (!init_class_impl(cx, global)) { return false; + } + + set_cookie_str = host_api::HostString("set-cookie"); auto comma_str = JS_NewStringCopyN(cx, ", ", 2); if (!comma_str) { @@ -888,6 +1040,10 @@ bool Headers::init_class(JSContext *cx, JS::HandleObject global) { } comma.init(cx, comma_str); + if (!HeadersIterator::init_class(cx, global)) { + return false; + } + JS::RootedValue entries(cx); if (!JS_GetProperty(cx, proto_obj, "entries", &entries)) return false; @@ -897,7 +1053,7 @@ bool Headers::init_class(JSContext *cx, JS::HandleObject global) { return JS_DefinePropertyById(cx, proto_obj, iteratorId, entries, 0); } -JSObject *Headers::get_entries(JSContext *cx, HandleObject self) { +Headers::HeadersList *Headers::get_list(JSContext *cx, HandleObject self) { MOZ_ASSERT(is_instance(self)); if (mode(self) == Mode::Uninitialized && !switch_mode(cx, self, Mode::ContentOnly)) { return nullptr; @@ -905,16 +1061,15 @@ JSObject *Headers::get_entries(JSContext *cx, HandleObject self) { if (mode(self) == Mode::HostOnly && !switch_mode(cx, self, Mode::CachedInContent)) { return nullptr; } - - return &GetReservedSlot(self, static_cast(Slots::Entries)).toObject(); + return headers_list(self); } -unique_ptr Headers::handle_clone(JSContext* cx, HandleObject self) { +unique_ptr Headers::handle_clone(JSContext *cx, HandleObject self) { auto mode = Headers::mode(self); // If this instance uninitialized, return an empty handle without initializing this instance. if (mode == Mode::Uninitialized) { - return std::make_unique(guard(self)); + return std::make_unique(); } if (mode == Mode::ContentOnly && !switch_mode(cx, self, Mode::CachedInContent)) { @@ -923,7 +1078,7 @@ unique_ptr Headers::handle_clone(JSContext* cx, HandleObj return nullptr; } - auto handle = unique_ptr(get_handle(self)->clone(guard(self))); + auto handle = unique_ptr(get_handle(self)->clone()); if (!handle) { api::throw_error(cx, FetchErrors::HeadersCloningFailed); return nullptr; @@ -931,4 +1086,164 @@ unique_ptr Headers::handle_clone(JSContext* cx, HandleObj return handle; } +BUILTIN_ITERATOR_METHODS(Headers) + +// Headers Iterator +JSObject *HeadersIterator::create(JSContext *cx, HandleObject headers, uint8_t type) { + JSObject *self = JS_NewObjectWithGivenProto(cx, &class_, proto_obj); + if (!self) { + return nullptr; + } + SetReservedSlot(self, static_cast(Slots::Type), + JS::Int32Value(static_cast(type))); + SetReservedSlot(self, static_cast(Slots::Cursor), JS::Int32Value(0)); + SetReservedSlot(self, static_cast(Slots::Headers), JS::ObjectValue(*headers)); + return self; +} + +const JSFunctionSpec HeadersIterator::static_methods[] = { + JS_FS_END, +}; + +const JSPropertySpec HeadersIterator::static_properties[] = { + JS_PS_END, +}; + +const JSFunctionSpec HeadersIterator::methods[] = { + JS_FN("next", HeadersIterator::next, 0, JSPROP_ENUMERATE), + JS_FS_END, +}; + +const JSPropertySpec HeadersIterator::properties[] = { + JS_PS_END, +}; + +bool HeadersIterator::init_class(JSContext *cx, JS::HandleObject global) { + JS::RootedObject iterator_proto(cx, JS::GetRealmIteratorPrototype(cx)); + if (!iterator_proto) + return false; + + if (!init_class_impl(cx, global, iterator_proto)) + return false; + + // Delete both the `HeadersIterator` global property and the + // `constructor` property on `HeadersIterator.prototype`. The latter + // because Iterators don't have their own constructor on the prototype. + return JS_DeleteProperty(cx, global, class_.name) && + JS_DeleteProperty(cx, proto_obj, "constructor"); +} + +std::tuple * +Headers::get_index(JSContext *cx, JS::HandleObject self, size_t index) { + MOZ_ASSERT(is_instance(self)); + std::vector *headers_sort_list = Headers::headers_sort_list(self); + HeadersList *headers_list = Headers::headers_list(self); + + ensure_updated_sort_list(headers_list, headers_sort_list); + MOZ_RELEASE_ASSERT(index < headers_sort_list->size()); + + return &headers_list->at(headers_sort_list->at(index)); +} + +std::optional Headers::lookup(JSContext *cx, HandleObject self, string_view key) { + MOZ_ASSERT(is_instance(self)); + const HeadersList *headers_list = Headers::headers_list(self); + std::vector *headers_sort_list = Headers::headers_sort_list(self); + + ensure_updated_sort_list(headers_list, headers_sort_list); + + // Now we know its sorted, we can binary search. + auto it = std::lower_bound(headers_sort_list->begin(), headers_sort_list->end(), key, + HeadersSortListLookupCompare(headers_list)); + if (it == headers_sort_list->end() || + header_compare(std::get<0>(headers_list->at(*it)), key) != Ordering::Equal) { + return std::nullopt; + } + return it - headers_sort_list->begin(); +} + +bool HeadersIterator::next(JSContext *cx, unsigned argc, Value *vp) { + METHOD_HEADER(0) + JS::RootedObject headers(cx, &JS::GetReservedSlot(self, Slots::Headers).toObject()); + + Headers::HeadersList *list = Headers::get_list(cx, headers); + + size_t index = JS::GetReservedSlot(self, Slots::Cursor).toInt32(); + size_t len = list->size(); + uint8_t type = static_cast(JS::GetReservedSlot(self, Slots::Type).toInt32()); + + JS::RootedObject result(cx, JS_NewPlainObject(cx)); + if (!result) + return false; + + if (index >= len) { + JS_DefineProperty(cx, result, "done", JS::TrueHandleValue, JSPROP_ENUMERATE); + JS_DefineProperty(cx, result, "value", JS::UndefinedHandleValue, JSPROP_ENUMERATE); + + args.rval().setObject(*result); + return true; + } + + JS_DefineProperty(cx, result, "done", JS::FalseHandleValue, JSPROP_ENUMERATE); + + JS::RootedValue key_val(cx); + JS::RootedValue val_val(cx); + + if (type != ITER_TYPE_VALUES) { + const host_api::HostString *key = &std::get<0>(*Headers::get_index(cx, headers, index)); + size_t len = key->len; + JS::Latin1Char *chars = reinterpret_cast(malloc(len)); + for (int i = 0; i < len; ++i) { + const unsigned char ch = key->ptr[i]; + // headers should already be validated by here + MOZ_ASSERT(ch <= 127 && VALID_NAME_CHARS[ch]); + // we store header keys with casing, so getter itself lowercases + if (ch >= 'A' && ch <= 'Z') { + chars[i] = ch - 'A' + 'a'; + } else { + chars[i] = ch; + } + } + key_val = JS::StringValue(JS_NewLatin1String(cx, JS::UniqueLatin1Chars(chars), len)); + } + + if (type != ITER_TYPE_KEYS) { + if (!retrieve_value_for_header_from_list(cx, headers, &index, &val_val, true)) { + return false; + } + } else { + skip_values_for_header_from_list(cx, headers, &index, true); + } + + JS::RootedValue result_val(cx); + + switch (type) { + case ITER_TYPE_ENTRIES: { + JS::RootedObject pair(cx, JS::NewArrayObject(cx, 2)); + if (!pair) + return false; + JS_DefineElement(cx, pair, 0, key_val, JSPROP_ENUMERATE); + JS_DefineElement(cx, pair, 1, val_val, JSPROP_ENUMERATE); + result_val = JS::ObjectValue(*pair); + break; + } + case ITER_TYPE_KEYS: { + result_val = key_val; + break; + } + case ITER_TYPE_VALUES: { + result_val = val_val; + break; + } + default: + MOZ_RELEASE_ASSERT(false, "Invalid iter type"); + } + + JS_DefineProperty(cx, result, "value", result_val, JSPROP_ENUMERATE); + + JS::SetReservedSlot(self, Slots::Cursor, JS::Int32Value(index + 1)); + args.rval().setObject(*result); + return true; +} + } // namespace builtins::web::fetch diff --git a/builtins/web/fetch/headers.h b/builtins/web/fetch/headers.h index 3beba950..50ae8737 100644 --- a/builtins/web/fetch/headers.h +++ b/builtins/web/fetch/headers.h @@ -9,14 +9,15 @@ namespace web { namespace fetch { class Headers final : public BuiltinImpl { - static bool get(JSContext *cx, unsigned argc, JS::Value *vp); - static bool set(JSContext *cx, unsigned argc, JS::Value *vp); - static bool has(JSContext *cx, unsigned argc, JS::Value *vp); static bool append(JSContext *cx, unsigned argc, JS::Value *vp); static bool delete_(JSContext *cx, unsigned argc, JS::Value *vp); - static bool forEach(JSContext *cx, unsigned argc, JS::Value *vp); static bool entries(JSContext *cx, unsigned argc, JS::Value *vp); + static bool forEach(JSContext *cx, unsigned argc, JS::Value *vp); + static bool get(JSContext *cx, unsigned argc, JS::Value *vp); + static bool getSetCookie(JSContext *cx, unsigned argc, JS::Value *vp); + static bool has(JSContext *cx, unsigned argc, JS::Value *vp); static bool keys(JSContext *cx, unsigned argc, JS::Value *vp); + static bool set(JSContext *cx, unsigned argc, JS::Value *vp); static bool values(JSContext *cx, unsigned argc, JS::Value *vp); public: @@ -53,47 +54,64 @@ class Headers final : public BuiltinImpl { /// If a header is added, deleted, or replaced on an instance in `CachedInContent` mode, the /// instance transitions to `ContentOnly` mode, and the underlying resource handle is discarded. enum class Mode { - HostOnly, // Headers are stored in the host. + HostOnly, // Headers are stored in the host. CachedInContent, // Host holds canonical headers, content a cached copy. - ContentOnly, // Headers are stored in a Map held by the `Entries` slot. - Uninitialized, // Headers have not been initialized. + ContentOnly, // Headers are stored in a Map held by the `Entries` slot. + Uninitialized, // Headers have not been initialized. }; + // Headers internal data structure is a list of key-value pairs, ready to go on the wire as + // owned host strings. + using HeadersList = std::vector>; + // A sort list is maintained of ordered indicies of the the sorted lowercase keys of main headers + // list, with each index of HeadersList always being present in this list once and only once. + // All lookups are done as indices in this list, which then map to indices in HeadersList. + // When this list is empty, that means the sort list is not valid and needs to be computed. For + // example, it is cleared after an insertion. It is recomputed lazily for every lookup. + using HeadersSortList = std::vector; + enum class Slots { Handle, - Entries, // Map holding headers if they are available in-content. + HeadersList, + HeadersSortList, Mode, Guard, Count, }; - /** - * Adds the given header name/value to `self`'s list of headers iff `self` - * doesn't already contain a header with that name. - */ - static bool set_if_undefined(JSContext *cx, JS::HandleObject self, string_view name, - string_view value); + enum class HeadersGuard { + None, + Request, + Response, + Immutable, + }; + + static HeadersList *headers_list(JSObject *self); + static HeadersSortList *headers_sort_list(JSObject *self); + static Mode mode(JSObject *self); + static HeadersGuard guard(JSObject *self); + + /// Adds the valid given header name/value to `self`'s list of headers iff `self` + /// doesn't already contain a header with that name. + static bool set_valid_if_undefined(JSContext *cx, JS::HandleObject self, string_view name, + string_view value); - /// Appends a value for a header name. - // /// Validates and normalizes the name and value. - static bool append_header_value(JSContext *cx, JS::HandleObject self, JS::HandleValue name, - JS::HandleValue value, const char *fun_name); - - static Mode mode(JSObject* self) { - MOZ_ASSERT(Headers::is_instance(self)); - Value modeVal = JS::GetReservedSlot(self, static_cast(Slots::Mode)); - if (modeVal.isUndefined()) { - return Mode::Uninitialized; - } - return static_cast(modeVal.toInt32()); - } - - static host_api::HttpHeadersGuard guard(JSObject* self) { - MOZ_ASSERT(Headers::is_instance(self)); - Value modeVal = JS::GetReservedSlot(self, static_cast(Slots::Guard)); - return static_cast(modeVal.toInt32()); - } + static host_api::HostString validate_header_name(JSContext *cx, HandleValue name_val, + const char *fun_name); + /// Appends a value for a header name. + static bool append_valid_header(JSContext *cx, JS::HandleObject self, + host_api::HostString valid_key, JS::HandleValue value, + const char *fun_name); + + /// Lookup the given header key, returning the sorted header index. + /// This index is guaranteed to be valid, so long as mutations are not made. + static std::optional lookup(JSContext *cx, JS::HandleObject self, string_view key); + + /// Get the header entry for a given index, ensuring that HeadersSortList is recomputed if + /// necessary in the process. + static std::tuple * + get_index(JSContext *cx, JS::HandleObject self, size_t idx); static const JSFunctionSpec static_methods[]; static const JSPropertySpec static_properties[]; @@ -105,19 +123,16 @@ class Headers final : public BuiltinImpl { static bool init_class(JSContext *cx, HandleObject global); static bool constructor(JSContext *cx, unsigned argc, Value *vp); - static JSObject *create(JSContext *cx, host_api::HttpHeadersGuard guard); - static JSObject *create(JSContext *cx, HandleValue init_headers, - host_api::HttpHeadersGuard guard); - static JSObject *create(JSContext *cx, host_api::HttpHeadersReadOnly *handle, - host_api::HttpHeadersGuard guard); + static JSObject *create(JSContext *cx, HeadersGuard guard); + static JSObject *create(JSContext *cx, HandleValue init_headers, HeadersGuard guard); + static JSObject *create(JSContext *cx, host_api::HttpHeadersReadOnly *handle, HeadersGuard guard); static bool init_entries(JSContext *cx, HandleObject self, HandleValue init_headers); - /// Returns a Map object containing the headers. - /// + /// Returns the headers list of entries, constructing it if necessary. /// Depending on the `Mode` the instance is in, this can be a cache or the canonical store for /// the headers. - static JSObject* get_entries(JSContext *cx, HandleObject self); + static HeadersList *get_list(JSContext *cx, HandleObject self); /** * Returns a cloned handle representing the contents of this Headers object. @@ -127,7 +142,30 @@ class Headers final : public BuiltinImpl { * * The handle is guaranteed to be uniquely owned by the caller. */ - static unique_ptr handle_clone(JSContext*, HandleObject self); + static unique_ptr handle_clone(JSContext *, HandleObject self); +}; + +class HeadersIterator final : public BuiltinNoConstructor { + static bool next(JSContext *cx, unsigned argc, JS::Value *vp); + +public: + static constexpr const char *class_name = "Headers Iterator"; + + enum Slots { + Type, + Cursor, + Headers, + Count, + }; + + static const JSFunctionSpec static_methods[]; + static const JSPropertySpec static_properties[]; + static const JSFunctionSpec methods[]; + static const JSPropertySpec properties[]; + + static bool init_class(JSContext *cx, HandleObject global); + + static JSObject *create(JSContext *cx, JS::HandleObject headers, uint8_t iter_type); }; } // namespace fetch diff --git a/builtins/web/fetch/request-response.cpp b/builtins/web/fetch/request-response.cpp index caf094fb..187e9e3d 100644 --- a/builtins/web/fetch/request-response.cpp +++ b/builtins/web/fetch/request-response.cpp @@ -369,8 +369,7 @@ bool RequestOrResponse::extract_body(JSContext *cx, JS::HandleObject self, if (!source) { return false; } - RootedObject body_stream(cx, JS::NewReadableDefaultStreamObject(cx, source, - nullptr, 0.0)); + RootedObject body_stream(cx, JS::NewReadableDefaultStreamObject(cx, source, nullptr, 0.0)); if (!body_stream) { return false; } @@ -379,13 +378,11 @@ bool RequestOrResponse::extract_body(JSContext *cx, JS::HandleObject self, MOZ_ASSERT(ReadableStreamIsDisturbed(cx, body_stream, &disturbed)); MOZ_ASSERT(!disturbed); - if (!ReadableStreamEnqueue(cx, body_stream, chunk) || - !ReadableStreamClose(cx, body_stream)) { + if (!ReadableStreamEnqueue(cx, body_stream, chunk) || !ReadableStreamClose(cx, body_stream)) { return false; } - JS_SetReservedSlot(self, static_cast(Slots::BodyStream), - ObjectValue(*body_stream)); + JS_SetReservedSlot(self, static_cast(Slots::BodyStream), ObjectValue(*body_stream)); content_length.emplace(length); } @@ -397,13 +394,14 @@ bool RequestOrResponse::extract_body(JSContext *cx, JS::HandleObject self, if (content_length.isSome()) { auto length_str = std::to_string(content_length.value()); - if (!Headers::set_if_undefined(cx, headers, "content-length", length_str)) { + if (!Headers::set_valid_if_undefined(cx, headers, "content-length", length_str)) { return false; } } // Step 36.3 of Request constructor / 8.4 of Response constructor. - if (content_type && !Headers::set_if_undefined(cx, headers, "content-type", content_type)) { + if (content_type && + !Headers::set_valid_if_undefined(cx, headers, "content-type", content_type)) { return false; } } @@ -417,8 +415,8 @@ JSObject *RequestOrResponse::maybe_headers(JSObject *obj) { return JS::GetReservedSlot(obj, static_cast(Slots::Headers)).toObjectOrNull(); } -unique_ptr RequestOrResponse::headers_handle_clone(JSContext* cx, - HandleObject self, host_api::HttpHeadersGuard guard) { +unique_ptr RequestOrResponse::headers_handle_clone(JSContext *cx, + HandleObject self) { MOZ_ASSERT(is_instance(self)); RootedObject headers(cx, maybe_headers(self)); @@ -428,7 +426,7 @@ unique_ptr RequestOrResponse::headers_handle_clone(JSCont auto handle = RequestOrResponse::handle(self); if (!handle) { - return std::make_unique(guard); + return std::make_unique(); } auto res = handle->headers(); @@ -436,10 +434,10 @@ unique_ptr RequestOrResponse::headers_handle_clone(JSCont HANDLE_ERROR(cx, *err); return nullptr; } - return unique_ptr(res.unwrap()->clone(guard)); + return unique_ptr(res.unwrap()->clone()); } -bool finish_outgoing_body_streaming(JSContext* cx, HandleObject body_owner) { +bool finish_outgoing_body_streaming(JSContext *cx, HandleObject body_owner) { // If no `body_owner` was passed, that means we sent a response: those aren't always // reified during `respondWith` processing, and we don't need the instance here. // That means, if we don't have the `body_owner`, we can advance the FetchState to @@ -497,9 +495,11 @@ bool RequestOrResponse::append_body(JSContext *cx, JS::HandleObject self, JS::Ha JSObject *RequestOrResponse::headers(JSContext *cx, JS::HandleObject obj) { JSObject *headers = maybe_headers(obj); if (!headers) { - host_api::HttpHeadersGuard guard = Request::is_instance(obj) - ? host_api::HttpHeadersGuard::Request - : host_api::HttpHeadersGuard::Response; + // Incoming request and incoming response headers are immutable per service worker + // and fetch specs respectively. + Headers::HeadersGuard guard = is_incoming(obj) ? Headers::HeadersGuard::Immutable + : Request::is_instance(obj) ? Headers::HeadersGuard::Request + : Headers::HeadersGuard::Response; host_api::HttpHeadersReadOnly *handle; if (is_incoming(obj) && (handle = headers_handle(obj))) { headers = Headers::create(cx, handle, guard); @@ -966,7 +966,6 @@ bool reader_for_outgoing_body_then_handler(JSContext *cx, JS::HandleObject body_ return false; } - // Read the next chunk. JS::RootedObject promise(cx, JS::ReadableStreamDefaultReaderRead(cx, reader)); if (!promise) { @@ -991,7 +990,7 @@ bool reader_for_outgoing_body_catch_handler(JSContext *cx, JS::HandleObject body } bool RequestOrResponse::maybe_stream_body(JSContext *cx, JS::HandleObject body_owner, - host_api::HttpOutgoingBodyOwner* destination, + host_api::HttpOutgoingBodyOwner *destination, bool *requires_streaming) { *requires_streaming = false; if (!has_body(body_owner)) { @@ -1053,13 +1052,15 @@ bool RequestOrResponse::maybe_stream_body(JSContext *cx, JS::HandleObject body_o // perform another operation in this way. JS::RootedObject catch_handler(cx); JS::RootedValue extra(cx, JS::ObjectValue(*reader)); - catch_handler = create_internal_method(cx, body_owner, extra); + catch_handler = + create_internal_method(cx, body_owner, extra); if (!catch_handler) return false; JS::RootedObject then_handler(cx); extra.setObject(*catch_handler); - then_handler = create_internal_method(cx, body_owner, extra); + then_handler = + create_internal_method(cx, body_owner, extra); if (!then_handler) return false; @@ -1212,17 +1213,17 @@ bool Request::clone(JSContext *cx, unsigned argc, JS::Value *vp) { RootedObject headers(cx, RequestOrResponse::maybe_headers(self)); if (headers) { RootedValue headers_val(cx, ObjectValue(*headers)); - JSObject *cloned_headers = - Headers::create(cx, headers_val, host_api::HttpHeadersGuard::Request); + JSObject *cloned_headers = Headers::create(cx, headers_val, Headers::guard(headers)); if (!cloned_headers) { return false; } cloned_headers_val.set(ObjectValue(*cloned_headers)); } else if (RequestOrResponse::handle(self)) { - auto handle = - RequestOrResponse::headers_handle_clone(cx, self, host_api::HttpHeadersGuard::Request); + auto handle = RequestOrResponse::headers_handle_clone(cx, self); JSObject *cloned_headers = - Headers::create(cx, handle.release(), host_api::HttpHeadersGuard::Request); + Headers::create(cx, handle.release(), + RequestOrResponse::is_incoming(self) ? Headers::HeadersGuard::Immutable + : Headers::HeadersGuard::Request); if (!cloned_headers) { return false; } @@ -1313,7 +1314,8 @@ bool Request::init_class(JSContext *cx, JS::HandleObject global) { } void Request::init_slots(JSObject *requestInstance) { - JS::SetReservedSlot(requestInstance, static_cast(Slots::Request), JS::PrivateValue(nullptr)); + JS::SetReservedSlot(requestInstance, static_cast(Slots::Request), + JS::PrivateValue(nullptr)); JS::SetReservedSlot(requestInstance, static_cast(Slots::Headers), JS::NullValue()); JS::SetReservedSlot(requestInstance, static_cast(Slots::BodyStream), JS::NullValue()); JS::SetReservedSlot(requestInstance, static_cast(Slots::HasBody), JS::FalseValue()); @@ -1330,7 +1332,7 @@ void Request::init_slots(JSObject *requestInstance) { * The places where we deviate from the spec are called out inline. */ bool Request::initialize(JSContext *cx, JS::HandleObject request, JS::HandleValue input, - JS::HandleValue init_val) { + JS::HandleValue init_val, Headers::HeadersGuard guard) { init_slots(request); JS::RootedString url_str(cx); JS::RootedString method_str(cx); @@ -1615,7 +1617,8 @@ bool Request::initialize(JSContext *cx, JS::HandleObject request, JS::HandleValu headers_val.setObject(*input_headers); } if (!headers_val.isUndefined()) { - headers = Headers::create(cx, headers_val, host_api::HttpHeadersGuard::Request); + // incoming request headers are always immutable + headers = Headers::create(cx, headers_val, guard); if (!headers) { return false; } @@ -1736,7 +1739,7 @@ JSObject *Request::create(JSContext *cx) { bool Request::constructor(JSContext *cx, unsigned argc, JS::Value *vp) { CTOR_HEADER("Request", 1); JS::RootedObject request(cx, JS_NewObjectForConstructor(cx, &class_, args)); - if (!request || !initialize(cx, request, args[0], args.get(1))) { + if (!request || !initialize(cx, request, args[0], args.get(1), Headers::HeadersGuard::Request)) { return false; } @@ -2044,15 +2047,17 @@ bool Response::redirect(JSContext *cx, unsigned argc, Value *vp) { return false; } - // 1. Let parsedURL be the result of parsing url with current settings object’s API base - // URL. + // 1. Let parsedURL be the result of parsing url with current settings object’s API base + // URL. jsurl::SpecString url_str = core::encode(cx, args.get(0)); if (!url_str.data) { return false; } - auto parsedURL = new_jsurl_with_base(&url_str, url::URL::url(worker_location::WorkerLocation::url)); + auto parsedURL = + new_jsurl_with_base(&url_str, url::URL::url(worker_location::WorkerLocation::url)); if (!parsedURL) { - return api::throw_error(cx, api::Errors::TypeError, "Response.redirect", "url", "be a valid URL"); + return api::throw_error(cx, api::Errors::TypeError, "Response.redirect", "url", + "be a valid URL"); } // 3. If status is not a redirect status, then throw a RangeError. @@ -2060,37 +2065,38 @@ bool Response::redirect(JSContext *cx, unsigned argc, Value *vp) { auto statusVal = args.get(1); uint16_t status; if (statusVal.isUndefined()) { - status = 302; + status = 302; } else { - if (!ToUint16(cx, statusVal, &status)) { - return false; - } + if (!ToUint16(cx, statusVal, &status)) { + return false; + } } if (status != 301 && status != 302 && status != 303 && status != 307 && status != 308) { auto status_str = std::to_string(status); return api::throw_error(cx, FetchErrors::InvalidStatus, "Response.redirect", - status_str.c_str()); + status_str.c_str()); } - // 4. Let responseObject be the result of creating a Response object, given a new response, - // "immutable", and this’s relevant Realm. + // 4. Let responseObject be the result of creating a Response object, given a new response, + // "immutable", and this’s relevant Realm. RootedObject responseObject(cx, create(cx)); if (!responseObject) { return false; } - // 5. Set responseObject’s response’s status to status. + // 5. Set responseObject’s response’s status to status. SetReservedSlot(responseObject, static_cast(Slots::Status), JS::Int32Value(status)); SetReservedSlot(responseObject, static_cast(Slots::StatusMessage), JS::StringValue(JS_GetEmptyString(cx))); // 6. Let value be parsedURL, serialized and isomorphic encoded. // 7. Append (`Location`, value) to responseObject’s response’s header list. + // TODO: redirect response headers should be immutable RootedObject headers(cx, RequestOrResponse::headers(cx, responseObject)); if (!headers) { return false; } - if (!Headers::set_if_undefined(cx, headers, "location", url_str)) { + if (!Headers::set_valid_if_undefined(cx, headers, "location", url_str)) { return false; } @@ -2127,7 +2133,8 @@ bool Response::redirect(JSContext *cx, unsigned argc, Value *vp) { // return false; // } // if (!callbackCalled) { -// return api::throw_error(cx, api::Errors::WrongType, "Response.json", "data", "be a valid JSON value"); +// return api::throw_error(cx, api::Errors::WrongType, "Response.json", "data", "be a valid JSON +// value"); // } // // 2. Let body be the result of extracting bytes. @@ -2325,7 +2332,8 @@ bool Response::constructor(JSContext *cx, unsigned argc, JS::Value *vp) { // `throw` a ``RangeError``. if (status < 200 || status > 599) { auto status_str = std::to_string(status); - return api::throw_error(cx, FetchErrors::InvalidStatus, "Response constructor", status_str.c_str()); + return api::throw_error(cx, FetchErrors::InvalidStatus, "Response constructor", + status_str.c_str()); } // 2. If `init`["statusText"] does not match the `reason-phrase` token @@ -2337,7 +2345,7 @@ bool Response::constructor(JSContext *cx, unsigned argc, JS::Value *vp) { // 7. (Reordered) If `init`["headers"] `exists`, then `fill` `this`’s `headers` with // `init`["headers"]. - JS::RootedObject headers(cx, Headers::create(cx, headers_val, host_api::HttpHeadersGuard::Response)); + JS::RootedObject headers(cx, Headers::create(cx, headers_val, Headers::HeadersGuard::Response)); if (!headers) { return false; } @@ -2381,7 +2389,7 @@ bool Response::constructor(JSContext *cx, unsigned argc, JS::Value *vp) { if (status == 204 || status == 205 || status == 304) { auto status_str = std::to_string(status); return api::throw_error(cx, FetchErrors::NonBodyResponseWithBody, "Response constructor", - status_str.c_str()); + status_str.c_str()); } // 2. Let `Content-Type` be null. @@ -2413,7 +2421,7 @@ bool Response::init_class(JSContext *cx, JS::HandleObject global) { (type_error_atom = JS_AtomizeAndPinString(cx, "error")); } -JSObject *Response::create(JSContext *cx){ +JSObject *Response::create(JSContext *cx) { RootedObject self(cx, JS_NewObjectWithGivenProto(cx, &class_, proto_obj)); if (!self) { return nullptr; @@ -2435,7 +2443,7 @@ JSObject *Response::init_slots(HandleObject response) { return response; } -JSObject * Response::create_incoming(JSContext *cx, host_api::HttpIncomingResponse *response) { +JSObject *Response::create_incoming(JSContext *cx, host_api::HttpIncomingResponse *response) { RootedObject self(cx, create(cx)); if (!self) { return nullptr; diff --git a/builtins/web/fetch/request-response.h b/builtins/web/fetch/request-response.h index a3d5bfaa..0a8400c3 100644 --- a/builtins/web/fetch/request-response.h +++ b/builtins/web/fetch/request-response.h @@ -1,9 +1,9 @@ #ifndef BUILTIN_REQUEST_RESPONSE #define BUILTIN_REQUEST_RESPONSE +#include "fetch-errors.h" #include "headers.h" #include "host_api.h" -#include "fetch-errors.h" namespace builtins { namespace web { @@ -59,8 +59,7 @@ class RequestOrResponse final { * * The handle is guaranteed to be uniquely owned by the caller. */ - static unique_ptr headers_handle_clone(JSContext *, HandleObject self, - host_api::HttpHeadersGuard guard); + static unique_ptr headers_handle_clone(JSContext *, HandleObject self); /** * Returns the RequestOrResponse's Headers, reifying it if necessary. @@ -159,7 +158,7 @@ class Request final : public BuiltinImpl { static JSObject *create(JSContext *cx); static bool initialize(JSContext *cx, JS::HandleObject requestInstance, JS::HandleValue input, - JS::HandleValue init_val); + JS::HandleValue init_val, Headers::HeadersGuard guard); static void init_slots(JSObject *requestInstance); }; @@ -208,7 +207,7 @@ class Response final : public BuiltinImpl { static JSObject *create(JSContext *cx); static JSObject *init_slots(HandleObject response); - static JSObject* create_incoming(JSContext * cx, host_api::HttpIncomingResponse* response); + static JSObject *create_incoming(JSContext *cx, host_api::HttpIncomingResponse *response); static host_api::HttpResponse *response_handle(JSObject *obj); static uint16_t status(JSObject *obj); diff --git a/builtins/web/url.cpp b/builtins/web/url.cpp index e5e96b23..f36f04ce 100644 --- a/builtins/web/url.cpp +++ b/builtins/web/url.cpp @@ -5,10 +5,6 @@ #include "sequence.hpp" #include "url.h" -constexpr int ITERTYPE_ENTRIES = 0; -constexpr int ITERTYPE_KEYS = 1; -constexpr int ITERTYPE_VALUES = 2; - namespace builtins { namespace web { namespace url { @@ -28,19 +24,19 @@ bool URLSearchParamsIterator::next(JSContext *cx, unsigned argc, JS::Value *vp) jsurl::params_at(params, index, ¶m); if (param.done) { - JS_DefineProperty(cx, result, "done", true, JSPROP_ENUMERATE); + JS_DefineProperty(cx, result, "done", JS::TrueHandleValue, JSPROP_ENUMERATE); JS_DefineProperty(cx, result, "value", JS::UndefinedHandleValue, JSPROP_ENUMERATE); args.rval().setObject(*result); return true; } - JS_DefineProperty(cx, result, "done", false, JSPROP_ENUMERATE); + JS_DefineProperty(cx, result, "done", JS::FalseHandleValue, JSPROP_ENUMERATE); JS::RootedValue key_val(cx); JS::RootedValue val_val(cx); - if (type != ITERTYPE_VALUES) { + if (type != ITER_TYPE_VALUES) { auto chars = JS::UTF8Chars((char *)param.name.data, param.name.len); JS::RootedString str(cx, JS_NewStringCopyUTF8N(cx, chars)); if (!str) @@ -48,7 +44,7 @@ bool URLSearchParamsIterator::next(JSContext *cx, unsigned argc, JS::Value *vp) key_val = JS::StringValue(str); } - if (type != ITERTYPE_KEYS) { + if (type != ITER_TYPE_KEYS) { auto chars = JS::UTF8Chars((char *)param.value.data, param.value.len); JS::RootedString str(cx, JS_NewStringCopyUTF8N(cx, chars)); if (!str) @@ -59,7 +55,7 @@ bool URLSearchParamsIterator::next(JSContext *cx, unsigned argc, JS::Value *vp) JS::RootedValue result_val(cx); switch (type) { - case ITERTYPE_ENTRIES: { + case ITER_TYPE_ENTRIES: { JS::RootedObject pair(cx, JS::NewArrayObject(cx, 2)); if (!pair) return false; @@ -68,11 +64,11 @@ bool URLSearchParamsIterator::next(JSContext *cx, unsigned argc, JS::Value *vp) result_val = JS::ObjectValue(*pair); break; } - case ITERTYPE_KEYS: { + case ITER_TYPE_KEYS: { result_val = key_val; break; } - case ITERTYPE_VALUES: { + case ITER_TYPE_VALUES: { result_val = val_val; break; } @@ -104,13 +100,6 @@ const JSPropertySpec URLSearchParamsIterator::properties[] = { JS_PS_END, }; -// This constructor will be deleted from the class prototype right after class -// initialization. -bool URLSearchParamsIterator::constructor(JSContext *cx, unsigned argc, JS::Value *vp) { - MOZ_RELEASE_ASSERT(false, "Should be deleted"); - return false; -} - bool URLSearchParamsIterator::init_class(JSContext *cx, JS::HandleObject global) { JS::RootedObject iterator_proto(cx, JS::GetRealmIteratorPrototype(cx)); if (!iterator_proto) @@ -127,7 +116,7 @@ bool URLSearchParamsIterator::init_class(JSContext *cx, JS::HandleObject global) } JSObject *URLSearchParamsIterator::create(JSContext *cx, JS::HandleObject params, uint8_t type) { - MOZ_RELEASE_ASSERT(type <= ITERTYPE_VALUES); + MOZ_RELEASE_ASSERT(type <= ITER_TYPE_VALUES); JS::RootedObject self(cx, JS_NewObjectWithGivenProto(cx, &class_, proto_obj)); if (!self) @@ -158,9 +147,9 @@ const JSFunctionSpec URLSearchParams::methods[] = { JS_FN("sort", URLSearchParams::sort, 0, JSPROP_ENUMERATE), JS_FN("toString", URLSearchParams::toString, 0, JSPROP_ENUMERATE), JS_FN("forEach", URLSearchParams::forEach, 0, JSPROP_ENUMERATE), - JS_FN("entries", URLSearchParams::entries, 0, 0), - JS_FN("keys", URLSearchParams::keys, 0, 0), - JS_FN("values", URLSearchParams::values, 0, 0), + JS_FN("entries", URLSearchParams::entries, 0, JSPROP_ENUMERATE), + JS_FN("keys", URLSearchParams::keys, 0, JSPROP_ENUMERATE), + JS_FN("values", URLSearchParams::values, 0, JSPROP_ENUMERATE), // [Symbol.iterator] added in init_class. JS_FS_END, }; @@ -175,19 +164,18 @@ jsurl::JSUrlSearchParams *URLSearchParams::get_params(JSObject *self) { } namespace { -bool append_impl(JSContext *cx, JS::HandleObject self, JS::HandleValue key, JS::HandleValue val, +jsurl::SpecString append_impl_validate(JSContext *cx, JS::HandleValue key, const char *_) { + return core::encode_spec_string(cx, key); +} +bool append_impl(JSContext *cx, JS::HandleObject self, jsurl::SpecString key, JS::HandleValue val, const char *_) { const auto params = URLSearchParams::get_params(self); - auto name = core::encode_spec_string(cx, key); - if (!name.data) - return false; - auto value = core::encode_spec_string(cx, val); if (!value.data) return false; - jsurl::params_append(params, name, value); + jsurl::params_append(params, key, value); return true; } } // namespace @@ -198,7 +186,8 @@ jsurl::SpecSlice URLSearchParams::serialize(JSContext *cx, JS::HandleObject self bool URLSearchParams::append(JSContext *cx, unsigned argc, JS::Value *vp) { METHOD_HEADER(2) - if (!append_impl(cx, self, args[0], args[1], "append")) + auto value = append_impl_validate(cx, args[0], "append"); + if (!append_impl(cx, self, value, args[1], "append")) return false; args.rval().setUndefined(); @@ -331,66 +320,7 @@ bool URLSearchParams::toString(JSContext *cx, unsigned argc, JS::Value *vp) { return true; } -bool URLSearchParams::forEach(JSContext *cx, unsigned argc, JS::Value *vp) { - METHOD_HEADER(1) - const auto params = get_params(self); - - if (!args[0].isObject() || !JS::IsCallable(&args[0].toObject())) { - return api::throw_error(cx, api::Errors::ForEachCallback, "URLSearchParams"); - } - - JS::HandleValue callback = args[0]; - JS::HandleValue thisv = args.get(1); - - JS::RootedValueArray<3> newArgs(cx); - newArgs[2].setObject(*self); - JS::RootedValue rval(cx); - - jsurl::JSSearchParam param{jsurl::SpecSlice(nullptr, 0), jsurl::SpecSlice(nullptr, 0), false}; - JS::RootedString name_str(cx); - JS::RootedString val_str(cx); - size_t index = 0; - while (true) { - jsurl::params_at(params, index, ¶m); - if (param.done) - break; - - name_str = JS_NewStringCopyUTF8N(cx, JS::UTF8Chars((char *)param.name.data, param.name.len)); - if (!name_str) - return false; - newArgs[1].setString(name_str); - - val_str = JS_NewStringCopyUTF8N(cx, JS::UTF8Chars((char *)param.value.data, param.value.len)); - if (!val_str) - return false; - newArgs[0].setString(val_str); - - if (!JS::Call(cx, thisv, callback, newArgs, &rval)) - return false; - - index++; - } - - args.rval().setUndefined(); - return true; -} - -#define ITERATOR_METHOD(name, type) \ - bool URLSearchParams::name(JSContext *cx, unsigned argc, JS::Value *vp) { \ - METHOD_HEADER(0) \ - \ - JS::RootedObject iter(cx, URLSearchParamsIterator::create(cx, self, type)); \ - if (!iter) \ - return false; \ - args.rval().setObject(*iter); \ - return true; \ - } - -ITERATOR_METHOD(entries, ITERTYPE_ENTRIES); -ITERATOR_METHOD(keys, ITERTYPE_KEYS); -ITERATOR_METHOD(values, ITERTYPE_VALUES); - -#undef ITERTYPE_VALUES +BUILTIN_ITERATOR_METHODS(URLSearchParams) bool URLSearchParams::constructor(JSContext *cx, unsigned argc, JS::Value *vp) { CTOR_HEADER("URLSearchParams", 0); @@ -424,8 +354,8 @@ JSObject *URLSearchParams::create(JSContext *cx, JS::HandleObject self, bool consumed = false; const char *alt_text = ", or a value that can be stringified"; - if (!core::maybe_consume_sequence_or_record(cx, params_val, self, &consumed, - "URLSearchParams", alt_text)) { + if (!core::maybe_consume_sequence_or_record( + cx, params_val, self, &consumed, "URLSearchParams", alt_text)) { return nullptr; } @@ -642,7 +572,7 @@ JSObject *URL::create(JSContext *cx, JS::HandleObject self, JS::HandleValue url_ JSObject *URL::create(JSContext *cx, JS::HandleObject self, JS::HandleValue url_val, JS::HandleObject base_obj) { - jsurl::JSUrl* base = nullptr; + jsurl::JSUrl *base = nullptr; if (is_instance(base_obj)) { base = static_cast(JS::GetReservedSlot(base_obj, Slots::Url).toPrivate()); } diff --git a/builtins/web/url.h b/builtins/web/url.h index 085e5c82..564568bc 100644 --- a/builtins/web/url.h +++ b/builtins/web/url.h @@ -25,7 +25,6 @@ class URLSearchParamsIterator : public BuiltinNoConstructor { diff --git a/componentize.sh b/componentize.sh index 017eb863..dfaddfd1 100755 --- a/componentize.sh +++ b/componentize.sh @@ -6,6 +6,7 @@ wizer="${WIZER:-wizer}" wasm_tools="${WASM_TOOLS:-wasm-tools}" weval="${WEVAL:-@WEVAL_BIN@}" aot=@AOT@ +preopen_dir="${PREOPEN_DIR:-}" usage() { echo "Usage: $(basename "$0") [input.js] [--verbose] [-o output.wasm]" @@ -60,10 +61,12 @@ then OUT_FILE="${BASENAME%.*}.wasm" fi -PREOPEN_DIR="" -if [ -n "$IN_FILE" ] -then - PREOPEN_DIR="--dir "$(dirname "$IN_FILE")"" +if [[ -n "$IN_FILE" ]]; then + if [[ -n $preopen_dir ]]; then + preopen_dir="--dir "$preopen_dir"" + else + preopen_dir="--dir "$(dirname "$IN_FILE")"" + fi echo "Componentizing $IN_FILE into $OUT_FILE" else echo "Creating runtime-script component $OUT_FILE" @@ -75,14 +78,14 @@ if [[ $aot -ne 0 ]]; then WEVAL_VERBOSE="--verbose --show-stats" fi - echo "$IN_FILE" | WASMTIME_BACKTRACE_DETAILS=1 $weval weval -w $PREOPEN_DIR \ + echo "$IN_FILE" | WASMTIME_BACKTRACE_DETAILS=1 $weval weval -w $preopen_dir \ --cache-ro "$(dirname "$0")/starling-ics.wevalcache" \ $WEVAL_VERBOSE \ -o "$OUT_FILE" \ -i "$(dirname "$0")/starling.wasm" else echo "$IN_FILE" | WASMTIME_BACKTRACE_DETAILS=1 $wizer --allow-wasi --wasm-bulk-memory true \ - --inherit-stdio true --inherit-env true $PREOPEN_DIR -o "$OUT_FILE" \ + --inherit-stdio true --inherit-env true $preopen_dir -o "$OUT_FILE" \ -- "$(dirname "$0")/starling.wasm" fi $wasm_tools component new -v --adapt "wasi_snapshot_preview1=$(dirname "$0")/preview1-adapter.wasm" --output "$OUT_FILE" "$OUT_FILE" diff --git a/crates/rust-url/rust-url.h b/crates/rust-url/rust-url.h index 4da6db1c..cb37c9eb 100644 --- a/crates/rust-url/rust-url.h +++ b/crates/rust-url/rust-url.h @@ -40,6 +40,8 @@ struct SpecString { cap(cap) {} + /// Conversion to a bool, testing for an empty pointer. + operator bool() const { return this->data != nullptr; } /// Conversion to a `jsurl::SpecString`. operator const std::string_view() const { diff --git a/host-apis/wasi-0.2.0/host_api.cpp b/host-apis/wasi-0.2.0/host_api.cpp index 4e6410b2..9a5b464b 100644 --- a/host-apis/wasi-0.2.0/host_api.cpp +++ b/host-apis/wasi-0.2.0/host_api.cpp @@ -32,7 +32,9 @@ typedef wasi_io_0_2_0_poll_borrow_pollable_t borrow_pollable_t; typedef wasi_io_0_2_0_poll_list_borrow_pollable_t list_borrow_pollable_t; #ifdef LOG_HANDLE_OPS -#define LOG_HANDLE_OP(...) fprintf(stderr, "%s", __PRETTY_FUNCTION__); fprintf(stderr, __VA_ARGS__) +#define LOG_HANDLE_OP(...) \ + fprintf(stderr, "%s", __PRETTY_FUNCTION__); \ + fprintf(stderr, __VA_ARGS__) #else #define LOG_HANDLE_OP(...) #endif @@ -52,8 +54,7 @@ class host_api::HandleState { template struct HandleOps {}; -template -class WASIHandle : public host_api::HandleState { +template class WASIHandle : public host_api::HandleState { #ifdef DEBUG static inline auto used_handles = std::set(); #endif @@ -95,13 +96,11 @@ class WASIHandle : public host_api::HandleState { #endif } - static WASIHandle* cast(HandleState* handle) { - return reinterpret_cast*>(handle); + static WASIHandle *cast(HandleState *handle) { + return reinterpret_cast *>(handle); } - typename HandleOps::borrowed borrow(HandleState *handle) { - return cast(handle)->borrow(); - } + typename HandleOps::borrowed borrow(HandleState *handle) { return cast(handle)->borrow(); } bool valid() const override { bool valid = handle_ != POISONED_HANDLE; @@ -119,7 +118,7 @@ class WASIHandle : public host_api::HandleState { MOZ_ASSERT(valid()); MOZ_ASSERT(owned_); LOG_HANDLE_OP("taking handle %d\n", handle_); - typename HandleOps::owned handle = { handle_ }; + typename HandleOps::owned handle = {handle_}; #ifdef DEBUG used_handles.erase(handle_); #endif @@ -128,8 +127,7 @@ class WASIHandle : public host_api::HandleState { } }; -template -struct Borrow { +template struct Borrow { static constexpr typename HandleOps::borrowed invalid{std::numeric_limits::max()}; typename HandleOps::borrowed handle_{invalid}; @@ -137,13 +135,9 @@ struct Borrow { handle_ = WASIHandle::cast(handle)->borrow(); } - explicit Borrow(typename HandleOps::borrowed handle) { - handle_ = handle; - } + explicit Borrow(typename HandleOps::borrowed handle) { handle_ = handle; } - explicit Borrow(typename HandleOps::owned handle) { - handle_ = {handle.__handle}; - } + explicit Borrow(typename HandleOps::owned handle) { handle_ = {handle.__handle}; } operator typename HandleOps::borrowed() const { return handle_; } }; @@ -221,8 +215,8 @@ class IncomingBodyHandle final : public WASIHandle { stream_handle_ = stream; } - static IncomingBodyHandle* cast(HandleState* handle) { - return reinterpret_cast(handle); + static IncomingBodyHandle *cast(HandleState *handle) { + return reinterpret_cast(handle); } }; @@ -242,8 +236,8 @@ class OutgoingBodyHandle final : public WASIHandle { stream_handle_ = stream; } - static OutgoingBodyHandle* cast(HandleState* handle) { - return reinterpret_cast(handle); + static OutgoingBodyHandle *cast(HandleState *handle) { + return reinterpret_cast(handle); } }; @@ -253,7 +247,7 @@ size_t api::AsyncTask::select(std::vector &tasks) { for (const auto task : tasks) { handles.emplace_back(task->id()); } - auto list = list_borrow_pollable_t{ handles.data(), count}; + auto list = list_borrow_pollable_t{handles.data(), count}; wasi_io_0_2_0_poll_list_u32_t result{nullptr, 0}; wasi_io_0_2_0_poll_poll(&list, &result); MOZ_ASSERT(result.len > 0); @@ -265,13 +259,6 @@ size_t api::AsyncTask::select(std::vector &tasks) { namespace host_api { -HostString::HostString(const char *c_str) { - len = strlen(c_str); - ptr = JS::UniqueChars(static_cast(malloc(len + 1))); - std::memcpy(ptr.get(), c_str, len); - ptr[len] = '\0'; -} - namespace { template HostString to_host_string(T str) { @@ -335,15 +322,15 @@ void MonotonicClock::unsubscribe(const int32_t handle_id) { wasi_io_0_2_0_poll_pollable_drop_own(own_pollable_t{handle_id}); } -HttpHeaders::HttpHeaders(HttpHeadersGuard guard, std::unique_ptr state) - : HttpHeadersReadOnly(std::move(state)), guard_(guard) {} +HttpHeaders::HttpHeaders(std::unique_ptr state) + : HttpHeadersReadOnly(std::move(state)) {} -HttpHeaders::HttpHeaders(HttpHeadersGuard guard) : guard_(guard) { - handle_state_ = std::make_unique>(wasi_http_0_2_0_types_constructor_fields()); +HttpHeaders::HttpHeaders() { + handle_state_ = + std::make_unique>(wasi_http_0_2_0_types_constructor_fields()); } -Result HttpHeaders::FromEntries(HttpHeadersGuard guard, - vector>& entries) { +Result HttpHeaders::FromEntries(vector> &entries) { std::vector pairs; pairs.reserve(entries.size()); @@ -357,15 +344,14 @@ Result HttpHeaders::FromEntries(HttpHeadersGuard guard, wasi_http_0_2_0_types_header_error_t err; if (!wasi_http_0_2_0_types_static_fields_from_list(&tuples, &ret, &err)) { // TODO: handle `err` - return Result::err(154); + return Result::err(154); } - auto headers = new HttpHeaders(guard, std::make_unique>(ret)); - return Result::ok(headers); + auto headers = new HttpHeaders(std::make_unique>(ret)); + return Result::ok(headers); } -HttpHeaders::HttpHeaders(HttpHeadersGuard guard, const HttpHeadersReadOnly &headers) - : HttpHeadersReadOnly(nullptr), guard_(guard) { +HttpHeaders::HttpHeaders(const HttpHeadersReadOnly &headers) : HttpHeadersReadOnly(nullptr) { Borrow borrow(headers.handle_state_.get()); auto handle = wasi_http_0_2_0_types_method_fields_clone(borrow); this->handle_state_ = std::unique_ptr(new WASIHandle(handle)); @@ -373,72 +359,26 @@ HttpHeaders::HttpHeaders(HttpHeadersGuard guard, const HttpHeadersReadOnly &head // We currently only guard against a single request header, instead of the full list in // https://fetch.spec.whatwg.org/#forbidden-request-header. -static std::list forbidden_request_headers = { - "host", +static const std::vector forbidden_request_headers = { + "host", }; // We currently only guard against a single response header, instead of the full list in // https://fetch.spec.whatwg.org/#forbidden-request-header. -static std::list forbidden_response_headers = { - "host", +static const std::vector forbidden_response_headers = { + "host", }; -bool HttpHeaders::check_guard(HttpHeadersGuard guard, string_view header_name) { - std::list* forbidden_headers = nullptr; - switch (guard) { - case HttpHeadersGuard::None: - return true; - case HttpHeadersGuard::Request: - forbidden_headers = &forbidden_request_headers; - break; - case HttpHeadersGuard::Response: - forbidden_headers = &forbidden_response_headers; - break; - default: - MOZ_ASSERT_UNREACHABLE(); - } - - if (!forbidden_headers) { - return true; - } - - for (auto header : *forbidden_headers) { - if (header_name.compare(header) == 0) { - return false; - } - } - - return true; +const std::vector &HttpHeaders::get_forbidden_request_headers() { + return forbidden_request_headers; } -HttpHeaders *HttpHeadersReadOnly::clone(HttpHeadersGuard guard) { - auto headers = new HttpHeaders(guard, *this); - std::list* forbidden_headers = nullptr; - switch (guard) { - case HttpHeadersGuard::Request: - forbidden_headers = &forbidden_request_headers; - break; - case HttpHeadersGuard::Response: - break; - case HttpHeadersGuard::None: - break; - default: - MOZ_ASSERT_UNREACHABLE(); - } - - if (!forbidden_headers) { - return headers; - } - - for (auto header : *forbidden_headers) { - if (headers->has(header).unwrap()) { - headers->remove(header).unwrap(); - } - } - - return headers; +const std::vector &HttpHeaders::get_forbidden_response_headers() { + return forbidden_response_headers; } +HttpHeaders *HttpHeadersReadOnly::clone() { return new HttpHeaders(*this); } + Result>> HttpHeadersReadOnly::entries() const { Result>> res; @@ -509,9 +449,6 @@ Result HttpHeadersReadOnly::has(string_view name) const { } Result HttpHeaders::set(string_view name, string_view value) { - if (!check_guard(guard_, name)) { - return {}; - } auto hdr = from_string_view(name); auto val = from_string_view(value); wasi_http_0_2_0_types_list_field_value_t host_values{&val, 1}; @@ -519,18 +456,14 @@ Result HttpHeaders::set(string_view name, string_view value) { wasi_http_0_2_0_types_header_error_t err; if (!wasi_http_0_2_0_types_method_fields_set(borrow, &hdr, &host_values, &err)) { - // TODO: handle `err` + // TODO: handle `err` return Result::err(154); } - return {}; } Result HttpHeaders::append(string_view name, string_view value) { - if (!check_guard(guard_, name)) { - return {}; - } auto hdr = from_string_view(name); auto val = from_string_view(value); Borrow borrow(this->handle_state_.get()); @@ -684,10 +617,10 @@ class BodyAppendTask final : public api::AsyncTask { PollableHandle outgoing_pollable_; api::TaskCompletionCallback cb_; - Heap cb_receiver_; + Heap cb_receiver_; State state_; - void set_state(JSContext* cx, const State state) { + void set_state(JSContext *cx, const State state) { MOZ_ASSERT(state_ != State::Done); state_ = state; if (state == State::Done && cb_) { @@ -699,13 +632,11 @@ class BodyAppendTask final : public api::AsyncTask { } public: - explicit BodyAppendTask(api::Engine *engine, - HttpIncomingBody *incoming_body, + explicit BodyAppendTask(api::Engine *engine, HttpIncomingBody *incoming_body, HttpOutgoingBody *outgoing_body, api::TaskCompletionCallback completion_callback, HandleObject callback_receiver) - : incoming_body_(incoming_body), outgoing_body_(outgoing_body), - cb_(completion_callback) { + : incoming_body_(incoming_body), outgoing_body_(outgoing_body), cb_(completion_callback) { auto res = incoming_body_->subscribe(); MOZ_ASSERT(!res.is_err()); incoming_pollable_ = res.unwrap(); @@ -816,7 +747,8 @@ class BodyAppendTask final : public api::AsyncTask { }; Result HttpOutgoingBody::append(api::Engine *engine, HttpIncomingBody *other, - api::TaskCompletionCallback callback, HandleObject callback_receiver) { + api::TaskCompletionCallback callback, + HandleObject callback_receiver) { engine->queue_async_task(new BodyAppendTask(engine, other, this, callback, callback_receiver)); return {}; } @@ -890,7 +822,9 @@ wasi_http_0_2_0_types_method_t http_method_to_host(string_view method_str) { return wasi_http_0_2_0_types_method_t{WASI_HTTP_0_2_0_TYPES_METHOD_OTHER, {val}}; } -HttpOutgoingRequest::HttpOutgoingRequest(std::unique_ptr state) { this->handle_state_ = std::move(state); } +HttpOutgoingRequest::HttpOutgoingRequest(std::unique_ptr state) { + this->handle_state_ = std::move(state); +} HttpOutgoingRequest *HttpOutgoingRequest::make(string_view method_str, optional url_str, std::unique_ptr headers) { @@ -927,8 +861,7 @@ HttpOutgoingRequest *HttpOutgoingRequest::make(string_view method_str, optional< } auto headers_handle = WASIHandle::cast(headers->handle_state_.get())->take(); - auto handle = - wasi_http_0_2_0_types_constructor_outgoing_request(headers_handle); + auto handle = wasi_http_0_2_0_types_constructor_outgoing_request(headers_handle); { auto borrow = wasi_http_0_2_0_types_borrow_outgoing_request(handle); @@ -953,9 +886,7 @@ HttpOutgoingRequest *HttpOutgoingRequest::make(string_view method_str, optional< return resp; } -Result HttpOutgoingRequest::method() { - return Result::ok(method_); -} +Result HttpOutgoingRequest::method() { return Result::ok(method_); } Result HttpOutgoingRequest::headers() { if (!headers_) { @@ -964,7 +895,8 @@ Result HttpOutgoingRequest::headers() { } Borrow borrow(handle_state_.get()); auto res = wasi_http_0_2_0_types_method_outgoing_request_headers(borrow); - headers_ = new HttpHeadersReadOnly(std::unique_ptr(new WASIHandle(res))); + headers_ = + new HttpHeadersReadOnly(std::unique_ptr(new WASIHandle(res))); } return Result::ok(headers_); @@ -991,7 +923,8 @@ Result HttpOutgoingRequest::send() { if (!wasi_http_0_2_0_outgoing_handler_handle(request_handle, nullptr, &ret, &err)) { return Res::err(154); } - auto res = new FutureHttpIncomingResponse(std::unique_ptr(new WASIHandle(ret))); + auto res = new FutureHttpIncomingResponse( + std::unique_ptr(new WASIHandle(ret))); return Result::ok(res); } @@ -999,7 +932,9 @@ void block_on_pollable_handle(PollableHandle handle) { wasi_io_0_2_0_poll_method_pollable_block({handle}); } -HttpIncomingBody::HttpIncomingBody(std::unique_ptr state) : Pollable() { handle_state_ = std::move(state); } +HttpIncomingBody::HttpIncomingBody(std::unique_ptr state) : Pollable() { + handle_state_ = std::move(state); +} Resource::~Resource() { if (handle_state_ != nullptr) { @@ -1050,7 +985,6 @@ FutureHttpIncomingResponse::FutureHttpIncomingResponse(std::unique_ptr> FutureHttpIncomingResponse::maybe_response() { typedef Result> Res; wasi_http_0_2_0_types_result_result_own_incoming_response_error_code_void_t res; @@ -1080,9 +1014,7 @@ void FutureHttpIncomingResponse::unsubscribe() { // TODO: implement } -HttpHeadersReadOnly::HttpHeadersReadOnly() { - handle_state_ = nullptr; -} +HttpHeadersReadOnly::HttpHeadersReadOnly() { handle_state_ = nullptr; } HttpHeadersReadOnly::HttpHeadersReadOnly(std::unique_ptr state) { handle_state_ = std::move(state); @@ -1132,9 +1064,12 @@ Result HttpIncomingResponse::body() { return Result::ok(body_); } -HttpOutgoingResponse::HttpOutgoingResponse(std::unique_ptr state) { this->handle_state_ = std::move(state); } +HttpOutgoingResponse::HttpOutgoingResponse(std::unique_ptr state) { + this->handle_state_ = std::move(state); +} -HttpOutgoingResponse *HttpOutgoingResponse::make(const uint16_t status, unique_ptr headers) { +HttpOutgoingResponse *HttpOutgoingResponse::make(const uint16_t status, + unique_ptr headers) { auto owned_headers = WASIHandle::cast(headers->handle_state_.get())->take(); auto handle = wasi_http_0_2_0_types_constructor_outgoing_response(owned_headers); @@ -1144,7 +1079,8 @@ HttpOutgoingResponse *HttpOutgoingResponse::make(const uint16_t status, unique_p // Set the status if (status != 200) { // The DOM implementation is expected to have validated the status code already. - MOZ_RELEASE_ASSERT(wasi_http_0_2_0_types_method_outgoing_response_set_status_code(state->borrow(), status)); + MOZ_RELEASE_ASSERT( + wasi_http_0_2_0_types_method_outgoing_response_set_status_code(state->borrow(), status)); } resp->status_ = status; diff --git a/include/builtin.h b/include/builtin.h index a7842a93..1f1d02e0 100644 --- a/include/builtin.h +++ b/include/builtin.h @@ -103,6 +103,60 @@ void markWizeningAsFinished(); return false; \ } +#define ITER_TYPE_ENTRIES 0 +#define ITER_TYPE_KEYS 1 +#define ITER_TYPE_VALUES 2 + +#define BUILTIN_ITERATOR_METHOD(class_name, method, type) \ + bool class_name::method(JSContext *cx, unsigned argc, JS::Value *vp) { \ + METHOD_HEADER(0) \ + JS::RootedObject iter(cx, class_name##Iterator::create(cx, self, type)); \ + if (!iter) \ + return false; \ + args.rval().setObject(*iter); \ + return true; \ + } + +// defines entries(), keys(), values(), and forEach(), assuming class_name##Iterator +#define BUILTIN_ITERATOR_METHODS(class_name) \ + BUILTIN_ITERATOR_METHOD(class_name, entries, ITER_TYPE_ENTRIES) \ + BUILTIN_ITERATOR_METHOD(class_name, keys, ITER_TYPE_KEYS) \ + BUILTIN_ITERATOR_METHOD(class_name, values, ITER_TYPE_VALUES) \ + \ +bool class_name::forEach(JSContext *cx, unsigned argc, JS::Value *vp) { \ + METHOD_HEADER(1) \ + if (!args[0].isObject() || !JS::IsCallable(&args[0].toObject())) { \ + return api::throw_error(cx, api::Errors::ForEachCallback, #class_name); \ + } \ + JS::RootedValueArray<3> newArgs(cx); \ + newArgs[2].setObject(*self); \ + JS::RootedValue rval(cx); \ + JS::RootedObject iter(cx, class_name##Iterator::create(cx, self, ITER_TYPE_ENTRIES)); \ + if (!iter) \ + return false; \ + JS::RootedValue iterable(cx, ObjectValue(*iter)); \ + JS::ForOfIterator it(cx); \ + if (!it.init(iterable)) \ + return false; \ + \ + JS::RootedValue entry_val(cx); \ + JS::RootedObject entry(cx); \ + while (true) { \ + bool done; \ + if (!it.next(&entry_val, &done)) \ + return false; \ + if (done) \ + break; \ + \ + entry = &entry_val.toObject(); \ + JS_GetElement(cx, entry, 1, newArgs[0]); \ + JS_GetElement(cx, entry, 0, newArgs[1]); \ + if (!JS::Call(cx, args.thisv(), args[0], newArgs, &rval)) \ + return false; \ + } \ + return true; \ +} + #define REQUEST_HANDLER_ONLY(name) \ if (isWizening()) { \ return api::throw_error(cx, api::Errors::RequestHandlerOnly, name); \ diff --git a/include/host_api.h b/include/host_api.h index 54ec4f38..434dd969 100644 --- a/include/host_api.h +++ b/include/host_api.h @@ -2,6 +2,7 @@ #define JS_RUNTIME_HOST_API_H #include +#include #include #include #include @@ -105,7 +106,17 @@ struct HostString final { HostString() = default; HostString(std::nullptr_t) : HostString() {} - HostString(const char *c_str); + HostString(const char *c_str) { + len = strlen(c_str); + ptr = JS::UniqueChars(static_cast(malloc(len + 1))); + std::memcpy(ptr.get(), c_str, len); + ptr[len] = '\0'; + } + HostString(const string_view &str) { + ptr = JS::UniqueChars( + static_cast(std::memcpy(malloc(str.size()), str.data(), str.size()))); + len = str.size(); + } HostString(JS::UniqueChars ptr, size_t len) : ptr{std::move(ptr)}, len{len} {} HostString(const HostString &other) = delete; @@ -218,7 +229,7 @@ class Resource { virtual ~Resource(); typedef uint8_t HandleNS; - static HandleNS next_handle_ns(const char* ns_name); + static HandleNS next_handle_ns(const char *ns_name); /// Returns true if this resource handle has been initialized and is still valid. bool valid() const; @@ -324,12 +335,6 @@ class FutureHttpIncomingResponse final : public Pollable { void unsubscribe() override; }; -enum class HttpHeadersGuard { - None, - Request, - Response, -}; - class HttpHeadersReadOnly : public Resource { friend HttpIncomingResponse; friend HttpIncomingRequest; @@ -346,12 +351,10 @@ class HttpHeadersReadOnly : public Resource { explicit HttpHeadersReadOnly(std::unique_ptr handle); HttpHeadersReadOnly(const HttpHeadersReadOnly &headers) = delete; - /// Clone the headers, while removing the headers not passing the given `guard`, as specified - /// here: https://fetch.spec.whatwg.org/#headers-validate. - HttpHeaders* clone(HttpHeadersGuard guard); + HttpHeaders *clone(); virtual bool is_writable() { return false; }; - virtual HttpHeaders* as_writable() { + virtual HttpHeaders *as_writable() { MOZ_ASSERT_UNREACHABLE(); return nullptr; }; @@ -368,28 +371,23 @@ class HttpHeaders final : public HttpHeadersReadOnly { friend HttpOutgoingResponse; friend HttpOutgoingRequest; - HttpHeadersGuard guard_; - - HttpHeaders(HttpHeadersGuard guard, std::unique_ptr handle); + HttpHeaders(std::unique_ptr handle); public: - HttpHeaders() = delete; - explicit HttpHeaders(HttpHeadersGuard guard); - explicit HttpHeaders(HttpHeadersGuard guard, const HttpHeadersReadOnly &headers); + HttpHeaders(); + HttpHeaders(const HttpHeadersReadOnly &headers); - static Result FromEntries(HttpHeadersGuard guard, - vector> &entries); + static Result FromEntries(vector> &entries); bool is_writable() override { return true; }; - HttpHeaders* as_writable() override { - return this; - }; + HttpHeaders *as_writable() override { return this; }; Result set(string_view name, string_view value); Result append(string_view name, string_view value); Result remove(string_view name); - static bool check_guard(HttpHeadersGuard guard, string_view header_name); + static const std::vector &get_forbidden_request_headers(); + static const std::vector &get_forbidden_response_headers(); }; class HttpRequestResponseBase : public Resource { @@ -442,7 +440,7 @@ class HttpRequest : public HttpRequestResponseBase { class HttpIncomingRequest final : public HttpRequest, public HttpIncomingBodyOwner { public: - using RequestHandler = bool (*)(HttpIncomingRequest* request); + using RequestHandler = bool (*)(HttpIncomingRequest *request); HttpIncomingRequest() = delete; explicit HttpIncomingRequest(std::unique_ptr handle); diff --git a/runtime/decode.cpp b/runtime/decode.cpp index 2bd7daf6..076d38cd 100644 --- a/runtime/decode.cpp +++ b/runtime/decode.cpp @@ -2,9 +2,15 @@ namespace core { -JSString* decode(JSContext* cx, string_view str) { +JSString *decode(JSContext *cx, string_view str) { JS::UTF8Chars ret_chars(str.data(), str.length()); return JS_NewStringCopyUTF8N(cx, ret_chars); } +JSString *decode_byte_string(JSContext *cx, string_view str) { + JS::UniqueLatin1Chars chars( + static_cast(std::memcpy(malloc(str.size()), str.data(), str.size()))); + return JS_NewLatin1String(cx, std::move(chars), str.length()); +} + } // namespace core diff --git a/runtime/decode.h b/runtime/decode.h index e473fcd7..73a695cc 100644 --- a/runtime/decode.h +++ b/runtime/decode.h @@ -4,6 +4,7 @@ namespace core { JSString* decode(JSContext *cx, string_view str); +JSString* decode_byte_string(JSContext* cx, string_view str); } // namespace core diff --git a/runtime/encode.cpp b/runtime/encode.cpp index 01184171..2d97cea5 100644 --- a/runtime/encode.cpp +++ b/runtime/encode.cpp @@ -8,6 +8,7 @@ namespace core { +using host_api::HostBytes; using host_api::HostString; HostString encode(JSContext *cx, JS::HandleString str) { @@ -31,6 +32,42 @@ HostString encode(JSContext *cx, JS::HandleValue val) { return encode(cx, str); } +HostString encode_byte_string(JSContext *cx, JS::HandleValue val) { + JS::RootedString str(cx, JS::ToString(cx, val)); + if (!str) { + return HostString{}; + } + size_t length; + if (!JS::StringHasLatin1Chars(str)) { + bool foundBadChar = false; + { + JS::AutoCheckCannotGC nogc; + const char16_t* chars = JS_GetTwoByteStringCharsAndLength(cx, nogc, str, &length); + if (!chars) { + foundBadChar = true; + } + else { + for (size_t i = 0; i < length; i++) { + if (chars[i] > 255) { + foundBadChar = true; + break; + } + } + } + } + if (foundBadChar) { + api::throw_error(cx, core::ByteStringEncodingError); + return host_api::HostString{}; + } + } else { + length = JS::GetStringLength(str); + } + char *buf = static_cast(malloc(length)); + if (!JS_EncodeStringToBuffer(cx, str, buf, length)) + MOZ_ASSERT_UNREACHABLE(); + return HostString(JS::UniqueChars(buf), length); +} + jsurl::SpecString encode_spec_string(JSContext *cx, JS::HandleValue val) { jsurl::SpecString slice(nullptr, 0, 0); auto chars = encode(cx, val); diff --git a/runtime/encode.h b/runtime/encode.h index dd42332c..bf13ddb7 100644 --- a/runtime/encode.h +++ b/runtime/encode.h @@ -2,14 +2,18 @@ #define JS_COMPUTE_RUNTIME_ENCODE_H #include "host_api.h" +#include "builtin.h" namespace core { +DEF_ERR(ByteStringEncodingError, JSEXN_TYPEERR, "Cannot convert JS string into byte string", 0) + // TODO(performance): introduce a version that writes into an existing buffer, // and use that with the hostcall buffer where possible. // https://github.com/fastly/js-compute-runtime/issues/215 host_api::HostString encode(JSContext *cx, JS::HandleString str); host_api::HostString encode(JSContext *cx, JS::HandleValue val); +host_api::HostString encode_byte_string(JSContext *cx, JS::HandleValue val); jsurl::SpecString encode_spec_string(JSContext *cx, JS::HandleValue val); diff --git a/runtime/sequence.hpp b/runtime/sequence.hpp index 82cb9394..358d2151 100644 --- a/runtime/sequence.hpp +++ b/runtime/sequence.hpp @@ -17,7 +17,7 @@ namespace core { * Extract pairs from the given value if it is either a * sequence or a record. */ -template +template bool maybe_consume_sequence_or_record(JSContext *cx, JS::HandleValue initv, JS::HandleObject target, bool *consumed, const char *ctor_name, const char *alt_text = "") { @@ -67,6 +67,10 @@ bool maybe_consume_sequence_or_record(JSContext *cx, JS::HandleValue initv, JS:: if (done) return api::throw_error(cx, api::Errors::InvalidSequence, ctor_name, alt_text); + T validated_key = validate(cx, key, ctor_name); + if (!validated_key) + return false; + // Extract value. if (!entr_iter.next(&value, &done)) return false; @@ -79,28 +83,37 @@ bool maybe_consume_sequence_or_record(JSContext *cx, JS::HandleValue initv, JS:: if (!done) return api::throw_error(cx, api::Errors::InvalidSequence, ctor_name, alt_text); - if (!apply(cx, target, key, value, ctor_name)) + if (!apply(cx, target, std::move(validated_key), value, ctor_name)) return false; } } *consumed = true; } else if (initv.isObject()) { // init isn't an iterator, so if it's an object, it must be a record to be - // valid input. + // valid input, following https://webidl.spec.whatwg.org/#js-record exactly. JS::RootedObject init(cx, &initv.toObject()); JS::RootedIdVector ids(cx); - if (!js::GetPropertyKeys(cx, init, JSITER_OWNONLY | JSITER_SYMBOLS, &ids)) + if (!js::GetPropertyKeys(cx, init, JSITER_OWNONLY | JSITER_HIDDEN | JSITER_SYMBOLS, &ids)) return false; JS::RootedId curId(cx); + + JS::Rooted> desc(cx); + for (size_t i = 0; i < ids.length(); ++i) { curId = ids[i]; key = js::IdToValue(curId); - + if (!JS_GetOwnPropertyDescriptorById(cx, init, curId, &desc)) + return false; + if (desc.isNothing() || !desc->enumerable()) + continue; + // Get call is observable and must come after any value validation + T validated_key = validate(cx, key, ctor_name); + if (!validated_key) + return false; if (!JS_GetPropertyById(cx, init, curId, &value)) return false; - - if (!apply(cx, target, key, value, ctor_name)) + if (!apply(cx, target, std::move(validated_key), value, ctor_name)) return false; } *consumed = true; @@ -111,6 +124,6 @@ bool maybe_consume_sequence_or_record(JSContext *cx, JS::HandleValue initv, JS:: return true; } -} +} // namespace core #endif diff --git a/tests/integration/assert.js b/tests/assert.js similarity index 100% rename from tests/integration/assert.js rename to tests/assert.js diff --git a/tests/e2e/headers/expect_serve_body.txt b/tests/e2e/headers/expect_serve_body.txt new file mode 100644 index 00000000..30d74d25 --- /dev/null +++ b/tests/e2e/headers/expect_serve_body.txt @@ -0,0 +1 @@ +test \ No newline at end of file diff --git a/tests/e2e/headers/expect_serve_headers.txt b/tests/e2e/headers/expect_serve_headers.txt new file mode 100644 index 00000000..de88862b --- /dev/null +++ b/tests/e2e/headers/expect_serve_headers.txt @@ -0,0 +1,8 @@ +HTTP/1.1 200 OK +example-header: Header Value +user-agent: test-agent +content-length: 4 +content-type: text/plain;charset=UTF-8 +set-cookie: A +set-cookie: B +another: A, B diff --git a/tests/e2e/headers/expect_serve_stderr.txt b/tests/e2e/headers/expect_serve_stderr.txt new file mode 100644 index 00000000..e69de29b diff --git a/tests/e2e/headers/headers.js b/tests/e2e/headers/headers.js new file mode 100644 index 00000000..0269f8ed --- /dev/null +++ b/tests/e2e/headers/headers.js @@ -0,0 +1,42 @@ +import { strictEqual, deepStrictEqual, throws } from "../../assert.js"; + +addEventListener("fetch", (evt) => + evt.respondWith( + (async () => { + strictEqual(evt.request.headers.get("EXAMPLE-HEADER"), "Header Value"); + throws( + () => { + evt.request.headers.delete("user-agent"); + }, + TypeError, + "Headers.delete: Headers are immutable" + ); + const response = new Response("test", { + headers: [...evt.request.headers.entries()].filter( + ([name]) => name !== "content-type" && name !== 'content-length' + ), + }); + + response.headers.append("Set-cookie", "A"); + response.headers.append("Another", "A"); + response.headers.append("set-cookie", "B"); + response.headers.append("another", "B"); + + deepStrictEqual( + [...response.headers], + [ + ["accept", "*/*"], + ["another", "A, B"], + ["content-length", "4"], + ["content-type", "text/plain;charset=UTF-8"], + ["example-header", "Header Value"], + ["set-cookie", "A"], + ["set-cookie", "B"], + ["user-agent", "test-agent"], + ] + ); + response.headers.delete("accept"); + return response; + })() + ) +); diff --git a/tests/integration/btoa/btoa.js b/tests/integration/btoa/btoa.js index ed3ac926..140047b3 100644 --- a/tests/integration/btoa/btoa.js +++ b/tests/integration/btoa/btoa.js @@ -1,5 +1,5 @@ import { serveTest } from '../test-server.js'; -import { strictEqual, throws } from '../assert.js'; +import { strictEqual, throws } from '../../assert.js'; export const handler = serveTest(async (t) => { t.test('btoa', () => { diff --git a/tests/integration/crypto/crypto.js b/tests/integration/crypto/crypto.js index f2fc9835..f9123b32 100644 --- a/tests/integration/crypto/crypto.js +++ b/tests/integration/crypto/crypto.js @@ -1,5 +1,5 @@ import { serveTest } from "../test-server.js"; -import { deepStrictEqual, strictEqual, throws, rejects } from "../assert.js"; +import { deepStrictEqual, strictEqual, throws, rejects } from "../../assert.js"; // From https://www.rfc-editor.org/rfc/rfc7517#appendix-A.1 const createPublicRsaJsonWebKeyData = () => ({ diff --git a/tests/integration/fetch/fetch.js b/tests/integration/fetch/fetch.js index 56c6b58e..997976cf 100644 --- a/tests/integration/fetch/fetch.js +++ b/tests/integration/fetch/fetch.js @@ -1,5 +1,5 @@ import { serveTest } from '../test-server.js'; -import { strictEqual, deepStrictEqual, throws } from '../assert.js'; +import { strictEqual, deepStrictEqual, throws } from '../../assert.js'; export const handler = serveTest(async (t) => { await t.test('headers-non-ascii-latin1-field-value', async () => { diff --git a/tests/integration/performance/performance.js b/tests/integration/performance/performance.js index 705a68e4..4b60935a 100644 --- a/tests/integration/performance/performance.js +++ b/tests/integration/performance/performance.js @@ -1,5 +1,5 @@ import { serveTest } from "../test-server.js"; -import { deepStrictEqual, strictEqual, throws } from "../assert.js"; +import { deepStrictEqual, strictEqual, throws } from "../../assert.js"; export const handler = serveTest(async (t) => { t.test("Performance-interface", () => { diff --git a/tests/integration/test-server.js b/tests/integration/test-server.js index 2e75d1f4..41947610 100644 --- a/tests/integration/test-server.js +++ b/tests/integration/test-server.js @@ -1,5 +1,5 @@ import * as handlers from './handlers.js'; -import { AssertionError } from './assert.js'; +import { AssertionError } from '../assert.js'; export function serveTest (handler) { return async function handle (evt) { diff --git a/tests/integration/tests/performance.js b/tests/integration/tests/performance.js deleted file mode 100644 index 2356e89d..00000000 --- a/tests/integration/tests/performance.js +++ /dev/null @@ -1,285 +0,0 @@ -import { serveTest } from "../test-server.js"; -import { deepStrictEqual, strictEqual, throws } from "../assert.js"; - -throw new Error('wat'); - -export const handler = serveTest(async (t) => { - t.test("Performance-interface", () => { - { - let actual = Reflect.ownKeys(Performance); - let expected = ["prototype", "length", "name"]; - deepStrictEqual(actual, expected, `Reflect.ownKeys(Performance)`); - } - - // Check the prototype descriptors are correct - { - let actual = Reflect.getOwnPropertyDescriptor(Performance, "prototype"); - let expected = { - value: Performance.prototype, - writable: false, - enumerable: false, - configurable: false, - }; - deepStrictEqual( - actual, - expected, - `Reflect.getOwnPropertyDescriptor(Performance, 'prototype')` - ); - } - - // Check the constructor function's defined parameter length is correct - { - let actual = Reflect.getOwnPropertyDescriptor(Performance, "length"); - let expected = { - value: 0, - writable: false, - enumerable: false, - configurable: true, - }; - deepStrictEqual( - actual, - expected, - `Reflect.getOwnPropertyDescriptor(Performance, 'length')` - ); - } - - // Check the constructor function's name is correct - { - let actual = Reflect.getOwnPropertyDescriptor(Performance, "name"); - let expected = { - value: "Performance", - writable: false, - enumerable: false, - configurable: true, - }; - deepStrictEqual( - actual, - expected, - `Reflect.getOwnPropertyDescriptor(Performance, 'name')` - ); - } - - // Check the prototype has the correct keys - { - let actual = Reflect.ownKeys(Performance.prototype); - let expected = ["constructor", "timeOrigin", "now", Symbol.toStringTag]; - deepStrictEqual( - actual, - expected, - `Reflect.ownKeys(Performance.prototype)` - ); - } - - // Check the constructor on the prototype is correct - { - let actual = Reflect.getOwnPropertyDescriptor( - Performance.prototype, - "constructor" - ); - let expected = { - writable: true, - enumerable: false, - configurable: true, - value: Performance.prototype.constructor, - }; - deepStrictEqual( - actual, - expected, - `Reflect.getOwnPropertyDescriptor(Performance.prototype, 'constructor')` - ); - - strictEqual( - typeof Performance.prototype.constructor, - "function", - `typeof Performance.prototype.constructor` - ); - } - - { - let actual = Reflect.getOwnPropertyDescriptor( - Performance.prototype.constructor, - "length" - ); - let expected = { - value: 0, - writable: false, - enumerable: false, - configurable: true, - }; - strictEqual( - actual, - expected, - `Reflect.getOwnPropertyDescriptor(Performance.prototype.constructor, 'length')` - ); - } - - { - let actual = Reflect.getOwnPropertyDescriptor( - Performance.prototype.constructor, - "name" - ); - let expected = { - value: "Performance", - writable: false, - enumerable: false, - configurable: true, - }; - strictEqual( - actual, - expected, - `Reflect.getOwnPropertyDescriptor(Performance.prototype.constructor, 'name')` - ); - } - - // Check the Symbol.toStringTag on the prototype is correct - { - let actual = Reflect.getOwnPropertyDescriptor( - Performance.prototype, - Symbol.toStringTag - ); - let expected = { - value: "performance", - writable: false, - enumerable: false, - configurable: true, - }; - strictEqual( - actual, - expected, - `Reflect.getOwnPropertyDescriptor(Performance.prototype, [Symbol.toStringTag])` - ); - - strictEqual( - typeof Performance.prototype[Symbol.toStringTag], - "string", - `typeof Performance.prototype[Symbol.toStringTag]` - ); - } - - // Check the timeOrigin property is correct - { - let descriptors = Reflect.getOwnPropertyDescriptor( - Performance.prototype, - "timeOrigin" - ); - let expected = { enumerable: true, configurable: true }; - strictEqual( - descriptors.enumerable, - true, - `Reflect.getOwnPropertyDescriptor(Performance, 'timeOrigin').enumerable` - ); - strictEqual( - descriptors.configurable, - true, - `Reflect.getOwnPropertyDescriptor(Performance, 'timeOrigin').configurable` - ); - strictEqual( - descriptors.value, - undefined, - `Reflect.getOwnPropertyDescriptor(Performance, 'timeOrigin').value` - ); - strictEqual( - descriptors.set, - undefined, - `Reflect.getOwnPropertyDescriptor(Performance, 'timeOrigin').set` - ); - strictEqual( - typeof descriptors.get, - "function", - `typeof Reflect.getOwnPropertyDescriptor(Performance, 'timeOrigin').get` - ); - - strictEqual( - typeof Performance.prototype.timeOrigin, - "number", - `typeof Performance.prototype.timeOrigin` - ); - } - - // Check the now property is correct - { - let actual = Reflect.getOwnPropertyDescriptor( - Performance.prototype, - "now" - ); - let expected = { - writable: true, - enumerable: true, - configurable: true, - value: Performance.prototype.now, - }; - strictEqual( - actual, - expected, - `Reflect.getOwnPropertyDescriptor(Performance, 'now')` - ); - - strictEqual( - typeof Performance.prototype.now, - "function", - `typeof Performance.prototype.now` - ); - } - - { - let actual = Reflect.getOwnPropertyDescriptor( - Performance.prototype.now, - "length" - ); - let expected = { - value: 0, - writable: false, - enumerable: false, - configurable: true, - }; - strictEqual( - actual, - expected, - `Reflect.getOwnPropertyDescriptor(Performance.prototype.now, 'length')` - ); - } - - { - let actual = Reflect.getOwnPropertyDescriptor( - Performance.prototype.now, - "name" - ); - let expected = { - value: "now", - writable: false, - enumerable: false, - configurable: true, - }; - strictEqual( - actual, - expected, - `Reflect.getOwnPropertyDescriptor(Performance.prototype.now, 'name')` - ); - } - }); - - t.test("globalThis.performance", () => { - strictEqual( - globalThis.performance instanceof Performance, - true, - `globalThis.performance instanceof Performance` - ); - }); - - t.test("globalThis.performance.now", () => { - throws(() => new performance.now()); - strictEqual(typeof performance.now(), "number"); - strictEqual(performance.now() > 0, true); - strictEqual(Number.isNaN(performance.now()), false); - strictEqual(Number.isFinite(performance.now()), true); - strictEqual(performance.now() < Date.now(), true); - }); - - t.test("globalThis.performance.timeOrigin", () => { - strictEqual(typeof performance.timeOrigin, "number"); - strictEqual(performance.timeOrigin > 0, true); - strictEqual(Number.isNaN(performance.timeOrigin), false); - strictEqual(Number.isFinite(performance.timeOrigin), true); - strictEqual(performance.timeOrigin < Date.now(), true); - }); -}); diff --git a/tests/integration/timers/timers.js b/tests/integration/timers/timers.js index 0017282b..051b0203 100644 --- a/tests/integration/timers/timers.js +++ b/tests/integration/timers/timers.js @@ -1,5 +1,5 @@ import { serveTest } from "../test-server.js"; -import { assert, strictEqual, throws, deepStrictEqual, AssertionError } from "../assert.js"; +import { assert, strictEqual, throws, deepStrictEqual, AssertionError } from "../../assert.js"; export const handler = serveTest(async (t) => { await t.asyncTest("clearTimeout invalid", (resolve, reject) => { diff --git a/tests/test.sh b/tests/test.sh index cc8340cd..43f28f5b 100755 --- a/tests/test.sh +++ b/tests/test.sh @@ -11,11 +11,13 @@ wasmtime="${WASMTIME:-wasmtime}" # Load test expectation fails to check, only those defined apply test_serve_body_expectation="$test_dir/expect_serve_body.txt" +test_serve_headers_expectation="$test_dir/expect_serve_headers.txt" test_serve_stdout_expectation="$test_dir/expect_serve_stdout.txt" test_serve_stderr_expectation="$test_dir/expect_serve_stderr.txt" test_serve_status_expectation=$(cat "$test_dir/expect_serve_status.txt" 2> /dev/null || echo "200") body_log="$test_dir/body.log" +headers_log="$test_dir/headers.log" stdout_log="$test_dir/stdout.log" stderr_log="$test_dir/stderr.log" @@ -25,7 +27,7 @@ if [ -z "$test_component" ]; then # Run Wizer set +e - "$test_runtime/componentize.sh" $componentize_flags "$test_dir/$test_name.js" "$test_component" 1> "$stdout_log" 2> "$stderr_log" + PREOPEN_DIR="$(dirname $(dirname "$test_dir"))" "$test_runtime/componentize.sh" $componentize_flags "$test_dir/$test_name.js" "$test_component" 1> "$stdout_log" 2> "$stderr_log" wizer_result=$? set -e @@ -96,16 +98,24 @@ fi port=$(cat "$stderr_log" | head -n 1 | tail -c 7 | head -c 5) -status_code=$(curl --write-out %{http_code} --silent --output "$body_log" "http://localhost:$port/$test_serve_path") +status_code=$(curl -A "test-agent" -H "eXample-hEader: Header Value" --write-out %{http_code} --silent -D "$headers_log" --output "$body_log" "http://localhost:$port/$test_serve_path") if [ ! "$status_code" = "$test_serve_status_expectation" ]; then echo "Bad status code $status_code, expected $test_serve_status_expectation" >&2 cat "$stderr_log" >&2 cat "$stdout_log" + >&2 cat "$headers_log" >&2 cat "$body_log" exit 1 fi +if [ -f "$test_serve_headers_expectation" ]; then + mv "$headers_log" "$headers_log.orig" + cat "$headers_log.orig" | head -n $(cat "$test_serve_headers_expectation" | wc -l) | sed 's/\r//g' > "$headers_log" + rm "$headers_log.orig" + cmp "$headers_log" "$test_serve_headers_expectation" +fi + if [ -f "$test_serve_body_expectation" ]; then cmp "$body_log" "$test_serve_body_expectation" fi @@ -120,6 +130,7 @@ if [ -f "$test_serve_stderr_expectation" ]; then fi rm "$body_log" +rm "$headers_log" rm "$stdout_log" rm "$stderr_log" diff --git a/tests/tests.cmake b/tests/tests.cmake index a863c68e..cf7dc60c 100644 --- a/tests/tests.cmake +++ b/tests/tests.cmake @@ -7,9 +7,9 @@ include("weval") function(test_e2e TEST_NAME) get_target_property(RUNTIME_DIR starling.wasm BINARY_DIR) - add_test(${TEST_NAME} ${BASH_PROGRAM} ${CMAKE_SOURCE_DIR}/tests/test.sh ${RUNTIME_DIR} ${CMAKE_SOURCE_DIR}/tests/e2e/${TEST_NAME}) - set_property(TEST ${TEST_NAME} PROPERTY ENVIRONMENT "WASMTIME=${WASMTIME};WIZER=${WIZER_DIR}/wizer;WASM_TOOLS=${WASM_TOOLS_DIR}/wasm-tools") - set_tests_properties(${TEST_NAME} PROPERTIES TIMEOUT 120) + add_test(e2e-${TEST_NAME} ${BASH_PROGRAM} ${CMAKE_SOURCE_DIR}/tests/test.sh ${RUNTIME_DIR} ${CMAKE_SOURCE_DIR}/tests/e2e/${TEST_NAME}) + set_property(TEST e2e-${TEST_NAME} PROPERTY ENVIRONMENT "WASMTIME=${WASMTIME};WIZER=${WIZER_DIR}/wizer;WASM_TOOLS=${WASM_TOOLS_DIR}/wasm-tools") + set_tests_properties(e2e-${TEST_NAME} PROPERTIES TIMEOUT 120) endfunction() add_custom_target(integration-test-server DEPENDS test-server.wasm) @@ -20,17 +20,18 @@ function(test_integration TEST_NAME) add_custom_command( OUTPUT test-server.wasm WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} - COMMAND ${CMAKE_COMMAND} -E env "WASM_TOOLS=${WASM_TOOLS_DIR}/wasm-tools" env "WIZER=${WIZER_DIR}/wizer" ${RUNTIME_DIR}/componentize.sh ${CMAKE_SOURCE_DIR}/tests/integration/test-server.js test-server.wasm + COMMAND ${CMAKE_COMMAND} -E env "WASM_TOOLS=${WASM_TOOLS_DIR}/wasm-tools" env "WIZER=${WIZER_DIR}/wizer" env "PREOPEN_DIR=${CMAKE_SOURCE_DIR}/tests" ${RUNTIME_DIR}/componentize.sh ${CMAKE_SOURCE_DIR}/tests/integration/test-server.js test-server.wasm DEPENDS ${ARG_SOURCES} ${RUNTIME_DIR}/componentize.sh starling.wasm VERBATIM ) - add_test(${TEST_NAME} ${BASH_PROGRAM} ${CMAKE_SOURCE_DIR}/tests/test.sh ${RUNTIME_DIR} ${CMAKE_SOURCE_DIR}/tests/integration/${TEST_NAME} ${RUNTIME_DIR}/test-server.wasm ${TEST_NAME}) - set_property(TEST ${TEST_NAME} PROPERTY ENVIRONMENT "WASMTIME=${WASMTIME};WIZER=${WIZER_DIR}/wizer;WASM_TOOLS=${WASM_TOOLS_DIR}/wasm-tools") - set_tests_properties(${TEST_NAME} PROPERTIES TIMEOUT 120) + add_test(integration-${TEST_NAME} ${BASH_PROGRAM} ${CMAKE_SOURCE_DIR}/tests/test.sh ${RUNTIME_DIR} ${CMAKE_SOURCE_DIR}/tests/integration/${TEST_NAME} ${RUNTIME_DIR}/test-server.wasm ${TEST_NAME}) + set_property(TEST integration-${TEST_NAME} PROPERTY ENVIRONMENT "WASMTIME=${WASMTIME};WIZER=${WIZER_DIR}/wizer;WASM_TOOLS=${WASM_TOOLS_DIR}/wasm-tools") + set_tests_properties(integration-${TEST_NAME} PROPERTIES TIMEOUT 120) endfunction() test_e2e(smoke) +test_e2e(headers) test_e2e(tla) test_e2e(syntax-err) test_e2e(tla-err) diff --git a/tests/wpt-harness/expectations/fetch/api/basic/request-headers-nonascii.any.js.json b/tests/wpt-harness/expectations/fetch/api/basic/request-headers-nonascii.any.js.json index a320d3ba..d06c8d5d 100644 --- a/tests/wpt-harness/expectations/fetch/api/basic/request-headers-nonascii.any.js.json +++ b/tests/wpt-harness/expectations/fetch/api/basic/request-headers-nonascii.any.js.json @@ -1,5 +1,5 @@ { "Non-ascii bytes in request headers": { - "status": "FAIL" + "status": "PASS" } } \ No newline at end of file diff --git a/tests/wpt-harness/expectations/fetch/api/headers/header-setcookie.any.js.json b/tests/wpt-harness/expectations/fetch/api/headers/header-setcookie.any.js.json index 4a04a604..01838e37 100644 --- a/tests/wpt-harness/expectations/fetch/api/headers/header-setcookie.any.js.json +++ b/tests/wpt-harness/expectations/fetch/api/headers/header-setcookie.any.js.json @@ -3,34 +3,34 @@ "status": "PASS" }, "Headers iterator does not combine set-cookie headers": { - "status": "FAIL" + "status": "PASS" }, "Headers iterator does not special case set-cookie2 headers": { "status": "PASS" }, "Headers iterator does not combine set-cookie & set-cookie2 headers": { - "status": "FAIL" + "status": "PASS" }, "Headers iterator preserves set-cookie ordering": { - "status": "FAIL" + "status": "PASS" }, "Headers iterator preserves per header ordering, but sorts keys alphabetically": { - "status": "FAIL" + "status": "PASS" }, "Headers iterator preserves per header ordering, but sorts keys alphabetically (and ignores value ordering)": { - "status": "FAIL" + "status": "PASS" }, "Headers iterator is correctly updated with set-cookie changes": { - "status": "FAIL" + "status": "PASS" }, "Headers iterator is correctly updated with set-cookie changes #2": { - "status": "FAIL" + "status": "PASS" }, "Headers.prototype.has works for set-cookie": { "status": "PASS" }, "Headers.prototype.append works for set-cookie": { - "status": "FAIL" + "status": "PASS" }, "Headers.prototype.set works for set-cookie": { "status": "PASS" @@ -39,31 +39,31 @@ "status": "PASS" }, "Headers.prototype.getSetCookie with no headers present": { - "status": "FAIL" + "status": "PASS" }, "Headers.prototype.getSetCookie with one header": { - "status": "FAIL" + "status": "PASS" }, "Headers.prototype.getSetCookie with one header created from an object": { - "status": "FAIL" + "status": "PASS" }, "Headers.prototype.getSetCookie with multiple headers": { - "status": "FAIL" + "status": "PASS" }, "Headers.prototype.getSetCookie with an empty header": { - "status": "FAIL" + "status": "PASS" }, "Headers.prototype.getSetCookie with two equal headers": { - "status": "FAIL" + "status": "PASS" }, "Headers.prototype.getSetCookie ignores set-cookie2 headers": { - "status": "FAIL" + "status": "PASS" }, "Headers.prototype.getSetCookie preserves header ordering": { - "status": "FAIL" + "status": "PASS" }, "Adding Set-Cookie headers normalizes their value": { - "status": "FAIL" + "status": "PASS" }, "Adding invalid Set-Cookie headers throws": { "status": "PASS" diff --git a/tests/wpt-harness/expectations/fetch/api/headers/headers-basic.any.js.json b/tests/wpt-harness/expectations/fetch/api/headers/headers-basic.any.js.json index 4fd7a460..2b7edf48 100644 --- a/tests/wpt-harness/expectations/fetch/api/headers/headers-basic.any.js.json +++ b/tests/wpt-harness/expectations/fetch/api/headers/headers-basic.any.js.json @@ -42,30 +42,30 @@ "status": "PASS" }, "Check keys method": { - "status": "FAIL" + "status": "PASS" }, "Check values method": { - "status": "FAIL" + "status": "PASS" }, "Check entries method": { - "status": "FAIL" + "status": "PASS" }, "Check Symbol.iterator method": { - "status": "FAIL" + "status": "PASS" }, "Check forEach method": { - "status": "FAIL" + "status": "PASS" }, "Iteration skips elements removed while iterating": { - "status": "FAIL" + "status": "PASS" }, "Removing elements already iterated over causes an element to be skipped during iteration": { - "status": "FAIL" + "status": "PASS" }, "Appending a value pair during iteration causes it to be reached during iteration": { - "status": "FAIL" + "status": "PASS" }, "Prepending a value pair before the current element position causes it to be skipped during iteration and adds the current element a second time": { - "status": "FAIL" + "status": "PASS" } } \ No newline at end of file diff --git a/tests/wpt-harness/expectations/fetch/api/headers/headers-combine.any.js.json b/tests/wpt-harness/expectations/fetch/api/headers/headers-combine.any.js.json index d65fb255..d7c07645 100644 --- a/tests/wpt-harness/expectations/fetch/api/headers/headers-combine.any.js.json +++ b/tests/wpt-harness/expectations/fetch/api/headers/headers-combine.any.js.json @@ -15,6 +15,6 @@ "status": "PASS" }, "Iterate combined values in sorted order": { - "status": "FAIL" + "status": "PASS" } } \ No newline at end of file diff --git a/tests/wpt-harness/expectations/fetch/api/headers/headers-errors.any.js.json b/tests/wpt-harness/expectations/fetch/api/headers/headers-errors.any.js.json index 26cc9e15..861575d4 100644 --- a/tests/wpt-harness/expectations/fetch/api/headers/headers-errors.any.js.json +++ b/tests/wpt-harness/expectations/fetch/api/headers/headers-errors.any.js.json @@ -9,7 +9,7 @@ "status": "PASS" }, "Create headers giving bad header value as init argument": { - "status": "FAIL" + "status": "PASS" }, "Check headers get with an invalid name invalidĀ": { "status": "PASS" @@ -36,7 +36,7 @@ "status": "PASS" }, "Check headers set with an invalid value invalidĀ": { - "status": "FAIL" + "status": "PASS" }, "Check headers append with an invalid name invalidĀ": { "status": "PASS" @@ -45,7 +45,7 @@ "status": "PASS" }, "Check headers append with an invalid value invalidĀ": { - "status": "FAIL" + "status": "PASS" }, "Headers forEach throws if argument is not callable": { "status": "PASS" diff --git a/tests/wpt-harness/expectations/fetch/api/headers/headers-record.any.js.json b/tests/wpt-harness/expectations/fetch/api/headers/headers-record.any.js.json index ef052831..8daf15c7 100644 --- a/tests/wpt-harness/expectations/fetch/api/headers/headers-record.any.js.json +++ b/tests/wpt-harness/expectations/fetch/api/headers/headers-record.any.js.json @@ -15,27 +15,27 @@ "status": "PASS" }, "Correct operation ordering with two properties": { - "status": "FAIL" + "status": "PASS" }, "Correct operation ordering with two properties one of which has an invalid name": { - "status": "FAIL" + "status": "PASS" }, "Correct operation ordering with two properties one of which has an invalid value": { - "status": "FAIL" + "status": "PASS" }, "Correct operation ordering with non-enumerable properties": { - "status": "FAIL" + "status": "PASS" }, "Correct operation ordering with undefined descriptors": { - "status": "FAIL" + "status": "PASS" }, "Correct operation ordering with repeated keys": { "status": "PASS" }, "Basic operation with Symbol keys": { - "status": "FAIL" + "status": "PASS" }, "Operation with non-enumerable Symbol keys": { - "status": "FAIL" + "status": "PASS" } } \ No newline at end of file diff --git a/tests/wpt-harness/expectations/fetch/api/request/request-headers.any.js.json b/tests/wpt-harness/expectations/fetch/api/request/request-headers.any.js.json index bddd1281..98c21422 100644 --- a/tests/wpt-harness/expectations/fetch/api/request/request-headers.any.js.json +++ b/tests/wpt-harness/expectations/fetch/api/request/request-headers.any.js.json @@ -63,7 +63,7 @@ "status": "FAIL" }, "Adding invalid request header \"Host: KO\"": { - "status": "FAIL" + "status": "PASS" }, "Adding invalid request header \"Keep-Alive: KO\"": { "status": "FAIL" diff --git a/tests/wpt-harness/expectations/fetch/api/response/response-headers-guard.any.js.json b/tests/wpt-harness/expectations/fetch/api/response/response-headers-guard.any.js.json index a7dde64c..3091327d 100644 --- a/tests/wpt-harness/expectations/fetch/api/response/response-headers-guard.any.js.json +++ b/tests/wpt-harness/expectations/fetch/api/response/response-headers-guard.any.js.json @@ -1,5 +1,5 @@ { "Ensure response headers are immutable": { - "status": "FAIL" + "status": "PASS" } } \ No newline at end of file diff --git a/tests/wpt-harness/tests.json b/tests/wpt-harness/tests.json index 0253e388..8032f813 100644 --- a/tests/wpt-harness/tests.json +++ b/tests/wpt-harness/tests.json @@ -65,7 +65,6 @@ "fetch/api/basic/mode-same-origin.any.js", "fetch/api/basic/referrer.any.js", "fetch/api/basic/referrer.any.js", - "fetch/api/basic/request-forbidden-headers.any.js", "fetch/api/basic/request-head.any.js", "fetch/api/basic/request-headers-case.any.js", "fetch/api/basic/request-headers-nonascii.any.js", @@ -111,6 +110,7 @@ "fetch/api/redirect/redirect-origin.any.js", "fetch/api/redirect/redirect-referrer-override.any.js", "fetch/api/redirect/redirect-referrer.any.js", + "fetch/api/basic/request-forbidden-headers.any.js", "fetch/api/redirect/redirect-schemes.any.js", "fetch/api/redirect/redirect-to-dataurl.any.js", "fetch/api/redirect/redirect-upload.h2.any.js", diff --git a/tests/wpt-harness/wpt.cmake b/tests/wpt-harness/wpt.cmake index 26a182c1..8b676d4e 100644 --- a/tests/wpt-harness/wpt.cmake +++ b/tests/wpt-harness/wpt.cmake @@ -10,7 +10,7 @@ else() endif() if(DEFINED ENV{WPT_ROOT}) - set(WPT_ROOT ENV{WPT_ROOT}) + set(WPT_ROOT $ENV{WPT_ROOT}) else() CPMAddPackage( NAME wpt-suite From 4e91e5c61accc0b122deeb8fe7fb432801496494 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 16 Aug 2024 23:59:46 +0200 Subject: [PATCH 11/12] ci: add WPT debug build, fix streams assertion failure (#113) --- .github/workflows/main.yml | 2 +- builtins/web/streams/transform-stream.cpp | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index d883aff5..10eb3619 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -76,7 +76,7 @@ jobs: name: Web Platform Tests strategy: matrix: - build: [release] + build: [release, debug] runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 diff --git a/builtins/web/streams/transform-stream.cpp b/builtins/web/streams/transform-stream.cpp index 73884e28..f5318979 100644 --- a/builtins/web/streams/transform-stream.cpp +++ b/builtins/web/streams/transform-stream.cpp @@ -280,6 +280,9 @@ void TransformStream::set_owner(JSObject *self, JSObject *owner) { JSObject *TransformStream::readable(JSObject *self) { MOZ_ASSERT(is_instance(self)); + if (!JS::GetReservedSlot(self, TransformStream::Slots::Readable).isObject()) { + return nullptr; + } return &JS::GetReservedSlot(self, TransformStream::Slots::Readable).toObject(); } @@ -323,6 +326,9 @@ void TransformStream::set_readable_used_as_body(JSContext *cx, JS::HandleObject JSObject *TransformStream::writable(JSObject *self) { MOZ_ASSERT(is_instance(self)); + if (!JS::GetReservedSlot(self, TransformStream::Slots::Writable).isObject()) { + return nullptr; + } return &JS::GetReservedSlot(self, TransformStream::Slots::Writable).toObject(); } @@ -372,13 +378,21 @@ void TransformStream::set_used_as_mixin(JSObject *self) { bool TransformStream::readable_get(JSContext *cx, unsigned argc, JS::Value *vp) { METHOD_HEADER_WITH_NAME(0, "get readable") - args.rval().setObject(*readable(self)); + auto readable_val = readable(self); + // TODO: determine why this isn't being set in some cases + if (readable_val) { + args.rval().setObject(*readable_val); + } return true; } bool TransformStream::writable_get(JSContext *cx, unsigned argc, JS::Value *vp) { METHOD_HEADER_WITH_NAME(0, "get writable") - args.rval().setObject(*writable(self)); + auto writable_val = writable(self); + // TODO: determine why this isn't being set in some cases + if (writable_val) { + args.rval().setObject(*writable_val); + } return true; } From 94104bfd3e3f823b8547ed281a2d33365057fc3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Istv=C3=A1n=20B=C3=ADr=C3=B3?= Date: Mon, 26 Aug 2024 09:58:53 +0200 Subject: [PATCH 12/12] fix assert import --- tests/integration/fetchsync/fetchsync.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/fetchsync/fetchsync.js b/tests/integration/fetchsync/fetchsync.js index c99fd063..f8f46785 100644 --- a/tests/integration/fetchsync/fetchsync.js +++ b/tests/integration/fetchsync/fetchsync.js @@ -1,5 +1,5 @@ import {serveTest} from "../test-server.js"; -import {assert, strictEqual, throws, deepStrictEqual} from "../assert.js"; +import {assert, strictEqual, throws, deepStrictEqual} from "../../assert.js"; export const handler = serveTest(async (t) => { t.test("fetch - json - small", () => {