StripesFramework / stripes

Stripes is a Java framework with the goal of making Servlet/JSP based web development in Java as easy, intuitive and straight-forward as it should be. It's stripey and it doesn't suck.
http://www.stripesframework.org/
171 stars 73 forks source link

StripesRequestWrapper ignores request's body when there is a "Transfer-Encoding: chunked" header instead of "Content-Length" when deciding to build the JsonContentTypeRequestWrapper #74

Open juanpablo-santos opened 5 years ago

juanpablo-santos commented 5 years ago

Issue #51 stated StripesRequestWrapper asumes that a request with json content-type should always have a body, with pull #52 containing and initial fix, which consisted in peeking into request.getReader() to see if the request contained a body or not.

Later on, this code was simplified to use instead request.getContentLength() to determine the request wrapper to use.

Fast-forward ~couple of years, we've stumbled upon a case in which there is a json body but there isn't a content-length header, which is when the Transfer-Enconding: chunked header is present on the request (i.e. a json structure with a base64 image exceding 8K size).

Suggested fix is to change line 126 on StripesRequestWrapper from:

} else if (contentType.toLowerCase().contains("json") && request.getContentLength() > 0) {

to:

} else if (contentType.toLowerCase().contains("json") && ( request.getContentLength() > 0 || "chunked".equals( request.getHeader( "Transfer-Encoding" ) ) ) ) {

I'd gladly prepare a PR with that, and would thank a lot if a new 1.7.0-beta5 release follows up with this, but don't know if it makes sense, as the Stripes' development seems to be staled :-?

thx in advance

iluvtr commented 5 years ago

Looks good to me.

El mar., 30 jul. 2019 a las 5:14, Juan Pablo Santos Rodríguez (< notifications@github.com>) escribió:

Issue #51 https://github.com/StripesFramework/stripes/issues/51 stated StripesRequestWrapper asumes that a request with json content-type should always have a body, with pull #52 https://github.com/StripesFramework/stripes/pull/52 containing and initial fix, which consisted in peeking into request.getReader() to see if the request contained a body or not.

Later on, this code was simplified https://github.com/StripesFramework/stripes/commit/9c13f9688e67c079b21a4058d16ad7f3cf2decb7#diff-84fade24cb7c31fd8f98aa004bae573a to use instead request.getContentLength() to determine the request wrapper to use.

Fast-forward ~couple of years, we've stumbled upon a case in which there is a json body but there isn't a content-length header, which is when the Transfer-Enconding: chunked header is present on the request (i.e. a json structure with a base64 image exceding 8K size).

Suggested fix is to change line 126 on StripesRequestWrapper https://github.com/StripesFramework/stripes/blob/1.7.0-beta4/stripes/src/main/java/net/sourceforge/stripes/controller/StripesRequestWrapper.java#L126 from:

} else if (contentType.toLowerCase().contains("json") && request.getContentLength() > 0) {

to:

} else if (contentType.toLowerCase().contains("json") && ( request.getContentLength() > 0 || "chunked".equals( request.getHeader( "Transfer-Encoding" ) ) ) ) {

I'd gladly prepare a PR with that, and would thank a lot if a new 1.7.0-beta5 release follows up with this, but don't know if it makes sense, as the Stripes' development seems to be staled :-?

thx in advance

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/StripesFramework/stripes/issues/74?email_source=notifications&email_token=AB4G64EYPAALCNIR3CF3BT3QCAH7LA5CNFSM4IH2WVKKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HCIIHHQ, or mute the thread https://github.com/notifications/unsubscribe-auth/AB4G64AJRCRSQJPG2WU2KK3QCAH7LANCNFSM4IH2WVKA .