From da723a15799590797b2df93fe871f3b3b5f2d5fe Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Wed, 6 Sep 2023 10:00:06 +0200 Subject: [PATCH] fix: show proper `req.url` in HTTPS mode (#5968) * fix: show proper `req.url` in HTTPS mode * fix: use x-nf-passthrough-proto * refactor: use settings.https to detect https * refactor: throw 500 error in test * chore: remove unused import * fix: update bootstrap url * fix: use .com --- src/lib/edge-functions/bootstrap.mjs | 2 +- src/lib/edge-functions/headers.mjs | 3 +++ src/lib/edge-functions/proxy.mjs | 14 +++++--------- src/utils/proxy.mjs | 1 + tests/integration/200.command.dev.test.cjs | 20 ++++++++++++++------ 5 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/lib/edge-functions/bootstrap.mjs b/src/lib/edge-functions/bootstrap.mjs index d37be10b591..881c66453ec 100644 --- a/src/lib/edge-functions/bootstrap.mjs +++ b/src/lib/edge-functions/bootstrap.mjs @@ -1,5 +1,5 @@ import { env } from 'process' -const latestBootstrapURL = 'https://64e7783fce8cfe0008496c72--edge.netlify.com/bootstrap/index-combined.ts' +const latestBootstrapURL = 'https://64f73321fdd56900083fa618--edge.netlify.com/bootstrap/index-combined.ts' export const getBootstrapURL = () => env.NETLIFY_EDGE_BOOTSTRAP || latestBootstrapURL diff --git a/src/lib/edge-functions/headers.mjs b/src/lib/edge-functions/headers.mjs index 6bdc58e6d6b..7aa85a2af24 100644 --- a/src/lib/edge-functions/headers.mjs +++ b/src/lib/edge-functions/headers.mjs @@ -5,10 +5,13 @@ export const headers = { DeployID: 'x-nf-deploy-id', FeatureFlags: 'x-nf-feature-flags', ForwardedHost: 'x-forwarded-host', + ForwardedProtocol: 'x-forwarded-proto', Functions: 'x-nf-edge-functions', InvocationMetadata: 'x-nf-edge-functions-metadata', Geo: 'x-nf-geo', Passthrough: 'x-nf-passthrough', + PassthroughHost: 'x-nf-passthrough-host', + PassthroughProtocol: 'x-nf-passthrough-proto', IP: 'x-nf-client-connection-ip', Site: 'X-NF-Site-Info', DebugLogging: 'x-nf-debug-logging', diff --git a/src/lib/edge-functions/proxy.mjs b/src/lib/edge-functions/proxy.mjs index f917d1b4161..08dd4495add 100644 --- a/src/lib/edge-functions/proxy.mjs +++ b/src/lib/edge-functions/proxy.mjs @@ -78,6 +78,7 @@ export const createAccountInfoHeader = (accountInfo = {}) => { * @param {boolean=} config.offline * @param {*} config.passthroughPort * @param {*} config.projectDir + * @param {*} config.settings * @param {*} config.siteInfo * @param {*} config.state * @returns @@ -96,6 +97,7 @@ export const initializeProxy = async ({ offline, passthroughPort, projectDir, + settings, siteInfo, state, }) => { @@ -167,28 +169,22 @@ export const initializeProxy = async ({ } const featureFlags = ['edge_functions_bootstrap_failure_mode'] - const forwardedHost = `localhost:${passthroughPort}` req[headersSymbol] = { [headers.FeatureFlags]: getFeatureFlagsHeader(featureFlags), - [headers.ForwardedHost]: forwardedHost, + [headers.ForwardedProtocol]: settings.https ? 'https:' : 'http:', [headers.Functions]: functionNames.join(','), [headers.InvocationMetadata]: getInvocationMetadataHeader(invocationMetadata), [headers.IP]: LOCAL_HOST, [headers.Passthrough]: 'passthrough', + [headers.PassthroughHost]: `localhost:${passthroughPort}`, + [headers.PassthroughProtocol]: 'http:', } if (debug) { req[headersSymbol][headers.DebugLogging] = '1' } - // If we're using a different port for passthrough requests, which is the - // case when the CLI is running on HTTPS, use it on the Host header so - // that the request URL inside the edge function is something accessible. - if (mainPort !== passthroughPort) { - req[headersSymbol].host = forwardedHost - } - return `http://${LOCAL_HOST}:${isolatePort}` } } diff --git a/src/utils/proxy.mjs b/src/utils/proxy.mjs index bd8a5ba8677..7bed08e2dad 100644 --- a/src/utils/proxy.mjs +++ b/src/utils/proxy.mjs @@ -698,6 +698,7 @@ export const startProxy = async function ({ mainPort: settings.port, offline, passthroughPort: secondaryServerPort || settings.port, + settings, projectDir, siteInfo, accountId, diff --git a/tests/integration/200.command.dev.test.cjs b/tests/integration/200.command.dev.test.cjs index 5e74e0c4477..e5fedb8be13 100644 --- a/tests/integration/200.command.dev.test.cjs +++ b/tests/integration/200.command.dev.test.cjs @@ -6,7 +6,6 @@ const path = require('path') // eslint-disable-next-line ava/use-test const avaTest = require('ava') const { isCI } = require('ci-info') -const { Response } = require('node-fetch') const { curl } = require('./utils/curl.cjs') const { withDevServer } = require('./utils/dev-server.cjs') @@ -239,10 +238,15 @@ export const handler = async function () { if (req.url.includes('?ef=fetch')) { const url = new URL('/origin', req.url) - const res = await fetch(url) - const text = await res.text() + try { + await fetch(url, {}) + } catch (error) { + return new Response(error, { status: 500 }) + } + } - return new Response(text.toUpperCase(), res) + if (req.url.includes('?ef=url')) { + return new Response(req.url) } }, name: 'hello', @@ -254,13 +258,17 @@ export const handler = async function () { copyFile(`${__dirname}/../../localhost.key`, `${builder.directory}/localhost.key`), ]) await withDevServer({ cwd: builder.directory, args }, async ({ port }) => { - const options = { https: { rejectUnauthorized: false } } + const options = { https: { rejectUnauthorized: false }, throwHttpErrors: false } + t.is(await got(`https://localhost:${port}/?ef=url`, options).text(), `https://localhost:${port}/?ef=url`) t.is(await got(`https://localhost:${port}`, options).text(), 'index') t.is(await got(`https://localhost:${port}?ef=true`, options).text(), 'INDEX') - t.is(await got(`https://localhost:${port}?ef=fetch`, options).text(), 'ORIGIN') t.deepEqual(await got(`https://localhost:${port}/api/hello`, options).json(), { rawUrl: `https://localhost:${port}/api/hello`, }) + + // the fetch will go against the `https://` url of the dev server, which isn't trusted system-wide. + // this is the expected behaviour for fetch, so we shouldn't change anything about it. + t.regex(await got(`https://localhost:${port}?ef=fetch`, options).text(), /invalid peer certificate/) }) }) })