Skip to content

Commit c102521

Browse files
author
Guy Bedford
authored
feat: support getSetCookie on new StarlingMonkey headers implementation (#844)
1 parent 9adfa4b commit c102521

20 files changed

+689
-1378
lines changed

‎integration-tests/js-compute/compare-headers.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ const compareHeaders = (configHeaders, wasmModuleHeaders) => {
6161
}
6262
else if (wasmModuleHeaderValue !== configHeaderValue) {
6363
throw new Error(
64-
`[Header Value mismatch] Expected: '${configHeaderKey}: ${configHeaderValue}' (${configHeaderValue.length}), got '${configHeaderKey}: ${wasmModuleHeaderValue}' (${wasmModuleHeaderValue.length})`
64+
`[Header Value mismatch] Expected: '${configHeaderKey}: ${configHeaderValue}' (${configHeaderValue.length}), got '${configHeaderKey}: ${wasmModuleHeaderValue}' (${wasmModuleHeaderValue?.length})`
6565
);
6666
}
6767
}

‎integration-tests/js-compute/fixtures/app/src/headers.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,13 @@
22

33
import { routes } from "./routes.js";
44
import { pass, assert } from "./assertions.js";
5+
import { CacheOverride } from "fastly:cache-override";
6+
7+
routes.set("/headers/construct", async () => {
8+
const headers = new Headers()
9+
headers.set('foo', 'bar')
10+
return new Response("check headers", { headers })
11+
})
512

613
routes.set("/headers/non-ascii-latin1-field-value", async () => {
714
let response = await fetch("https://http-me.glitch.me/meow?header=cat:é", {
@@ -15,3 +22,32 @@ routes.set("/headers/non-ascii-latin1-field-value", async () => {
1522
if (error) { return error }
1623
return pass("ok")
1724
})
25+
26+
routes.set("/headers/from-response/set", async () => {
27+
const response = await fetch("https://httpbin.org/stream-bytes/11", {
28+
backend: "httpbin",
29+
cacheOverride: new CacheOverride('pass')
30+
});
31+
response.headers.set("cuStom", "test")
32+
return response;
33+
})
34+
35+
routes.set("/headers/from-response/delete-invalid", async () => {
36+
const response = await fetch("https://httpbin.org/stream-bytes/11", {
37+
backend: "httpbin",
38+
cacheOverride: new CacheOverride('pass')
39+
});
40+
response.headers.delete("none")
41+
return response;
42+
})
43+
44+
45+
routes.set("/headers/from-response/set-delete", async () => {
46+
const response = await fetch("https://httpbin.org/stream-bytes/11", {
47+
backend: "httpbin",
48+
cacheOverride: new CacheOverride('pass')
49+
});
50+
response.headers.set("custom", "test")
51+
response.headers.delete("access-control-allow-origin")
52+
return response;
53+
})

‎integration-tests/js-compute/fixtures/app/tests.json

Lines changed: 51 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4301,15 +4301,7 @@
43014301
"headers": [
43024302
[
43034303
"Set-Cookie",
4304-
"test=1; expires=Tue, 06-Dec-2022 12:34:56 GMT; Max-Age=60; Path=/; HttpOnly; Secure"
4305-
],
4306-
[
4307-
"Set-Cookie",
4308-
"test_id=1; Max-Age=60; Path=/; expires=Tue, 06-Dec-2022 12:34:56 GMT"
4309-
],
4310-
[
4311-
"Set-Cookie",
4312-
"test_id=1; Max-Age=60; Path=/; expires=Tue, 06-Dec-2022 12:34:56 GMT"
4304+
"test=1; expires=Tue, 06-Dec-2022 12:34:56 GMT; Max-Age=60; Path=/; HttpOnly; Secure, test_id=1; Max-Age=60; Path=/; expires=Tue, 06-Dec-2022 12:34:56 GMT, test_id=1; Max-Age=60; Path=/; expires=Tue, 06-Dec-2022 12:34:56 GMT"
43134305
],
43144306
["Set-Cookie", "test2=2"],
43154307
["Set-Cookie", "test3=3"],
@@ -4335,15 +4327,7 @@
43354327
"headers": [
43364328
[
43374329
"Set-Cookie",
4338-
"test=1; expires=Tue, 06-Dec-2022 12:34:56 GMT; Max-Age=60; Path=/; HttpOnly; Secure"
4339-
],
4340-
[
4341-
"Set-Cookie",
4342-
"test_id=1; Max-Age=60; Path=/; expires=Tue, 06-Dec-2022 12:34:56 GMT"
4343-
],
4344-
[
4345-
"Set-Cookie",
4346-
"test_id=1; Max-Age=60; Path=/; expires=Tue, 06-Dec-2022 12:34:56 GMT"
4330+
"test=1; expires=Tue, 06-Dec-2022 12:34:56 GMT; Max-Age=60; Path=/; HttpOnly; Secure, test_id=1; Max-Age=60; Path=/; expires=Tue, 06-Dec-2022 12:34:56 GMT, test_id=1; Max-Age=60; Path=/; expires=Tue, 06-Dec-2022 12:34:56 GMT"
43474331
],
43484332
["Set-Cookie", "test2=2"],
43494333
["Set-Cookie", "test3=3"],
@@ -5290,6 +5274,55 @@
52905274
"body": "ok"
52915275
}
52925276
},
5277+
"GET /headers/construct": {
5278+
"environments": ["compute", "viceroy"],
5279+
"downstream_request": {
5280+
"method": "GET",
5281+
"pathname": "/headers/construct"
5282+
},
5283+
"downstream_response": {
5284+
"status": 200,
5285+
"headers": {
5286+
"foo": "bar"
5287+
}
5288+
}
5289+
},
5290+
"GET /headers/from-response/set": {
5291+
"environments": ["compute", "viceroy"],
5292+
"downstream_request": {
5293+
"method": "GET",
5294+
"pathname": "/headers/from-response/set"
5295+
},
5296+
"downstream_response": {
5297+
"status": 200,
5298+
"headers": [
5299+
["cuStom", "test"]
5300+
]
5301+
}
5302+
},
5303+
"GET /headers/from-response/delete-invalid": {
5304+
"environments": ["compute", "viceroy"],
5305+
"downstream_request": {
5306+
"method": "GET",
5307+
"pathname": "/headers/from-response/delete-invalid"
5308+
},
5309+
"downstream_response": {
5310+
"status": 200
5311+
}
5312+
},
5313+
"GET /headers/from-response/set-delete": {
5314+
"environments": ["compute", "viceroy"],
5315+
"downstream_request": {
5316+
"method": "GET",
5317+
"pathname": "/headers/from-response/set-delete"
5318+
},
5319+
"downstream_response": {
5320+
"status": 200,
5321+
"headers": [
5322+
["cuStom", "test"]
5323+
]
5324+
}
5325+
},
52935326

52945327
"GET /FastlyBody/interface": {
52955328
"environments": ["compute"],

‎runtime/fastly/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ add_builtin(fastly::dictionary SRC builtins/dictionary.cpp)
1515
add_builtin(fastly::edge_rate_limiter SRC builtins/edge-rate-limiter.cpp)
1616
add_builtin(fastly::config_store SRC builtins/config-store.cpp)
1717
add_builtin(fastly::secret_store SRC builtins/secret-store.cpp)
18-
add_builtin(fastly::fetch SRC builtins/fetch/fetch.cpp builtins/fetch/request-response.cpp builtins/fetch/headers.cpp)
18+
add_builtin(fastly::fetch SRC builtins/fetch/fetch.cpp builtins/fetch/request-response.cpp builtins/fetch/request-response.cpp ../StarlingMonkey/builtins/web/fetch/headers.cpp)
1919
add_builtin(fastly::cache_override SRC builtins/cache-override.cpp)
2020
add_builtin(fastly::fetch_event SRC builtins/fetch-event.cpp DEPENDENCIES OpenSSL)
2121

‎runtime/fastly/builtins/cache-core.cpp

Lines changed: 43 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,24 @@
11
#include "cache-core.h"
2+
#include "../../../StarlingMonkey/builtins/web/fetch/headers.h"
23
#include "../../../StarlingMonkey/builtins/web/streams/native-stream-source.h"
34
#include "../../../StarlingMonkey/runtime/encode.h"
45
#include "../host-api/host_api_fastly.h"
5-
#include "./fetch/headers.h"
66
#include "body.h"
77
#include "builtin.h"
88
#include "fastly.h"
99
#include "host_api.h"
1010
#include "js/Stream.h"
1111
#include <iostream>
1212

13+
using builtins::web::fetch::Headers;
1314
using builtins::web::streams::NativeStreamSource;
1415
using fastly::body::FastlyBody;
1516
using fastly::fastly::convertBodyInit;
16-
using fastly::fetch::Headers;
1717
using fastly::fetch::Request;
1818
using fastly::fetch::RequestOrResponse;
1919

20+
using builtins::web::fetch::Headers;
21+
2022
namespace fastly::cache_core {
2123

2224
namespace {
@@ -52,24 +54,30 @@ JS::Result<host_api::CacheLookupOptions> parseLookupOptions(JSContext *cx,
5254
"a [ ['name', 'value'], ... ] sequence, or a { 'name' : 'value', ... } record.");
5355
return JS::Result<host_api::CacheLookupOptions>(JS::Error());
5456
}
55-
JS::RootedObject request_opts(cx, JS_NewPlainObject(cx));
56-
if (!JS_SetProperty(cx, request_opts, "headers", headers_val)) {
57+
58+
auto request_handle_res = host_api::HttpReq::make();
59+
if (auto *err = request_handle_res.to_err()) {
60+
HANDLE_ERROR(cx, *err);
5761
return JS::Result<host_api::CacheLookupOptions>(JS::Error());
5862
}
59-
JS::RootedObject requestInstance(cx, Request::create_instance(cx));
60-
if (!requestInstance) {
63+
auto request_handle = request_handle_res.unwrap();
64+
65+
JS::RootedObject headers(cx,
66+
Headers::create(cx, headers_val, Headers::HeadersGuard::Request));
67+
if (!headers) {
6168
return JS::Result<host_api::CacheLookupOptions>(JS::Error());
6269
}
63-
64-
JS::RootedValue request_opts_val(cx, ObjectValue(*request_opts));
65-
66-
// We need to convert the supplied HeadersInit in the `headers` property into a host-backed
67-
// Request which contains the same headers Request::create does exactly that
68-
// however, it also expects a fully valid URL for the Request. We don't ever use the Request
69-
// URL, so we hard-code a valid URL
70-
JS::RootedValue input(cx, JS::StringValue(JS_NewStringCopyZ(cx, "http://example.com")));
71-
JS::RootedObject request(cx, Request::create(cx, requestInstance, input, request_opts_val));
72-
options.request_headers = host_api::HttpReq(Request::request_handle(request));
70+
if (Headers::mode(headers) == Headers::Mode::Uninitialized) {
71+
return options;
72+
}
73+
MOZ_ASSERT(Headers::mode(headers) == Headers::Mode::ContentOnly);
74+
Headers::HeadersList *headers_list = Headers::get_list(cx, headers);
75+
MOZ_ASSERT(headers_list);
76+
auto res = host_api::write_headers(request_handle.headers_writable(), *headers_list);
77+
if (auto *err = res.to_err()) {
78+
return JS::Result<host_api::CacheLookupOptions>(JS::Error());
79+
}
80+
options.request_headers = host_api::HttpReq(request_handle);
7381
}
7482
}
7583
return options;
@@ -337,23 +345,29 @@ JS::Result<host_api::CacheWriteOptions> parseInsertOptions(JSContext *cx,
337345
"a [ ['name', 'value'], ... ] sequence, or a { 'name' : 'value', ... } record.");
338346
return JS::Result<host_api::CacheWriteOptions>(JS::Error());
339347
}
340-
JS::RootedObject request_opts(cx, JS_NewPlainObject(cx));
341-
if (!JS_SetProperty(cx, request_opts, "headers", headers_val)) {
348+
349+
auto request_handle_res = host_api::HttpReq::make();
350+
if (auto *err = request_handle_res.to_err()) {
351+
HANDLE_ERROR(cx, *err);
352+
return JS::Result<host_api::CacheWriteOptions>(JS::Error());
353+
}
354+
auto request_handle = request_handle_res.unwrap();
355+
356+
JS::RootedObject headers(cx, Headers::create(cx, headers_val, Headers::HeadersGuard::Request));
357+
if (!headers) {
342358
return JS::Result<host_api::CacheWriteOptions>(JS::Error());
343359
}
344-
JS::RootedObject requestInstance(cx, Request::create_instance(cx));
345-
if (!requestInstance) {
360+
if (Headers::mode(headers) == Headers::Mode::Uninitialized) {
361+
return options;
362+
}
363+
MOZ_ASSERT(Headers::mode(headers) == Headers::Mode::ContentOnly);
364+
Headers::HeadersList *headers_list = Headers::get_list(cx, headers);
365+
MOZ_ASSERT(headers_list);
366+
auto res = host_api::write_headers(request_handle.headers_writable(), *headers_list);
367+
if (auto *err = res.to_err()) {
346368
return JS::Result<host_api::CacheWriteOptions>(JS::Error());
347369
}
348-
JS::RootedValue request_opts_val(cx, ObjectValue(*request_opts));
349-
350-
// We need to convert the supplied HeadersInit in the `headers` property into a host-backed
351-
// Request which contains the same headers Request::create does exactly that however,
352-
// it also expects a fully valid URL for the Request. We don't ever use the Request URL, so we
353-
// hard-code a valid URL
354-
JS::RootedValue input(cx, JS::StringValue(JS_NewStringCopyZ(cx, "http://example.com")));
355-
JS::RootedObject request(cx, Request::create(cx, requestInstance, input, request_opts_val));
356-
options.request_headers = host_api::HttpReq(Request::request_handle(request));
370+
options.request_headers = host_api::HttpReq(request_handle);
357371
}
358372
return options;
359373
}

‎runtime/fastly/builtins/fetch-event.cpp

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,10 @@
1919
using std::chrono::microseconds;
2020
using std::chrono::system_clock;
2121
using namespace std::literals::string_view_literals;
22+
using builtins::web::fetch::Headers;
2223
using builtins::web::url::URL;
2324
using builtins::web::worker_location::WorkerLocation;
2425
using fastly::fastly::Fastly;
25-
using fastly::fetch::Headers;
2626
using fastly::fetch::RequestOrResponse;
2727
using fastly::fetch::Response;
2828

@@ -568,6 +568,10 @@ bool start_response(JSContext *cx, JS::HandleObject response_obj, bool streaming
568568
auto response = Response::response_handle(response_obj);
569569
auto body = RequestOrResponse::body_handle(response_obj);
570570

571+
// write all the headers
572+
if (!RequestOrResponse::commit_headers(cx, response_obj))
573+
return false;
574+
571575
auto res = response.send_downstream(body, streaming);
572576
if (auto *err = res.to_err()) {
573577
HANDLE_ERROR(cx, *err);
@@ -601,12 +605,10 @@ bool response_promise_then_handler(JSContext *cx, JS::HandleObject event, JS::Ha
601605
// very different.)
602606
JS::RootedObject response_obj(cx, &args[0].toObject());
603607

604-
// Ensure that all headers are stored client-side, so we retain access to them
605-
// after sending the response off.
606608
if (Response::is_upstream(response_obj)) {
607-
JS::RootedObject headers(cx);
608-
headers = RequestOrResponse::headers<Headers::Mode::ProxyToResponse>(cx, response_obj);
609-
if (!Headers::delazify(cx, headers))
609+
JS::RootedObject headers(cx, Response::headers(cx, response_obj));
610+
// Calling get_list() transitions to Mode::ContentOnly or Mode::CachedInContent.
611+
if (!Headers::get_list(cx, headers))
610612
return false;
611613
}
612614

‎runtime/fastly/builtins/fetch/fetch.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
#include "fetch.h"
2+
#include "../../../StarlingMonkey/builtins/web/fetch/headers.h"
23
#include "../backend.h"
34
#include "../fastly.h"
45
#include "../fetch-event.h"
5-
#include "./headers.h"
66
#include "./request-response.h"
77
#include "encode.h"
88
#include "extension-api.h"
@@ -73,6 +73,10 @@ bool fetch(JSContext *cx, unsigned argc, Value *vp) {
7373
return ReturnPromiseRejectedWithPendingError(cx, args);
7474
}
7575

76+
if (!RequestOrResponse::commit_headers(cx, request)) {
77+
return false;
78+
}
79+
7680
RootedString backend(cx, Request::backend(request));
7781
if (!backend) {
7882
if (Fastly::allowDynamicBackends) {
@@ -171,7 +175,7 @@ bool install(api::Engine *engine) {
171175
if (!Response::init_class(ENGINE->cx(), ENGINE->global())) {
172176
return false;
173177
}
174-
if (!Headers::init_class(ENGINE->cx(), ENGINE->global())) {
178+
if (!builtins::web::fetch::Headers::init_class(ENGINE->cx(), ENGINE->global())) {
175179
return false;
176180
}
177181
return true;

‎runtime/fastly/builtins/fetch/fetch.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#include "headers.h"
1+
#include "../../../StarlingMonkey/builtins/web/fetch/headers.h"
22
#include "request-response.h"
33

44
namespace fastly::fetch {

0 commit comments

Comments
 (0)