-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Add Dialer.ProxyConnectHeader #605
base: main
Are you sure you want to change the base?
Conversation
@@ -274,7 +277,7 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h | |||
return nil, nil, err | |||
} | |||
if proxyURL != nil { | |||
dialer, err := proxy_FromURL(proxyURL, netDialerFunc(netDial)) | |||
dialer, err := proxy_FromURL(proxyURL, &netDialer{d.ProxyConnectHeader, netDial}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should check if the header is set before using it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Custom header information, can be empty
proxy.go
Outdated
} | ||
|
||
func init() { | ||
proxy_RegisterDialerType("http", func(proxyURL *url.URL, forwardDialer proxy_Dialer) (proxy_Dialer, error) { | ||
return &httpProxyDialer{proxyURL: proxyURL, forwardDial: forwardDialer.Dial}, nil | ||
p, _ := forwardDialer.(*netDialer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We really should not just discard errors here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, i fix it
7ce4db9
to
aa46640
Compare
LGTM but someone else must approve. Just a quick comment: force pushing on a PR makes it difficult to review your new changes to the PR; I'd suggest just pushing followup commits and if you want to squash them after review that's a good idea. In this case it was easy enough to scroll through and find the change but it can be quite cumbersome in large PRs. |
Yes, thanks for the kind reminder |
LGTM - if the merge conflicts can be resolved then we can merge this PR :) |
Fixes #479. Add a field
ProxyConnectHeader
inDialer
for user to costomize the proxy headers