fix: disable transfer-encoding leading to problems in fastly (#333)

* fix: disregard accept-encoding at origin

* chore: format with prettier

* fix: disable buggy transfer-encoding behaviour in fastly

* chore: format with prettier

* chore: add e2e test

* fix: rename util

* fix: types

* chore: update comment

* refactor: rename function

* Update src/run/handlers/server.ts

Co-authored-by: Matt Kane <[email protected]>

---------

Co-authored-by: Skn0tt <[email protected]>
Co-authored-by: Michal Piechowiak <[email protected]>
Co-authored-by: Matt Kane <[email protected]>

Author: 3 people

src/run/handlers/server.ts

@@ -1,4 +1,6 @@
-import { toComputeResponse, toReqRes } from '@fastly/http-compute-js'
+import type { OutgoingHttpHeaders } from 'http'
+
+import { toComputeResponse, toReqRes, ComputeJsOutgoingMessage } from '@fastly/http-compute-js'
 import { SpanStatusCode, trace } from '@opentelemetry/api'
 import type { NextConfigComplete } from 'next/dist/server/config-shared.js'
 import type { WorkerRequestHandler } from 'next/dist/server/lib/types.js'
@@ -18,6 +20,29 @@ import { createRequestContext, runWithRequestContext } from './request-context.c
 
 let nextHandler: WorkerRequestHandler, nextConfig: NextConfigComplete, tagsManifest: TagsManifest
 
+/**
+ * When Next.js proxies requests externally, it writes the response back as-is.
+ * In some cases, this includes Transfer-Encoding: chunked.
+ * This triggers behaviour in @fastly/http-compute-js to separate chunks with chunk delimiters, which is not what we want at this level.
+ * We want Lambda to control the behaviour around chunking, not this.
+ * This workaround removes the Transfer-Encoding header, which makes the library send the response as-is.
+ */
+const disableFaultyTransferEncodingHandling = (res: ComputeJsOutgoingMessage) => {
+  const originalStoreHeader = res._storeHeader
+  res._storeHeader = function _storeHeader(firstLine, headers) {
+    if (headers) {
+      if (Array.isArray(headers)) {
+        // eslint-disable-next-line no-param-reassign
+        headers = headers.filter(([header]) => header.toLowerCase() !== 'transfer-encoding')
+      } else {
+        delete (headers as OutgoingHttpHeaders)['transfer-encoding']
+      }
+    }
+
+    return originalStoreHeader.call(this, firstLine, headers)
+  }
+}
+
 export default async (request: Request) => {
   const tracer = trace.getTracer('Next.js Runtime')
 
@@ -53,6 +78,8 @@ export default async (request: Request) => {
 
     const requestContext = createRequestContext()
 
+    disableFaultyTransferEncodingHandling(res as unknown as ComputeJsOutgoingMessage)
+
     const resProxy = nextResponseProxy(res, requestContext)
 
     // We don't await this here, because it won't resolve until the response is finished.

tests/e2e/simple-app.test.ts

@@ -205,3 +205,11 @@ test('requesting a non existing page route that needs to be fetched from the blo
   )
   expect(headers['cache-control']).toBe('public,max-age=0,must-revalidate')
 })
+
+test('Compressed rewrites are readable', async ({ simpleNextApp }) => {
+  const resp = await fetch(`${simpleNextApp.url}/rewrite-no-basepath`)
+  expect(resp.headers.get('content-length')).toBeNull()
+  expect(resp.headers.get('transfer-encoding')).toEqual('chunked')
+  expect(resp.headers.get('content-encoding')).toEqual('br')
+  expect(await resp.text()).toContain('<title>Example Domain</title>')
+})

tests/fixtures/simple-next-app/next.config.js

@@ -17,6 +17,15 @@ const nextConfig = {
     ],
     domains: ['images.pexels.com'],
   },
+  async rewrites() {
+    return [
+      {
+        source: '/rewrite-no-basepath',
+        destination: 'https://example.vercel.sh',
+        basePath: false,
+      },
+    ]
+  },
 }
 
 module.exports = nextConfig

tests/integration/simple-app.test.ts

@@ -14,6 +14,7 @@ import {
   getBlobEntries,
   startMockBlobStore,
 } from '../utils/helpers.js'
+import { gunzipSync } from 'node:zlib'
 
 // Disable the verbose logging of the lambda-local runtime
 getLogger().level = 'alert'
@@ -139,3 +140,19 @@ test<FixtureTestContext>('handlers can add cookies in route handlers with the co
   expect(setCookieHeader).toContain('test1=value1; Path=/; Secure')
   expect(setCookieHeader).toContain('test2=value2; Path=/handler; HttpOnly')
 })
+
+// there's a bug where requests accept-encoding header
+// result in corrupted bodies
+// while that bug stands, we want to ignore accept-encoding
+test<FixtureTestContext>('rewrites to external addresses dont use compression', async (ctx) => {
+  await createFixture('simple-next-app', ctx)
+  await runPlugin(ctx)
+  const page = await invokeFunction(ctx, {
+    url: '/rewrite-no-basepath',
+    headers: { 'accept-encoding': 'gzip' },
+  })
+  expect(page.headers).not.toHaveProperty('content-length')
+  expect(page.headers).not.toHaveProperty('transfer-encoding')
+  expect(page.headers['content-encoding']).toBe('gzip')
+  expect(gunzipSync(page.bodyBuffer).toString('utf-8')).toContain('<title>Example Domain</title>')
+})

tests/utils/fixture.ts

@@ -17,7 +17,7 @@ import { env } from 'node:process'
 import { fileURLToPath } from 'node:url'
 import { v4 } from 'uuid'
 import { LocalServer } from './local-server.js'
-import { streamToString } from './stream-to-string.js'
+import { streamToBuffer } from './stream-to-buffer.js'
 
 import { glob } from 'fast-glob'
 import {
@@ -393,9 +393,12 @@ export async function invokeFunction(
       response.headers || {},
     )
 
+    const bodyBuffer = await streamToBuffer(response.body)
+
     return {
       statusCode: response.statusCode,
-      body: await streamToString(response.body),
+      bodyBuffer,
+      body: bodyBuffer.toString('utf-8'),
       headers: responseHeaders,
       isBase64Encoded: response.isBase64Encoded,
     }

tests/utils/index.ts

@@ -1,3 +1,3 @@
 export * from './helpers.js'
 export * from './mock-file-system.js'
-export * from './stream-to-string.js'
+export * from './stream-to-buffer.js'

tests/utils/stream-to-buffer.ts

@@ -0,0 +1,14 @@
+/**
+ * Converts a readable stream to a buffer
+ * @param stream Node.js Readable stream
+ * @returns
+ */
+export function streamToBuffer(stream: NodeJS.ReadableStream) {
+  const chunks: Buffer[] = []
+
+  return new Promise<Buffer>((resolve, reject) => {
+    stream.on('data', (chunk) => chunks.push(Buffer.from(chunk)))
+    stream.on('error', (err) => reject(err))
+    stream.on('end', () => resolve(Buffer.concat(chunks)))
+  })
+}