Skip to content

fix(grpc-web): don't override Access-Control-Allow-Origin header in response#6842

Merged
spacewander merged 1 commit into
apache:masterfrom
tzssangglass:Issue6834
Apr 15, 2022
Merged

fix(grpc-web): don't override Access-Control-Allow-Origin header in response#6842
spacewander merged 1 commit into
apache:masterfrom
tzssangglass:Issue6834

Conversation

@tzssangglass

@tzssangglass tzssangglass commented Apr 14, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Description

Fixes #6834

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@tzssangglass tzssangglass changed the title fix(grpc-web): don't override Access-Control-Allow-Origin header in r… fix(grpc-web): don't override Access-Control-Allow-Origin header in response Apr 14, 2022
@HalilOzkan

HalilOzkan commented Apr 14, 2022

Copy link
Copy Markdown

I think we should also respect and don't override all CORS headers in the grpc-web plugin if already set by the CORS plugin

@tzssangglass

tzssangglass commented Apr 15, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

I think we should also respect and don't override all CORS headers in the grpc-web plugin if already set by the CORS plugin

yes, this is how this PR is handled.

According to the grpc-web requirements for CORS: https://github.com/grpc/grpc-web/blob/master/doc/browser-features.md#cors-support

Access-Control-Allow-Credentials, Access-Control-Allow-Methods, Access-Control-Allow-Headers is clear, I understand what you mean, the cors plugin may have more precise control over the three header.

@spacewander
spacewander merged commit d6a4afd into apache:master Apr 15, 2022
Liu-Junlin pushed a commit to Liu-Junlin/apisix that referenced this pull request May 20, 2022
@tzssangglass @Liu-Junlin
fix(grpc-web): don't override Access-Control-Allow-Origin header in r…
cda9f4e 
…esponse (apache#6842)
@tzssangglass
tzssangglass deleted the Issue6834 branch May 24, 2022 06:25
spacewander pushed a commit to spacewander/incubator-apisix that referenced this pull request May 27, 2022
@tzssangglass @spacewander
fix(grpc-web): don't override Access-Control-Allow-Origin header in r…
8f9f8d6 
…esponse (apache#6842)
spacewander pushed a commit to spacewander/incubator-apisix that referenced this pull request May 29, 2022
@tzssangglass @spacewander
fix(grpc-web): don't override Access-Control-Allow-Origin header in r…
8b55e36 
…esponse (apache#6842)
spacewander pushed a commit that referenced this pull request May 29, 2022
@tzssangglass @spacewander
fix(grpc-web): don't override Access-Control-Allow-Origin header in r…
173afe3 
…esponse (#6842)
spacewander pushed a commit that referenced this pull request Jun 30, 2022
@tzssangglass @spacewander
fix(grpc-web): don't override Access-Control-Allow-Origin header in r…
4d3f79f 
…esponse (#6842)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@spacewander spacewander spacewander approved these changes
@shuaijinchao shuaijinchao shuaijinchao approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug: Access-Control-Allow-Origin is manipulated incorrect when using grpc-web and cors plugins in the same route

4 participants

@tzssangglass @HalilOzkan @spacewander @shuaijinchao