Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Presence of middleware prevents access to raw request bodies greater than or equal to 16,384 bytes (16 KiB) #39262

Closed
1 task done
jhahn opened this issue Aug 2, 2022 · 44 comments · Fixed by Sitecore/jss#1134 or #41270
Labels
bug Issue was opened via the bug report template. Runtime Related to Node.js or Edge Runtime with Next.js.

Comments

@jhahn
Copy link

jhahn commented Aug 2, 2022

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 21.6.0: Sat Jun 18 17:07:22 PDT 2022; root:xnu-8020.140.41~1/RELEASE_ARM64_T6000
    Binaries:
      Node: 18.7.0
      npm: 8.15.0
      Yarn: 1.22.19
      pnpm: 7.8.0
    Relevant packages:
      next: 12.2.4-canary.9
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0

What browser are you using? (if relevant)

N/A

How are you deploying your application? (if relevant)

N/A

Describe the Bug

When attempting to upload a file over a few kilobytes (e.g. sending a POST request with a binary body and a Content-Type of multipart/form-data) via fetch or curl, the request stalls, then fails with the error:

error - Error: aborted
    at connResetException (node:internal/errors:704:14)
    at abortIncoming (node:_http_server:700:17)
    at socketOnClose (node:_http_server:694:3)
    at Socket.emit (node:events:525:35)
    at TCP.<anonymous> (node:net:757:14) {
  middleware: true
}

This occurs only for API pages with…

export const config = {
  api: {
    bodyParser: {
      bodyParser: false,
    },
  },
}

and only when middleware is present; even something as basic as:

import {NextRequest, NextResponse} from "next/server"

export async function middleware(req: NextRequest) {
    return NextResponse.next()
}

Removing the middleware fixes the issue. Of note, very small request bodies (e.g. < 1kb files) work even in the presence of middleware.

Expected Behavior

Sending a POST request to an API endpoint with a Content-Type of multipart/form-data along with a reasonably sized (~200kB) binary payload should work and not stall.

Link to reproduction

https://github.com/jhahn/nextjs-upload-issue

To Reproduce

pages/index.tsx

import type { NextPage } from 'next'
import Head from 'next/head'
import Image from 'next/image'
import styles from '../styles/Home.module.css'

const Home: NextPage = () => {
  const uploadFile = async (files: FileList | null) => {
    if (!files) return
    const formData = new FormData()
    formData.append("file", files[0])
    const response = await fetch("/api/hello", {
      method: "POST",
      body: formData,
    })
    console.log(await response.json())
  }

  return <input type="file" onChange={(e) => uploadFile(e.target.files)} />
}

export default Home

pages/api/hello.ts:

import type { Readable } from 'node:stream';
import type { NextApiRequest, NextApiResponse } from 'next'

export const config = {
  api: {
    bodyParser: {
      bodyParser: false,
    },
  },
}

async function buffer(readable: Readable) {
  const chunks = [];
  for await (const chunk of readable) {
    chunks.push(typeof chunk === 'string' ? Buffer.from(chunk) : chunk);
  }
  return Buffer.concat(chunks);
}

export default async function (req: NextApiRequest, res: NextApiResponse) {
  if (req.method === 'POST') {
    const buf = await buffer(req.body);
    const rawBody = buf.toString('utf8');

    // Can do something here...
    res.json({ rawBody });
  } else {
    res.setHeader('Allow', 'POST');
    res.status(405).end('Method Not Allowed');
  }
}

The code for pages/api/hello.ts was adapted from https://vercel.com/support/articles/how-do-i-get-the-raw-body-of-a-serverless-function. However, I had to change const buf = await buffer(req); to const buf = await buffer(req.body);

@jhahn jhahn added the bug Issue was opened via the bug report template. label Aug 2, 2022
@Evilscaught
Copy link

Sorry, I'm not necessarily here to answer your issue, but I did notice a potential bug when skimming through your code,

export const config = {
  api: {
    bodyParser: {
      bodyParser: false,
    },
  },
}

According to these NextJS' guides, shouldn't this be:

export const config = {
  api: {
    bodyParser: false,
  },
}

@jhahn
Copy link
Author

jhahn commented Aug 2, 2022

@Evilscaught good catch! (The perils of hacking away until well after midnight…)

I’ve updated the code in the sample repo; this is what the corrected API page looks like:

pages/api/hello.ts:

import type { Readable } from 'node:stream';
import type { NextApiRequest, NextApiResponse } from 'next'

export const config = {
    api: {
        bodyParser: false,
    },
}

async function buffer(readable: Readable) {
    const chunks = [];
    for await (const chunk of readable) {
        chunks.push(typeof chunk === 'string' ? Buffer.from(chunk) : chunk);
    }
    return Buffer.concat(chunks);
}

export default async function (req: NextApiRequest, res: NextApiResponse) {
    if (req.method === 'POST') {
        const buf = await buffer(req);
        const rawBody = buf.toString('utf8');

        // Can do something here...
        res.json({ rawBody });
    } else {
        res.setHeader('Allow', 'POST');
        res.status(405).end('Method Not Allowed');
    }
}

Fixing the config export also meant changing back to const buf = await buffer(req);, bringing the code into alignment with the example provide by Vercel.

Unfortunately, the issue as described originally is still present; any sort of middleware seems to prevent the processing of a raw body over 1-2kB.

@levinhtin
Copy link

levinhtin commented Aug 3, 2022

Same issue, post a multipart/form-data is prevented by middleware. When I remove middleware.ts file, it's work.

@jhahn
Copy link
Author

jhahn commented Aug 3, 2022

After some additional digging, I’ve realized the multipart/form-data bit is probably a red herring.

I’ve reproduced the issue more precisely with a second test case at https://github.com/jhahn/nextjs-upload-issue/blob/main/pages/large-post.tsx:

/** Add your relevant code here for the issue to reproduce */
export default function Home() {
    const largePost = async () => {
        const formData = new FormData()
        const succeeds = new Uint8Array(16383);
        const fails = new Uint8Array(16384);
        self.crypto.getRandomValues(succeeds);
        self.crypto.getRandomValues(fails);
        const response = await fetch("/api/hello", {
            method: "POST",
            body: fails,
        })
        console.log(await response.json())
    }

    return <button onClick={largePost}>Send Large Post</button>
}

A POST of the succeeds array, with a size of 16,383 bytes, works when middleware is present.

Adding a single byte (the fails array) for a size of 16,384 bytes, causes the POST to hang when middleware is present.

(Again – when there’s no middleware, things work as they should)

Fascinatingly, 16,384 bytes (16 KiB) corresponds exactly to the default highWaterMark option for NodeJS Readable and Writable streams. (https://nodejs.org/api/stream.html#new-streamreadableoptions)

@jhahn jhahn changed the title Presence of middleware prevents access to raw body of multipart-form/data requests Presence of middleware prevents access to raw body of multipart/form-data requests Aug 3, 2022
@jhahn
Copy link
Author

jhahn commented Aug 4, 2022

I’ve updated the repo to 12.2.4-canary.11; this issue is still present.

@jhahn jhahn changed the title Presence of middleware prevents access to raw body of multipart/form-data requests Presence of middleware prevents access to raw request bodies greater than or equal to 16,384 bytes (16 KiB) Aug 4, 2022
@jhahn
Copy link
Author

jhahn commented Aug 4, 2022

I’ve updated the repo to 12.2.4-canary.12 and confirmed the issue is still present.

@balazsorban44 balazsorban44 added the Runtime Related to Node.js or Edge Runtime with Next.js. label Aug 8, 2022
@jhahn
Copy link
Author

jhahn commented Aug 9, 2022

I’ve updated the repo to 12.2.5-canary.0 and confirmed the issue is still present.

@Marcelx8
Copy link

Marcelx8 commented Aug 9, 2022

Could the issue be related to this change?
d7e8370

@dmgawel
Copy link

dmgawel commented Aug 9, 2022

Apparently, it worked in v12.0.0 (middleware introduced). I did a bisect to narrow down which version introduced this bug and I can confirm it stopped working in 12.1.1-canary.1. Looking at the version changelog, it's almost certainly caused by #34519.

I did some testing and while sending application/json requests the issue is even more pronounced because it's present even with an empty or very small body. Again, without the middleware requests hits the API endpoint correctly.

Example request:

const response = await fetch("/api/hello", {
    method: "POST",
    headers: {
        "Content-Type": "application/json",
    },
    body: JSON.stringify({}),
});

EDIT: Looks like that small JSON payload issue was resolved by #35131, but the original issue still persists.

Following up, in my testing the issue occurs regardless of bodyParser setting.

@jhahn
Copy link
Author

jhahn commented Aug 10, 2022

🤖 I’ve updated the repo to 12.2.5-canary.1 and confirmed the issue is still present. (I hope confirming with each canary release is somewhat helpful!)

@Marcelx8
Copy link

Marcelx8 commented Aug 10, 2022

🤖 I’ve updated the repo to 12.2.5-canary.1 and confirmed the issue is still present. (I hope confirming with each canary release is somewhat helpful!)

@jhahn - I believe everyone following this issue appreciates you testing it with every release, thank you. It might be helpful to have a look at the canary release changelog to see whether there were any changes made to the related affected area.

@Pamboli
Copy link

Pamboli commented Aug 10, 2022

This problem appears on the 12.2.3 version. In the 12.2.2 there is no problem.
I hope this information may be useful.

@Marcelx8
Copy link

This problem appears on the 12.2.3 version. In the 12.2.2 there is no problem. I hope this information may be useful.

Verified on my end and working when I changed the version to 12.2.2

Thank you @Pamboli !

@dmgawel
Copy link

dmgawel commented Aug 11, 2022

Great find @Pamboli, looks like the issue was first introduced in 12.1.1-canary.1, then fixed, and then a similar bug was reintroduced in 12.2.3-canary.17 (see new commits comparing to canary 16).

@jhahn
Copy link
Author

jhahn commented Aug 12, 2022

I’ve updated the repo to the final release of 12.2.5 and confirmed the issue is still present.

I also did a bit of additional digging and confirmed the findings of @dmgawel:

  • The last official release that doesn’t exhibit the issue described here is 12.1.0
  • The last pre-release that doesn’t exhibit the issue is 12.1.1-canary.0

If I had to hazard a guess, #34519 is the PR that introduced the issue. #34966 looks relevant, as does its fix #35131.

@sloonz
Copy link

sloonz commented Aug 15, 2022

This occurs only for API pages with…

  api: {
    bodyParser: false,
  },
}```

Can you confirm this part ? Commenting away bodyParser: false (or the config) does not solve the issue for me. In fact it’s not even for API pages only, I can reproduce the issue on a test page ("/test" just returning a page containing "Hello world") fine.

@jhahn
Copy link
Author

jhahn commented Aug 15, 2022

@sloonz good catch; thanks! While our use-case (uploaded files) requires access to the raw request body, I can confirm your findings that the issue occurs with or without the bodyParser config set.

@hmdnprks
Copy link

I have the same issue with http-proxy in my API route and a middleware file. The request will stall if I upload a file larger than 64kb. I think it's the default chunk size for the stream mentioned here. Removing the middleware file will resolve the issue.

@Gawdfrey
Copy link

Thanks for updating @jhahn ! Sadly downgrading did not help me, a bit unsure why but I will have to conduct further tests tomorrow. Removing the middleware worked great, but is necessary for the project I am working on so that is not an option.

@jhahn
Copy link
Author

jhahn commented Aug 15, 2022

@Gawdfrey FWIW, the workaround proposed by @Pamboli (downgrading to 12.2.2) did not work for us, either.

@Gawdfrey
Copy link

Gawdfrey commented Aug 15, 2022

Actually, I just found and attempted with the matcher filter and I was able upload a 295Kb file without deleting the middleware on version 12.2.4! This is of course only an option if you can do without middleware on the paths you exclude.

export const config = {
  runtime: "nodejs",
  matcher: "/about/:path*",
}

A bit late so I will confirm further tomorrow.

@jhahn
Copy link
Author

jhahn commented Aug 17, 2022

I’ve updated the sample repo to 12.2.6-canary.1 and confirmed this issue is still present.

I can also confirm @Gawdfrey’s "matcher filter" workaround solves the issue, assuming you don’t need middleware on the excluded path(s).

@jhahn
Copy link
Author

jhahn commented Aug 23, 2022

I’ve updated the sample repo to 12.2.6-canary.2 and confirmed the issue is still present.

@Gawdfrey
Copy link

Thanks for the snippet works great for my use case! @ambrauer

@noc2spam
Copy link

noc2spam commented Sep 7, 2022

Same issue here. Only downgrading to 12.2.2 helps. After downgrading as well as applying

export const config = {
    api: {
      bodyParser: {
        sizeLimit: '4mb',
      },
    },
  }

the API works as expected.

@hoersamu
Copy link

Has anybody tested it with 12.3 yet?

@AndyChinViolet
Copy link

Just tested on 12.3 and have the same issue

Has anybody tested it with 12.3 yet?

@malindap92
Copy link

Just tested on 12.3 and have the same issue

Has anybody tested it with 12.3 yet?

@hoersamu @AndyChinViolet I can confirm that we are also having the same issue with 12.3.0. Downgrading to 12.2.0 works.

@manuschillerdev
Copy link
Contributor

Downgrading to 12.2.0 does not solve the problem for me while using withAuth from next-auth.

@Dinesh485
Copy link

It worked after downgrading to 12.2.2 and with next-auth default export in middleware and with matcher routes.

@seanislegend
Copy link

I experienced the same issue. It took me a while to figure out, completely forgetting that middleware may be a factor. After downgrading from 12.3.0 to 12.2.2 I had no issues with uploads.

@clementpoiret
Copy link

I hope it'll be fixed soon... Downgrading to 12.2.2 also fixed the issue for me as well

@manuschillerdev
Copy link
Contributor

can confirm that downgrading to nextjs 12.2.2 (exact version, not ^12.2.2) fixes the issue for me as well
nextjs + next-auth (^4.10.3) middleware.

@bcheidemann
Copy link
Contributor

I hope it'll be fixed soon... Downgrading to 12.2.2 also fixed the issue for me as well

Me too! 😅 Has anyone been able to identify the actual cause of the issue - in theory I'd be happy to raise a PR but don't currently have time to dig about trying to find the cause.

@dmgawel
Copy link

dmgawel commented Oct 7, 2022

@bcheidemann the issue was almost certainly (re)introduced in this PR: #38862

Because it contains a lot of changes regarding body streams and given I'm not familiar with Next.js internal code, I'm not able to point a single line or root cause. However, for Contributors it should be much easier to spot ;-) I hope pointing the PR helps.

@aprendendofelipe
Copy link
Contributor

I proposed a solution (#41270). Any collaboration or suggestion is welcome.

ijjk added a commit that referenced this issue Oct 17, 2022
Fixes #39262

The solution is to call `stream.push(null)` to trigger the `end` event
which allows `getRawBody` to run completely.

<!--
Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change that you're making:
-->

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see `contributing.md`

## Documentation / Examples

- [x] Make sure the linting passes by running `pnpm lint`
- [x] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)

Co-authored-by: JJ Kasper <jj@jjsweb.site>
Kikobeats pushed a commit to Kikobeats/next.js that referenced this issue Oct 24, 2022
…1270)

Fixes vercel#39262

The solution is to call `stream.push(null)` to trigger the `end` event
which allows `getRawBody` to run completely.

<!--
Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change that you're making:
-->

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see `contributing.md`

## Documentation / Examples

- [x] Make sure the linting passes by running `pnpm lint`
- [x] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)

Co-authored-by: JJ Kasper <jj@jjsweb.site>
@integral-llc
Copy link

updating to version 13 solved the same issue I had.

@group-buex
Copy link

group-buex commented Nov 10, 2022

I solved this issue above Next@12.2.2
and I can upload large(?) file(up to 16kb)

You can ignore middleware to specific routes through the Matcher

// api route example
/sample-route
// middleware.js or .ts

export const config = {
  matcher: [
    '/((?!sample-route|another-routes).*)', // << Add external routes here
  ],
}

@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. Runtime Related to Node.js or Edge Runtime with Next.js.
Projects
None yet