Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix!: immutable headers guards for incoming response and downstream request #1006

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
4 changes: 2 additions & 2 deletions integration-tests/cli/help.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ test('--help should return help on stdout and zero exit code', async function (t
'--engine-wasm <engine-wasm> The JS engine Wasm file path',
'--module-mode [experimental] Run all sources as native modules,',
'with full error stack support.',
'--enable-experimental-aot Enable experimental AOT compilation for performance',
'--enable-aot Enable AOT compilation for performance',
'--enable-experimental-high-resolution-time-methods Enable experimental high-resolution fastly.now() method',
'--enable-experimental-top-level-await Enable experimental top level await',
'ARGS:',
Expand Down Expand Up @@ -61,7 +61,7 @@ test('-h should return help on stdout and zero exit code', async function (t) {
'--engine-wasm <engine-wasm> The JS engine Wasm file path',
'--module-mode [experimental] Run all sources as native modules,',
'with full error stack support.',
'--enable-experimental-aot Enable experimental AOT compilation for performance',
'--enable-aot Enable AOT compilation for performance',
'--enable-experimental-high-resolution-time-methods Enable experimental high-resolution fastly.now() method',
'--enable-experimental-top-level-await Enable experimental top level await',
'ARGS:',
Expand Down
20 changes: 13 additions & 7 deletions integration-tests/js-compute/fixtures/app/src/headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,25 +26,31 @@ routes.set('/headers/from-response/set', async () => {
backend: 'httpbin',
cacheOverride: new CacheOverride('pass'),
});
response.headers.set('cuStom', 'test');
return response;
return new Response(response.body, {
headers: {
cuStom: 'test',
},
});
});

routes.set('/headers/from-response/delete-invalid', async () => {
const response = await fetch('https://httpbin.org/stream-bytes/11', {
backend: 'httpbin',
cacheOverride: new CacheOverride('pass'),
});
response.headers.delete('none');
return response;
const outResponse = new Response(response.body, response);
outResponse.headers.delete('none');
return outResponse;
});

routes.set('/headers/from-response/set-delete', async () => {
const response = await fetch('https://httpbin.org/stream-bytes/11', {
backend: 'httpbin',
cacheOverride: new CacheOverride('pass'),
});
response.headers.set('custom', 'test');
response.headers.delete('access-control-allow-origin');
return response;
const outResponse = new Response(response.body, response);
outResponse.headers.set('custom', 'test');
outResponse.headers.set('another', 'test');
outResponse.headers.delete('access-control-allow-origin');
return outResponse;
});
5 changes: 4 additions & 1 deletion integration-tests/js-compute/fixtures/app/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,10 @@ async function app(event) {
}
}
} finally {
res.headers.set('fastly_service_version', FASTLY_SERVICE_VERSION);
try {
// this will fail if headers were same headers object from original upstream
res.headers.set('fastly_service_version', FASTLY_SERVICE_VERSION);
} catch {}
}

return res;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,14 @@ async function responseMethod(setManualFramingHeaders) {
backend: 'httpbin',
cacheOverride: new CacheOverride('pass'),
});
response.setManualFramingHeaders(setManualFramingHeaders);
response.headers.set('content-length', '11');
response.headers.delete('transfer-encoding');
return response;
const outResponse = new Response(response.body, {
headers: response.headers,
status: response.status,
});
outResponse.setManualFramingHeaders(setManualFramingHeaders);
outResponse.headers.set('content-length', '11');
outResponse.headers.delete('transfer-encoding');
return outResponse;
}

routes.set('/override-content-length/response/method/false', async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-env serviceworker */
import { assertThrows } from './assertions.js';
import { routes } from './routes.js';

{
Expand All @@ -7,6 +8,11 @@ import { routes } from './routes.js';
* @type {Request} request
**/
const request = event.request;

assertThrows(() => {
request.headers.set('should-be', 'immutable');
}, TypeError);

const headers = {};
for (const [name, value] of request.headers.entries()) {
if (!headers[name]) {
Expand Down
7 changes: 6 additions & 1 deletion integration-tests/js-compute/fixtures/app/src/response.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-env serviceworker */

import { routes } from './routes.js';
import { assert } from './assertions.js';
import { assert, assertThrows } from './assertions.js';
import { allowDynamicBackends } from 'fastly:experimental';

routes.set('/response/stall', async (event) => {
Expand Down Expand Up @@ -64,6 +64,11 @@ routes.set('/response/request-body-init', async () => {
accept: 'image/webp',
},
});

assertThrows(() => {
downloadResp.headers.set('should-be', 'immutable');
}, TypeError);

// stream it through an echo proxy
const postResp = await fetch(
new Request('https://httpbin.org/anything', {
Expand Down
5 changes: 4 additions & 1 deletion integration-tests/js-compute/fixtures/app/tests.json
Original file line number Diff line number Diff line change
Expand Up @@ -1322,7 +1322,10 @@
"GET /headers/from-response/set-delete": {
"downstream_response": {
"status": 200,
"headers": [["cuStom", "test"]]
"headers": [
["cuStom", "test"],
["another", "test"]
]
}
},

Expand Down
2 changes: 1 addition & 1 deletion integration-tests/js-compute/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ const config = TOML.parse(
config.name = serviceName;
if (aot) {
const buildArgs = config.scripts.build.split(' ');
buildArgs.splice(-1, null, '--enable-experimental-aot');
buildArgs.splice(-1, null, '--enable-aot');
config.scripts.build = buildArgs.join(' ');
}
if (debugBuild) {
Expand Down
5 changes: 3 additions & 2 deletions runtime/fastly/builtins/fetch/request-response.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,8 @@ JSObject *Request::headers(JSContext *cx, JS::HandleObject obj) {
if (!headers) {
MOZ_ASSERT(is_instance(obj));
if (is_downstream(obj)) {
headers = Headers::create(cx, request_handle(obj).headers(), Headers::HeadersGuard::Request);
headers =
Headers::create(cx, request_handle(obj).headers(), Headers::HeadersGuard::Immutable);
} else {
headers = Headers::create(cx, Headers::HeadersGuard::Request);
}
Expand All @@ -482,7 +483,7 @@ JSObject *Response::headers(JSContext *cx, JS::HandleObject obj) {
MOZ_ASSERT(is_instance(obj));
if (is_upstream(obj)) {
headers =
Headers::create(cx, response_handle(obj).headers(), Headers::HeadersGuard::Response);
Headers::create(cx, response_handle(obj).headers(), Headers::HeadersGuard::Immutable);
} else {
headers = Headers::create(cx, Headers::HeadersGuard::Response);
}
Expand Down
8 changes: 8 additions & 0 deletions src/parseInputs.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,15 @@ export async function parseInputs(cliInputs) {
bundle = true;
break;
}
case '--enable-aot': {
enableAOT = true;
break;
}
case '--enable-experimental-aot': {
console.error(
'Warning: --enable-experimental-aot flag is now --enable-aot. The old flag continues\n' +
'to work for now, but please update your build invocation!',
);
enableAOT = true;
break;
}
Expand Down
2 changes: 1 addition & 1 deletion src/printHelp.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ OPTIONS:
--engine-wasm <engine-wasm> The JS engine Wasm file path
--module-mode [experimental] Run all sources as native modules,
with full error stack support.
--enable-experimental-aot Enable experimental AOT compilation for performance
--enable-aot Enable AOT compilation for performance
--enable-experimental-high-resolution-time-methods Enable experimental high-resolution fastly.now() method
--enable-experimental-top-level-await Enable experimental top level await

Expand Down
Loading