NotificationsYou must be signed in to change notification settings
Fork3.6k
Star24.3k

Add Dialer.ProxyConnectHeader#605

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Open

AllenX2018 wants to merge2 commits intogorilla:main

base:main

Choose a base branch

fromAllenX2018:fix-issue-479

Open

Add Dialer.ProxyConnectHeader#605

AllenX2018 wants to merge2 commits intogorilla:mainfromAllenX2018:fix-issue-479

+27 −5

Conversation

Copy link

AllenX2018 commentedJun 29, 2020

Fixes#479. Add a fieldProxyConnectHeader inDialer for user to costomize the proxy headers

IngCr3at1on suggested changes

Sep 8, 2020

View reviewed changes

client.go

		}
		ifproxyURL!=nil {
		dialer,err:=proxy_FromURL(proxyURL,netDialerFunc(netDial))
		dialer,err:=proxy_FromURL(proxyURL,&netDialer{d.ProxyConnectHeader,netDial})

Copy link

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.

Copy link

Author

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

		funcinit() {
		proxy_RegisterDialerType("http",func(proxyURL*url.URL,forwardDialerproxy_Dialer) (proxy_Dialer,error) {
		return&httpProxyDialer{proxyURL:proxyURL,forwardDial:forwardDialer.Dial},nil
		p,_:=forwardDialer.(*netDialer)

Copy link

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.

Copy link

Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Yes, i fix it

fix issuegorilla#479

aa46640

AllenX2018 force-pushed thefix-issue-479 branch from7ce4db9 toaa46640Compare

September 11, 2020 03:31

Copy link

IngCr3at1on commentedSep 11, 2020

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.