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

Ensure responses are context.ResponseWriters #19843

Merged

Conversation

zeripath
Copy link
Contributor

In order for web.Wrap to be able to detect if a response has been written
we need to wrap any non-context.ResponseWriters as a such. Otherwise
responses will be incorrectly detected as non-written to and handlers can
double run.

In the case of GZip this handler will change the response to a non-context.RW
and this failure to correctly detect response writing causes fallthrough and
a NPE.

Fix #19839

Signed-off-by: Andrew Thornton art27@cantab.net

In order for web.Wrap to be able to detect if a response has been written
we need to wrap any non-context.ResponseWriters as a such. Otherwise
responses will be incorrectly detected as non-written to and handlers can
double run.

In the case of GZip this handler will change the response to a non-context.RW
and this failure to correctly detect response writing causes fallthrough and
a NPE.

Fix go-gitea#19839

Signed-off-by: Andrew Thornton <art27@cantab.net>
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label May 31, 2022
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 31, 2022
@zeripath
Copy link
Contributor Author

make lgtm work

@zeripath zeripath merged commit 730b9a5 into go-gitea:main May 31, 2022
@zeripath zeripath deleted the fix-19839-resp-needs-to-be-respWriter branch May 31, 2022 20:27
zeripath added a commit to zeripath/gitea that referenced this pull request May 31, 2022
Backport go-gitea#19843

In order for web.Wrap to be able to detect if a response has been written
we need to wrap any non-context.ResponseWriters as a such. Otherwise
responses will be incorrectly detected as non-written to and handlers can
double run.

In the case of GZip this handler will change the response to a non-context.RW
and this failure to correctly detect response writing causes fallthrough and
a NPE.

Fix go-gitea#19839

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added the backport/done All backports for this PR have been created label May 31, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jun 1, 2022
* giteaofficial/main:
  Fix spelling mistake in ProcReceive (go-gitea#19853)
  Ensure responses are context.ResponseWriters (go-gitea#19843)
  Fix inconsistency in doctor output (go-gitea#19836)
  docs: recommendations to ensure backup consistency and reliability (go-gitea#19845)
  feat: Add support for extra sshd_config parameters via 'Include' file (go-gitea#19842)
  [skip ci] Updated translations via Crowdin
  docs: update the ROOT documentation and error messages (go-gitea#19832)
techknowlogick added a commit that referenced this pull request Jun 3, 2022
* Ensure responses are context.ResponseWriters (#19843)

Backport #19843

In order for web.Wrap to be able to detect if a response has been written
we need to wrap any non-context.ResponseWriters as a such. Otherwise
responses will be incorrectly detected as non-written to and handlers can
double run.

In the case of GZip this handler will change the response to a non-context.RW
and this failure to correctly detect response writing causes fallthrough and
a NPE.

Fix #19839

Signed-off-by: Andrew Thornton <art27@cantab.net>

* fix test

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>
zeripath added a commit to zeripath/gitea that referenced this pull request Jun 20, 2022
## [1.16.9](https://github.com/go-gitea/gitea/releases/tag/1.16.9) - 2022-06-20

* BUGFIXES
  * Fix permission check for delete tag (go-gitea#19985) (go-gitea#20001)
  * Only log non ErrNotExist errors in git.GetNote  (go-gitea#19884) (go-gitea#19905)
  *  Use exact search instead of fuzzy search for branch filter dropdown (go-gitea#19885) (go-gitea#19893)
  * Set Setpgid on child git processes (go-gitea#19865) (go-gitea#19881)
  * Import git from alpine 3.16 repository as 2.30.4 is needed for `safe.directory = '*'` to work but alpine 3.13 has 2.30.3 (go-gitea#19876)
  * Ensure responses are context.ResponseWriters (go-gitea#19843) (go-gitea#19859)
  * Fix count bug (go-gitea#19850)
  * Fix raw endpoint PDF file headers (go-gitea#19825) (go-gitea#19826)
  * Make WIP prefixes case insensitive, e.g. allow `Draft` as a WIP prefix (go-gitea#19780) (go-gitea#19811)
  * Fix NotificationUnreadCount (go-gitea#19802)
  * Prevent NPE when cache service is disabled (go-gitea#19703) (go-gitea#19783)
  * Detect truncated utf-8 characters at the end of content as still representing utf-8 (go-gitea#19773) (go-gitea#19774)
  * Fix doctor pq: syntax error at or near "." quote user table name (go-gitea#19765) (go-gitea#19770)
  * Fix bug (go-gitea#19757)

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Jun 20, 2022
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
In order for web.Wrap to be able to detect if a response has been written
we need to wrap any non-context.ResponseWriters as a such. Otherwise
responses will be incorrectly detected as non-written to and handlers can
double run.

In the case of GZip this handler will change the response to a non-context.RW
and this failure to correctly detect response writing causes fallthrough and
a NPE.

Fix go-gitea#19839

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting ENABLE_GZIP=true makes swagger API URL 500 (NPE panic)
5 participants