fix: resemble Netlify Production logic for base64 encoding more close…

…ly (#3631) * 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 <[email protected]> * chore: fix test Co-authored-by: Netlify Team Account 1 <[email protected]> Co-authored-by: Eduardo Bouças <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
netlify · Nov 15, 2021 · efc0b48 · efc0b48 · github-actions · Nov 15, 2021
1 parent 72927da
commit efc0b48
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 8 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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) => {