From 68adcdaddb48b24dc005cbad20a2ad2e09279b84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20Bou=C3=A7as?= Date: Mon, 4 Apr 2022 16:19:32 +0100 Subject: [PATCH] feat: generate ETag header (#11) * chore: change package details * chore: update package * chore: bump * chore: add @netlify/edge-bundler * chore: bump * chore: remove npmrc line * chore: bump version * chore: update @netlify/edge-bundler * feat: deploy Edge Handlers to internal path * feat: add local development experience * chore: bump version * chore: update @netlify/edge-bundler * refactor: move `watcher` util * feat: add `configWatcher` * feat: updates to Edge Handlers dev server * chore: update @netlify/edge-bundler * refactor: remove unused error binding * chore: update package name * chore: update deps * fix: use correct URL path when matching Edge Handlers * chore: bump version * chore: bump version * feat: add support for internal handlers in local dev (#1) * feat: add support for internal handlers in local dev * refactor: correctly merge declarations * chore: bump version * fix: run config watcher only on certain commands (#2) * chore: bump version * chore: bump version * feat: add support for user import maps (#3) * feat: add support for user import maps * feat: warn when failing to read import map file * chore: bump version * chore: bump version * fix: improve error handling for Edge Handlers (#4) * chore: bump version * feat: listen for `NETLIFY_DENO_DEBUG` environment variable (#5) * feat: listen for `NETLIFY_DENO_DEBUG` environment variable * chore: upgrade @netlify-labs/edge-bundler * chore: bump version * fix: use `0.0.0.0` for Deno server host (#6) * fix: wait for Edge Handlers server to become available (#7) * fix: wait for Edge Handlers server to become available * refactor: wait for server inside request * refactor: increase timeout * refactor: use watcher from correct location * chore: update package.json * chore: disable release-please * feat: replace @netlify/build with @netlify-labs/build-internal * chore: use token in CI * refactor: use existing config watcher * chore: remove unused variables * chore: remove actions * chore: remove action * chore: update test * chore: add tests * chore: move env to job * chore: remove duplicate env * chore: fix tests * chore: remove unused imports * fix: check if EH directory exists before deploy * feat: rename Edge Handlers to Edge Functions (#9) * chore: fix linting issue * chore: fix tests * chore: fix test * chore: add `withEdgeFunction` test helper * chore: remove `console.log` * refactor: remove old Edge Handlers code * chore: update packages * fix: remove reference to traffic-mesh * chore: fix linting issue * chore: fix linting issue * refactor: remove unused code * refactor: remove `dev:trace` command * chore: setup Deno in the CI * refactor: use different localhost IP * chore: simplify test * chore: simplify test * feat: generate ETag header * chore: remove debug statement * chore: add etag module * refactor: use `const` * refactor: make `shouldGenerateETag` a function * chore: add tests * chore: use node-fetch in test --- npm-shrinkwrap.json | 5 +- package.json | 1 + src/utils/proxy.js | 58 ++++++++++++++++++++--- tests/integration/0.command.dev.test.js | 50 +++++++++++++++++++ tests/integration/200.command.dev.test.js | 2 + 5 files changed, 107 insertions(+), 9 deletions(-) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index a4957d77996..e0aa6a9d55f 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,12 +1,12 @@ { "name": "@netlify-labs/cli-internal", - "version": "2.0.2", + "version": "2.1.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@netlify-labs/cli-internal", - "version": "2.0.2", + "version": "2.1.0", "hasInstallScript": true, "license": "MIT", "dependencies": { @@ -45,6 +45,7 @@ "dotenv": "^16.0.0", "env-paths": "^2.2.0", "envinfo": "^7.3.1", + "etag": "^1.8.1", "execa": "^5.0.0", "express": "^4.17.1", "express-logging": "^1.1.1", diff --git a/package.json b/package.json index a3c5661a8cb..2c77ed52744 100644 --- a/package.json +++ b/package.json @@ -241,6 +241,7 @@ "dotenv": "^16.0.0", "env-paths": "^2.2.0", "envinfo": "^7.3.1", + "etag": "^1.8.1", "execa": "^5.0.0", "express": "^4.17.1", "express-logging": "^1.1.1", diff --git a/src/utils/proxy.js b/src/utils/proxy.js index 7fe07250dd5..48b745dfefe 100644 --- a/src/utils/proxy.js +++ b/src/utils/proxy.js @@ -8,6 +8,7 @@ const path = require('path') const contentType = require('content-type') const cookie = require('cookie') const { get } = require('dot-prop') +const generateETag = require('etag') const httpProxy = require('http-proxy') const { createProxyMiddleware } = require('http-proxy-middleware') const jwtDecode = require('jwt-decode') @@ -25,6 +26,8 @@ const { createStreamPromise } = require('./create-stream-promise') const { headersForPath, parseHeaders } = require('./headers') const { createRewriter, onChanges } = require('./rules-proxy') +const shouldGenerateETag = Symbol('Internal: response should generate ETag') + const isInternal = function (url) { return url.startsWith('/.netlify/') } @@ -345,16 +348,44 @@ const initializeProxy = async function ({ configPath, distDir, port, projectDir return serveRedirect({ req, res, proxy: handlers, match: null, options: req.proxyOptions }) } + const responseData = [] const requestURL = new URL(req.url, `http://${req.headers.host || 'localhost'}`) const headersRules = headersForPath(headers, requestURL.pathname) - Object.entries(headersRules).forEach(([key, val]) => { - res.setHeader(key, val) - }) - res.writeHead(req.proxyOptions.status || proxyRes.statusCode, proxyRes.headers) + proxyRes.on('data', function onData(data) { - res.write(data) + responseData.push(data) }) + proxyRes.on('end', function onEnd() { + const responseBody = Buffer.concat(responseData) + + let responseStatus = req.proxyOptions.status || proxyRes.statusCode + + // `req[shouldGenerateETag]` may contain a function that determines + // whether the response should have an ETag header. + if ( + typeof req[shouldGenerateETag] === 'function' && + req[shouldGenerateETag]({ statusCode: responseStatus }) === true + ) { + const etag = generateETag(responseBody) + + if (req.headers['if-none-match'] === etag) { + responseStatus = 304 + } + + res.setHeader('etag', etag) + } + + Object.entries(headersRules).forEach(([key, val]) => { + res.setHeader(key, val) + }) + + res.writeHead(responseStatus, proxyRes.headers) + + if (responseStatus !== 304) { + res.write(responseBody) + } + res.end() }) }) @@ -382,6 +413,9 @@ const onRequest = async ({ addonsUrls, edgeFunctionsProxy, functionsServer, prox const edgeFunctionsProxyURL = await edgeFunctionsProxy(req, res) if (edgeFunctionsProxyURL !== undefined) { + // We always want to generate an ETag for Edge Functions requests. + req[shouldGenerateETag] = () => true + return proxy.web(req, res, { target: edgeFunctionsProxyURL }) } @@ -405,7 +439,17 @@ const onRequest = async ({ addonsUrls, edgeFunctionsProxy, functionsServer, prox framework: settings.framework, } - if (match) return serveRedirect({ req, res, proxy, match, options }) + if (match) { + // We don't want to generate an ETag for 3xx redirects. + req[shouldGenerateETag] = ({ statusCode }) => statusCode < 300 || statusCode >= 400 + + return serveRedirect({ req, res, proxy, match, options }) + } + + // The request will be served by the framework server, which means we want to + // generate an ETag unless we're rendering an error page. The only way for + // us to know that is by looking at the status code + req[shouldGenerateETag] = ({ statusCode }) => statusCode >= 200 && statusCode < 300 const ct = req.headers['content-type'] ? contentType.parse(req).type : '' if ( @@ -464,4 +508,4 @@ const startProxy = async function ({ addonsUrls, config, configPath, configWatch const BYTES_LIMIT = 30 -module.exports = { startProxy } +module.exports = { shouldGenerateETag, startProxy } diff --git a/tests/integration/0.command.dev.test.js b/tests/integration/0.command.dev.test.js index e7773821425..a53c90bf120 100644 --- a/tests/integration/0.command.dev.test.js +++ b/tests/integration/0.command.dev.test.js @@ -4,6 +4,7 @@ const http = require('http') // eslint-disable-next-line ava/use-test const avaTest = require('ava') const { isCI } = require('ci-info') +const fetch = require('node-fetch') const { withDevServer } = require('./utils/dev-server') const { startExternalServer } = require('./utils/external-server') @@ -23,6 +24,7 @@ test('should return 404.html if exists for non existing routes', async (t) => { await withDevServer({ cwd: builder.directory }, async (server) => { const response = await got(`${server.url}/non-existent`, { throwHttpErrors: false }) + t.is(response.headers.etag, undefined) t.is(response.body, '

404 - Page not found

') }) }) @@ -48,6 +50,7 @@ test('should return 404.html from publish folder if exists for non existing rout await withDevServer({ cwd: builder.directory }, async (server) => { const response = await got(`${server.url}/non-existent`, { throwHttpErrors: false }) t.is(response.statusCode, 404) + t.is(response.headers.etag, undefined) t.is(response.body, '

404 - My Custom 404 Page

') }) }) @@ -70,6 +73,7 @@ test('should return 404 for redirect', async (t) => { await withDevServer({ cwd: builder.directory }, async (server) => { const response = await got(`${server.url}/test-404`, { throwHttpErrors: false }) + t.truthy(response.headers.etag) t.is(response.statusCode, 404) t.is(response.body, '

foo') }) @@ -303,3 +307,49 @@ test('should not shadow an existing file that has unsafe URL characters', async }) }) }) + +test('should generate an ETag for static assets', async (t) => { + await withSiteBuilder('site-with-static-assets', async (builder) => { + builder + .withContentFile({ + path: 'public/index.html', + content: 'index', + }) + .withNetlifyToml({ + config: { + build: { publish: 'public' }, + redirects: [{ from: '/*', to: '/index.html', status: 200 }], + }, + }) + + await builder.buildAsync() + + await withDevServer({ cwd: builder.directory }, async (server) => { + const res1 = await fetch(`${server.url}`) + const etag = res1.headers.get('etag') + + t.truthy(etag) + t.is(res1.status, 200) + t.truthy(await res1.text()) + + const res2 = await fetch(`${server.url}`, { + headers: { + 'if-none-match': etag, + }, + }) + + t.is(res2.status, 304) + t.falsy(await res2.text()) + + const res3 = await fetch(`${server.url}`, { + headers: { + 'if-none-match': 'stale-etag', + }, + }) + + t.truthy(res3.headers.get('etag')) + t.is(res3.status, 200) + t.truthy(await res3.text()) + }) + }) +}) diff --git a/tests/integration/200.command.dev.test.js b/tests/integration/200.command.dev.test.js index 479e7c518e2..c9d0e5bd90b 100644 --- a/tests/integration/200.command.dev.test.js +++ b/tests/integration/200.command.dev.test.js @@ -358,9 +358,11 @@ export const handler = async function () { await withDevServer({ cwd: builder.directory, args }, async (server) => { const response = await got(`${server.url}/.netlify/functions/custom-headers`) + t.falsy(response.headers.etag) t.is(response.headers['single-value-header'], 'custom-value') t.is(response.headers['multi-value-header'], 'custom-value1, custom-value2') const builderResponse = await got(`${server.url}/.netlify/builders/custom-headers`) + t.falsy(builderResponse.headers.etag) t.is(builderResponse.headers['single-value-header'], 'custom-value') t.is(builderResponse.headers['multi-value-header'], 'custom-value1, custom-value2') })