Skip to content

fix bug with transfer-encoding: chunked#220

Open
d-t-w wants to merge 1 commit intojuxt:masterfrom
d-t-w:chunked-encoding
Open

fix bug with transfer-encoding: chunked#220
d-t-w wants to merge 1 commit intojuxt:masterfrom
d-t-w:chunked-encoding

Conversation

@d-t-w
Copy link

@d-t-w d-t-w commented Feb 21, 2018

Doc for yada.interceptors/process-request-body says:

"Therefore we process the request body if the request contains a Content-Length or Transfer-Encoding header, regardless of the method semantics."

Current implementation does not process the request body if there is a Transfer-Encoding header (indicating chunked) and no Content-Length header, valid per HTTP 1.1 (see: conditional on line 174).

This PR causes yada.request-body/process-request-body to be called if the request contains either header, in line with the doc-string.

If the consumer in the original impl was intended to deal with chunked encoding, be aware that as long as the underlying Netty pipeline includes the HttpObjectAggregator then Netty will take care of that chunking aggregation for you.

@d-t-w
Copy link
Author

d-t-w commented Feb 21, 2018

I should add I found this issue when attempting to receive data via a PUT with chunked encoding, e.g.

curl -v -X PUT --data-binary "some-data" "host:port/resource" -H "Transfer-Encoding: chunked"

And noting the request-body/process-request-body defmethod defined for the content type was not being called.

@malcolmsparks
Copy link

The reason for the :consumer was where yada users wanted to get access to the stream itself. There are reasons for keeping it. Let me leave this open to consider.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants