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

Add middleware with support proxy headers #2453

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

padremortius
Copy link

Add middleware for support proxy headers (based on code gorilla mux handler https://github.com/gorilla/handlers/blob/master/proxy_headers.go)

}

if c.Request().Header.Get(echo.HeaderXForwardedHost) != "" {
c.Request().Host = c.Request().Header.Get(echo.HeaderXForwardedHost)
Copy link
Contributor

@aldas aldas May 15, 2023

Choose a reason for hiding this comment

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

Getting host from proxied request is little bit more complex and has security nuances. Please see https://github.com/labstack/echo/blob/master/ip.go

@aldas
Copy link
Contributor

aldas commented May 15, 2023

NB: it is missing tests

@lammel
Copy link
Contributor

lammel commented Jul 11, 2023

@padremortius Do you want to continue on this PR or shall we close it?

@padremortius
Copy link
Author

@lammel, yes. Added some changes and unit-test

@aldas
Copy link
Contributor

aldas commented Jul 13, 2023

unit tests are only for scheme parsing. this is only small part of that middleware.


Currently it introduces new header "forwarded" RFC7239 which I think - if implemented should be integrated into c.RealIP() and IPExtractor code as well.

about RFC7239 and Go standard library golang/go#30963

maybe instead of that Regex we could have small parser that takes in header value, goes over that rune/char by rune/char and outputs parsed parts or an error. RFC7239 should not be that complex as spec is not that complex.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants