unjs / httpxy

πŸ”€ A Full-Featured HTTP and WebSocket Proxy for Node.js
Other
194 stars 12 forks source link

DELETE requests add a "Content-Length: 0" header, causing undici to error "Request body length does not match content-length header" #7

Open dan-hammond opened 1 year ago

dan-hammond commented 1 year ago

Environment

Reproduction

I confess I haven't made one up-front. I think the explanation below might be enough, but if not I can try to figure out a reproduction.

Describe the bug

When making a DELETE request, the request has a "Content-Length: 0" header added to it, which causes Node's undici to respond with the following error:

TypeError: fetch failed
    at Object.fetch (node:internal/deps/undici/undici:11576:11)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async sendProxy (file:///myprojectpath/node_modules/h3/dist/index.mjs:1049:20)
    at async Object.handler (file:///myprojectpath/node_modules/h3/dist/index.mjs:1646:19)
    at async Server.toNodeHandle (file:///myprojectpath/node_modules/h3/dist/index.mjs:1857:7) {
  cause: RequestContentLengthMismatchError: Request body length does not match content-length header
      at write (node:internal/deps/undici/undici:10059:41)
      at _resume (node:internal/deps/undici/undici:10037:33)
      at resume (node:internal/deps/undici/undici:9938:7)
      at [dispatch] (node:internal/deps/undici/undici:9286:11)
      at Client.Intercept (node:internal/deps/undici/undici:9017:20)
      at Client.dispatch (node:internal/deps/undici/undici:7772:44)
      at [dispatch] (node:internal/deps/undici/undici:7991:32)
      at Pool.dispatch (node:internal/deps/undici/undici:7772:44)
      at [dispatch] (node:internal/deps/undici/undici:10556:27)
      at Agent.Intercept (node:internal/deps/undici/undici:9017:20) {
    code: 'UND_ERR_REQ_CONTENT_LENGTH_MISMATCH'
  }

More context has been provided here: https://github.com/nodejs/undici/issues/2046#issuecomment-1496932582

Here's the same code in this fork: https://github.com/unjs/httpxy/blob/f8cde141ece42e5a8d243a2963d6644ba3f83212/src/middleware/web-incoming.ts#L10

This is all somewhat beyond me, but I would think it's not the responsibility of the proxy to be adding HTTP headers that weren't provided in the initial request, unless it's intentional behaviour written in middleware or proxy route rules.

I appreciate that Node may end up fixing their underlying behaviour to allow DELETE requests with a Content-Length, but that may not solve the problem for people proxying to other non-JS back-ends.

Similarly, I have read in the linked undici issue that Safari does send Content-Length: 0 with DELETE requests, so it would be useful to know how I can filter those out. I am using extendRouteRules in a Nuxt module to register my proxy rules, and am doing so as a wildcard for a whole path prefix. Is that currently possible?

Additional context

No response

Logs

No response

pi0 commented 1 year ago

Thanks for the issue. Related upstream issues https://github.com/http-party/node-http-proxy/issues/1343 and https://github.com/nodejs/node/issues/27880

This feature was originally introduced in http-proxy via https://github.com/http-party/node-http-proxy/pull/742 as part of bug fix for nginx and chunked streaming. I will have to check it more in defails.

Anyway i think it is a valid bug if we are overriding the existing content-length.

wug-ge commented 1 year ago

I'm facing the same issue using Nuxt,js routeRules proxy and my DELETE requests don't work.

Putting

delete opts.fetchOptions.headers['content-length']

before this line

https://github.com/unjs/h3/blob/12ccb81432a62783fe951a8170976c8162bce44a/src/utils/proxy.ts#L77

solves my issue, otherwise it fails with a UND_ERR_REQ_CONTENT_LENGTH_MISMATCH.

I'm haven't digged deep enough into the ecosystem to understand fully what's going on, I'd really appreciate if someone could explain further what exactly is happening / provide a workaround.

For now I might have to use a different proxy.

If wanted, I'm happy to do some digging myself. (not sure how efficient I'll be here though)

brc-dd commented 12 months ago

undici fixed this in https://github.com/nodejs/undici/issues/2046 , but not sure if it'll be backported to LTS. I'm using nitro.experimental.nodeFetchCompat till that fix makes to a node LTS πŸ‘€

wug-ge commented 10 months ago

@brc-dd can you explain what you mean by using nitro.experimental.nodeFetchCompat?

I can't find much about it, what it is and how do I set it in my Nuxt config? :smile:

I tested this issue with node 18, 20 and 21 and it persists in all versions. My current workaround is to add something random in the body of the DELETE request, which is quiet ugly...

Denoder commented 9 months ago

@wug-ge


export default defineNuxtConfig({
    nitro: {
        experimental: {
            nodeFetchCompat: true
        }
    }
})
brc-dd commented 9 months ago

You might also need to disable built-in fetch. I'm using this kind of setup:

// package.json

  "scripts": {
    "dev": "pnpm nuxt dev",
    "build": "pnpm nuxt build",
    "generate": "pnpm nuxt generate",
    "preview": "pnpm nuxt preview",
    "postinstall": "pnpm nuxt prepare",
    "nuxt": "NODE_OPTIONS='--no-experimental-fetch' nuxt"
  }
// nuxt.config.ts

export default defineNuxtConfig({
  nitro: {
    experimental: {
      nodeFetchCompat: true
    }
  }
})

At the time I posted that comment, it was working fine just by setting in nuxt.config, but broke between some Nuxt minor IG. So you might need to add both. I have confirmed that it works fine in latest Node.js without any extra config, so probably this issue can be closed as I think this was always an upstream issue.

PS: I just read node@20.11.0 release notes and looks like it should now work fine on latest LTS too. πŸŽ‰