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

check port is valid #2501

Merged
merged 1 commit into from Nov 4, 2013
Merged

check port is valid #2501

merged 1 commit into from Nov 4, 2013

Conversation

@vieux
Copy link
Contributor

vieux commented Nov 1, 2013

Fixes #2480

if _, err := strconv.ParseUint(containerPort, 10, 16); err != nil {
return nil, nil, fmt.Errorf("Invalid containerPort: %s", containerPort)
}
if _, err := strconv.ParseUint(hostPort, 10, 16); hostPort != "" && err != nil {

This comment has been minimized.

Copy link
@crosbymichael

crosbymichael Nov 1, 2013

Member

Shouldn't you check if hostPort is != "" before even trying the strconv?

This comment has been minimized.

Copy link
@vieux

vieux Nov 1, 2013

Author Contributor

It's the same, and it smaller that way.
Otherwise you should add a new if

@crosbymichael

This comment has been minimized.

Copy link
Member

crosbymichael commented Nov 4, 2013

LGTM

1 similar comment
@creack

This comment has been minimized.

Copy link
Contributor

creack commented Nov 4, 2013

LGTM

creack added a commit that referenced this pull request Nov 4, 2013
@creack creack merged commit 261c2e2 into master Nov 4, 2013
@creack creack deleted the 2480-check_port-fix branch Nov 4, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.