Skip to content

Commit 1856998

Browse files
aryan-p03aryan-p03
committed
updated builder to better handle internal and external links, more logging
1 parent 28ad67f commit 1856998

File tree

2 files changed

+71
-133
lines changed

2 files changed

+71
-133
lines changed

‎links/links.go

+20-15
Original file line numberDiff line numberDiff line change
@@ -20,33 +20,38 @@ func FromHeadersOrDefault(h *http.Header, r *http.Request, defaultURL *url.URL)
2020
"defaultURL": defaultURL.String(),
2121
"url": r.URL.String(),
2222
})
23+
2324
path := h.Get("X-Forwarded-Path-Prefix")
2425

2526
host := h.Get("X-Forwarded-Host")
26-
if host == "" {
27-
if r.Host != "" {
28-
log.Info(r.Context(), "using request host instead of default host", log.Data{
29-
"host": r.Host,
30-
})
31-
defaultURL.Host = r.Host
32-
}
27+
if host == "" || r.Host == "" {
3328
defaultURL = defaultURL.JoinPath(path)
34-
log.Info(r.Context(), "internal request, using default URL", log.Data{
35-
"defaultURL": defaultURL.String(),
29+
log.Info(r.Context(), "X-Forwarded-Host or r.Host is empty, using default URL", log.Data{
30+
"X-Forwarded-Host": host,
31+
"r.Host": r.Host,
3632
})
3733
return &Builder{
3834
URL: defaultURL,
3935
}
4036
}
37+
if !strings.HasPrefix(host, "api") {
38+
log.Info(r.Context(), "X-Forwarded-Host is not an external host, using incoming request host", log.Data{
39+
"X-Forwarded-Host": host,
40+
"r.Host": r.Host,
41+
})
42+
host = r.Host
43+
}
4144

4245
scheme := h.Get("X-Forwarded-Proto")
4346
if scheme == "" {
44-
scheme = "https"
45-
}
46-
47-
port := h.Get("X-Forwarded-Port")
48-
if port != "" {
49-
host += ":" + port
47+
log.Info(r.Context(), "X-Forwarded-Proto is empty, using http or https based on host", log.Data{
48+
"host": host,
49+
})
50+
if !strings.HasPrefix(host, "api") {
51+
scheme = "http"
52+
} else {
53+
scheme = "https"
54+
}
5055
}
5156

5257
url := &url.URL{

‎links/links_test.go

+51-118
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,13 @@ import (
88
. "github.com/smartystreets/goconvey/convey"
99
)
1010

11-
func Test_FromHeadersOrDefault_With_Forwarded_Headers(t *testing.T) {
11+
func Test_FromHeadersOrDefault(t *testing.T) {
1212

1313
Convey("Given a list of test cases", t, func() {
1414
tests := []struct {
1515
defaultURL string
1616
fwdProto string
1717
fwdHost string
18-
fwdPort string
1918
fwdPathPrefix string
2019
want string
2120
}{
@@ -25,143 +24,95 @@ func Test_FromHeadersOrDefault_With_Forwarded_Headers(t *testing.T) {
2524
"",
2625
"",
2726
"",
28-
"",
2927
"http://localhost:8080/",
3028
},
3129
// With all forwarded headers
3230
{
3331
"http://localhost:8080/",
3432
"https",
35-
"forwardedhost",
36-
"9090",
33+
"api.external.host",
3734
"/prefix",
38-
"https://forwardedhost:9090/prefix",
35+
"https://api.external.host/prefix",
3936
},
4037
// With only forwarded proto
4138
{
4239
"http://localhost:8080/",
4340
"https",
4441
"",
4542
"",
46-
"",
4743
"http://localhost:8080/",
4844
},
4945
// With only forwarded host
5046
{
5147
"http://localhost:8080/",
5248
"",
53-
"forwardedhost",
54-
"",
55-
"",
56-
"https://forwardedhost/",
57-
},
58-
// With only forwarded port
59-
{
60-
"http://localhost:8080/",
61-
"",
49+
"api.external.host",
6250
"",
63-
"9090",
64-
"",
65-
"http://localhost:8080/",
51+
"https://api.external.host/",
6652
},
6753
// With only forwarded path prefix
6854
{
6955
"http://localhost:8080/",
7056
"",
7157
"",
72-
"",
7358
"/prefix",
7459
"http://localhost:8080/prefix",
7560
},
7661
// Without all headers except forwarded proto
7762
{
7863
"http://localhost:8080/",
7964
"",
80-
"forwardedhost",
81-
"9090",
65+
"api.external.host",
8266
"/prefix",
83-
"https://forwardedhost:9090/prefix",
67+
"https://api.external.host/prefix",
8468
},
8569
// Without all headers except forwarded host
8670
{
8771
"http://localhost:8080/",
8872
"https",
8973
"",
90-
"9090",
9174
"/prefix",
9275
"http://localhost:8080/prefix",
9376
},
94-
// Without all headers except forwarded port
95-
{
96-
"http://localhost:8080/",
97-
"https",
98-
"forwardedhost",
99-
"",
100-
"/prefix",
101-
"https://forwardedhost/prefix",
102-
},
10377
// Without all headers except forwarded path prefix
10478
{
10579
"http://localhost:8080/",
10680
"https",
107-
"forwardedhost",
108-
"9090",
81+
"api.external.host",
10982
"",
110-
"https://forwardedhost:9090/",
83+
"https://api.external.host/",
11184
},
11285
// With only forwarded proto and host
11386
{
11487
"http://localhost:8080/",
11588
"https",
116-
"forwardedhost",
89+
"api.external.host",
11790
"",
118-
"",
119-
"https://forwardedhost/",
120-
},
121-
// With only forwarded port and host
122-
{
123-
"http://localhost:8080/",
124-
"",
125-
"forwardedhost",
126-
"9090",
127-
"",
128-
"https://forwardedhost:9090/",
91+
"https://api.external.host/",
12992
},
13093
// With only forwarded prefix and host
13194
{
13295
"http://localhost:8080/",
13396
"",
134-
"forwardedhost",
135-
"",
97+
"api.external.host",
13698
"/prefix",
137-
"https://forwardedhost/prefix",
138-
},
139-
// With only forwarded proto and port
140-
{
141-
"http://localhost:8080/",
142-
"https",
143-
"",
144-
"9090",
145-
"",
146-
"http://localhost:8080/",
99+
"https://api.external.host/prefix",
147100
},
148101
// With only forwarded proto and prefix
149102
{
150103
"http://localhost:8080/",
151104
"https",
152105
"",
153-
"",
154106
"/prefix",
155107
"http://localhost:8080/prefix",
156108
},
157-
// With only forwarded port and prefix
109+
// With non-external forwarded host
158110
{
159111
"http://localhost:8080/",
160112
"",
113+
"internalhost",
161114
"",
162-
"9090",
163-
"/prefix",
164-
"http://localhost:8080/prefix",
115+
"http://localhost:8080/",
165116
},
166117
}
167118

@@ -176,16 +127,14 @@ func Test_FromHeadersOrDefault_With_Forwarded_Headers(t *testing.T) {
176127
if tt.fwdHost != "" {
177128
h.Add("X-Forwarded-Host", tt.fwdHost)
178129
}
179-
if tt.fwdPort != "" {
180-
h.Add("X-Forwarded-Port", tt.fwdPort)
181-
}
182130
if tt.fwdPathPrefix != "" {
183131
h.Add("X-Forwarded-Path-Prefix", tt.fwdPathPrefix)
184132
}
185133

186134
du.JoinPath()
187135
r := &http.Request{
188-
URL: &url.URL{Scheme: "http", Host: "example.com"},
136+
URL: &url.URL{},
137+
Host: "localhost:8080",
189138
}
190139
builder := FromHeadersOrDefault(&h, r, du)
191140
So(builder, ShouldNotBeNil)
@@ -196,64 +145,48 @@ func Test_FromHeadersOrDefault_With_Forwarded_Headers(t *testing.T) {
196145

197146
})
198147

199-
}
200-
201-
func Test_FromHeadersOrDefault_Without_Forwarded_Headers(t *testing.T) {
202-
203-
Convey("Given a list of test cases", t, func() {
204-
tests := []struct {
205-
incomingRequestHost string
206-
defaultURL string
207-
want string
208-
}{
209-
// Without incoming request host
210-
{
211-
"",
212-
"http://localhost:8080/",
213-
"http://localhost:8080/",
214-
},
215-
// With incoming request host and no port
216-
{
217-
"localhost",
218-
"http://localhost:8080/",
219-
"http://localhost/",
220-
},
221-
// With incoming request host and different port
222-
{
223-
"localhost:6789",
224-
"http://localhost:8080/",
225-
"http://localhost:6789/",
226-
},
227-
{
228-
"10.30.100.123:4567",
229-
"http://localhost:8080/",
230-
"http://10.30.100.123:4567/",
231-
},
232-
// With incoming request host and default URL with path
233-
{
234-
"localhost",
235-
"http://localhost:8080/some/path",
236-
"http://localhost/some/path",
237-
},
148+
Convey("Given an empty incoming request host", t, func() {
149+
r := &http.Request{
150+
URL: &url.URL{},
151+
Host: "",
238152
}
239153

240-
for _, tt := range tests {
241-
du, err := url.Parse(tt.defaultURL)
154+
Convey("When the builder is created with forwarded headers", func() {
155+
h := &http.Header{}
156+
h.Add("X-Forwarded-Proto", "https")
157+
h.Add("X-Forwarded-Host", "api.newhost")
158+
h.Add("X-Forwarded-Path-Prefix", "v1")
159+
160+
defaultURL, err := url.Parse("http://localhost:8080/")
242161
So(err, ShouldBeNil)
243162

244-
incomingRequest := &http.Request{
245-
Host: tt.incomingRequestHost,
246-
URL: &url.URL{Scheme: "http", Host: "example.com"},
247-
}
163+
builder := FromHeadersOrDefault(h, r, defaultURL)
248164

249-
builder := FromHeadersOrDefault(&http.Header{}, incomingRequest, du)
250165
So(builder, ShouldNotBeNil)
251166
So(builder.URL, ShouldNotBeNil)
252-
So(builder.URL.String(), ShouldEqual, tt.want)
253167

254-
}
168+
Convey("Then the builder URL should be the default URL with the path prefix", func() {
169+
So(builder.URL.String(), ShouldEqual, "http://localhost:8080/v1")
170+
})
171+
})
172+
173+
Convey("When the builder is created without forwarded headers", func() {
174+
h := &http.Header{}
175+
176+
defaultURL, err := url.Parse("http://localhost:8080/")
177+
So(err, ShouldBeNil)
178+
179+
builder := FromHeadersOrDefault(h, r, defaultURL)
255180

181+
So(builder, ShouldNotBeNil)
182+
So(builder.URL, ShouldNotBeNil)
183+
184+
Convey("Then the builder URL should be the default URL", func() {
185+
So(builder.URL.String(), ShouldEqual, "http://localhost:8080/")
186+
})
187+
})
256188
})
189+
257190
}
258191

259192
func TestBuilder_BuildLink(t *testing.T) {
@@ -349,7 +282,7 @@ func TestBuilder_BuildLink(t *testing.T) {
349282
})
350283

351284
Convey("When an invalid old URL is provided", t, func() {
352-
builder := &Builder{URL: &url.URL{Scheme: "http", Host: "localhost:8080"}}
285+
builder := &Builder{URL: &url.URL{}}
353286
invalidURL := ":invalid/url"
354287
newurl, err := builder.BuildLink(invalidURL)
355288
So(err, ShouldNotBeNil)

0 commit comments

Comments
 (0)