square / okhttp

Square’s meticulous HTTP client for the JVM, Android, and GraalVM.
https://square.github.io/okhttp/
Apache License 2.0
45.83k stars 9.16k forks source link

MockWebserver fails for empty body requests with "Transfer-Encoding: chunked" header #2309

Closed bclozel closed 4 years ago

bclozel commented 8 years ago

I'm using an HTTP client that sends a "Transfer-Encoding: chunked" HTTP request header for all requests, including with empty bodies. When testing that client with MockWebServer, I'm getting the following:

MockWebServer[54824] connection from /127.0.0.1 crashed
java.lang.IllegalArgumentException: Request must not have a body: GET /pojos HTTP/1.1
    at okhttp3.mockwebserver.MockWebServer.readRequest(MockWebServer.java:631)
    at okhttp3.mockwebserver.MockWebServer.access$1500(MockWebServer.java:99)
    at okhttp3.mockwebserver.MockWebServer$4.processOneRequest(MockWebServer.java:498)
    at okhttp3.mockwebserver.MockWebServer$4.processConnection(MockWebServer.java:460)
    at okhttp3.mockwebserver.MockWebServer$4.execute(MockWebServer.java:401)
    at okhttp3.internal.NamedRunnable.run(NamedRunnable.java:33)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)

Why is this enforced? Is there a strong reason or something in the HTTP spec that qualifies those requests as invalid? Could we relax that check if not necessary?

JakeWharton commented 8 years ago

The RFC states that some servers might reject a GET with a body but that it otherwise has no defined semantics. Does the body have contents of 0\r\n?

bclozel commented 8 years ago

I guess you're right. The RFC states that:

The presence of a message body in a request is signaled by a Content-Length or Transfer-Encoding header field. Request message framing is independent of method semantics, even if the method does not define any use for a message body.

So in that case, I guess the client should at least send a 0\r\n body to send an empty body if nothing has been sent.

Right now the client sends the following:

GET /greeting HTTP/1.1\r\n
transfer-encoding: chunked\r\n
host: localhost:50419\r\n
\r\n
0\r\n
\r\n

Is there something wrong with this request?

JakeWharton commented 8 years ago

Doesn't look like it, no. We can potentially update MWS to correctly ignore bodies on non-body HTTP methods, but we'll have to look more closely at the RFC to see if there are specific HTTP methods where the body is completely forbidden before doing so.

bclozel commented 8 years ago

The spec is actually quite liberal. GET requests may have bodies but they can be ignored by the server. AFAIK, the only request method that forbids request bodies is HEAD.

anderius commented 7 years ago

I have the same problem. Would it be a solution to make it configurable?

yschimke commented 4 years ago

Won't fix. We reject for clients because we are opionated, doing the same here.