slurp doesn't properly handle URLs that contain authentication information

Description

Because of limitations in the underlying Java classes [[1]|https://stackoverflow.com/questions/496651/connecting-to-remote-url-which-requires-authentication-using-java], clojure.core/slurp doesn't support URLs that contain authentication information i.e. https://user:password@hostname.domain/path/to/resource. It would be great if slurp did support this (e.g. by using Java's UrlConnection class, as suggested in [1]), so that application code doesn't have to include special cases for different types of input string that will be sent to slurp.

[1] https://stackoverflow.com/questions/496651/connecting-to-remote-url-which-requires-authentication-using-java

Approach: Check to see if url contains userInfo, if so, open a connection, set authorisation on it and return its input stream. Otherwise call `openStream` on the url as before
Limitations: This approach uses java.xml.bind.DatatypeConverter, which is known to not work with java9

Environment

n/a

Activity

Show:
import
June 27, 2018, 11:35 PM

Comment made by: pmonks

Alex it would be good to better understand why you've chosen to draw this somewhat arbitrary line through HTTP retrieval support in slurp.

I don't think anyone is proposing that slurp should "handle every possible remote resource retrieval", but there are clear pragmatic advantages in properly supporting at least HTTP (given it's become the lingua franca of remote resource retrieval).

Alex Miller
June 28, 2018, 3:12 AM

If you're not saying that it should handle every possible option then you are implicitly admitting there is a line. I'm drawing that line here. This is not a burning need or priority for anyone's production app. Use an http client lib if you need it.

import
June 28, 2018, 3:42 AM

Comment made by: pmonks

Nice straw man you've constructed there Alex - nowhere did I say that there wasn't a line. Rather, and at the risk of sounding like a broken record, I am advocating for that line being around HTTP - all of it, not some arbitrary, Java-historical-oddity defined line.

And yes, this is a burning priority for several production apps I developed and operate. Sufficiently burning that I've hacked around this bug in those apps myself, something I would rather not have had to do. And as the patches above demonstrate, an HTTP client library is not needed for this - the core Java libraries can already handle this case, they just don't fully support the URI RFC when it comes to URIs provided as string values.

Alex Miller
June 28, 2018, 4:02 AM

The point is, I'm not willing to sign up to support more than it does now and continue supporting it forever. You can handle this yourself by extending the protocol on the user side (which is the point of open extensions) or by using other libs, or by using Java directly.

import
June 28, 2018, 5:39 AM

Comment made by: pmonks

We'll just have to agree to disagree that the maintenance cost of this ~16 line patch is greater than the benefit of slurp acting in a reasonable, spec-compliant way.

Declined

Assignee

Unassigned

Reporter

import

Labels

Approval

Triaged

Patch

Code

Affects versions

Priority

Minor