Conversation
mbyczkowski
left a comment
mbyczkowski
left a comment
There was a problem hiding this comment.
Some nits and comments, but overall ✅
Also, I agree that it would be nice to have more tests, but a proper integration test harness is out of scope for this.
| gopkg.in/yaml.v2 v2.2.1 // indirect | ||
| ) | ||
|
|
||
| go 1.13 |
There was a problem hiding this comment.
Looks like change is the same as #198, but the commit SHA is different. Maybe it needs another rebase?
There was a problem hiding this comment.
yeah, will rebase or drop this commit
There was a problem hiding this comment.
rebased and it went poof
| if connectProxy.User != nil { | ||
| password, ok := connectProxy.User.Password() | ||
| if !ok { | ||
| return nil, fmt.Errorf("proxy username without password not currently supported") |
There was a problem hiding this comment.
is anything special needed for proxying using a username without password? or does anyone even use that?
There was a problem hiding this comment.
It's potentially possible (you could specify http://username@proxy/) but the upstream go-http-dialer package doesn't appear to support it (it always takes a username and password).
This only supports the simplest case: a username and password in the proxy URL, and http basic auth.
mcpherrinm
force-pushed
the
proxy-auth
branch
from
ec2208f to
e336bdd
Compare
2 participants
This uses credentials, if present, from the URL.
This code is manually tested, but unfortunately there's not really a test suite setup for connecting to things. I think that's a much bigger project than just adding this, so I'm punting on tests til later.