Skip to content
This repository has been archived by the owner on Jul 13, 2021. It is now read-only.

adding support for ignoring credentials in URL #76

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tjb1982
Copy link

@tjb1982 tjb1982 commented Jan 20, 2016

Related to #75

This "fix" isn't ideal in that it just ignores the credentials instead of doing something useful with them, e.g., when you curl with credentials in the url, curl adds them as a base64 encoded string to an Authorization: Basic header automatically.

@jech
Copy link
Owner

jech commented Jan 22, 2016

Which RFC is this described in?

@tjb1982
Copy link
Author

tjb1982 commented Jan 22, 2016

RFC3986 describes a "userinfo" subcomponent of the authority component. It also explains that the password portion of it is deprecated and should be ignored. It also explains instances where the username piece of the userinfo subcomponent could be used for "semantic attacks," but doesn't explicitly deprecate the use of the username piece. However, I would argue that it's both implied and a good idea to ignore both.

cf. https://tools.ietf.org/html/rfc3986#section-3.2.1, https://tools.ietf.org/html/rfc3986#appendix-A, https://tools.ietf.org/html/rfc3986#section-7.5, and https://tools.ietf.org/html/rfc3986#section-7.6

@jech
Copy link
Owner

jech commented Jan 30, 2016

Shouldn't we be sending the user info to the server somewhere?

@tjb1982
Copy link
Author

tjb1982 commented Feb 1, 2016

I think so, and the server should be allowed to handle it however it would like. I tried to address that with riptano@37a9fef, so that x isn't reset to the position after the '@' char. Instead, parseUrl leaves x pointing to the start of the "authority" component (so-called by RFC-3986). E.g.:

http://user:[email protected]:8080/foo?bar#baz
       ^ x points here

That's what I meant by "passthrough userinfo subcomponent."

@tjb1982
Copy link
Author

tjb1982 commented Mar 7, 2016

@jech what do you want to do with this?

@jech
Copy link
Owner

jech commented Mar 23, 2016

I'm still waiting for an explanation why this is useful, and why it is the business of the proxy to do that.

@tjb1982
Copy link
Author

tjb1982 commented Sep 8, 2016

@jech all the information you need has been presented to you in my comments above, the pull request, in the RFCs referenced above, and in this ticket: #75. If you don't agree that you should support all conforming URIs, that's not something I care to argue with you. We (DataStax) had a desire for polipo to support all types of conforming URIs, but because polipo is incomplete, rather than contort our systems around polipo's shortcomings, we chose to use a different proxy that does support the complete URI spec as per the RFC instead.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants