Raise DockerException for invalid ports in parse_host()#3421
Open
semx wants to merge 1 commit into
Open
Conversation
parse_host() read the port via parsed_url.port, whose urllib property raises ValueError for out-of-range or non-numeric ports. That error escaped unwrapped, so a bad DOCKER_HOST surfaced as a raw ValueError instead of the DockerException the rest of parse_host() (and the invalid_hosts test contract) guarantees. Additionally, 'parsed_url.port or 0' collapsed an explicit ':0' into the no-port case. For ssh that appended ':22' to a netloc that already ended in ':0', producing a malformed 'ssh://host:0:22'; tcp://host:0 was already rejected, so the two protocols were inconsistent. Wrap the port access, distinguish a missing port (None) from an explicitly invalid one, and reject port 0 for both protocols. Signed-off-by: Sergey Sannikov <sergey@sannikov.dev>
7945fee to
2b67132
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
parse_host()mishandles invalid ports in thetcp/sshbranch.1. A raw
ValueErrorescapes instead ofDockerException.The port is read via
parsed_url.port, and urllib's.portproperty raisesValueErrorfor an out-of-range or non-numeric port. That error propagates unwrapped, even though the rest ofparse_host()— and theinvalid_hoststest contract intests/unit/utils_test.py— guarantees invalid input raisesDockerException. A user with a malformedDOCKER_HOSTgets an opaqueValueErrorthatexcept DockerExceptionwon't catch.2.
ssh://host:0silently produces a malformed result.parsed_url.port or 0collapses an explicit:0into the "no port given" case. Forsshthe code then rebuildsnetloc = f'{parsed_url.netloc}:{port}', butparsed_url.netlocstill ends in:0, so:22is appended to it:tcp://host:0is already rejected withDockerException, so the two protocols were inconsistent.Fix
Wrap the
parsed_url.portaccess, distinguish a missing port (None) from an explicitly invalid one, and reject port0for both protocols:After the change:
Valid hosts are unaffected (
ssh://host→ssh://host:22,tcp://host:2375→http://host:2375, empty host →127.0.0.1, IPv6, etc.).Tests
Added
tcp://host:0,ssh://host:0,tcp://host:99999andtcp://host:notaportto theinvalid_hostslist intests/unit/utils_test.py. The fullutils_test.pysuite (73 tests) passes.