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.

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

fix(grpc-web): fix missing allowed headers for grpc deadlines #11368

Closed
wants to merge 3 commits into from
Closed

fix(grpc-web): fix missing allowed headers for grpc deadlines #11368

wants to merge 3 commits into from

Conversation

TheImpressiveDonut
Copy link

Acces-Control-Allow-Headers is missing an header which make it impossible to use deadlines with grpc calls, the header is "grpc-timeout".

#11185
https://github.com/grpc/grpc-web/blob/9c48d290b2a978c0662dabe5123ed05923054bce/javascript/net/grpc/web/grpcwebclientbase.js#L332

Summary

Fix that make deadline usable with grpc-web plugin.

Checklist

Full changelog

  • add "grpc-timeout": Acces-Control-Allow-Headers is missing an header which make it impossible to use deadlines with grpc calls, the header is "grpc-timeout".

Issue reference

Fix #11185

@CLAassistant
Copy link

CLAassistant commented Aug 8, 2023

CLA assistant check
All committers have signed the CLA.

@kikito kikito requested a review from gszr August 8, 2023 17:05
@hanshuebner hanshuebner added the author/community PRs from the open-source community (not Kong Inc) label Aug 30, 2023
@chronolaw
Copy link
Contributor

Could you add a change log entry for this PR. See: https://github.com/Kong/kong/blob/master/CHANGELOG/README.md

@chronolaw chronolaw added the pending author feedback Waiting for the issue author to get back to a maintainer with findings, more details, etc... label Sep 14, 2023
@@ -178,5 +178,19 @@ for _, strategy in helpers.each_strategy() do
assert.same({ reply = "hello heya" }, cjson.decode((res:read_body())))
assert.is_nil(err)
end)

test("Call binary gRCP via HTTP with a deadline", function()
local res, err = proxy_client:post("/hello.HelloService/SayHello", {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it might be better to provide two tests, one is about the test case that grpc_timeout can take effect, and the other is using the OPTIONS request to get the Access-Control-Allow-Headers response with grpc_timeout.

Copy link
Member

Choose a reason for hiding this comment

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

@Water-Melon Would you mind taking this over so we can get the fix merged?

Copy link
Contributor

github-actions bot commented Feb 2, 2024

This PR is marked as stale because it has been open for 14 days with no activity.

@github-actions github-actions bot added the stale label Feb 2, 2024
Copy link
Contributor

Dear contributor,

We are automatically closing this pull request because it has not seen any activity for three weeks.
We're sorry that we could not merge it. If you still want to pursure your patch, please feel free to
reopen it and address any remaining issues.

Your contribution is greatly appreciated!

Please have a look
our pledge to the community
for more information.

Sincerely,
Your Kong Gateway team

@github-actions github-actions bot closed this Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author/community PRs from the open-source community (not Kong Inc) pending author feedback Waiting for the issue author to get back to a maintainer with findings, more details, etc... plugins/grpc-web size/S stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugin gRPC-web missing allow-headers
6 participants