HTTP/1 Clear Text Requests to HTTP/2 servers wait forever, if called with DELETE method and Body

ckosmowski commented 3 years ago

Thanks for reporting an issue, please review the task list below before submitting the issue. Your issue report will be closed if the issue is incomplete and the below tasks not completed.

NOTE: If you are unsure about something and the issue is more of a question a better place to ask questions is on Stack Overflow (https://stackoverflow.com/tags/micronaut) or Gitter (https://gitter.im/micronautfw/). DO NOT use the issue tracker to ask questions.

Task List

[X] Steps to reproduce provided
[-] Stacktrace (if present) provided
[-] Example that reproduces the problem uploaded to Github
[X] Full description of the issue provided (see below)

Steps to Reproduce

Create a Http Service with micronaut
Implement a simple GET route
Implement a second route with DELETE OR PUT with body parameters
Test the routes in any order (with simple HTTP/1.1 clients) -> it works as expected
Configure it to use Http/2
Test the routes again (with simple HTTP/1.1 clients)

Expected Behaviour

The routes will still respond correctly in any order

Actual Behaviour

If we start with the DELETE or PUT route in our test, it will wait forever, no response is received, no exception is raised.
If we start with the GET request first, and use the other route afterwards, all subsequent calls to DELETE and PUT will succeed.
After waiting a while without calling, DELETE and PUT will fail again, until another GET request is issued.

Environment Information

Operating System: Linux
Micronaut Version: 2.2.0
JDK Version: 8

Example Application

volkerrichert commented 3 years ago

I also had this issue with "PUT", as I described on gitter / questions on Nov. 25 18:16

graemerocher commented 3 years ago

Can one of you attach an example

volkerrichert commented 3 years ago

check out https://github.com/volkerrichert/mnIssus4643 and run tests. It will fail on put.

reorder test wit get first or switch to http 1.1

volkerrichert commented 3 years ago

current test will fail with:

Read Timeout
io.micronaut.http.client.exceptions.ReadTimeoutException: Read Timeout
    at io.micronaut.http.client.exceptions.ReadTimeoutException.<clinit>(ReadTimeoutException.java:26)
    at io.micronaut.http.client.netty.DefaultHttpClient$12.exceptionCaught(DefaultHttpClient.java:2258)
    at io.netty.channel.AbstractChannelHandlerContext.invokeExceptionCaught(AbstractChannelHandlerContext.java:302)
    at io.netty.channel.AbstractChannelHandlerContext.invokeExceptionCaught(AbstractChannelHandlerContext.java:281)

graemerocher commented 3 years ago

@volkerrichert the example works if you inject an HTTP/2 client

@Inject
@Client(id = "/", httpVersion = HttpVersion.HTTP_2_0)
RxHttpClient client;

So the actually problem is HTTP 1.1 clients can't talk to an HTTP/2 server, that clears things up. Investigating... not sure if a bug yet.

volkerrichert commented 3 years ago

But why does it work after doing the GET - request first? This request shouldn't work also. Thats's stange.

ckosmowski commented 3 years ago

If this is the case, i would suggest a downgrade feature or something. Because at least in my case you rarely are aware of all the clients using your server and you are not in control of changing the clients.

graemerocher commented 3 years ago

@volkerrichert there is a bug in the way the Netty channel is downgraded to HTTP/1.1 which I am looking into. But the root cause is attempting to send HTTP/1.1 requests over HTTP/2

@ckosmowski in this case the downgrade support is not working but only if the request has a body (like a PUT or a GET)

It seems to be a core Netty issue because the CleartextHttp2ServerUpgradeHandler which we are using (a Netty class) discards the body and prevents it from being read again when attempting to do the connection upgrade.

ckosmowski commented 3 years ago

@graemerocher Thank you for claryfiing.

When CleartextHttp2ServerUpgradeHandler is involved would you expect this to work with self signed certificates and SSL enabled?

graemerocher commented 3 years ago

Yes SSL uses a completely different codepath and doesn't involve CleartextHttp2ServerUpgradeHandler

yawkat commented 3 years ago

This is fixed by #6300 for the test case by @volkerrichert (thanks!). I have not tested it with DELETE, but the patch should apply to any first request with a body.

micronaut-projects / micronaut-core