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 incorrect CORS default values #24206

Merged
merged 2 commits into from
Apr 19, 2023
Merged

Fix incorrect CORS default values #24206

merged 2 commits into from
Apr 19, 2023

Conversation

wxiaoguang
Copy link
Contributor

Document:

;ALLOW_DOMAIN = *
;METHODS = GET,HEAD,POST,PUT,PATCH,DELETE,OPTIONS

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 19, 2023
@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 Apr 19, 2023
@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 Apr 19, 2023
@lunny lunny added type/bug outdated/backport/v1.19 This PR should be backported to Gitea 1.19 labels Apr 19, 2023
@lunny lunny added this to the 1.20.0 milestone Apr 19, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 19, 2023
@silverwind silverwind enabled auto-merge (squash) April 19, 2023 19:12
@silverwind silverwind merged commit 9421063 into go-gitea:main Apr 19, 2023
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 19, 2023
@GiteaBot GiteaBot mentioned this pull request Apr 19, 2023
Merged
GiteaBot added a commit to GiteaBot/gitea that referenced this pull request Apr 19, 2023
@wxiaoguang @GiteaBot
Fix incorrect CORS default values (go-gitea#24206)
a1dde2c 
Document: 

```
;ALLOW_DOMAIN = *
;METHODS = GET,HEAD,POST,PUT,PATCH,DELETE,OPTIONS
```

Co-authored-by: Giteabot <teabot@gitea.io>
@GiteaBot GiteaBot added the backport/done All backports for this PR have been created label Apr 19, 2023
silverwind pushed a commit that referenced this pull request Apr 19, 2023
@GiteaBot @wxiaoguang
Fix incorrect CORS default values (#24206) (#24217)
2e6e5bc 
Backport #24206 by @wxiaoguang

Document: 

```
;ALLOW_DOMAIN = *
;METHODS = GET,HEAD,POST,PUT,PATCH,DELETE,OPTIONS
```

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@wxiaoguang wxiaoguang deleted the fix-cors-default branch April 20, 2023 01:44
@delvh
Copy link
Member

delvh commented Apr 20, 2023

Should we mark this PR as breaking so that we will not forget it when writing the release blog?

@wxiaoguang
Copy link
Contributor Author

At least, I don't see it would affect end users.

@silverwind
Copy link
Member

Previous default config did not set any CORS headers, right? I recall CORS config is confusingly split into at least 2-3 config options, some work on some endpoints, some on another.

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Apr 20, 2023 •
edited
Loading

Previous default config did not set any CORS headers, right?

I guess yes. Before if there is only ENABLED=true, then CORS seems not working.

The METHODS is optional, leaving it empty is still fine. So the minimal config for CORS is like this now:

[cors]
ENABLED=true

And it matches the document.

But I think this PR only fixes the bug, it doesn't really introduce "breaking" behavior because the document already said that by default ALLOW_DOMAIN="*"

zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 20, 2023
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
0b7bce9 
* giteaofficial/main: (26 commits)
  Make HTML template functions support context (go-gitea#24056)
  Fix issue attachment handling (go-gitea#24202)
  Update JS dependencies (go-gitea#24218)
  Remove most path-based golangci exclusions (go-gitea#24214)
  [skip ci] Updated translations via Crowdin
  Fix internal sever error when visiting a PR that bound to the deleted team (go-gitea#24127)
  Add owner team permission check test (go-gitea#24096)
  Fix incorrect CORS default values (go-gitea#24206)
  Add repository counter badge to repository tab (go-gitea#24205)
  Fix template error in pull request with deleted head repo (go-gitea#24192)
  Make wiki title supports dashes and improve wiki name related features (go-gitea#24143)
  Fix Resolve Conversation not working in Conversation view (go-gitea#24191)
  Vertical widths of containers removed (go-gitea#24184)
  Don't list root repository on compare page if pulls not allowed (go-gitea#24183)
  Add unset default project column (go-gitea#23531)
  Allow adding new files to an empty repo (go-gitea#24164)
  Add runner check in repo action page (go-gitea#24124)
  Use same action status svg icons on actions list as on action page (go-gitea#24178)
  [skip ci] Updated translations via Crowdin
  fix calReleaseNumCommitsBehind (go-gitea#24148)
  ...

# Conflicts:
#	templates/repo/wiki/view.tmpl
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jul 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lunny lunny lunny approved these changes

@silverwind silverwind silverwind 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. outdated/backport/v1.19 This PR should be backported to Gitea 1.19 type/bug
Projects
None yet
Milestone
1.20.0
Development

Successfully merging this pull request may close these issues.
5 participants
@wxiaoguang @delvh @silverwind @lunny @GiteaBot