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 host string passed to PerRPCCredentials #1433

Merged
merged 3 commits into from
Aug 17, 2017
Merged

Conversation

xinzhangx
Copy link
Contributor

callHdr.Host already contains the port. So the behavior is broken that:

  • when default port 443 is used, the audience string still contains a port
  • when non-default port is used, the port number appears twice in the audience string

callHdr.Host already contains the port. So the behavior is broken that:
- when default port 443 is used, the audience string still contains a port
- when non-default port is used, the port number appears twice in the audience string
pos := strings.LastIndex(callHdr.Method, "/")
if pos == -1 {
pos = len(callHdr.Method)
}
audience = "https://" + callHdr.Host + port + callHdr.Method[:pos]
audience = "https://" + host + callHdr.Method[:pos]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove the trailing space? It's causing gofmt failures.

@menghanl
Copy link
Contributor

menghanl commented Aug 11, 2017

Can you please run gofmt to fix the formatting issues? Thanks.

@dfawley
Copy link
Member

dfawley commented Aug 17, 2017

Ping -- could you please update this so that Travis passes (gofmt/goimports)? Thanks.

@menghanl menghanl merged commit 5d9b09e into grpc:master Aug 17, 2017
@xinzhangx xinzhangx deleted the patch-2 branch August 17, 2017 22:59
@dfawley dfawley modified the milestone: 1.6 Release Aug 28, 2017
menghanl pushed a commit to menghanl/grpc-go that referenced this pull request Aug 30, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants