Rapidoid crashes when processing requests that use Transfer-Encoding.

[Jezza]

Ok, so after an hour or two of smashing my face against this problem, it's a bit of a doozy...

I've written a shitty http client that can send Multipart POST requests to a server.
The server I was using to test against was Rapidoid, as it's insanely easy to do anything. (Props for that).

The library I'm writing uses the new Java 11 HttpClient, and as far as I can tell, the problem doesn't lie with that.

The problem:

ERROR | 05/Jan/2019 19:27:03:122 | server1 | org.rapidoid.net.impl.RapidoidWorker | Failed to process message! | error = java.lang.RuntimeException: Error in the response order control! Expected handle: 1, real: 0
java.lang.RuntimeException: Error in the response order control! Expected handle: 1, real: 0
	at org.rapidoid.u.U.rte(U.java:423)
	at org.rapidoid.u.U.rte(U.java:435)
	at org.rapidoid.net.impl.RapidoidConnection.processedSeq(RapidoidConnection.java:276)
	at org.rapidoid.net.impl.RapidoidWorker.processNext(RapidoidWorker.java:281)
	at org.rapidoid.net.impl.RapidoidWorker.processMsgs(RapidoidWorker.java:216)
	at org.rapidoid.net.impl.RapidoidWorker.process(RapidoidWorker.java:208)
	at org.rapidoid.net.impl.RapidoidWorker.readOP(RapidoidWorker.java:159)
	at org.rapidoid.net.impl.AbstractEventLoop.processKey(AbstractEventLoop.java:93)
	at org.rapidoid.net.impl.AbstractEventLoop.insideLoop(AbstractEventLoop.java:146)
	at org.rapidoid.net.impl.AbstractLoop.run(AbstractLoop.java:71)
	at org.rapidoid.net.impl.RapidoidWorkerThread.run(RapidoidWorkerThread.java:58)

An attempt to describe said problem:

After poking it for a bit, I've determined that the HttpManagedHandlerDecorator promotes the connection to async, when it shouldn't.
But it does so in an alternating fashion.

So, I chunk up the request, because, well, that's a good thing to do.
When the first chunk comes through, rapidoid processes it without issue.
As it was processing the first chunk, the connection seems to be set to sync, and it tells the connection
that the seq 0 was handled.

The problem arises when the next seq is handled.
For some reason, HttpManagedHandlerDecorator sets it to async:

Breakpoint reached
	  at org.rapidoid.net.impl.RapidoidConnection.async(RapidoidConnection.java:490)
	  at org.rapidoid.http.handler.HttpManagedHandlerDecorator.handle(HttpManagedHandlerDecorator.java:63)
	  at org.rapidoid.http.handler.AbstractDecoratingHttpHandler.handle(AbstractDecoratingHttpHandler.java:55)
	  at org.rapidoid.http.FastHttp.handleIfFound(FastHttp.java:285)
	  at org.rapidoid.http.FastHttp.onRequest(FastHttp.java:136)
	  at org.rapidoid.http.FastHttpProtocol.process(FastHttpProtocol.java:59)
	  at org.rapidoid.net.impl.RapidoidWorker.processNext(RapidoidWorker.java:271)
	  at org.rapidoid.net.impl.RapidoidWorker.processMsgs(RapidoidWorker.java:216)
	  at org.rapidoid.net.impl.RapidoidWorker.process(RapidoidWorker.java:208)
	  at org.rapidoid.net.impl.RapidoidWorker.readOP(RapidoidWorker.java:159)
	  at org.rapidoid.net.impl.AbstractEventLoop.processKey(AbstractEventLoop.java:93)
	  at org.rapidoid.net.impl.AbstractEventLoop.insideLoop(AbstractEventLoop.java:146)
	  at org.rapidoid.net.impl.AbstractLoop.run(AbstractLoop.java:71)
	  at org.rapidoid.net.impl.RapidoidWorkerThread.run(RapidoidWorkerThread.java:58)

So the connection is now marked as async, and the RapidoidWorker checks if the current request is async, if so, it doesn't care, and continues processing.

Meanwhile, it sets it back to sync, so when the second chunk comes in, the connection errors out, because
it was marked as sync, and as a result, tries to tell the connection that seq 2 was handled, but the
connection was never informed about seq 1, so it throws the error.

Beyond that, I have no idea.
If this wall of text helps in some way, awesome.
Otherwise I'm fine answering any questions you might have.

The client I've written isn't public yet, but I'd be happy to give you the code to take a look.

[Jezza]

After returning to have another go at this issue, I've been able to stop the issue if I just send all of the data at once.

This means that there's actually two more things that could be doing something.

1. It could be Java's new HttpClient and their InputStream BodyPublisher. (I shouldn't have dismissed it immediately.)
2. It could be my code on top of that, because I map a bunch of multipart items to a single InputStream.
   The resulting code should be correct, but I could have missed something.
3. It could be a problem within rapidoid.

[Jezza]

After poking it more, I'm about 99% sure it doesn't come from my InputStream code.
I transformed the output into an array, and then compared that against the collected output of the chunked InputStream, and they both came out the same.

This leaves Java or Rapidoid.

[Jezza]

Having poked it a lot more, I think I've determined the issue.
Java's Client uses "Transfer-Encoding" when using an InputStream and Rapidoid doesn't support that.

So, I guess after all of that, I should probably change this ticket to support Requests that use that.

[Jezza]

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Transfer-Encoding

[Jezza]

So, yeah, I think the biggest thing right now is the fact I can bring down any rapidoid server just by sending a request that uses this...
Whether or not that's REALLY bad, or just bad, well, could be debated.

[nmihajlovski]

@Jezza Thanks a lot for the detailed investigation and for reporting this!

This looks like a serious problem, indeed. I managed to simulate the problem by throwing the exception intentionally, and I received the same error trace.

However, I didn't manage to crash the server. The response ordering is done independently for each connection, so in theory receiving one problematic request from one client connection shouldn't affect the other clients / connections, unless I am missing something.

Can you please confirm that after receiving such request, the server cannot process any other regular requests from the other connections? If that is the case, it would mean that I didn't manage to reproduce the exact problem.

I would really appreciate if you can send me the exact HTTP request - preferably by email at nikolce.mihajlovski@gmail.com, considering the possibility of exploits, if it really crashes the server.

[Jezza]

Heyo, thanks for the speedy response.
I sent you an email with more information.

[nmihajlovski]

Thanks again for helping with the investigation and for providing all the details!

I managed to reproduce the exceptions with the program that you provided. However, the server doesn't crash, the exceptions are just being logged.

Additionally, I wrote a test that sends the same request (in a bit different way, though), but the effect is the same - the same exceptions occur, but the server doesn't crash.

So far I didn't manage to find a scenario where the server/worker would actually crash. Can you please check if a worker will actually crash after processing your request? I propose the following test:

1. Running Rapidoid with only 1 worker:

Conf.NET.set("workers", 1);
On.get("/ping").serve((req, res) -> "Pong!");

2. Sending the chunked request (one or more times).

3. Simply checking if the server still works fine by visiting http://localhost:8080/hello

Thanks!

[Jezza]

No problem!

Hm, that's really weird.
I guess that means there's probably a problem elsewhere...
Might be on my side. (or possibly a JDK issue... Most likely my side though...)
I'll take another look into it.

Either way, I'm glad it's reproducible.