Skip to content

Commit

Permalink
fix: show proper req.url in HTTPS mode (#5968)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Skn0tt authored Sep 6, 2023
1 parent 4a7d9df commit da723a1
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 16 deletions.
2 changes: 1 addition & 1 deletion src/lib/edge-functions/bootstrap.mjs
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions src/lib/edge-functions/headers.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
14 changes: 5 additions & 9 deletions src/lib/edge-functions/proxy.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -96,6 +97,7 @@ export const initializeProxy = async ({
offline,
passthroughPort,
projectDir,
settings,
siteInfo,
state,
}) => {
Expand Down Expand Up @@ -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}`
}
}
Expand Down
1 change: 1 addition & 0 deletions src/utils/proxy.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,7 @@ export const startProxy = async function ({
mainPort: settings.port,
offline,
passthroughPort: secondaryServerPort || settings.port,
settings,
projectDir,
siteInfo,
accountId,
Expand Down
20 changes: 14 additions & 6 deletions tests/integration/200.command.dev.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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',
Expand All @@ -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/)
})
})
})
Expand Down

2 comments on commit da723a1

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📊 Benchmark results

  • Dependency count: 1,331
  • Package size: 295 MB

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📊 Benchmark results

  • Dependency count: 1,331
  • Package size: 295 MB

Please sign in to comment.