Http11Processor's keep alive state and input buffer's swallow state must be synchronized #550
Conversation
…g the input from the request stream. When a request comes in with a 100 Continue expectation, and the server immediately responds with a 200 status without reading the request body at all, the next request on the connection will fail because Tomcat cannot parse the HTTP verb. The new unit test replicates the issue and is addressed by the code changes to keep the keepAlive and input buffer swallow state in sync with each other.
|
A request with an expectation is not supposed to be performed the same way as an identical request without it. So you have to be careful and I don't think your patch is correct. |
|
The TestSwallowAbortedUploads test fails so I need to look into that. We ran into this issue in production and were able to reproduce with curl or postman requests to an API that returns 200 without ever reading the request body. I don't think that the core logic is wrong in When a request is started, |
|
I think you can just remove the |
|
@markt-asf That does work and is a much simpler solution! It also doesn't break TestSwallowAbortedUploads (which I now understand why). I will update my PR with that change. What is the other call that you think could be removed? |
…on. If keepAlive is true, setSwallowInput must also be true.
|
I suspect the call in |
|
Are you referring to the call to |
Yes. That was the one. |
|
I've included that change too. Please let me know if there's anything else that should be done. Thanks! |
…k or reading request body. (#550) * Fixing a bug where a connection would be kept alive without swallowing the input from the request stream. When a request comes in with a 100 Continue expectation, and the server immediately responds with a 200 status without reading the request body at all, the next request on the connection will fail because Tomcat cannot parse the HTTP verb. The new unit test replicates the issue and is addressed by removing the unnecessary changes to the swallowInput field of the Http11InputBuffer.
…k or reading request body. (#550) * Fixing a bug where a connection would be kept alive without swallowing the input from the request stream. When a request comes in with a 100 Continue expectation, and the server immediately responds with a 200 status without reading the request body at all, the next request on the connection will fail because Tomcat cannot parse the HTTP verb. The new unit test replicates the issue and is addressed by removing the unnecessary changes to the swallowInput field of the Http11InputBuffer.
…k or reading request body. (#550) * Fixing a bug where a connection would be kept alive without swallowing the input from the request stream. When a request comes in with a 100 Continue expectation, and the server immediately responds with a 200 status without reading the request body at all, the next request on the connection will fail because Tomcat cannot parse the HTTP verb. The new unit test replicates the issue and is addressed by removing the unnecessary changes to the swallowInput field of the Http11InputBuffer.
|
@markt-asf Thanks for reviewing and merging. Could this please also be included in 9.0.x? |
Fixing a bug where a connection would be kept alive without swallowing the input from the request stream.
When a request comes in with a 100 Continue expectation, and the server immediately responds with a 200 status without reading the request body at all, the next request on the connection will fail because Tomcat cannot parse the HTTP verb. The first request left Http11Processor in a state where it kept the connection alive but did not discard the request body, thus reading the next request began at the wrong place in the stream.
The new unit test replicates the issue and is addressed by the code changes to keep the keepAlive and input buffer swallow state in sync with each other.