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

Support (or don't support) including a port number in --trusted-host #6886

Closed
frostming opened this issue Aug 16, 2019 · 5 comments · Fixed by #6909
Closed

Support (or don't support) including a port number in --trusted-host #6886

frostming opened this issue Aug 16, 2019 · 5 comments · Fixed by #6909
Labels
auto-locked Outdated issues that have been locked by automation C: finder PackageFinder and index related code type: enhancement Improvements to functionality

Comments

@frostming
Copy link
Contributor

What's the problem this feature will solve?
Refer to #6705
for HTTP indexes, --trusted-host option should be sans-port, if you give a port part then it aborts:

$ pip install -i http://localtest.me:5000 urllib3 --trusted-host localtest.me:5000
Looking in indexes: http://localtest.me:5000
Collecting urllib3
  The repository located at localtest.me is not a trusted or secure host and is being ignored. If this repository is available via HTTPS we recommend you use HTTPS instead, otherwise you may silence this warning and allow it anyway with '--trusted-host localtest.me'.
  Could not find a version that satisfies the requirement urllib3 (from versions: )
No matching distribution found for urllib3

While for HTTPS indexes, --trusted-host should carry the port. (Now a fix is landed on master to support both, with port and without port).

Describe the solution you'd like
Pip should handle trusted-host correctly, whether the host has a port part or not.
with --trusted-host example.org you trust all the subpaths and wildcard ports on the same host.
with --trusted-host example.org:8080 you only trust requests to 8080 port.

Alternative Solutions

Additional context

@triage-new-issues triage-new-issues bot added the S: needs triage Issues/PRs that need to be triaged label Aug 16, 2019
@cjerdonek cjerdonek changed the title Pip should handle trusted-host correctly whether it contains port part. Support (or don't support) including a port number in --trusted-host Aug 16, 2019
@cjerdonek cjerdonek added C: finder PackageFinder and index related code type: enhancement Improvements to functionality labels Aug 16, 2019
@triage-new-issues triage-new-issues bot removed the S: needs triage Issues/PRs that need to be triaged label Aug 16, 2019
@cjerdonek
Copy link
Member

cjerdonek commented Aug 16, 2019

To add more info to the description above, currently, pip "happens" to behave correctly for HTTPS URL's when --trusted-host includes a port number. I say "happens" because it's not supported for HTTP URL's, and the support is undocumented for HTTPS URL's; the documentation just says "hostname":

--trusted-host <hostname> Mark this host as trusted, even though it does not have valid or any HTTPS.

So the question is whether to officially support port numbers or not. If so, we should add support for HTTP and update the documentation to say that port numbers are allowed. If not, we should probably deprecate allowing port numbers to be included in --trusted-host values (for both HTTP and HTTPS).

@cjerdonek cjerdonek added the state: needs discussion This needs some more discussion label Aug 16, 2019
@cjerdonek
Copy link
Member

@frostming I'm thinking we should add official support for this, in part because pip's code is already structured to support it. Would you be able to work up a PR? It's probably easiest to do it after PR #6903 is merged (or build the changes on top of those changes).

@cjerdonek
Copy link
Member

PR #6903 has been merged, FYI.

@frostming
Copy link
Contributor Author

@cjerdonek Great, I will take a look when I get a chance.

@cjerdonek cjerdonek added state: awaiting PR Feature discussed, PR is needed and removed state: needs discussion This needs some more discussion labels Aug 23, 2019
@frostming
Copy link
Contributor Author

@cjerdonek I have created a PR #6909 together with some test cases to illustrate the changes.

@cjerdonek cjerdonek removed the state: awaiting PR Feature discussed, PR is needed label Aug 24, 2019
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Sep 24, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Sep 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation C: finder PackageFinder and index related code type: enhancement Improvements to functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants