From efc0b48db300f0338482290b9a72bc9eb99504a1 Mon Sep 17 00:00:00 2001 From: Netlify Team Account 1 <90322326+netlify-team-account-1@users.noreply.github.com> Date: Mon, 15 Nov 2021 16:46:02 +0100 Subject: [PATCH] fix: resemble Netlify Production logic for base64 encoding more closely (#3631) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore: add reproduction test for multipart/form-data encoding * fix: resemble proxy's base64 encoding logic see https://github.com/netlify/proxy/blob/main/pkg/functions/request.go#L119 * chore: update tests requests without a content-type will be base64-encoded this also applies to HTTP GET * Add comment to production code Co-authored-by: Eduardo Bouças * chore: fix test Co-authored-by: Netlify Team Account 1 Co-authored-by: Eduardo Bouças Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- src/lib/functions/utils.js | 37 ++++++++++++++++++++++++++++++--- tests/command.dev.test.js | 6 +++--- tests/eleventy.test.js | 4 ++-- tests/serving-functions.test.js | 36 ++++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 8 deletions(-) diff --git a/src/lib/functions/utils.js b/src/lib/functions/utils.js index 39d4e842372..fce3428466c 100644 --- a/src/lib/functions/utils.js +++ b/src/lib/functions/utils.js @@ -3,8 +3,6 @@ const chalk = require('chalk') const { warn } = require('../../utils/command-helpers') const { getLogMessage } = require('../log') -const BASE_64_MIME_REGEXP = /image|audio|video|application\/pdf|application\/zip|applicaton\/octet-stream/i - const DEFAULT_LAMBDA_OPTIONS = { verboseLevel: 3, } @@ -21,8 +19,41 @@ const detectAwsSdkError = ({ error }) => { const formatLambdaError = (err) => chalk.red(`${err.errorType}: ${err.errorMessage}`) +// should be equivalent to https://github.com/netlify/proxy/blob/main/pkg/functions/request.go#L105 +const exceptionsList = new Set([ + 'application/csp-report', + 'application/graphql', + 'application/json', + 'application/javascript', + 'application/x-www-form-urlencoded', + 'application/x-ndjson', + 'application/xml', +]) + +/** + * @param {string | undefined} contentType + * @returns {boolean} + */ const shouldBase64Encode = function (contentType) { - return Boolean(contentType) && BASE_64_MIME_REGEXP.test(contentType) + if (!contentType) { + return true + } + + contentType = contentType.toLowerCase() + + if (contentType.startsWith('text/')) { + return false + } + + if (contentType.endsWith('+json') || contentType.endsWith('+xml')) { + return false + } + + if (exceptionsList.has(contentType)) { + return false + } + + return true } const styleFunctionName = (name) => chalk.magenta(name) diff --git a/tests/command.dev.test.js b/tests/command.dev.test.js index 7bef4c7aa05..9f5641258f3 100644 --- a/tests/command.dev.test.js +++ b/tests/command.dev.test.js @@ -487,7 +487,7 @@ testMatrix.forEach(({ args }) => { t.is(response.body, undefined) t.is(response.headers.host, `${server.host}:${server.port}`) t.is(response.httpMethod, 'GET') - t.is(response.isBase64Encoded, false) + t.is(response.isBase64Encoded, true) t.is(response.path, '/api/echo') t.deepEqual(response.queryStringParameters, { ding: 'dong' }) }) @@ -591,7 +591,7 @@ testMatrix.forEach(({ args }) => { form.append('some', 'thing') const expectedBoundary = form.getBoundary() - const expectedResponseBody = form.getBuffer().toString() + const expectedResponseBody = form.getBuffer().toString('base64') const response = await got .post(`${server.url}/api/echo?ding=dong`, { @@ -603,7 +603,7 @@ testMatrix.forEach(({ args }) => { t.is(response.headers['content-type'], `multipart/form-data; boundary=${expectedBoundary}`) t.is(response.headers['content-length'], '164') t.is(response.httpMethod, 'POST') - t.is(response.isBase64Encoded, false) + t.is(response.isBase64Encoded, true) t.is(response.path, '/api/echo') t.deepEqual(response.queryStringParameters, { ding: 'dong' }) t.is(response.body, expectedResponseBody) diff --git a/tests/eleventy.test.js b/tests/eleventy.test.js index 62fd21ddb98..cc06c063a92 100644 --- a/tests/eleventy.test.js +++ b/tests/eleventy.test.js @@ -66,7 +66,7 @@ test('functions rewrite echo without body', async (t) => { 'x-forwarded-for': '::ffff:127.0.0.1', }) t.is(response.httpMethod, 'GET') - t.is(response.isBase64Encoded, false) + t.is(response.isBase64Encoded, true) t.is(response.path, '/api/echo') t.deepEqual(response.queryStringParameters, { ding: 'dong' }) }) @@ -114,7 +114,7 @@ test('functions echo with multiple query params', async (t) => { 'x-forwarded-for': '::ffff:127.0.0.1', }) t.is(response.httpMethod, 'GET') - t.is(response.isBase64Encoded, false) + t.is(response.isBase64Encoded, true) t.is(response.path, '/.netlify/functions/echo') t.deepEqual(response.queryStringParameters, { category: 'a, b' }) t.deepEqual(response.multiValueQueryStringParameters, { category: ['a', 'b'] }) diff --git a/tests/serving-functions.test.js b/tests/serving-functions.test.js index 704dc1e54c3..1e9ccb08b78 100644 --- a/tests/serving-functions.test.js +++ b/tests/serving-functions.test.js @@ -743,6 +743,42 @@ exports.handler = () => ({ }) }) }) + + test(testName('Resembles base64 encoding of production', args), async (t) => { + await withSiteBuilder('function-base64-encoding', async (builder) => { + const bundlerConfig = args.includes('esbuild') ? { node_bundler: 'esbuild' } : {} + + await builder + .withNetlifyToml({ + config: { + build: { publish: 'public' }, + functions: { directory: 'functions' }, + ...bundlerConfig, + }, + }) + .withFunction({ + path: 'echoEncoding.js', + handler: async (event) => ({ + statusCode: 200, + body: event.isBase64Encoded ? 'base64' : 'plain', + }), + }) + .buildAsync() + + await withDevServer({ cwd: builder.directory, args }, async ({ outputBuffer, port }) => { + await tryAndLogOutput(async () => { + t.is( + await got(`http://localhost:${port}/.netlify/functions/echoEncoding`, { + headers: { + 'Content-Type': 'multipart/form-data', + }, + }).text(), + 'base64', + ) + }, outputBuffer) + }) + }) + }) }) test('Serves functions that dynamically load files included in the `functions.included_files` config property', async (t) => {