From 090d94c5f0e50cc8c1fcae80059c88c8306e6875 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 28 Aug 2023 12:09:11 +0200 Subject: [PATCH 1/7] fix: show proper `req.url` in HTTPS mode --- src/lib/edge-functions/bootstrap.mjs | 2 +- src/lib/edge-functions/headers.mjs | 1 + src/lib/edge-functions/proxy.mjs | 16 ++++++---------- src/utils/proxy.mjs | 17 +---------------- tests/integration/200.command.dev.test.cjs | 10 ++++++++++ 5 files changed, 19 insertions(+), 27 deletions(-) diff --git a/src/lib/edge-functions/bootstrap.mjs b/src/lib/edge-functions/bootstrap.mjs index c21a26268cf..dbe6cf2e511 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://64c264287e9cbb0008621df3--edge.netlify.com/bootstrap/index-combined.ts' +const latestBootstrapURL = 'https://deploy-preview-294--edge.netlify.app/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..48c3825c748 100644 --- a/src/lib/edge-functions/headers.mjs +++ b/src/lib/edge-functions/headers.mjs @@ -5,6 +5,7 @@ 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', diff --git a/src/lib/edge-functions/proxy.mjs b/src/lib/edge-functions/proxy.mjs index f6c1b55f95a..2b7bc958701 100644 --- a/src/lib/edge-functions/proxy.mjs +++ b/src/lib/edge-functions/proxy.mjs @@ -94,8 +94,8 @@ export const initializeProxy = async ({ inspectSettings, mainPort, offline, - passthroughPort, projectDir, + settings, siteInfo, state, }) => { @@ -122,6 +122,7 @@ export const initializeProxy = async ({ internalFunctions, port: isolatePort, projectDir, + settings, }) const hasEdgeFunctions = userFunctionsPath !== undefined || internalFunctionsPath @@ -167,11 +168,11 @@ 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.ForwardedHost]: req.headers.host, + [headers.ForwardedProtocol]: req.socket.encrypted ? 'https:' : 'http:', [headers.Functions]: functionNames.join(','), [headers.InvocationMetadata]: getInvocationMetadataHeader(invocationMetadata), [headers.IP]: LOCAL_HOST, @@ -182,13 +183,6 @@ export const initializeProxy = async ({ 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}` } } @@ -207,6 +201,7 @@ const prepareServer = async ({ internalFunctions, port, projectDir, + settings, }) => { // Merging internal with user-defined import maps. const importMapPaths = [...importMaps, config.functions['*'].deno_import_map] @@ -227,6 +222,7 @@ const prepareServer = async ({ importMapPaths, inspectSettings, port, + certificatePath: settings?.https?.certFilePath, }) const registry = new EdgeFunctionsRegistry({ bundler, diff --git a/src/utils/proxy.mjs b/src/utils/proxy.mjs index 54968fe48dc..587e7507693 100644 --- a/src/utils/proxy.mjs +++ b/src/utils/proxy.mjs @@ -13,7 +13,6 @@ import contentType from 'content-type' import cookie from 'cookie' import { getProperty } from 'dot-prop' import generateETag from 'etag' -import getAvailablePort from 'get-port' import httpProxy from 'http-proxy' import { createProxyMiddleware } from 'http-proxy-middleware' import jwtDecode from 'jwt-decode' @@ -681,7 +680,6 @@ export const startProxy = async function ({ siteInfo, state, }) { - const secondaryServerPort = settings.https ? await getAvailablePort() : null const functionsServer = settings.functionsPort ? `http://127.0.0.1:${settings.functionsPort}` : null const edgeFunctionsProxy = await initializeEdgeFunctionsProxy({ config, @@ -694,10 +692,10 @@ export const startProxy = async function ({ inspectSettings, mainPort: settings.port, offline, - passthroughPort: secondaryServerPort || settings.port, projectDir, siteInfo, accountId, + settings, state, }) const proxy = await initializeProxy({ @@ -742,19 +740,6 @@ export const startProxy = async function ({ const eventQueue = [once(primaryServer, 'listening')] - // If we're running the main server on HTTPS, we need to start a secondary - // server on HTTP for receiving passthrough requests from edge functions. - // This lets us run the Deno server on HTTP and avoid the complications of - // Deno talking to Node on HTTPS with potentially untrusted certificates. - if (secondaryServerPort) { - const secondaryServer = http.createServer(onRequestWithOptions) - - secondaryServer.on('upgrade', onUpgrade) - secondaryServer.listen({ port: secondaryServerPort }) - - eventQueue.push(once(secondaryServer, 'listening')) - } - await Promise.all(eventQueue) return getProxyUrl(settings) diff --git a/tests/integration/200.command.dev.test.cjs b/tests/integration/200.command.dev.test.cjs index 5e74e0c4477..45f69ee296a 100644 --- a/tests/integration/200.command.dev.test.cjs +++ b/tests/integration/200.command.dev.test.cjs @@ -244,6 +244,10 @@ export const handler = async function () { return new Response(text.toUpperCase(), res) } + + if (req.url.includes('?ef=url')) { + return new Response(req.url) + } }, name: 'hello', }) @@ -255,6 +259,12 @@ export const handler = async function () { ]) await withDevServer({ cwd: builder.directory, args }, async ({ port }) => { const options = { https: { rejectUnauthorized: 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') From 58b40b3af29a015434b99d854a841f146e291125 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 4 Sep 2023 13:46:21 +0200 Subject: [PATCH 2/7] fix: use x-nf-passthrough-proto --- src/lib/edge-functions/headers.mjs | 2 ++ src/lib/edge-functions/proxy.mjs | 8 +++----- src/utils/proxy.mjs | 17 ++++++++++++++++- tests/integration/200.command.dev.test.cjs | 21 ++++++++++----------- 4 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/lib/edge-functions/headers.mjs b/src/lib/edge-functions/headers.mjs index 48c3825c748..7aa85a2af24 100644 --- a/src/lib/edge-functions/headers.mjs +++ b/src/lib/edge-functions/headers.mjs @@ -10,6 +10,8 @@ export const headers = { 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 2b7bc958701..51960366e98 100644 --- a/src/lib/edge-functions/proxy.mjs +++ b/src/lib/edge-functions/proxy.mjs @@ -94,8 +94,8 @@ export const initializeProxy = async ({ inspectSettings, mainPort, offline, + passthroughPort, projectDir, - settings, siteInfo, state, }) => { @@ -122,7 +122,6 @@ export const initializeProxy = async ({ internalFunctions, port: isolatePort, projectDir, - settings, }) const hasEdgeFunctions = userFunctionsPath !== undefined || internalFunctionsPath @@ -171,12 +170,13 @@ export const initializeProxy = async ({ req[headersSymbol] = { [headers.FeatureFlags]: getFeatureFlagsHeader(featureFlags), - [headers.ForwardedHost]: req.headers.host, [headers.ForwardedProtocol]: req.socket.encrypted ? '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) { @@ -201,7 +201,6 @@ const prepareServer = async ({ internalFunctions, port, projectDir, - settings, }) => { // Merging internal with user-defined import maps. const importMapPaths = [...importMaps, config.functions['*'].deno_import_map] @@ -222,7 +221,6 @@ const prepareServer = async ({ importMapPaths, inspectSettings, port, - certificatePath: settings?.https?.certFilePath, }) const registry = new EdgeFunctionsRegistry({ bundler, diff --git a/src/utils/proxy.mjs b/src/utils/proxy.mjs index 587e7507693..54968fe48dc 100644 --- a/src/utils/proxy.mjs +++ b/src/utils/proxy.mjs @@ -13,6 +13,7 @@ import contentType from 'content-type' import cookie from 'cookie' import { getProperty } from 'dot-prop' import generateETag from 'etag' +import getAvailablePort from 'get-port' import httpProxy from 'http-proxy' import { createProxyMiddleware } from 'http-proxy-middleware' import jwtDecode from 'jwt-decode' @@ -680,6 +681,7 @@ export const startProxy = async function ({ siteInfo, state, }) { + const secondaryServerPort = settings.https ? await getAvailablePort() : null const functionsServer = settings.functionsPort ? `http://127.0.0.1:${settings.functionsPort}` : null const edgeFunctionsProxy = await initializeEdgeFunctionsProxy({ config, @@ -692,10 +694,10 @@ export const startProxy = async function ({ inspectSettings, mainPort: settings.port, offline, + passthroughPort: secondaryServerPort || settings.port, projectDir, siteInfo, accountId, - settings, state, }) const proxy = await initializeProxy({ @@ -740,6 +742,19 @@ export const startProxy = async function ({ const eventQueue = [once(primaryServer, 'listening')] + // If we're running the main server on HTTPS, we need to start a secondary + // server on HTTP for receiving passthrough requests from edge functions. + // This lets us run the Deno server on HTTP and avoid the complications of + // Deno talking to Node on HTTPS with potentially untrusted certificates. + if (secondaryServerPort) { + const secondaryServer = http.createServer(onRequestWithOptions) + + secondaryServer.on('upgrade', onUpgrade) + secondaryServer.listen({ port: secondaryServerPort }) + + eventQueue.push(once(secondaryServer, 'listening')) + } + await Promise.all(eventQueue) return getProxyUrl(settings) diff --git a/tests/integration/200.command.dev.test.cjs b/tests/integration/200.command.dev.test.cjs index 45f69ee296a..0dbb17a9aba 100644 --- a/tests/integration/200.command.dev.test.cjs +++ b/tests/integration/200.command.dev.test.cjs @@ -239,10 +239,11 @@ 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() - - return new Response(text.toUpperCase(), res) + try { + await fetch(url, {}) + } catch (error) { + return new Response(error) + } } if (req.url.includes('?ef=url')) { @@ -259,18 +260,16 @@ export const handler = async function () { ]) await withDevServer({ cwd: builder.directory, args }, async ({ port }) => { const options = { https: { rejectUnauthorized: false } } - t.is( - await got(`https://localhost:${port}/?ef=url`, { - ...options, - }).text(), - `https://localhost:${port}/?ef=url`, - ) + 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/) }) }) }) From dfa73c9881fb44405d6fcf04e8ea738b0bf1c451 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Tue, 5 Sep 2023 11:26:30 +0200 Subject: [PATCH 3/7] refactor: use settings.https to detect https --- src/lib/edge-functions/proxy.mjs | 4 +++- src/utils/proxy.mjs | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/lib/edge-functions/proxy.mjs b/src/lib/edge-functions/proxy.mjs index 51960366e98..85b9a15f3b8 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, }) => { @@ -170,7 +172,7 @@ export const initializeProxy = async ({ req[headersSymbol] = { [headers.FeatureFlags]: getFeatureFlagsHeader(featureFlags), - [headers.ForwardedProtocol]: req.socket.encrypted ? 'https:' : 'http:', + [headers.ForwardedProtocol]: settings.https ? 'https:' : 'http:', [headers.Functions]: functionNames.join(','), [headers.InvocationMetadata]: getInvocationMetadataHeader(invocationMetadata), [headers.IP]: LOCAL_HOST, diff --git a/src/utils/proxy.mjs b/src/utils/proxy.mjs index 54968fe48dc..865152b5e9b 100644 --- a/src/utils/proxy.mjs +++ b/src/utils/proxy.mjs @@ -695,6 +695,7 @@ export const startProxy = async function ({ mainPort: settings.port, offline, passthroughPort: secondaryServerPort || settings.port, + settings, projectDir, siteInfo, accountId, From 18fe0a19e981cdfbbeb329b7b85a5d8580345d34 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Tue, 5 Sep 2023 11:28:42 +0200 Subject: [PATCH 4/7] refactor: throw 500 error in test --- tests/integration/200.command.dev.test.cjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/200.command.dev.test.cjs b/tests/integration/200.command.dev.test.cjs index 0dbb17a9aba..69d8b5a1bd4 100644 --- a/tests/integration/200.command.dev.test.cjs +++ b/tests/integration/200.command.dev.test.cjs @@ -242,7 +242,7 @@ export const handler = async function () { try { await fetch(url, {}) } catch (error) { - return new Response(error) + return new Response(error, { status: 500 }) } } @@ -259,7 +259,7 @@ 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') From 3c9a98c0dd51b778f1cea572b15bee107f2fc6df Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Tue, 5 Sep 2023 11:29:40 +0200 Subject: [PATCH 5/7] chore: remove unused import --- tests/integration/200.command.dev.test.cjs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/200.command.dev.test.cjs b/tests/integration/200.command.dev.test.cjs index 69d8b5a1bd4..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') From bb5f238fcd8ce2926679167ec1daf84844968490 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Tue, 5 Sep 2023 16:08:14 +0200 Subject: [PATCH 6/7] fix: update bootstrap url --- src/lib/edge-functions/bootstrap.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/edge-functions/bootstrap.mjs b/src/lib/edge-functions/bootstrap.mjs index dbe6cf2e511..951635b45df 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://deploy-preview-294--edge.netlify.app/bootstrap/index-combined.ts' +const latestBootstrapURL = 'https://64f73321fdd56900083fa618--edge.netlify.app/bootstrap/index-combined.ts' export const getBootstrapURL = () => env.NETLIFY_EDGE_BOOTSTRAP || latestBootstrapURL From a60764caf22054f50c270981ced5a56af1ac8e19 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Tue, 5 Sep 2023 16:09:34 +0200 Subject: [PATCH 7/7] fix: use .com --- src/lib/edge-functions/bootstrap.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/edge-functions/bootstrap.mjs b/src/lib/edge-functions/bootstrap.mjs index 951635b45df..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://64f73321fdd56900083fa618--edge.netlify.app/bootstrap/index-combined.ts' +const latestBootstrapURL = 'https://64f73321fdd56900083fa618--edge.netlify.com/bootstrap/index-combined.ts' export const getBootstrapURL = () => env.NETLIFY_EDGE_BOOTSTRAP || latestBootstrapURL