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

[http-probe] Add HTTP headers to CONNECT requests #404

Closed
matt-malarkey opened this issue Jun 8, 2023 · 3 comments · Fixed by #763
Closed

[http-probe] Add HTTP headers to CONNECT requests #404

matt-malarkey opened this issue Jun 8, 2023 · 3 comments · Fixed by #763
Labels
enhancement New feature or request
Milestone

Comments

@matt-malarkey
Copy link

Issue

When sending HTTPS requests with http-probe through a proxy, using proxy_url, user defined HTTP headers are not added to the CONNECT request.

This appears to be a limitation of https://pkg.go.dev/net/http where HTTP headers are only added to CONNECT requests if you specify them when creating the Transport object.

Fix

In http.go:L457 we could add any specified HTTP headers also to the CONNECT request like so:

for _, header := range p.c.GetHeaders() {
    if header.GetName() == "Host" {
        continue
    }
    t.ProxyConnectHeader = http.Header{header.GetName(): {header.GetValue()}}
}

It seems fairly simple to implement this but I'm not sure if adding user defined headers also to the CONNECT request has any other implications.

@matt-malarkey matt-malarkey added the enhancement New feature or request label Jun 8, 2023
@manugarg
Copy link
Contributor

manugarg commented Jun 8, 2023

@matt-malarkey Thank you for reporting the issue and researching it.

Off the top of my head, this sounds reasonable to me. I'll think more about it.

@manugarg
Copy link
Contributor

manugarg commented Jun 4, 2024

Sorry for dropping the ball on this one, but I was looking at it again (yes, I know after a year :() and realized that connect headers will likely look very different from the general request headers. It will be better to add another field called proxy_connect_header for this.

@manugarg manugarg linked a pull request Jun 5, 2024 that will close this issue
@manugarg
Copy link
Contributor

manugarg commented Jun 5, 2024

#763 adds proxy_connect_headers field.

@manugarg manugarg closed this as completed Jun 5, 2024
@manugarg manugarg added this to the v0.13.6 milestone Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants