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

Question about the format of PerRPCCredential.GetRequestMetadata()'s parameter #1435

Closed
mitake opened this issue Aug 10, 2017 · 6 comments
Closed

Comments

@mitake
Copy link

mitake commented Aug 10, 2017

Please answer these questions before submitting your issue.

What version of gRPC are you using?

v1.5.1

What version of Go are you using (go version)?

1.8.1

What operating system (Linux, Windows, …) and version?

linux

What did you do?

I checked PerRPCCredential.GetRequestMetadata()'s second parameter, uri.

What did you expect to see?

I expected that the uri is something like thishttps://<host>:<port>/<method>.

What did you see instead?

I saw https://<host>:<port1>:<port2>/<method>.

question

My question: is the above format that contains 2 port numbers intentional? I expected that the URI is information of the target: host, port and method . It is a little bit strange and according to the URI construction code (https://github.com/grpc/grpc-go/blob/master/transport/http2_client.go#L351), it is constructed like this: audience = "https://" + callHdr.Host + port + callHdr.Method[:pos]. And callHdr.Host contains its own port number that comes from here: https://github.com/grpc/grpc-go/blob/master/clientconn.go#L378 .

@dfawley
Copy link
Member

dfawley commented Aug 10, 2017

This is a bug. See PR #1433 for a possible fix. However, we still aren't completely sure if using the port from the host is correct, or if we should use the target's port instead (which is what the code was attempting to do before).

@mitake
Copy link
Author

mitake commented Aug 21, 2017

@dfawley thanks for the pointer and I'm glad to see the problem was fixed. Will the format of the parameter be changed in the future?

@dfawley
Copy link
Member

dfawley commented Aug 21, 2017

There are no plans to change this beyond what #1433 did. Please comment here if you think there's any issues using it as implemented. Otherwise, I'll close this, as the PR has been merged. Thanks.

@dfawley dfawley closed this as completed Aug 21, 2017
@mitake
Copy link
Author

mitake commented Aug 23, 2017

@dfawley thanks for the answer. And I don't have problems for now. Will #1433 be backported to 1.5.x?

@dfawley
Copy link
Member

dfawley commented Aug 23, 2017

We will not be backporting it unless there is an urgent need. We will be doing the 1.6 release next Wednesday.

@mitake
Copy link
Author

mitake commented Aug 23, 2017

I understand. Thanks a lot for your help!

@lock lock bot locked as resolved and limited conversation to collaborators Sep 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants