All DELETE requests are failling when using HTTP2 #253

delucca · 2022-06-17T21:46:06Z

Prerequisites

I have written a descriptive issue title
I have searched existing issues to ensure the bug has not already been reported

Fastify version

3.x.x

Plugin version

7.1.0

Node.js version

16.x.x

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

Fedora 35

Description

When running any DELETE request without a body with HTTP2 we got the following error:

{
	"statusCode": 500,
	"code": "UND_ERR_REQ_CONTENT_LENGTH_MISMATCH",
	"error": "Internal Server Error",
	"message": "Request body length does not match content-length header"
}

If we run the same request with HTTP1, the error doesn't happen

Even if we use http2: false on the proxy options, we still got that error if the request was executed with HTTP2

Also, seems like Undici doesn't support HTTP2 yet, maybe that's the reason?

Steps to Reproduce

Create a new application
Add a proxy to another application that has a valid DELETE endpoint
Execute a request to that DELETE endpoint using HTTP2

Expected Behavior

The request should be executed as expected

The text was updated successfully, but these errors were encountered:

mcollina · 2022-06-17T22:01:41Z

Thanks for reporting!

Can you provide steps to reproduce? We often need a reproducible example, e.g. some code that allows someone else to recreate your problem by just copying and pasting it. If it involves more than a couple of different file, create a new repository on GitHub and add a link to that.

I'm not sure what the problem is but it would be awesome if you'd try a fix!

delucca · 2022-06-18T05:16:41Z

@mcollina sure 😄

I was trying to debug it for a while and just stopped. Next monday I'll try to create a sample repo with it and share with you over here 😄

annahassel · 2023-09-27T13:33:38Z

@mcollina The browser always sends content-length: 0 for DELETE requests. But @fastify/http-proxy can't do this request, because of undici issue. So I can't use proxy (without rewriteRequestHeaders options) for DELETE requests. Is it bug or not?

replyOptions: {
  rewriteRequestHeaders(originalReq, headers) {
    const rewrittenHeaders = { ...headers }
    
    // undici issue https://github.com/nodejs/undici/issues/2046
    if (originalReq.method === 'DELETE') {
      rewrittenHeaders['content-length'] = undefined
    }

    return rewrittenHeaders
  },
}

mcollina · 2023-09-28T10:53:53Z

The browser always sends content-length: 0 for DELETE requests

Is this documented somewhere? is there a repro I can check?

annahassel · 2023-09-28T17:16:56Z

Is this documented somewhere?

Sorry, my fault, looks like it's only Safari problem.

is there a repro I can check?

This example works in Chrome, but fails in Safari (Version 16.4 (18615.1.26.110.1)).

Console output for Safari

[Info] status – 500 (index.html, line 8)
[Error] error – "Request body length does not match content-length header" (anonymous function) (index.html:13)

Console output for Chrome

status 200
index.html:13 error undefined

// server
const Fastify = require('fastify');
const server = Fastify();

server.register(require('@fastify/cors'), {
  origin: "*",
  methods: ["DELETE"]
});

server.register(require('@fastify/http-proxy'), {
  upstream: 'http://localhost:3000/api2',
  prefix: '/api', // optional
  http2: false, // optional
  // NOTE: server fix for Safari
  // replyOptions: {
  //   rewriteRequestHeaders(originalReq, headers) {
  //     const rewrittenHeaders = { ...headers }

  //     // undici issue https://github.com/nodejs/undici/issues/2046
  //     if (originalReq.method === 'DELETE') {
  //       rewrittenHeaders['content-length'] = undefined
  //     }

  //     return rewrittenHeaders
  //   },
  // },
});

server.route({
  method: 'DELETE',
  path: '/api2',
  handler(request, reply) {
    reply.send({ status: 'deleted' });
  },
});

server.listen({ port: 3000 });

<!-- client -->
<html>
  <head></head>
  <body>
    <script>
      window.onload = function onLoad() {
        fetch('http://localhost:3000/api', { method: 'DELETE' })
          .then((result) => {
            console.info('status', result.status)

            return result.json()
          })
          .then((error) => {
            console.error('error', error.message)
          })
      }
    </script>
  </body>
</html>

mcollina · 2023-10-03T09:51:28Z

This matches our expectations... I guess no one uses Safari. I think we should just remove the content-length for delete by default, at least until safari gets updated.

Alternatively we should just document this oddity, but I think it's better to have the patch for everyone.

I think this should be the default implementation of replyOptions.rewriteRequestHeaders, meaning that an end user that overwrite that method would need to take care of this.
In this way we allow a fallback to prevsiou behavior.

@annahassel @delucca Would you like to send a PR along those lines?

stercoris · 2024-01-29T15:20:10Z

I have a kinda similar issue, all DELETE requests failing with an error:

{
  "statusCode":500,
  "code":"FST_REPLY_FROM_INTERNAL_SERVER_ERROR",
  "error":"Internal Server Error",
  "message":"write after end"
}

GET, POST, PATCH, PUT methods work as intended.

Reproduce:

import fastify from 'fastify';
import proxy from '@fastify/http-proxy';

const proxyServer = fastify();

proxyServer.register(proxy, {
  upstream: 'http://localhost:3001/example2',
  prefix: '/example1',
  http2: true,
});

proxyServer.listen({ port: 3000 });

const http2Server = fastify({ http2: true });

http2Server.route({
  method: ['GET', 'PATCH', 'POST', 'PUT', 'DELETE'],
  url: '/example2',
  handler(_, reply) {
    reply.send({ status: 'you got it!' });
  },
});

http2Server.listen({ port: 3001 });

Curl:

curl --request DELETE --url http://localhost:3000/example1 # fail

curl --request POST --url http://localhost:3000/example1 # ok

Is it even correct to use a proxy this way? HTTP/1.1 => Proxy => Http/2 and reverse. Or am I doing something wrong?

PS: The upstream on which I received this behaviour supports versions 1.1 and 2, but all proxied requests are sent with version 2. It would be great if I could choose the version depending on the request being proxied.

mcollina · 2024-01-29T17:59:10Z

Does curl includes a Content-Length header?

stercoris · 2024-01-29T23:09:32Z

Nope, here's two traces of POST and DELETE requests from reproduce above:
DELETE:

DELETE /example1 HTTP/1.1
Host: localhost:3000
User-Agent: curl/7.88.1
Accept: */*

HTTP/1.1 500 Internal Server Error
content-type: application/json; charset=utf-8
content-length: 124
Date: Mon, 29 Jan 2024 22:52:42 GMT
Connection: keep-alive
Keep-Alive: timeout=72

{"statusCode":500,"code":"FST_REPLY_FROM_INTERNAL_SERVER_ERROR","error":"Internal Server Error","message":"write after end"}

POST:

POST /example1 HTTP/1.1
Host: localhost:3000
User-Agent: curl/7.88.1
Accept: */*

HTTP/1.1 200 OK
content-type: application/json; charset=utf-8
content-length: 24
date: Mon, 29 Jan 2024 22:54:27 GMT
Connection: keep-alive
Keep-Alive: timeout=72

{"status":"you got it!"}

mcollina · 2024-01-30T18:33:43Z

I currently do not have much time to debug this. However a PR would be nice.

karl-power · 2024-04-10T09:09:49Z

I've had a look at this and it seems to be caused by this call to req.end in the HTTP2 request handler from @fastify/reply-from.

The DELETE request does succeed without that call to .end, but it's not clear to me why it is needed, and why all other non-GET (i.e. POST, PATCH...) requests work fine with it.

Update: It seems that is indeed only GET and DELETE for which .end should not be called. I have opened a PR in fastify/fastify-reply-from to address this.

mcollina added enhancement good first issue labels Oct 3, 2023

karl-power mentioned this issue Apr 10, 2024

fix: DELETE requests should not call stream.end fastify/fastify-reply-from#362

Merged

4 tasks

mcollina closed this as completed in fastify/fastify-reply-from#362 Apr 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

All DELETE requests are failling when using HTTP2 #253

All DELETE requests are failling when using HTTP2 #253

delucca commented Jun 17, 2022

mcollina commented Jun 17, 2022

delucca commented Jun 18, 2022 •

edited

annahassel commented Sep 27, 2023 •

edited

mcollina commented Sep 28, 2023

annahassel commented Sep 28, 2023 •

edited

mcollina commented Oct 3, 2023

stercoris commented Jan 29, 2024

mcollina commented Jan 29, 2024

stercoris commented Jan 29, 2024

mcollina commented Jan 30, 2024

karl-power commented Apr 10, 2024 •

edited

All DELETE requests are failling when using HTTP2 #253

All DELETE requests are failling when using HTTP2 #253

Comments

delucca commented Jun 17, 2022

Prerequisites

Fastify version

Plugin version

Node.js version

Operating system

Operating system version (i.e. 20.04, 11.3, 10)

Description

Steps to Reproduce

Expected Behavior

mcollina commented Jun 17, 2022

delucca commented Jun 18, 2022 • edited

annahassel commented Sep 27, 2023 • edited

mcollina commented Sep 28, 2023

annahassel commented Sep 28, 2023 • edited

mcollina commented Oct 3, 2023

stercoris commented Jan 29, 2024

mcollina commented Jan 29, 2024

stercoris commented Jan 29, 2024

mcollina commented Jan 30, 2024

karl-power commented Apr 10, 2024 • edited

delucca commented Jun 18, 2022 •

edited

annahassel commented Sep 27, 2023 •

edited

annahassel commented Sep 28, 2023 •

edited

karl-power commented Apr 10, 2024 •

edited