Skip to content

Don't panic if we fail to parse U2FRegistration data #17304

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
Merged
merged 3 commits into from
Oct 14, 2021
Merged

Don't panic if we fail to parse U2FRegistration data #17304

merged 3 commits into from
Oct 14, 2021

Conversation

dvejmz
Copy link
Contributor

@dvejmz dvejmz commented Oct 14, 2021

Downgrade logging statement from Fatal to Error so that errors parsing
U2FRegistration data does not panic; instead, the invalid key will be
skipped and we will attempt to parse the next one, if available.

This closes #17293.

@dvejmz
Don't panic if we fail to parse a U2FRegistration data
656d4b1 
Downgrade logging statement from Fatal to Error so that errors parsing
U2FRegistration data does not panic; instead, the invalid key will be
skipped and we will attempt to parse the next one, if available.

Signed-off-by: David Jimenez <dvejmz@sgfault.com>
@wxiaoguang
Copy link
Contributor

CI failure is related. Otherwise LGTM

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Oct 14, 2021
@lunny lunny added this to the 1.16.0 milestone Oct 14, 2021
@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 Oct 14, 2021
@6543
Copy link
Member

6543 commented Oct 14, 2021

@dvejmz please do make fmt and commit changes

@6543
Copy link
Member

6543 commented Oct 14, 2021

🚀

@6543 6543 merged commit fa8b8c0 into go-gitea:main Oct 14, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #17304 (57c2336) into main (d47798c) will increase coverage by 0.02%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #17304      +/-   ##
==========================================
+ Coverage   45.24%   45.27%   +0.02%     
==========================================
  Files         781      781              
  Lines       87922    87922              
==========================================
+ Hits        39784    39805      +21     
+ Misses      41667    41640      -27     
- Partials     6471     6477       +6
Impacted Files Coverage Δ
routers/web/user/auth.go 12.18% <0.00%> (ø)
models/login/u2f.go 85.00% <100.00%> (+20.00%) ⬆️
modules/notification/mail/mail.go 35.29% <0.00%> (-2.95%) ⬇️
modules/notification/ui/ui.go 60.86% <0.00%> (-1.45%) ⬇️
models/notification.go 65.28% <0.00%> (-0.88%) ⬇️
services/pull/pull.go 41.78% <0.00%> (-0.41%) ⬇️
models/issue_comment.go 51.89% <0.00%> (+0.29%) ⬆️
modules/git/commit.go 60.76% <0.00%> (+0.76%) ⬆️
models/gpg_key_common.go 64.51% <0.00%> (+4.83%) ⬆️
modules/queue/workerpool.go 53.05% <0.00%> (+4.96%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d47798c...57c2336. Read the comment docs.

@dvejmz
Copy link
Contributor Author

dvejmz commented Oct 14, 2021

Thanks for sorting the source formatting @6543 . I thought I'd run it prior to raising the PR but it didn't turn out to be the case. Thanks for reviewing this so quickly!

@6543
Copy link
Member

6543 commented Oct 14, 2021

@dvejmz can you do a backport :) ?

Chianina pushed a commit to Chianina/gitea that referenced this pull request Oct 15, 2021
@dvejmz
Don't panic if we fail to parse U2FRegistration data (go-gitea#17304)
f0833c5 
* Don't panic if we fail to parse a U2FRegistration data

Downgrade logging statement from Fatal to Error so that errors parsing
U2FRegistration data does not panic; instead, the invalid key will be
skipped and we will attempt to parse the next one, if available.

Signed-off-by: David Jimenez <dvejmz@sgfault.com>
zeripath pushed a commit to zeripath/gitea that referenced this pull request Oct 20, 2021
@dvejmz @zeripath
Don't panic if we fail to parse U2FRegistration data (go-gitea#17304)
81fda7a 
Backport go-gitea#17304

Downgrade logging statement from Fatal to Error so that errors parsing
U2FRegistration data does not panic; instead, the invalid key will be
skipped and we will attempt to parse the next one, if available.

Signed-off-by: David Jimenez <dvejmz@sgfault.com>
@zeripath zeripath mentioned this pull request Oct 20, 2021
Merged
@zeripath zeripath added the backport/done All backports for this PR have been created label Oct 20, 2021
6543 pushed a commit that referenced this pull request Oct 20, 2021
@zeripath @dvejmz
Don't panic if we fail to parse U2FRegistration data (#17304) (#17371)
79a3d27 
Backport #17304

Downgrade logging statement from Fatal to Error so that errors parsing
U2FRegistration data does not panic; instead, the invalid key will be
skipped and we will attempt to parse the next one, if available.

Signed-off-by: David Jimenez <dvejmz@sgfault.com>

Co-authored-by: David Jimenez <dvejmz@users.noreply.github.com>
zeripath added a commit to zeripath/gitea that referenced this pull request Oct 21, 2021
@zeripath
Changelog 1.15.5
9ae77e1 
* SECURITY
  * Upgrade Bluemonday to v1.0.16 (go-gitea#17372) (go-gitea#17374)
  * Ensure correct SSH permissions check for private and restricted users (go-gitea#17370) (go-gitea#17373)
* BUGFIXES
  * Prevent NPE in CSV diff rendering when column removed (go-gitea#17018) (go-gitea#17377)
  * Offer rsa-sha2-512 and rsa-sha2-256 algorithms in internal SSH (go-gitea#17281) (go-gitea#17376)
  * Don't panic if we fail to parse U2FRegistration data (go-gitea#17304) (go-gitea#17371)
  * Ensure popup text is aligned left (backport for 1.15) (go-gitea#17343)
  * Ensure that git daemon export ok is created for mirrors (go-gitea#17243) (go-gitea#17306)
  * Disable core.protectNTFS (go-gitea#17300) (go-gitea#17302)
  * Use pointer for wrappedConn methods (go-gitea#17295) (go-gitea#17296)
  * AutoRegistration is supposed to be working with disabled registration (backport) (go-gitea#17292)
  * Handle duplicate keys on GPG key ring (go-gitea#17242) (go-gitea#17284)
  * Fix SVG side by side comparison link (go-gitea#17375) (go-gitea#17391)

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Oct 21, 2021
Merged
6543 pushed a commit that referenced this pull request Oct 21, 2021
@zeripath
Changelog 1.15.5 (#17392)
3aecea2 
* SECURITY
  * Upgrade Bluemonday to v1.0.16 (#17372) (#17374)
  * Ensure correct SSH permissions check for private and restricted users (#17370) (#17373)
* BUGFIXES
  * Prevent NPE in CSV diff rendering when column removed (#17018) (#17377)
  * Offer rsa-sha2-512 and rsa-sha2-256 algorithms in internal SSH (#17281) (#17376)
  * Don't panic if we fail to parse U2FRegistration data (#17304) (#17371)
  * Ensure popup text is aligned left (backport for 1.15) (#17343)
  * Ensure that git daemon export ok is created for mirrors (#17243) (#17306)
  * Disable core.protectNTFS (#17300) (#17302)
  * Use pointer for wrappedConn methods (#17295) (#17296)
  * AutoRegistration is supposed to be working with disabled registration (backport) (#17292)
  * Handle duplicate keys on GPG key ring (#17242) (#17284)
  * Fix SVG side by side comparison link (#17375) (#17391)

Signed-off-by: Andrew Thornton <art27@cantab.net>
zeripath added a commit to zeripath/gitea that referenced this pull request Oct 22, 2021
@zeripath
Changelog 1.15.5 (go-gitea#17392)
e3365ce 
Frontport go-gitea#17392

* SECURITY
  * Upgrade Bluemonday to v1.0.16 (go-gitea#17372) (go-gitea#17374)
  * Ensure correct SSH permissions check for private and restricted users (go-gitea#17370) (go-gitea#17373)
* BUGFIXES
  * Prevent NPE in CSV diff rendering when column removed (go-gitea#17018) (go-gitea#17377)
  * Offer rsa-sha2-512 and rsa-sha2-256 algorithms in internal SSH (go-gitea#17281) (go-gitea#17376)
  * Don't panic if we fail to parse U2FRegistration data (go-gitea#17304) (go-gitea#17371)
  * Ensure popup text is aligned left (backport for 1.15) (go-gitea#17343)
  * Ensure that git daemon export ok is created for mirrors (go-gitea#17243) (go-gitea#17306)
  * Disable core.protectNTFS (go-gitea#17300) (go-gitea#17302)
  * Use pointer for wrappedConn methods (go-gitea#17295) (go-gitea#17296)
  * AutoRegistration is supposed to be working with disabled registration (backport) (go-gitea#17292)
  * Handle duplicate keys on GPG key ring (go-gitea#17242) (go-gitea#17284)
  * Fix SVG side by side comparison link (go-gitea#17375) (go-gitea#17391)

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Oct 22, 2021
Merged
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@6543 6543 6543 approved these changes

Assignees
No one assigned
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
Milestone
1.16.0
Development

Successfully merging this pull request may close these issues.

Unreachable continue in loop
7 participants
@dvejmz @wxiaoguang @6543 @codecov-commenter @lunny @zeripath @GiteaBot