Skip to content

Commit 2da4beb

Browse files
haineshaines
authored
fix: handle X-Forwarded-* headers correctly (#4334)
Signed-off-by: Andrew Haines <[email protected]>
1 parent 9660e4a commit 2da4beb

File tree

2 files changed

+103
-15
lines changed

2 files changed

+103
-15
lines changed

‎runtime/context.go‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,12 @@ func annotateContext(ctx context.Context, mux *ServeMux, req *http.Request, rpcM
148148
var pairs []string
149149
for key, vals := range req.Header {
150150
key = textproto.CanonicalMIMEHeaderKey(key)
151+
switch key {
152+
case xForwardedFor, xForwardedHost:
153+
// Handled separately below
154+
continue
155+
}
156+
151157
for _, val := range vals {
152158
// For backwards-compatibility, pass through 'authorization' header with no prefix.
153159
if key == "Authorization" {
@@ -181,15 +187,15 @@ func annotateContext(ctx context.Context, mux *ServeMux, req *http.Request, rpcM
181187
pairs = append(pairs, strings.ToLower(xForwardedHost), req.Host)
182188
}
183189

190+
xff := req.Header.Values(xForwardedFor)
184191
if addr := req.RemoteAddr; addr != "" {
185192
if remoteIP, _, err := net.SplitHostPort(addr); err == nil {
186-
if fwd := req.Header.Get(xForwardedFor); fwd == "" {
187-
pairs = append(pairs, strings.ToLower(xForwardedFor), remoteIP)
188-
} else {
189-
pairs = append(pairs, strings.ToLower(xForwardedFor), fmt.Sprintf("%s, %s", fwd, remoteIP))
190-
}
193+
xff = append(xff, remoteIP)
191194
}
192195
}
196+
if len(xff) > 0 {
197+
pairs = append(pairs, strings.ToLower(xForwardedFor), strings.Join(xff, ", "))
198+
}
193199

194200
if timeout != 0 {
195201
ctx, _ = context.WithTimeout(ctx, timeout)

‎runtime/context_test.go‎

Lines changed: 92 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -113,17 +113,20 @@ func TestAnnotateContext_ForwardGrpcBinaryMetadata(t *testing.T) {
113113
}
114114
}
115115

116-
func TestAnnotateContext_XForwardedFor(t *testing.T) {
116+
func TestAnnotateContext_AddsXForwardedHeaders(t *testing.T) {
117117
ctx := context.Background()
118118
expectedRPCName := "/example.Example/Example"
119119
request, err := http.NewRequestWithContext(ctx, "GET", "http://bar.foo.example.com", nil)
120120
if err != nil {
121121
t.Fatalf("http.NewRequestWithContext(ctx, %q, %q, nil) failed with %v; want success", "GET", "http://bar.foo.example.com", err)
122122
}
123-
request.Header.Add("X-Forwarded-For", "192.0.2.100") // client
124-
request.RemoteAddr = "192.0.2.200:12345" // proxy
123+
request.RemoteAddr = "192.0.2.100:12345" // client
125124

126-
annotated, err := runtime.AnnotateContext(ctx, runtime.NewServeMux(), request, expectedRPCName)
125+
serveMux := runtime.NewServeMux(runtime.WithIncomingHeaderMatcher(func(key string) (string, bool) {
126+
return key, true
127+
}))
128+
129+
annotated, err := runtime.AnnotateContext(ctx, serveMux, request, expectedRPCName)
127130
if err != nil {
128131
t.Errorf("runtime.AnnotateContext(ctx, %#v) failed with %v; want success", request, err)
129132
return
@@ -135,8 +138,46 @@ func TestAnnotateContext_XForwardedFor(t *testing.T) {
135138
if got, want := md["x-forwarded-host"], []string{"bar.foo.example.com"}; !reflect.DeepEqual(got, want) {
136139
t.Errorf(`md["host"] = %v; want %v`, got, want)
137140
}
141+
if got, want := md["x-forwarded-for"], []string{"192.0.2.100"}; !reflect.DeepEqual(got, want) {
142+
t.Errorf(`md["x-forwarded-for"] = %v want %v`, got, want)
143+
}
144+
if m, ok := runtime.RPCMethod(annotated); !ok {
145+
t.Errorf("runtime.RPCMethod(annotated) failed with no value; want %s", expectedRPCName)
146+
} else if m != expectedRPCName {
147+
t.Errorf("runtime.RPCMethod(annotated) failed with %s; want %s", m, expectedRPCName)
148+
}
149+
}
150+
151+
func TestAnnotateContext_AppendsToExistingXForwardedHeaders(t *testing.T) {
152+
ctx := context.Background()
153+
expectedRPCName := "/example.Example/Example"
154+
request, err := http.NewRequestWithContext(ctx, "GET", "http://bar.foo.example.com", nil)
155+
if err != nil {
156+
t.Fatalf("http.NewRequestWithContext(ctx, %q, %q, nil) failed with %v; want success", "GET", "http://bar.foo.example.com", err)
157+
}
158+
request.Header.Add("X-Forwarded-Host", "qux.example.com")
159+
request.Header.Add("X-Forwarded-For", "192.0.2.100") // client
160+
request.Header.Add("X-Forwarded-For", "192.0.2.101, 192.0.2.102") // intermediate proxies
161+
request.RemoteAddr = "192.0.2.200:12345" // final proxy
162+
163+
serveMux := runtime.NewServeMux(runtime.WithIncomingHeaderMatcher(func(key string) (string, bool) {
164+
return key, true
165+
}))
166+
167+
annotated, err := runtime.AnnotateContext(ctx, serveMux, request, expectedRPCName)
168+
if err != nil {
169+
t.Errorf("runtime.AnnotateContext(ctx, %#v) failed with %v; want success", request, err)
170+
return
171+
}
172+
md, ok := metadata.FromOutgoingContext(annotated)
173+
if !ok || len(md) != emptyForwardMetaCount+1 {
174+
t.Errorf("Expected %d metadata items in context; got %v", emptyForwardMetaCount+1, md)
175+
}
176+
if got, want := md["x-forwarded-host"], []string{"qux.example.com"}; !reflect.DeepEqual(got, want) {
177+
t.Errorf(`md["host"] = %v; want %v`, got, want)
178+
}
138179
// Note: it must be in order client, proxy1, proxy2
139-
if got, want := md["x-forwarded-for"], []string{"192.0.2.100, 192.0.2.200"}; !reflect.DeepEqual(got, want) {
180+
if got, want := md["x-forwarded-for"], []string{"192.0.2.100, 192.0.2.101, 192.0.2.102, 192.0.2.200"}; !reflect.DeepEqual(got, want) {
140181
t.Errorf(`md["x-forwarded-for"] = %v want %v`, got, want)
141182
}
142183
if m, ok := runtime.RPCMethod(annotated); !ok {
@@ -356,17 +397,20 @@ func TestAnnotateIncomingContext_ForwardGrpcBinaryMetadata(t *testing.T) {
356397
}
357398
}
358399

359-
func TestAnnotateIncomingContext_XForwardedFor(t *testing.T) {
400+
func TestAnnotateIncomingContext_AddsXForwardedHeaders(t *testing.T) {
360401
ctx := context.Background()
361402
expectedRPCName := "/example.Example/Example"
362403
request, err := http.NewRequestWithContext(ctx, "GET", "http://bar.foo.example.com", nil)
363404
if err != nil {
364405
t.Fatalf("http.NewRequestWithContext(ctx, %q, %q, nil) failed with %v; want success", "GET", "http://bar.foo.example.com", err)
365406
}
366-
request.Header.Add("X-Forwarded-For", "192.0.2.100") // client
367-
request.RemoteAddr = "192.0.2.200:12345" // proxy
407+
request.RemoteAddr = "192.0.2.100:12345" // client
368408

369-
annotated, err := runtime.AnnotateIncomingContext(ctx, runtime.NewServeMux(), request, expectedRPCName)
409+
serveMux := runtime.NewServeMux(runtime.WithIncomingHeaderMatcher(func(key string) (string, bool) {
410+
return key, true
411+
}))
412+
413+
annotated, err := runtime.AnnotateIncomingContext(ctx, serveMux, request, expectedRPCName)
370414
if err != nil {
371415
t.Errorf("runtime.AnnotateIncomingContext(ctx, %#v) failed with %v; want success", request, err)
372416
return
@@ -378,8 +422,46 @@ func TestAnnotateIncomingContext_XForwardedFor(t *testing.T) {
378422
if got, want := md["x-forwarded-host"], []string{"bar.foo.example.com"}; !reflect.DeepEqual(got, want) {
379423
t.Errorf(`md["host"] = %v; want %v`, got, want)
380424
}
425+
if got, want := md["x-forwarded-for"], []string{"192.0.2.100"}; !reflect.DeepEqual(got, want) {
426+
t.Errorf(`md["x-forwarded-for"] = %v want %v`, got, want)
427+
}
428+
if m, ok := runtime.RPCMethod(annotated); !ok {
429+
t.Errorf("runtime.RPCMethod(annotated) failed with no value; want %s", expectedRPCName)
430+
} else if m != expectedRPCName {
431+
t.Errorf("runtime.RPCMethod(annotated) failed with %s; want %s", m, expectedRPCName)
432+
}
433+
}
434+
435+
func TestAnnotateIncomingContext_AppendsToExistingXForwardedHeaders(t *testing.T) {
436+
ctx := context.Background()
437+
expectedRPCName := "/example.Example/Example"
438+
request, err := http.NewRequestWithContext(ctx, "GET", "http://bar.foo.example.com", nil)
439+
if err != nil {
440+
t.Fatalf("http.NewRequestWithContext(ctx, %q, %q, nil) failed with %v; want success", "GET", "http://bar.foo.example.com", err)
441+
}
442+
request.Header.Add("X-Forwarded-Host", "qux.example.com")
443+
request.Header.Add("X-Forwarded-For", "192.0.2.100") // client
444+
request.Header.Add("X-Forwarded-For", "192.0.2.101, 192.0.2.102") // intermediate proxies
445+
request.RemoteAddr = "192.0.2.200:12345" // final proxy
446+
447+
serveMux := runtime.NewServeMux(runtime.WithIncomingHeaderMatcher(func(key string) (string, bool) {
448+
return key, true
449+
}))
450+
451+
annotated, err := runtime.AnnotateIncomingContext(ctx, serveMux, request, expectedRPCName)
452+
if err != nil {
453+
t.Errorf("runtime.AnnotateIncomingContext(ctx, %#v) failed with %v; want success", request, err)
454+
return
455+
}
456+
md, ok := metadata.FromIncomingContext(annotated)
457+
if !ok || len(md) != emptyForwardMetaCount+1 {
458+
t.Errorf("Expected %d metadata items in context; got %v", emptyForwardMetaCount+1, md)
459+
}
460+
if got, want := md["x-forwarded-host"], []string{"qux.example.com"}; !reflect.DeepEqual(got, want) {
461+
t.Errorf(`md["host"] = %v; want %v`, got, want)
462+
}
381463
// Note: it must be in order client, proxy1, proxy2
382-
if got, want := md["x-forwarded-for"], []string{"192.0.2.100, 192.0.2.200"}; !reflect.DeepEqual(got, want) {
464+
if got, want := md["x-forwarded-for"], []string{"192.0.2.100, 192.0.2.101, 192.0.2.102, 192.0.2.200"}; !reflect.DeepEqual(got, want) {
383465
t.Errorf(`md["x-forwarded-for"] = %v want %v`, got, want)
384466
}
385467
if m, ok := runtime.RPCMethod(annotated); !ok {

0 commit comments

Comments
 (0)