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

Fix NPE /repos/issues/search when not signed in #19154

Merged
merged 1 commit into from
Mar 20, 2022
Merged

Fix NPE /repos/issues/search when not signed in #19154

merged 1 commit into from
Mar 20, 2022

Conversation

Gusted
Copy link
Contributor

@Gusted Gusted commented Mar 20, 2022

Fix NPE /repos/issues/search when not signed in
bdf4c61 
- Don't panic when on
`/repos/issues/search?{created,assigned,mentioned,review_requested}=true`
when client didn't pass any authentication.
- Resolves go-gitea#19115
@Gusted Gusted added this to the 1.16.5 milestone Mar 20, 2022
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 20, 2022
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 20, 2022 •
edited
Loading

But .... should we just deny the anonymous user from querying with these created options? For example, 400 Bad Request.

What's the meaning of issues created by anonymous user.

Gusted pushed a commit to Gusted/gitea that referenced this pull request Mar 20, 2022
Fix NPE /repos/issues/search when not signed in (go-gitea#19154)
034839e 
- Backport go-gitea#19154

  - Don't panic when on `/repos/issues/search?{created,assigned,mentioned,review_requested}=true` when client didn't pass any authentication.
  - Resolves go-gitea#19115
@Gusted
Copy link
Contributor Author

Gusted commented Mar 20, 2022

But .... should we just deny the anonymous user from querying with these created options?

What's the meaning of issues created by anonymous user.

Deny? Do you propose to return empty list and some kind of status code? Currently what it will do is filter out all issues, because none are created, commented, mentioned or review requested by userID of zero. #19115 mentions:

It should return an empty list.

Which I think is indeed the correct return output.

wxiaoguang
wxiaoguang approved these changes Mar 20, 2022
View reviewed changes
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, but I think it's better to return 400 Bad Request when anonymous requests for created=true, it's meaningless.

Either is fine to me.

@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 Mar 20, 2022
@Gusted Gusted mentioned this pull request Mar 20, 2022
Merged
@Gusted Gusted modified the milestones: 1.16.5, 1.17.0 Mar 20, 2022
@Gusted Gusted added the backport/done All backports for this PR have been created label Mar 20, 2022
@6543 6543 merged commit 49c5fc5 into go-gitea:main Mar 20, 2022
6543 pushed a commit that referenced this pull request Mar 20, 2022
Fix NPE /repos/issues/search when not signed in (#19154) (#19155)
ee234af 
- Backport #19154

  - Don't panic when on `/repos/issues/search?{created,assigned,mentioned,review_requested}=true` when client didn't pass any authentication.
  - Resolves #19115
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 21, 2022
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
aa07464 
* giteaofficial/main:
  Add 1.18 (go-gitea#19151)
  [skip ci] Updated translations via Crowdin
  Fix NPE `/repos/issues/search` when not signed in (go-gitea#19154)
  [skip ci] Updated licenses and gitignores
  Use custom favicon when viewing static files if it exists (go-gitea#19130)
  not send notification emails to inactive users (part 2) (go-gitea#19142)
  Make migrations SKIP_TLS_VERIFY apply to git too (go-gitea#19132)
  Do not send notification emails to inactive users (go-gitea#19131)
a1012112796 added a commit to a1012112796/gitea that referenced this pull request Mar 21, 2022
@a1012112796
Merge branch 'main' into a1012/fix-break-act-link
9a572e4 
* main:
  Reorder issue templates and automatically add labels (go-gitea#18875)
  Use IterateBufferSize whilst querying repositories during adoption check (go-gitea#19140)
  Add 1.18 (go-gitea#19151)
  [skip ci] Updated translations via Crowdin
  Fix NPE `/repos/issues/search` when not signed in (go-gitea#19154)
zeripath added a commit to zeripath/gitea that referenced this pull request Mar 23, 2022
@zeripath
Changelog for 1.16.5
fb7de1c 
 ## [1.16.5](https://github.com/go-gitea/gitea/releases/tag/1.16.5) - 2022-03-23

* BREAKING
  * Bump to build with go1.18 (go-gitea#19120 et al) (go-gitea#19127)
* SECURITY
  * Prevent redirect to Host (2) (go-gitea#19175) (go-gitea#19186)
  * Try to prevent autolinking of displaynames by email readers (go-gitea#19169) (go-gitea#19183)
  * Clean paths when looking in Storage (go-gitea#19124) (go-gitea#19179)
  * Do not send notification emails to inactive users (go-gitea#19131) (go-gitea#19139)
  * Do not send activation email if manual confirm is set (go-gitea#19119) (go-gitea#19122)
* ENHANCEMENTS
  * Use the new/choose link for New Issue on project page (go-gitea#19172) (go-gitea#19176)
* BUGFIXES
  * Fix compare link in active feeds for new branch (go-gitea#19149) (go-gitea#19185)
  * Redirect .wiki/* ui link to /wiki (go-gitea#18831) (go-gitea#19184)
  * Ensure deploy keys with write access can push (go-gitea#19010) (go-gitea#19182)
  * Ensure that setting.LocalURL always has a trailing slash (go-gitea#19171) (go-gitea#19177)
  * Cleanup protected branches when deleting users & teams (go-gitea#19158) (go-gitea#19174)
  * Use IterateBufferSize whilst querying repositories during adoption check (go-gitea#19140) (go-gitea#19160)
  * Fix NPE /repos/issues/search when not signed in (go-gitea#19154) (go-gitea#19155)
  * Use custom favicon when viewing static files if it exists (go-gitea#19130) (go-gitea#19152)
  * Fix the editor height in review box (go-gitea#19003) (go-gitea#19147)
  * Ensure isSSH is set whenever DISABLE_HTTP_GIT is set (go-gitea#19028) (go-gitea#19146)
  * Fix wrong scopes caused by empty scope input (go-gitea#19029) (go-gitea#19145)
  * Make migrations SKIP_TLS_VERIFY apply to git too (go-gitea#19132) (go-gitea#19141)
  * Handle email address not exist (go-gitea#19089) (go-gitea#19121)
* MISC
  * Update json-iterator to allow compilation with go1.18 (go-gitea#18644) (go-gitea#19100)
  * Update golang.org/x/crypto (go-gitea#19097) (go-gitea#19098)

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Mar 23, 2022
Merged
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
Fix NPE /repos/issues/search when not signed in (go-gitea#19154)
07007d6 
- Don't panic when on
`/repos/issues/search?{created,assigned,mentioned,review_requested}=true`
when client didn't pass any authentication.
- Resolves go-gitea#19115
@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

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@zeripath zeripath zeripath 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.17.0
Development

Successfully merging this pull request may close these issues.

Issue search API with created param returns 500 if not logged in
5 participants
@Gusted @wxiaoguang @zeripath @GiteaBot @6543