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

Sending a DELETE request with "Content-Length: 0" fails with a RequestContentLengthMismatchError #2046

Closed
ouuan opened this issue Apr 4, 2023 · 19 comments · Fixed by #2305
Closed
Labels
bug Something isn't working

Comments

@ouuan
Copy link

ouuan commented Apr 4, 2023

Bug Description

Sending a DELETE request with "content-length: 0" fails with a RequestContentLengthMismatchError.

The specification says "A user agent SHOULD NOT send a Content-Length header field when the request message does not contain a payload body and the method semantics do not anticipate such a body", but according to RFC 2119, "SHOULD NOT" means "NOT RECOMMENDED", so it's not reasonable to fail a request for this reason.

Even if the Content-Length header "MUST NOT" be sent, the current implementation would still be problematic:

  1. A better behavior could be removing the header instead of failing the request.
  2. The request only fails when Content-Length is set to zero, but not when it is not zero and matches the length of the body. Note that the DELETE method does not anticipate a body.
  3. The error message is unhelpful. It says "Request body length does not match content-length header" instead of "Content-Length should not be set for the DELETE method".

Reproducible By

fetch('https://example.com', { method: 'DELETE', headers: { 'content-length': '0' } })

Expected Behavior

The request succeeds, either as it is or with the Content-Length header removed.

Logs & Screenshots

> Uncaught TypeError: fetch failed
    at Object.fetch (node:internal/deps/undici/undici:11413:11) {
  cause: RequestContentLengthMismatchError: Request body length does not match content-length header
      at write (node:internal/deps/undici/undici:9907:41)
      at _resume (node:internal/deps/undici/undici:9885:33)
      at resume (node:internal/deps/undici/undici:9787:7)
      at connect (node:internal/deps/undici/undici:9776:7) {
    code: 'UND_ERR_REQ_CONTENT_LENGTH_MISMATCH'
  }
}

Environment

Reproducible on both Node v18.15.0 and v19.8.1

Additional Context

Related codes:

undici/lib/client.js

Lines 1333 to 1366 in 5f3b8e1

const expectsPayload = (
method === 'PUT' ||
method === 'POST' ||
method === 'PATCH'
)
if (body && typeof body.read === 'function') {
// Try to read EOF in order to get length.
body.read(0)
}
let contentLength = util.bodyLength(body)
if (contentLength === null) {
contentLength = request.contentLength
}
if (contentLength === 0 && !expectsPayload) {
// https://tools.ietf.org/html/rfc7230#section-3.3.2
// A user agent SHOULD NOT send a Content-Length header field when
// the request message does not contain a payload body and the method
// semantics do not anticipate such a body.
contentLength = null
}
if (request.contentLength !== null && request.contentLength !== contentLength) {
if (client[kStrictContentLength]) {
errorRequest(client, request, new RequestContentLengthMismatchError())
return false
}
process.emitWarning(new RequestContentLengthMismatchError())
}

@ouuan ouuan added the bug Something isn't working label Apr 4, 2023
@ronag
Copy link
Member

ronag commented Apr 5, 2023

PR welcome

@ronag
Copy link
Member

ronag commented Apr 5, 2023

FYI I'm not sure atm what I think is right or wrong here. What is the realistic scenario where this is a problem? @KhafraDev shouldn't content-length be removed by fetch anyway? Why isn't it?

@ouuan
Copy link
Author

ouuan commented Apr 5, 2023

What is the realistic scenario where this is a problem?

I'm using Nuxt, which is using Nitro, whose proxy is based on node-http-proxy. And the problem comes from this code:

https://github.com/http-party/node-http-proxy/blob/9b96cd725127a024dabebec6c7ea8c807272223d/lib/http-proxy/passes/web-incoming.js#L34-L40

node-http-proxy is not actively maintained, and it cost me a few hours to find where the bug was. It's certainly a bug of node-http-proxy in my case, but I just think it doesn't worth the effort (to locate and) to fix the bug when the Content-Length header is mistakenly added somewhere.

@KhafraDev
Copy link
Member

KhafraDev commented Apr 5, 2023

content-length is removed on redirect in fetch, this bug would also affect undici.request

@ronag
Copy link
Member

ronag commented Apr 5, 2023

content-length is removed on redirect in fetch, this bug would also affect undici.request

I'm aware it affects undici.request, however I wouldn't consider it a bug per se in that context. Might be different for fetch of the spec says to remove it.

@KhafraDev
Copy link
Member

I think failing the request is fine in fetch too.

@ronag
Copy link
Member

ronag commented Apr 14, 2023

@ouuan If you want to make a PR to change this behavior, we'd consider merging it.

@kaloyanBozhkov
Copy link

kaloyanBozhkov commented Apr 22, 2023

I encountered this error as well. Using tRPC with Next.JS:

 TRPCClientError: fetch failed
    at TRPCClientError.from (file:///var/task/node_modules/@trpc/client/dist/transformResult-6fb67924.mjs:13:16)
    at file:///var/task/node_modules/@trpc/client/dist/links/httpBatchLink.mjs:211:64 {
  meta: undefined,
  shape: undefined,
  data: undefined,
  [cause]: TypeError: fetch failed
      at Object.fetch (node:internal/deps/undici/undici:14062:11) {
    cause: RequestContentLengthMismatchError: Request body length does not match content-length header
        at AsyncWriter.end (node:internal/deps/undici/undici:9704:19)
        at writeIterable (node:internal/deps/undici/undici:9614:16) {
      code: 'UND_ERR_REQ_CONTENT_LENGTH_MISMATCH'
    }
  }
}

@kmvan
Copy link

kmvan commented Aug 26, 2023

Should the server ignore content-length when it receives a DELETE request?

@metcoder95
Copy link
Member

Servers most likely can decline the request if a content-length and/or body of the request is received. It is highly recommended to not do it at all at the client side. From RFC-9110:

Although request message framing is independent of the method used, content received in a DELETE request has no generally defined semantics, cannot alter the meaning or target of the request, and might lead some implementations to reject the request and close the connection because of its potential as a request smuggling attack (Section 11.2 of [HTTP/1.1]). A client SHOULD NOT generate content in a DELETE request unless it is made directly to an origin server that has previously indicated, in or out of band, that such a request has a purpose and will be adequately supported

@pxue
Copy link
Contributor

pxue commented Oct 3, 2023

Well just spent the entire day to dig through this issue, and also just realized all of our users couldn't DELETE things in the app for a good few months.

The discrepancy is where undici expects content length to be 'null'

contentLength = null
and not zero

now I'm fixing this by sending an empty body through all DELETE calls.

@KhafraDev
Copy link
Member

I think we should allow this. If anyone wants to send in a PR, that would be great.

@pxue
Copy link
Contributor

pxue commented Oct 3, 2023

I think we should allow this. If anyone wants to send in a PR, that would be great.

@KhafraDev is the fix here to allow 0-ish values in client.js? If so I can open a PR and write some tests

@KhafraDev
Copy link
Member

I would only expect it to be set to 0 (outside of where it's already being done) if the method is DELETE and content-length is 0, and definitely a comment linking to this issue.

@pxue
Copy link
Contributor

pxue commented Oct 3, 2023

Ok got it, just to confirm, would I have to check for both integer 0 and string '0'?

For example, the source of the issue comes from node-http-proxy where the content-length is being forced to '0'
https://github.com/http-party/node-http-proxy/blob/9b96cd725127a024dabebec6c7ea8c807272223d/lib/http-proxy/passes/web-incoming.js#L37

@KhafraDev
Copy link
Member

no, it's an integer (the code in the linked issue only checks for 0)

@pxue
Copy link
Contributor

pxue commented Oct 3, 2023

PR opened, let me know if that's a passable fix @KhafraDev

@sahanxdissanayake
Copy link

Getting this same error when calling a python/flask service hosted on vercel from a server action with fetch

{
  error: TypeError: fetch failed
      at Object.fetch (node:internal/deps/undici/undici:11457:11) {
    cause: RequestContentLengthMismatchError: Request body length does not match content-length header
        at AsyncWriter.end (/Users/sahan/dashboard/node_modules/next/dist/compiled/undici/index.js:1:82040)
        at writeIterable (/Users/sahan/dashboard/node_modules/next/dist/compiled/undici/index.js:1:80712) {
      code: 'UND_ERR_REQ_CONTENT_LENGTH_MISMATCH'
    }
  }
}

@sahanxdissanayake
Copy link

Moved to axios and the request seems to be working now. Will report back if there are further issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants