Skip to content

fix: grpc-web trailers#10851

Merged
juststillthinking merged 8 commits into
apache:masterfrom
sheharyaar:fix-grpc-web-trailers
Jan 26, 2024
Merged

fix: grpc-web trailers#10851
juststillthinking merged 8 commits into
apache:masterfrom
sheharyaar:fix-grpc-web-trailers

Conversation

@sheharyaar

@sheharyaar sheharyaar commented Jan 18, 2024

Copy link
Copy Markdown
Contributor

Description

Fixes #10573 and closes #10613

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)

@sheharyaar

Copy link
Copy Markdown
Contributor Author

This PR is not yet complete, need to add some features.
@Affanmir, please checkout this PR and confirm if this fixes your issue ?

@sheharyaar sheharyaar changed the title fix grpc-web trailers fix: grpc-web trailers Jan 18, 2024
@Vacant2333

Copy link
Copy Markdown
Contributor

@sheharyaar hello~ have u test on your enviorment? u can add some testcase that can help us to review~~

@sheharyaar

Copy link
Copy Markdown
Contributor Author

@Vacant2333 , I have added test-cases, please approve workflow

@juststillthinking

Copy link
Copy Markdown
Contributor

@sheharyaar Is this pr ready?

@sheharyaar

Copy link
Copy Markdown
Contributor Author

Yes, the PR is ready @monkeyDluffy6017

@Affanmir

Copy link
Copy Markdown

This PR is not yet complete, need to add some features. @Affanmir, please checkout this PR and confirm if this fixes your issue ?

Missing trailers issue fixed but path based testing cannot be done until the cors issue is resolved.

Access to XMLHttpRequest at 'https://XXXXXXXX/RANDOM.RANDOM.SERVICENAME.service.ServiceExternal/SERVICEMETHOF' from origin 'http://localhost:8080/' has been blocked by CORS policy: Request header field REDACTED_FIELD is not allowed by Access-Control-Allow-Headers in preflight response.
SUBDOMAON.DOMAIN/RANDOM.RANDOM.SERVICENAME.service.ServiceExternal/SERVICEMETHOD:1

The issue persistents even when using cors plugin alongside this plugin. Please replicate on your end

Comment thread apisix/plugins/grpc-web.lua Outdated
Comment thread apisix/plugins/grpc-web.lua
Comment thread apisix/plugins/grpc-web.lua

@Vacant2333 Vacant2333 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

hi, the ci was failed mostly, u can merge the master after the pr get merged~ it will fix it

@Vacant2333

Vacant2333 commented Jan 23, 2024

Copy link
Copy Markdown
Contributor

r u ready to review and merge now? @shreemaan-abhishek can u help take a look~

@sheharyaar

Copy link
Copy Markdown
Contributor Author

Yes, I am ready

Comment thread apisix/plugins/grpc-web.lua
Comment thread t/plugin/grpc-web.t

@shreemaan-abhishek shreemaan-abhishek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment thread t/plugin/grpc-web.t Outdated
Comment thread t/plugin/grpc-web.t Outdated
@Revolyssup

Copy link
Copy Markdown
Contributor

please fix ci

@sheharyaar

Copy link
Copy Markdown
Contributor Author

I am having issues with this. Locally it works fine for me. I need help with this.

Signed-off-by: Mohammad Shehar Yaar Tausif <[email protected]>
Signed-off-by: Mohammad Shehar Yaar Tausif <[email protected]>
Signed-off-by: Mohammad Shehar Yaar Tausif <[email protected]>
Signed-off-by: Mohammad Shehar Yaar Tausif <[email protected]>
Signed-off-by: Mohammad Shehar Yaar Tausif <[email protected]>
Signed-off-by: Mohammad Shehar Yaar Tausif <[email protected]>
Signed-off-by: Mohammad Shehar Yaar Tausif <[email protected]>
Signed-off-by: Mohammad Shehar Yaar Tausif <[email protected]>
@juststillthinking
juststillthinking merged commit 0bea3db into apache:master Jan 26, 2024
shreemaan-abhishek added a commit to shreemaan-abhishek/apisix that referenced this pull request Jan 2, 2026
## Description

backports: apache#10904 and
apache#10851

   
## Checklist
- [ ] I have explained **WHY** we need this PR and **HOW** the problem
it solves
- [ ] The title of the PR concisely describes the code modification and
complies with APISIX specifications.
 - [ ] I have explained the changes or the new features added to this PR
 - [ ] I have added **e2e test cases** corresponding to this change
 - [ ] I have updated the documentation to reflect this change

**NOTE: This PR will not be reviewed and merged until the contents of
these checklists are completed.**
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: grpc-web plugin causing missing trailers issue

6 participants