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 portFromUrl when double protocol is provided #7838

Merged
merged 1 commit into from
Aug 7, 2019

Conversation

Mythra
Copy link
Member

@Mythra Mythra commented Aug 6, 2019

revival of #7823

Description:

this fixes a particular exception case where an end-user configures
a socket address to point at: https://google.com instead of just
the hostname: google.com.

instead of throwing an (stoi) error. because it can't turn:
//google.com into a port. we now through a slightly more sane:
malformed url. it isn't as good as a protoc-gen-validate message
since it's not clear why: tcp:// gets prepended, but I think it's
digestable enough to know, you shouldn't be putting https there.
not to mention it is a step forward from: stoi.

Risk Level: Low

Testing:

Not only are there unit tests, but you can run through the following configurations and
see the new error messages.

Docs Changes: None
Release Notes: None

revival of envoyproxy#7823

this fixes a particular exception case where an end-user configures
a socket address to point at: `https://google.com` instead of just
the hostname: `google.com`.

instead of throwing an (stoi) error. because it can't turn:
`//google.com` into a port. we now through a slightly more sane:
`malformed url`. it isn't as good as a protoc-gen-validate message
since it's not clear why: `tcp://` gets prepended, but I think it's
digestable enough to know, you shouldn't be putting https there.
not to mention it is a step forward from: `stoi`.

Signed-off-by: Cynthia Coan <ccoan@instructure.com>
@jmarantz
Copy link
Contributor

jmarantz commented Aug 7, 2019

@lizan for senior committer approval.

@jmarantz
Copy link
Contributor

jmarantz commented Aug 7, 2019

/azp run envoy-macos

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lizan lizan merged commit 7e21623 into envoyproxy:master Aug 7, 2019
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.

3 participants