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

Use maintained gziphandler #27894

Closed
wants to merge 1 commit into from

Conversation

earl-warren
Copy link
Contributor

(cherry picked from commit 45a16f8c3c6f7d5e4aab8fdde6a621cf36e4801c)

- https://github.com/NYTimes/gziphandler doesn't seems to be maintained
anymore and Forgejo already includes
https://github.com/klauspost/compress which provides a maintained and
faster gzip handler fork.
- Enables Jitter to prevent BREACH attacks, as this *seems* to be
possible in the context of Forgejo.

(cherry picked from commit 45a16f8c3c6f7d5e4aab8fdde6a621cf36e4801c)
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 3, 2023
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 3, 2023
"github.com/prometheus/client_golang/prometheus"
)

const (
// GzipMinSize represents min size to compress for the body size of response
GzipMinSize = 1400
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about GzipMinSize = gzhttp.DefaultMinSize?

@lunny
Copy link
Member

lunny commented Nov 3, 2023

The library's License looks like some complicated.

@jolheiser
Copy link
Member

The library's License looks like some complicated.

It looks like the gzhttp package simply uses its own apache2 license vs the base module also including the Go authors.

@KN4CK3R
Copy link
Member

KN4CK3R commented Nov 5, 2023

The removed license should be equal to the added one because the new code is a fork of the replaced code.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Dec 30, 2023
@lunny lunny closed this Apr 19, 2024
lunny added a commit that referenced this pull request Apr 21, 2024
Replace #27894

---------

Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
lunny added a commit to lunny/gitea that referenced this pull request Apr 22, 2024
Replace go-gitea#27894

---------

Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
lunny added a commit to lunny/gitea that referenced this pull request Apr 22, 2024
Replace go-gitea#27894

---------

Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
lunny added a commit that referenced this pull request Apr 23, 2024
Replace #27894
Backport #30592

Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
lunny added a commit that referenced this pull request Apr 23, 2024
Replace #27894
Backport #30592

Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 1 This PR needs approval from one additional maintainer to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants