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

feat: add Forwarded header parsing and real IP extraction with tests #2744

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 75 additions & 4 deletions context.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ type Context interface {
// Scheme returns the HTTP protocol scheme, `http` or `https`.
Scheme() string

SchemeForwarded() *Forwarded
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding new SchemeForwarded method to Context interface is a backwards incompatible change. It has to be some utility function or specific IPExtractor implementation


// RealIP returns the client's network address based on `X-Forwarded-For`
// or `X-Real-IP` request header.
// The behavior can be configured using `Echo#IPExtractor`.
Expand Down Expand Up @@ -234,6 +236,14 @@ const (
ContextKeyHeaderAllow = "echo_header_allow"
)

// Forwarded represents the structured format of the Forwarded HTTP header.
type Forwarded struct {
By []string
For []string
Host []string
Proto []string
}

const (
defaultMemory = 32 << 20 // 32 MB
indexPage = "index.html"
Expand Down Expand Up @@ -293,24 +303,85 @@ func (c *context) Scheme() string {
return "http"
}

func (c *context) SchemeForwarded() *Forwarded {
// Parse and get "Forwarded" header.
// See : https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded
if scheme := c.request.Header.Get(HeaderForwarded); scheme != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

This only parses single Header. From mozilla article I understand that you can have actually multiple Forwarded Headers in request.

RFC says https://datatracker.ietf.org/doc/html/rfc7239

A proxy server that wants to add a new "Forwarded" header field value
can either append it to the last existing "Forwarded" header field
after a comma separator or add a new field at the end of the header
block
.

Mozilla rephrases this as
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded

If there are multiple proxy servers between the client and server, they may each specify their own forwarding information. This can be done by adding a new Forwarded header to the end of the header block, or by appending the information to the end of the last Forwarded header in a comma-separated list.

There is Nginx issue https://trac.nginx.org/nginx/ticket/1316 that has example for multiple headers in request

f, err := c.parseForwarded(scheme)
if err != nil {
return nil
}
return &f
}
return nil
}

func (c *context) parseForwarded(input string) (Forwarded, error) {
forwarded := Forwarded{}
entries := strings.Split(input, ",")
Copy link
Contributor

Choose a reason for hiding this comment

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

To be honest - I do not understand when is semicolon used and when is colon used.
Mozilla documentation says that Directives are key=value pairs, separated by a semicolon. and This can be done by adding a new Forwarded header to the end of the header block, or by appending the information to the end of the last Forwarded header in a comma-separated list.

Can semicolon and comma be mixed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally - what about quoted strings that contain comma or semicolon?
RFC says that field value can be value = token / quoted-string where

These 2 are defined as

token = <Defined in [RFC7230], Section 3.2.6>
quoted-string = <Defined in [RFC7230], Section 3.2.6>

I think rfc7230 section-3.2.6 allows unescaped commas in quoted field value


for _, entry := range entries {
entry = strings.TrimSpace(entry)
pairs := strings.Split(entry, ";")

for _, pair := range pairs {
parts := strings.SplitN(pair, "=", 2)
if len(parts) != 2 {
return forwarded, fmt.Errorf("invalid pair: %s", pair)
}

key := strings.TrimSpace(parts[0])
value, err := url.QueryUnescape(strings.TrimSpace(parts[1]))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is url.QueryUnescape correct by rfc7230 section-3.2.6?

RFC says that value is defined as:

forwarded-pair = token "=" value
value = token / quoted-string
token = <Defined in [RFC7230], Section 3.2.6>
quoted-string = <Defined in [RFC7230], Section 3.2.6>

if err != nil {
return forwarded, fmt.Errorf("failed to unescape value: %w", err)
}
value = strings.Trim(value, "\"[]")
switch key {
case "by":
forwarded.By = append(forwarded.By, value)
case "for":
forwarded.For = append(forwarded.For, value)
case "host":
forwarded.Host = append(forwarded.Host, value)
case "proto":
forwarded.Proto = append(forwarded.Proto, value)
default:
return forwarded, fmt.Errorf("unknown key: %s", key)
}
}
}

return forwarded, nil
}

func (c *context) RealIP() string {
if c.echo != nil && c.echo.IPExtractor != nil {
return c.echo.IPExtractor(c.request)
}
// Check if the "Forwarded" header is present in the request.
Copy link
Contributor

Choose a reason for hiding this comment

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

After seeing how complex or how less understood Forwarded header is - I think it is better not to add it directly to RealIP method. It is better suited as separate IPExtractor implementation

if d := c.request.Header.Get(HeaderForwarded); d != "" {
// Parse the "Forwarded" header.
scheme, err := c.parseForwarded(d)
if err != nil {
return "" // Return an empty string if parsing fails.
}
if len(scheme.For) > 0 {
return scheme.For[0] // Return first for item
}
return ""
}
// Fall back to legacy behavior
if ip := c.request.Header.Get(HeaderXForwardedFor); ip != "" {
i := strings.IndexAny(ip, ",")
if i > 0 {
xffip := strings.TrimSpace(ip[:i])
xffip = strings.TrimPrefix(xffip, "[")
xffip = strings.TrimSuffix(xffip, "]")
xffip = strings.Trim(xffip, "\"[]")
return xffip
}
return ip
}
if ip := c.request.Header.Get(HeaderXRealIP); ip != "" {
ip = strings.TrimPrefix(ip, "[")
ip = strings.TrimSuffix(ip, "]")
ip = strings.Trim(ip, "\"[]")
return ip
}
ra, _, _ := net.SplitHostPort(c.request.RemoteAddr)
Expand Down
60 changes: 60 additions & 0 deletions context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,50 @@ func TestContext_Scheme(t *testing.T) {
}
}

func TestContext_SchemeForwarded(t *testing.T) {
tests := []struct {
c Context
s *Forwarded
}{
{
&context{
request: &http.Request{
Header: http.Header{HeaderForwarded: []string{"for=192.0.2.60;proto=http;by=203.0.113.43"}},
},
},
&Forwarded{
For: []string{"192.0.2.60"},
Proto: []string{"http"},
By: []string{"203.0.113.43"},
},
},
{
&context{
request: &http.Request{
Header: http.Header{HeaderForwarded: []string{"for=192.0.2.43, for=198.51.100.17"}},
},
},
&Forwarded{
For: []string{"192.0.2.43", "198.51.100.17"},
},
},
{
&context{
request: &http.Request{
Header: http.Header{HeaderForwarded: []string{"for=192.0.2.43, for=[2001:db8:cafe::17]"}},
},
},
&Forwarded{
For: []string{"192.0.2.43", "2001:db8:cafe::17"},
},
},
}

for _, tt := range tests {
assert.Equal(t, tt.s, tt.c.SchemeForwarded())
}
}

func TestContext_IsWebSocket(t *testing.T) {
tests := []struct {
c Context
Expand Down Expand Up @@ -1062,6 +1106,22 @@ func TestContext_RealIP(t *testing.T) {
},
"127.0.0.1",
},
{
&context{
request: &http.Request{
Header: http.Header{HeaderForwarded: []string{"for=192.0.2.43, for=198.51.100.17"}},
},
},
"192.0.2.43",
},
{
&context{
request: &http.Request{
Header: http.Header{HeaderForwarded: []string{"for=[2001:db8:85a3:8d3:1319:8a2e:370:7348], for=2001:db8::1"}},
},
},
"2001:db8:85a3:8d3:1319:8a2e:370:7348",
},
{
&context{
request: &http.Request{
Expand Down
1 change: 1 addition & 0 deletions echo.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ const (
HeaderUpgrade = "Upgrade"
HeaderVary = "Vary"
HeaderWWWAuthenticate = "WWW-Authenticate"
HeaderForwarded = "Forwarded"
HeaderXForwardedFor = "X-Forwarded-For"
HeaderXForwardedProto = "X-Forwarded-Proto"
HeaderXForwardedProtocol = "X-Forwarded-Protocol"
Expand Down