-
Notifications
You must be signed in to change notification settings - Fork 272
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
[bug] COmpressHandler is broken #194
Comments
Can you please share the request & response headers for the failed connection? The relevant part of your application code will help too. We merged #187 last night but unless your site is doing something with the |
This zip has sammple web app , golang code with gorilla mux and dockerfile. I am changing the X-Frame-Options header, but this is required to avoid the clickjacking. Code is available in zip No header information is shown when this error occurs. please find screenshot below. |
Can you please provide your (minimal) code in code blocks here? I would prefer not to download a zip file.
|
This is the server code
|
It is serving one index.html and minified javascript which is of size 38k lines , so cant copy it here |
@elithrar Are you able to replicate the issue? Is there any workaround? |
Same problem here. After upgrading handlers 1.4.2 to 1.5.0 my application is broken. Local requests are working fine (no gzip), all gzip requests are broken (appear red in Safari). In that sense, 1.5.0 is broken! This is the Safari Request:
Response headers look ok, but response body apparently isn't. I would suspect either #187 or #193. Update: I'm not able to force the problem on localhost, not sure why:
...is always working fine. |
Agree with andig, if the easy solution is not in sight then alteast handler code rollback shold happen. It takes a lot of time for user to understand what is wrong with the application. Nobody thinks that there can be anything wrong with golang or http provider. We start checking current code, old code, current deployment , old deployment , environment configuration and so on. It is very time consuming. |
@max202021 can you repro this when running on localhost? If yes we could git revert the two prs selectively and see which one is the culprit. You can always rollback yourself using 1.4.2 for time being. |
How do you execute your test to repro? |
@andig , Remove the old gorilla Handler from local gopath . ( Take Backup ) I have attached a zip file which has dummy website and server.go with Dockerfile which uses gorilla mux. when we run that it will take the latest gorilla mux and then you can replicate the issue |
@andig use firefox browser . it will show content encoding error. |
After using httpsnoop to preserve interfaces, the compress response writer now implemented ReaderFrom. ReaderFrom is used by net/http to use sendfile when serving *os.Files. This breaks compression because it serves directly to the underlying response writer, skipping the compressor. Fix by implementing ReadFrom on our resposne writer to copy to our compressor. Fixes gorilla#194.
We are running the same problem in our setup where we run the testapp behind nginx. The endpoint that broke is also the
|
After using httpsnoop to preserve interfaces, the compress response writer now implemented ReaderFrom. ReaderFrom is used by net/http to use sendfile when serving *os.Files. This breaks compression because it serves directly to the underlying response writer, skipping the compressor. Fix by implementing ReadFrom on our resposne writer to copy to our compressor. Fixes gorilla#194.
After using httpsnoop to preserve interfaces, the compress response writer now implemented ReaderFrom. ReaderFrom is used by net/http to use sendfile when serving *os.Files. This breaks compression because it serves directly to the underlying response writer, skipping the compressor. Fix by implementing ReadFrom on our resposne writer to copy to our compressor. Fixes #194.
@elithrar it would be good to release this asap. In my case I had dependabot running and upgraded when the pr was opened (stupid me) bit I think it would be good to release before more users might get bitten. |
same problem and several hour lost, please release 1.5.1 |
@yangjuncode How did you find that it is issue with handler and not anything else? |
|
I can only emphasize the need for a release. Its really urgent as it breaks production systems (and has just done again) ... |
Please pin to the latest commit hash. I do not have time to make a release
this week.
e.g.
go get github.com/gorilla/handlers@abc123
|
I've put a warning on r/golang to help spread the word. @elithrar if I can help with releasing (looks like tagging + cutting a github release) please let me know if I can support. I've also setup a couple of projects with Travis CI and golang releaser and could provide the steps here (requires org access though). |
Everything was working till yesterday. it starts failing today. I am using the google cloud build , so it takes the latest mux from repo. Local mux which is old is working fine.
I am using the compress handler like this
http.ListenAndServe(":8080", handlers.CompressHandler(r))
and serving static files.
but all the browser are not able to read the content. I tested with Mozzila , chrome and Microsoft EDGE.
Following error is shown
When I use without compress , things works but now my server slow without compression
http.ListenAndServe(":8080",r)
The text was updated successfully, but these errors were encountered: