Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling Accept-Encoding request #331

Closed
munendrasn opened this issue Dec 6, 2016 · 15 comments
Closed

Handling Accept-Encoding request #331

munendrasn opened this issue Dec 6, 2016 · 15 comments

Comments

@munendrasn
Copy link
Collaborator

How to handle Accept-Encoding header in request?
Suppose Accept-Encoding=gzip then, response header should contain Content-Encoding=gzip.
How to achieve this?
Just adding header to response wouldn't be sufficient. Is there way already available in pippo to handle this or custom implementation is required for this??

@decebals
Copy link
Member

decebals commented Dec 6, 2016

Just adding header to response wouldn't be sufficient.

Can you give me more details? Can you supply a code snippet to show me what you try to achieve?

@decebals
Copy link
Member

decebals commented Dec 6, 2016

You can wrap the response's output stream in a GzipOutputStream and write to it.

response.contentType(...);
GzipOutputStream outputStream = new GzipOutputStream(response.getOutputStream());
// write to outputStream

For some servlet containers (like Tomcat) you can obtain this feature automatically using configuration (a parameter).

If my code snippet is OK for you, I can add a new method setGzip (or something similar) in Response class that set automatically the content type and wrap the output stream in a GzipOutputStream.

@munendrasn
Copy link
Collaborator Author

I think creating a method in Response class would be better, so that this can be automatically set

@decebals
Copy link
Member

decebals commented Dec 6, 2016

Can you confirm that the above code snippet resolves your problem?

@decebals
Copy link
Member

My idea here is to supply an out of the box solution, something that is transparent for end user/developer and that could be implemented via HttpServletResponseWrapper or by introduction of a new ResponseWrapper concept.

@munendrasn
Copy link
Collaborator Author

munendrasn commented Dec 13, 2016

Apologizes for the delayed response. The above snippet solved my problem.
Instead of introducing new ResponseWrapper, this can be handled in Response class right??
To handle it in Response class, access to request is needed.
Or is there any advantages in using ResponseWrapper ??

@munendrasn
Copy link
Collaborator Author

I think your idea is introduce ResponseWrapper something like in this link right?

@decebals
Copy link
Member

decebals commented Dec 13, 2016

Yes, something like that.

The idea is to inject GzipOutputStream directly in HttpServletRequest (via HttpServletRequestWrapper.getOutputStream method) if the request contains gzip in Accept-Encoding, to set gzip in Content-Encoding in HttpServletResponse and to do nothing on Pippo's Response (gzip is transparent). So all responses will be "gzipped" out of the box.

@munendrasn
Copy link
Collaborator Author

I think this would be awesome solution

decebals added a commit that referenced this issue Dec 15, 2016
Use gzip compression where it's possible; see #331
@decebals
Copy link
Member

I merged the PR that comes with an out of the box support for gzip. Also I created a new SNAPSHOT. Please test the modifications using your application and let me know if you encounter any problems.

@munendrasn
Copy link
Collaborator Author

munendrasn commented Dec 16, 2016

@decebals , gzip content-encoding is working as expected. But regression has occured due to this. Now, responses are not getting committed.

routeContext.send(<some-content>);
routeContext.send(<some-content>); // this should give response committed error. Its not happening after gzip support

@munendrasn
Copy link
Collaborator Author

@decebals, Is there any update on this??
Thank you in advance

@decebals
Copy link
Member

I will try to test on demo application.

@decebals
Copy link
Member

I think that now, the code is OK. The idea is that you must finish the GZipResponseWrapper (call finish method) but the big problem is where to call this method. Initial I tried to finish the response onPostDispatch using a RoutePostDispatchListener but wasn't OK. After this, I tried to call this function at the end of RouteDispatcher.onRouteDispatch method but after some elaborated tests I saw that on error/exception, the template that displays the error is not committed. The third attempt (and I hope that is the last) was to call the method that finish the GZip, at the end of Response.commit. Now everything seems to be OK.
@munendrasn Please try the new code. Also, I updated the SNAPSHOT.

@decebals decebals reopened this Dec 16, 2016
@munendrasn
Copy link
Collaborator Author

@decebals , I have tested new code..the issue is resolved

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

No branches or pull requests

2 participants