From 780067d429dbd90bd529f42169c2c1af6c139bb7 Mon Sep 17 00:00:00 2001 From: Jake Champion Date: Tue, 25 Apr 2023 20:12:53 +0100 Subject: [PATCH] feat: implement Response.json static method (#499) Co-authored-by: Trevor Elliott --- .github/workflows/main.yml | 2 + .../builtins/request-response.cpp | 154 +++++++++++++++++- .../builtins/request-response.h | 4 +- .../js-compute-runtime/error-numbers.msg | 2 + .../fixtures/response-json/bin/index.js | 85 ++++++++++ .../fixtures/response-json/fastly.toml.in | 12 ++ .../fixtures/response-json/tests.json | 12 ++ .../fixtures/response-redirect/bin/index.js | 2 +- .../bin => }/test-harness.js | 2 +- .../response/response-static-json.any.js.json | 41 +++++ tests/wpt-harness/tests.json | 1 + 11 files changed, 313 insertions(+), 4 deletions(-) create mode 100644 integration-tests/js-compute/fixtures/response-json/bin/index.js create mode 100644 integration-tests/js-compute/fixtures/response-json/fastly.toml.in create mode 100644 integration-tests/js-compute/fixtures/response-json/tests.json rename integration-tests/js-compute/{fixtures/response-redirect/bin => }/test-harness.js (95%) create mode 100644 tests/wpt-harness/expectations/fetch/api/response/response-static-json.any.js.json diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index d1a51709e9..39cdc5ce46 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -248,6 +248,7 @@ jobs: - request-upstream - response - response-headers + - response-json - response-redirect - secret-store - status @@ -396,6 +397,7 @@ jobs: - 'request-upstream' - 'response' - 'response-headers' + - 'response-json' - 'response-redirect' - 'secret-store' - 'status' diff --git a/c-dependencies/js-compute-runtime/builtins/request-response.cpp b/c-dependencies/js-compute-runtime/builtins/request-response.cpp index 4fc67b135b..4395a0f15c 100644 --- a/c-dependencies/js-compute-runtime/builtins/request-response.cpp +++ b/c-dependencies/js-compute-runtime/builtins/request-response.cpp @@ -2373,7 +2373,7 @@ bool Response::redirect(JSContext *cx, unsigned argc, JS::Value *vp) { if (!headers) { return false; } - if (!builtins::Headers::maybe_add(cx, headers, "Location", value)) { + if (!builtins::Headers::maybe_add(cx, headers, "location", value)) { return false; } JS::SetReservedSlot(response, static_cast(Slots::Headers), JS::ObjectValue(*headers)); @@ -2384,8 +2384,160 @@ bool Response::redirect(JSContext *cx, unsigned argc, JS::Value *vp) { return true; } +namespace { +bool callbackCalled; +bool write_json_to_buf(const char16_t *str, uint32_t strlen, void *out) { + callbackCalled = true; + auto outstr = static_cast(out); + outstr->append(str, strlen); + + return true; +} +} // namespace + +bool Response::json(JSContext *cx, unsigned argc, JS::Value *vp) { + JS::CallArgs args = JS::CallArgsFromVp(argc, vp); + if (!args.requireAtLeast(cx, "json", 1)) { + return false; + } + JS::RootedValue data(cx, args.get(0)); + JS::RootedValue init_val(cx, args.get(1)); + JS::RootedObject replacer(cx); + JS::RootedValue space(cx); + + std::u16string out; + // 1. Let bytes the result of running serialize a JavaScript value to JSON bytes on data. + callbackCalled = false; + if (!JS::ToJSON(cx, data, replacer, space, &write_json_to_buf, &out)) { + return false; + } + if (!callbackCalled) { + JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_RESPONSE_JSON_INVALID_VALUE); + return false; + } + // 2. Let body be the result of extracting bytes. + + // 3. Let responseObject be the result of creating a Response object, given a new response, + // "response", and this’s relevant Realm. + JS::RootedValue status_val(cx); + uint16_t status = 200; + + JS::RootedValue statusText_val(cx); + JS::RootedString statusText(cx, JS_GetEmptyString(cx)); + JS::RootedValue headers_val(cx); + + if (init_val.isObject()) { + JS::RootedObject init(cx, init_val.toObjectOrNull()); + if (!JS_GetProperty(cx, init, "status", &status_val) || + !JS_GetProperty(cx, init, "statusText", &statusText_val) || + !JS_GetProperty(cx, init, "headers", &headers_val)) { + return false; + } + + if (!status_val.isUndefined() && !JS::ToUint16(cx, status_val, &status)) { + return false; + } + + if (status == 204 || status == 205 || status == 304) { + JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, + JSMSG_RESPONSE_NULL_BODY_STATUS_WITH_BODY); + return false; + } + + if (!statusText_val.isUndefined() && !(statusText = JS::ToString(cx, statusText_val))) { + return false; + } + + } else if (!init_val.isNullOrUndefined()) { + JS_ReportErrorLatin1(cx, "Response constructor: |init| parameter can't be converted to " + "a dictionary"); + return false; + } + + fastly_response_handle_t response_handle = INVALID_HANDLE; + fastly_error_t err; + if (!fastly_http_resp_new(&response_handle, &err)) { + HANDLE_ERROR(cx, err); + return false; + } + if (response_handle == INVALID_HANDLE) { + return false; + } + + auto make_res = HttpBody::make(); + if (auto *err = make_res.to_err()) { + HANDLE_ERROR(cx, *err); + return false; + } + + auto body = make_res.unwrap(); + JS::RootedString string(cx, JS_NewUCStringCopyN(cx, out.c_str(), out.length())); + size_t encoded_len; + auto stringChars = encode(cx, string, &encoded_len); + + auto write_res = body.write_all(reinterpret_cast(stringChars.get()), encoded_len); + if (auto *err = write_res.to_err()) { + HANDLE_ERROR(cx, *err); + return false; + } + JS::RootedObject response_instance(cx, JS_NewObjectWithGivenProto(cx, &builtins::Response::class_, + builtins::Response::proto_obj)); + if (!response_instance) { + return false; + } + JS::RootedObject response(cx, create(cx, response_instance, response_handle, body.handle, false)); + if (!response) { + return false; + } + + // Set `this`’s `response`’s `status` to `init`["status"]. + if (!fastly_http_resp_status_set(response_handle, status, &err)) { + HANDLE_ERROR(cx, err); + return false; + } + // To ensure that we really have the same status value as the host, + // we always read it back here. + if (!fastly_http_resp_status_get(response_handle, &status, &err)) { + HANDLE_ERROR(cx, err); + return false; + } + + JS::SetReservedSlot(response, static_cast(Slots::Status), JS::Int32Value(status)); + + // Set `this`’s `response`’s `status message` to `init`["statusText"]. + JS::SetReservedSlot(response, static_cast(Slots::StatusMessage), + JS::StringValue(statusText)); + + // If `init`["headers"] `exists`, then `fill` `this`’s `headers` with + // `init`["headers"]. + JS::RootedObject headers(cx); + JS::RootedObject headersInstance( + cx, JS_NewObjectWithGivenProto(cx, &builtins::Headers::class_, builtins::Headers::proto_obj)); + if (!headersInstance) + return false; + + headers = builtins::Headers::create(cx, headersInstance, builtins::Headers::Mode::ProxyToResponse, + response, headers_val); + if (!headers) { + return false; + } + // 4. Perform initialize a response given responseObject, init, and (body, "application/json"). + if (!builtins::Headers::maybe_add(cx, headers, "content-type", "application/json")) { + return false; + } + JS::SetReservedSlot(response, static_cast(Slots::Headers), JS::ObjectValue(*headers)); + JS::SetReservedSlot(response, static_cast(Slots::Redirected), JS::FalseValue()); + JS::SetReservedSlot(response, static_cast(Slots::HasBody), JS::TrueValue()); + RequestOrResponse::set_url(response, JS_GetEmptyStringValue(cx)); + + // 5. Return responseObject. + args.rval().setObjectOrNull(response); + return true; +} + const JSFunctionSpec Response::static_methods[] = { JS_FN("redirect", redirect, 1, JSPROP_ENUMERATE), + JS_FN("json", json, 1, JSPROP_ENUMERATE), JS_FS_END, }; diff --git a/c-dependencies/js-compute-runtime/builtins/request-response.h b/c-dependencies/js-compute-runtime/builtins/request-response.h index e48bc3f5c6..46b84d42bc 100644 --- a/c-dependencies/js-compute-runtime/builtins/request-response.h +++ b/c-dependencies/js-compute-runtime/builtins/request-response.h @@ -179,6 +179,9 @@ class Response final : public BuiltinImpl { static bool body_get(JSContext *cx, unsigned argc, JS::Value *vp); static bool bodyUsed_get(JSContext *cx, unsigned argc, JS::Value *vp); + static bool redirect(JSContext *cx, unsigned argc, JS::Value *vp); + static bool json(JSContext *cx, unsigned argc, JS::Value *vp); + public: static constexpr const char *class_name = "Response"; @@ -205,7 +208,6 @@ class Response final : public BuiltinImpl { static bool init_class(JSContext *cx, JS::HandleObject global); static bool constructor(JSContext *cx, unsigned argc, JS::Value *vp); - static bool redirect(JSContext *cx, unsigned argc, JS::Value *vp); static JSObject *create(JSContext *cx, JS::HandleObject response, fastly_response_handle_t response_handle, fastly_body_handle_t body_handle, bool is_upstream); diff --git a/c-dependencies/js-compute-runtime/error-numbers.msg b/c-dependencies/js-compute-runtime/error-numbers.msg index afcd729057..05a49af9cd 100644 --- a/c-dependencies/js-compute-runtime/error-numbers.msg +++ b/c-dependencies/js-compute-runtime/error-numbers.msg @@ -105,4 +105,6 @@ MSG_DEF(JSMSG_SUBTLE_CRYPTO_INVALID_JWK_KTY_VALUE, 1, JSEXN_ERR, "Th MSG_DEF(JSMSG_SUBTLE_CRYPTO_INVALID_KEY_USAGES_VALUE, 0, JSEXN_TYPEERR, "Invalid keyUsages argument") MSG_DEF(JSMSG_RESPONSE_REDIRECT_INVALID_URI, 0, JSEXN_TYPEERR, "Response.redirect: url parameter is not a valid URL.") MSG_DEF(JSMSG_RESPONSE_REDIRECT_INVALID_STATUS, 0, JSEXN_RANGEERR, "Response.redirect: Invalid redirect status code.") +MSG_DEF(JSMSG_RESPONSE_NULL_BODY_STATUS_WITH_BODY, 0, JSEXN_TYPEERR, "Response with null body status cannot have body") +MSG_DEF(JSMSG_RESPONSE_JSON_INVALID_VALUE, 0, JSEXN_TYPEERR, "Redirect.json: The data is not JSON serializable") //clang-format on \ No newline at end of file diff --git a/integration-tests/js-compute/fixtures/response-json/bin/index.js b/integration-tests/js-compute/fixtures/response-json/bin/index.js new file mode 100644 index 0000000000..e892778ea0 --- /dev/null +++ b/integration-tests/js-compute/fixtures/response-json/bin/index.js @@ -0,0 +1,85 @@ +/* eslint-env serviceworker */ +import { pass, assert, assertThrows } from "../../../assertions.js"; +import { routes } from "../../../test-harness.js"; + +let error; +routes.set("/response/json", async () => { + const APPLICATION_JSON = "application/json"; + const FOO_BAR = "foo/bar"; + + const INIT_TESTS = [ + [undefined, 200, "", APPLICATION_JSON, {}], + [{ status: 400 }, 400, "", APPLICATION_JSON, {}], + [{ statusText: "foo" }, 200, "foo", APPLICATION_JSON, {}], + [{ headers: {} }, 200, "", APPLICATION_JSON, {}], + [{ headers: { "content-type": FOO_BAR } }, 200, "", FOO_BAR, {}], + [{ headers: { "x-foo": "bar" } }, 200, "", APPLICATION_JSON, { "x-foo": "bar" }], + ]; + + for (const [init, expectedStatus, expectedStatusText, expectedContentType, expectedHeaders] of INIT_TESTS) { + const response = Response.json("hello world", init); + error = assert(response.type, "default", 'response.type'); + if (error) { return error; } + error = assert(response.status, expectedStatus, 'response.status'); + if (error) { return error; } + error = assert(response.statusText, expectedStatusText, 'response.statusText'); + if (error) { return error; } + error = assert(response.headers.get("content-type"), expectedContentType, 'response.headers.get("content-type")'); + if (error) { return error; } + for (const key in expectedHeaders) { + error = assert(response.headers.get(key), expectedHeaders[key], 'response.headers.get(key)'); + if (error) { return error; } + } + const data = await response.json(); + error = assert(data, "hello world", 'data'); + if (error) { return error; } + } + + const nullBodyStatus = [204, 205, 304]; + for (const status of nullBodyStatus) { + error = assertThrows( + function () { + Response.json("hello world", { status: status }); + }, + TypeError, + ); + if (error) { return error; } + } + + const response = Response.json({ foo: "bar" }); + const data = await response.json(); + error = assert(typeof data, "object", 'typeof data'); + if (error) { return error; } + error = assert(data.foo, "bar", "data.foo"); + if (error) { return error; } + + error = assertThrows( + function () { + Response.json(Symbol("foo")); + }, + TypeError + ); + if (error) { return error; } + + const a = { b: 1 }; + a.a = a; + error = assertThrows( + function () { + Response.json(a); + }, + TypeError, + ); + if (error) { return error; } + + class CustomError extends Error { + name = "CustomError"; + } + error = assertThrows( + function () { + Response.json({ get foo() { throw new CustomError("bar") } }); + }, + CustomError, + ) + if (error) { return error; } + return pass() +}); diff --git a/integration-tests/js-compute/fixtures/response-json/fastly.toml.in b/integration-tests/js-compute/fixtures/response-json/fastly.toml.in new file mode 100644 index 0000000000..4fe579e791 --- /dev/null +++ b/integration-tests/js-compute/fixtures/response-json/fastly.toml.in @@ -0,0 +1,12 @@ +# This file describes a Fastly Compute@Edge package. To learn more visit: +# https://developer.fastly.com/reference/fastly-toml/ + +authors = ["jchampion@fastly.com"] +description = "" +language = "other" +manifest_version = 2 +name = "response-json" +service_id = "" + +[scripts] + build = "node ../../../../js-compute-runtime-cli.js" diff --git a/integration-tests/js-compute/fixtures/response-json/tests.json b/integration-tests/js-compute/fixtures/response-json/tests.json new file mode 100644 index 0000000000..c618701759 --- /dev/null +++ b/integration-tests/js-compute/fixtures/response-json/tests.json @@ -0,0 +1,12 @@ +{ + "GET /response/json": { + "environments": ["viceroy", "c@e"], + "downstream_request": { + "method": "GET", + "pathname": "/response/json" + }, + "downstream_response": { + "status": 200 + } + } +} diff --git a/integration-tests/js-compute/fixtures/response-redirect/bin/index.js b/integration-tests/js-compute/fixtures/response-redirect/bin/index.js index 420e8084ed..d6bdf6e752 100644 --- a/integration-tests/js-compute/fixtures/response-redirect/bin/index.js +++ b/integration-tests/js-compute/fixtures/response-redirect/bin/index.js @@ -1,6 +1,6 @@ /* eslint-env serviceworker */ import { pass, assert, assertThrows } from "../../../assertions.js"; -import { routes } from "./test-harness.js"; +import { routes } from "../../../test-harness.js"; routes.set("/response/redirect", async () => { const url = "http://test.url:1234/"; diff --git a/integration-tests/js-compute/fixtures/response-redirect/bin/test-harness.js b/integration-tests/js-compute/test-harness.js similarity index 95% rename from integration-tests/js-compute/fixtures/response-redirect/bin/test-harness.js rename to integration-tests/js-compute/test-harness.js index 9108df99e1..272a5f1f6f 100644 --- a/integration-tests/js-compute/fixtures/response-redirect/bin/test-harness.js +++ b/integration-tests/js-compute/test-harness.js @@ -1,6 +1,6 @@ /* eslint-env serviceworker */ import { env } from 'fastly:env'; -import { fail } from "../../../assertions.js"; +import { fail } from "./assertions.js"; addEventListener("fetch", event => { event.respondWith(app(event)) diff --git a/tests/wpt-harness/expectations/fetch/api/response/response-static-json.any.js.json b/tests/wpt-harness/expectations/fetch/api/response/response-static-json.any.js.json new file mode 100644 index 0000000000..c2ea58a4a9 --- /dev/null +++ b/tests/wpt-harness/expectations/fetch/api/response/response-static-json.any.js.json @@ -0,0 +1,41 @@ +{ + "Check response returned by static json() with init undefined": { + "status": "PASS" + }, + "Check response returned by static json() with init {\"status\":400}": { + "status": "PASS" + }, + "Check response returned by static json() with init {\"statusText\":\"foo\"}": { + "status": "PASS" + }, + "Check response returned by static json() with init {\"headers\":{}}": { + "status": "PASS" + }, + "Check response returned by static json() with init {\"headers\":{\"content-type\":\"foo/bar\"}}": { + "status": "PASS" + }, + "Check response returned by static json() with init {\"headers\":{\"x-foo\":\"bar\"}}": { + "status": "PASS" + }, + "Throws TypeError when calling static json() with a status of 204": { + "status": "PASS" + }, + "Throws TypeError when calling static json() with a status of 205": { + "status": "PASS" + }, + "Throws TypeError when calling static json() with a status of 304": { + "status": "PASS" + }, + "Check static json() encodes JSON objects correctly": { + "status": "PASS" + }, + "Check static json() throws when data is not encodable": { + "status": "PASS" + }, + "Check static json() throws when data is circular": { + "status": "PASS" + }, + "Check static json() propagates JSON serializer errors": { + "status": "PASS" + } +} \ No newline at end of file diff --git a/tests/wpt-harness/tests.json b/tests/wpt-harness/tests.json index fe4bb78e09..f443c95886 100644 --- a/tests/wpt-harness/tests.json +++ b/tests/wpt-harness/tests.json @@ -97,6 +97,7 @@ "fetch/api/response/response-init-002.any.js", "fetch/api/response/response-init-contenttype.any.js", "fetch/api/response/response-static-error.any.js", + "fetch/api/response/response-static-json.any.js", "fetch/api/response/response-static-redirect.any.js", "fetch/api/response/response-stream-disturbed-1.any.js", "fetch/api/response/response-stream-disturbed-2.any.js",