Add option to specify proxy CONNECT headers

[jjiang-stripe]

Hello! 👋 I'd like to set some custom headers on proxy CONNECT requests to be able to some log info on the proxy server based on the header. Would it be possible to add an option somewhere on the request datum to add custom headers (e.g. something like Go's http clients ProxyConnectHeader)?

The CONNECT request is constructed over here, but I'm not entirely sure how @data is actually populated at request time. My ruby is pretty limited, but happy to help in any way I can!

[geemus]

Certainly sounds pretty straightforward. Could you supply an example of what headers might look like to ensure I know what you intend before getting too far into this? Thanks!

[jjiang-stripe]

Sure! I'm mainly looking to add in a basic trace header so that our proxy can log it when it receives a CONNECT, so something like x-trace-header: some-uuid. Does that help?

[geemus]

Got it, thanks. Yeah that helps, my guess was that it was something relatively simple like that, but just wanted to confirm before I potentially headed down a dead end. I think this should be fairly easy to do, but not sure how quickly I'll find time to do it.

[stale]

This issue has been automatically marked stale due to inactivity. It will be closed if no further activity occurs. Thank you for your contributions.

[geemus]

Oops, thought I had pinned this before.

[cds2-stripe]

Thanks @geemus! We are definitely still interested in this feature.

[hans-stripe]

Hey @geemus! I put together an experimental branch that adds support for these headers to both the default Excon client and any middlewares that might want to tweak the headers (which is how we'd like to use this feature internally). The hash handling is rudimentary but I wanted to put a demo together! Does this seem like a reasonable approach?

master...hans-stripe:hans-ssl-proxy-headers

[geemus]

@hans-stripe Hey, thanks for reaching out. Definitely seems like a step in the right direction. I think the key is probably adding the header insertion stuff in the socket, as you did.

I was wondering for the other part about the data.delete, which seemed a little weird to me. I think if you had data[:proxy][:headers] it would just work without needing to manipulate the data hash (and I think maybe manipulating it in that way might interfere with middleware). Does that make sense?

It might also be good to build the headers like the connection normally does: request << key.to_s << ': ' << value.to_s << CR_NL (at least historically this was more performant). We might consider adding this to the shared utilities also, rather than repeating it, but we can get into these little details once the broad strokes are sorted out.

Hope that helps, certainly happy to continue discussing and supporting this effort.

[jjiang-stripe]

hi @geemus! I ended up grabbing this from hans, and made a PR over at #742 to make it a lil easier to gather feedback. I still need to go wire this up to our proxy and see if it works as intended, but the unit test seems to work and early feedback is welcome (esp around where you'd like this documented/more places I could test)!

edit: I wired it up, and it seems to be properly setting the headers on the CONNECT request based on our logs!

[geemus]

@jjiang-stripe sounds good, I'll check it out.