Skip to content

reverseproxy: PROXY protocol support for http transport#4855

Closed
WeidiDeng wants to merge 3 commits intocaddyserver:masterfrom
WeidiDeng:proxy-protocol-reverse-proxy
Closed

reverseproxy: PROXY protocol support for http transport#4855
WeidiDeng wants to merge 3 commits intocaddyserver:masterfrom
WeidiDeng:proxy-protocol-reverse-proxy

Conversation

@WeidiDeng
Copy link
Member

Add proxy protocol support for reverseproxy http transport

fix 2724.

@CLAassistant
Copy link

CLAassistant commented Jun 28, 2022

CLA assistant check
All committers have signed the CLA.

@mholt
Copy link
Member

mholt commented Jun 28, 2022

Cool, I'm excited to review this! Currently traveling but will try to squeeze it in.

var proxyProtocolInfo ProxyProtocolInfo
// using X-Forwarded-For header which is already filtered by trusted proxies
if xff := r.Header.Get("X-Forwarded-For"); xff != "" {
ip := net.ParseIP(xff)
Copy link
Member

Choose a reason for hiding this comment

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

This isn't safe, X-Forwarded-For could contain a comma separated list of IP addresses.

Copy link
Member Author

Choose a reason for hiding this comment

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

I forget that caddy set it as a string 😇, will change it.

@francislavoie francislavoie added the feature ⚙️ New feature or request label Jun 28, 2022
@francislavoie francislavoie added this to the v2.6.0 milestone Jun 28, 2022
Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

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

Thanks for implementing this! I agree with Francis' comments so far. I've only done a first pass at this point; I think I have more to say about the ProxyProtocolInfo stuff -- or maybe not, but I do need to spend more time when I have a chance and understand it a little better, and make sure it's the best way to do things.

Thanks again for working on this and for your patience!

@mholt mholt modified the milestones: v2.6.0, v2.7.0 Sep 16, 2022
@francislavoie
Copy link
Member

francislavoie commented Oct 3, 2022

I think this needs a rebase @WeidiDeng if you could update the PR!

Please remember to use the new netip package instead of net for IP address stuff.

@francislavoie francislavoie changed the title feat: proxy protocol support for reverseproxy http transport reverseproxy: PROXY protocol support for http transport Feb 26, 2023
@WeidiDeng
Copy link
Member Author

Superseded by 5424.

@WeidiDeng WeidiDeng closed this Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature ⚙️ New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

proxy module should support the proxy protocols for upstreams

4 participants