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

Basic auth #7

Merged
merged 2 commits into from
Oct 16, 2015
Merged

Basic auth #7

merged 2 commits into from
Oct 16, 2015

Conversation

natebrennand
Copy link
Contributor

I'm confused as to why Drone lets the test pass and local doesn't:

$ go test
--- FAIL: TestURLwithBasicAuth (0.00s)
    discovery_test.go:61: Unexpected result, expected: https://user:[email protected]:80, received: https://user:pass%40api.google.com:80
FAIL
exit status 1
FAIL    github.com/Clever/discovery-go  0.005s

This just changes how we parse the URL to handle things like @ in the host.

@christiangriset
Copy link

Are you still having the problem? I think one possibility is it has to do with default encoding. At the command line I get the following

>>> echo $LANG
en_US.UTF-8

It turns out you're not the only one having this problem. I wonder if this should be a general concern and if Clever should general fix encoding to keep bugs like this from ever occurring.

I also wonder if you just happen to be using an outdated version of net/url. It's possible you, me and drone all have different versions of net/url. Seems unlikely to work, but maybe do go get -u then retry the test.

@natebrennand
Copy link
Contributor Author

Well oddly I'm using 1.5 locally whereas drone is on 1.4 and I'm still getting the error. My $LANG is also set to en_US.UTF-8 fwiw.
I think this approach is better to just let the lib parse everything anyways, otherwise we'd need to make env keys for each possible section of the URL.

natebrennand added a commit that referenced this pull request Oct 16, 2015
@natebrennand natebrennand merged commit 91bc0dd into master Oct 16, 2015
@natebrennand natebrennand deleted the basic-auth branch October 16, 2015 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants