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.

Sign up for GitHub

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

Jump to bottom

ready release gin 1.9.1 #3620

Closed
wants to merge 5 commits into from
Closed

ready release gin 1.9.1 #3620

wants to merge 5 commits into from

Conversation

thinkerou
Copy link
Member

as title, @appleboy please review, thanks!

@codecov
Copy link

codecov bot commented May 29, 2023 •
edited

Codecov Report

Merging #3620 (2b1fd65) into master (bb1fc2e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3620   +/-   ##
=======================================
  Coverage   99.01%   99.01%           
=======================================
  Files          42       42           
  Lines        3157     3157           
=======================================
  Hits         3126     3126           
  Misses         21       21           
  Partials       10       10
Flag Coverage Δ
99.01% <ø> (ø)
go-1.18 98.92% <ø> (ø)
go-1.19 99.01% <ø> (ø)
go-1.20 99.01% <ø> (ø)
macos-latest 99.01% <ø> (ø)
ubuntu-latest 99.01% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@thinkerou thinkerou mentioned this pull request May 29, 2023
Merged
likaned
likaned previously approved these changes May 29, 2023
View reviewed changes
tharun-d
tharun-d previously approved these changes May 29, 2023
View reviewed changes
herrberk
herrberk reviewed May 29, 2023
View reviewed changes
go.mod Outdated
golang.org/x/arch v0.3.0 // indirect
golang.org/x/crypto v0.8.0 // indirect
golang.org/x/arch v0.0.0-20210923205945-b76863e36670 // indirect
golang.org/x/crypto v0.7.0 // indirect
Copy link

Choose a reason for hiding this comment

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

Looks like these two got downgraded?

Copy link
Member Author

Choose a reason for hiding this comment

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

@herrberk thanks! fixed.

@thinkerou thinkerou dismissed stale reviews from tharun-d and likaned via db5cc90 May 29, 2023 11:29
herrberk
herrberk previously approved these changes May 29, 2023
View reviewed changes
@adrianosela adrianosela mentioned this pull request May 29, 2023
Closed
@adrianosela
Copy link
Contributor

RE: CVE-2023-29401 | GO-2023-1737 | GHSA-2c4m-59x9-fr2g | Fix PR | Original Issue

Hey @thinkerou, for starters: thanks so much for moving this forward! Your effort is noticed.

Gin is one of the most, if not the most, popular http framework for Go. Many organizations use Gin as their framework of choice, including my workplace and myself for personal projects.

I understand its hard, and in most cases not possible, to hold a community-ran project to strict SLAs; however the sentiment of the comments thread in #3556 is shared by many of us security-oriented folks who use Gin.

It's been ~60 days since the issue was originally reported, with the reporter claiming to have tried to contact the maintainer numerous times via multiple different channels - and the fix is still not released.

It would be awesome to have a better security story around fixing issues quickly with Gin... The timeline of this change was a bit too lengthy. Organizations out there in the best case have compliance goals to meet, and in the worst case have things running in the wild with tangible risk related to this CVE.

Perhaps the Gin maintainers would consider widening the list of folks who can approve/merge and push releases, in particular for security related changes and releases. Perhaps Gin documenting a soft SLA to meet when it comes to security issues?

Once again, thank you and the rest of the Gin team <3

@herrberk
Copy link

Perhaps the Gin maintainers would consider widening the list of folks who can approve/merge and push releases, in particular for security related changes and releases. Perhaps Gin documenting a soft SLA to meet when it comes to security issues?

I second this, if a maintainer is away on vacation, etc. the project (especially the one that is used by 136k and has ~400 contributors) should be able to proceed without having to wait on them. Thoughts? @thinkerou @javierprovecho

vitordm
vitordm previously approved these changes May 29, 2023
View reviewed changes
Copy link

@vitordm vitordm left a comment

Choose a reason for hiding this comment

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

Lets go!

Laotree
Laotree reviewed May 30, 2023
View reviewed changes
Copy link

@Laotree Laotree left a comment

Choose a reason for hiding this comment

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

fixme pls

CHANGELOG.md Outdated
### ENHANCEMENTS

* refactor: use bytes.ReplaceAll directly [#3455](https://github.com/gin-gonic/gin/pull/3455)
* Convert strings and slices using the officially recommended way [#3344](https://github.com/gin-gonic/gin/pull/3344)
Copy link

Choose a reason for hiding this comment

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

Convert -> convert start with lower case as context

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@thinkerou thinkerou dismissed stale reviews from vitordm and herrberk via 2b1fd65 May 30, 2023 09:17
@thinkerou
Copy link
Member Author

thinkerou commented May 30, 2023 •
edited

thanks everyone!
@appleboy and I have only limited permission, now do more things need @javierprovecho and @manucorporat

vitordm
vitordm approved these changes May 30, 2023
View reviewed changes
Copy link

@vitordm vitordm left a comment

Choose a reason for hiding this comment

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

Please @appleboy, We need this fix!

@cpusoft
Copy link

cpusoft commented May 31, 2023

We need this fix!

This was referenced May 31, 2023
Closed
Closed
Closed
@thinkerou thinkerou mentioned this pull request May 31, 2023
Closed
@amandalal
Copy link

amandalal commented May 31, 2023 •
edited

❗ ❗ ❗ Who is authorized to merge this PR? We need this ASAP please @javierprovecho @manucorporat

@dayvsonsales
Copy link

Hello, please, some administrator could please merge this PR, we need this

@thinkerou
Copy link
Member Author

hi, who can copy the pull request and commit it, I approve and merge. thanks!

@adrianosela adrianosela mentioned this pull request Jun 1, 2023
Merged
@adrianosela
Copy link
Contributor

@thinkerou #3630

@thinkerou
Copy link
Member Author

@adrianosela thanks a lot! let's go!

@thinkerou thinkerou closed this Jun 1, 2023
@thinkerou
Copy link
Member Author

v1.9.1 have released, please see https://github.com/gin-gonic/gin/releases/tag/v1.9.1, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Laotree Laotree Laotree left review comments

@herrberk herrberk herrberk approved these changes

@vitordm vitordm vitordm approved these changes

@PascalBourdier PascalBourdier PascalBourdier approved these changes

@tharun-d tharun-d tharun-d left review comments

@likaned likaned likaned left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet