git.iko.soy/typeable/oauth2-proxy
on master help sign in
Ensure upstreams are sorted by longest first
commit075cb9c3a0411ad56110766ee869e68e4b67d6b5 authorJoel Speed <joel.speed@hotmail.co.uk> dateSun 7 Mar 2021, 14:51 (6 years ago) parent8a06779
2 files changed+371−220
pkg/upstream/proxy.go+32−1 pkg/upstream/proxy_test.go+339−219
pkg/upstream/proxy.go+32−1
@@ -6,6 +6,7 @@ import (
66 "net/http"
77 "net/url"
88 "regexp"
9+ "sort"
910 "strings"
1011
1112 "github.com/gorilla/mux"
@@ -26,7 +27,7 @@ func NewProxy(upstreams options.Upstreams, sigData *options.SignatureData, write
2627 serveMux: mux.NewRouter(),
2728 }
2829
29− for _, upstream := range upstreams {
30+ for _, upstream := range sortByPathLongest(upstreams) {
3031 if upstream.Static {
3132 if err := m.registerStaticResponseHandler(upstream, writer); err != nil {
3233 return nil, fmt.Errorf("could not register static upstream %q: %v", upstream.ID, err)
@@ -145,3 +146,33 @@ func registerTrailingSlashHandler(serveMux *mux.Router) {
145146 http.Redirect(rw, req, req.URL.String()+"/", http.StatusMovedPermanently)
146147 }))
147148}
149+
150+// sortByPathLongest ensures that the upstreams are sorted by longest path.
151+// If rewrites are involved, a rewrite takes precedence over a non-rewrite.
152+// When two upstreams define rewrites, whichever has the longest path will take
153+// precedence (note this is the input to the rewrite logic).
154+// This does not account for when a rewrite would actually make the path shorter.
155+// This should maintain the sorting behaviour of the standard go serve mux.
156+func sortByPathLongest(in options.Upstreams) options.Upstreams {
157+ sort.Slice(in, func(i, j int) bool {
158+ iRW := in[i].RewriteTarget
159+ jRW := in[j].RewriteTarget
160+
161+ switch {
162+ case iRW != "" && jRW != "":
163+ // If both have a rewrite target, whichever has the longest pattern
164+ // should go first
165+ return len(in[i].Path) > len(in[j].Path)
166+ case iRW != "" && jRW == "":
167+ // Only one has rewrite, it goes first
168+ return true
169+ case iRW == "" && jRW != "":
170+ // Only one has rewrite, it goes first
171+ return false
172+ default:
173+ // Default to longest Path wins
174+ return len(in[i].Path) > len(in[j].Path)
175+ }
176+ })
177+ return in
178+}
pkg/upstream/proxy_test.go+339−219
@@ -18,249 +18,369 @@ import (
1818var _ = Describe("Proxy Suite", func() {
1919 var upstreamServer http.Handler
2020
21− BeforeEach(func() {
22− sigData := &options.SignatureData{Hash: crypto.SHA256, Key: "secret"}
21+ Context("multiUpstreamProxy", func() {
22+ BeforeEach(func() {
23+ sigData := &options.SignatureData{Hash: crypto.SHA256, Key: "secret"}
2324
24− writer := &pagewriter.WriterFuncs{
25− ProxyErrorFunc: func(rw http.ResponseWriter, _ *http.Request, _ error) {
26− rw.WriteHeader(502)
27− rw.Write([]byte("Proxy Error"))
28− },
29− }
25+ writer := &pagewriter.WriterFuncs{
26+ ProxyErrorFunc: func(rw http.ResponseWriter, _ *http.Request, _ error) {
27+ rw.WriteHeader(502)
28+ rw.Write([]byte("Proxy Error"))
29+ },
30+ }
3031
31− ok := http.StatusOK
32− accepted := http.StatusAccepted
32+ ok := http.StatusOK
33+ accepted := http.StatusAccepted
3334
34− upstreams := options.Upstreams{
35− {
36− ID: "http-backend",
37− Path: "/http/",
38− URI: serverAddr,
39− },
40− {
41− ID: "file-backend",
42− Path: "/files/",
43− URI: fmt.Sprintf("file:///%s", filesDir),
44− },
45− {
46− ID: "static-backend",
47− Path: "/static/",
48− Static: true,
49− StaticCode: &ok,
50− },
51− {
52− ID: "static-backend-no-trailing-slash",
53− Path: "/static",
54− Static: true,
55− StaticCode: &accepted,
56− },
57− {
58− ID: "bad-http-backend",
59− Path: "/bad-http/",
60− URI: "http://::1",
61− },
62− {
63− ID: "single-path-backend",
64− Path: "/single-path",
65− Static: true,
66− StaticCode: &ok,
67− },
68− {
69− ID: "backend-with-rewrite-prefix",
70− Path: "^/rewrite-prefix/(.*)",
71− RewriteTarget: "/different/backend/path/$1",
72− URI: serverAddr,
73− },
74− }
35+ upstreams := options.Upstreams{
36+ {
37+ ID: "http-backend",
38+ Path: "/http/",
39+ URI: serverAddr,
40+ },
41+ {
42+ ID: "file-backend",
43+ Path: "/files/",
44+ URI: fmt.Sprintf("file:///%s", filesDir),
45+ },
46+ {
47+ ID: "static-backend",
48+ Path: "/static/",
49+ Static: true,
50+ StaticCode: &ok,
51+ },
52+ {
53+ ID: "static-backend-no-trailing-slash",
54+ Path: "/static",
55+ Static: true,
56+ StaticCode: &accepted,
57+ },
58+ {
59+ ID: "static-backend-long",
60+ Path: "/static/long",
61+ Static: true,
62+ StaticCode: &accepted,
63+ },
64+ {
65+ ID: "bad-http-backend",
66+ Path: "/bad-http/",
67+ URI: "http://::1",
68+ },
69+ {
70+ ID: "single-path-backend",
71+ Path: "/single-path",
72+ Static: true,
73+ StaticCode: &ok,
74+ },
75+ {
76+ ID: "backend-with-rewrite-prefix",
77+ Path: "^/rewrite-prefix/(.*)",
78+ RewriteTarget: "/different/backend/path/$1",
79+ URI: serverAddr,
80+ },
81+ {
82+ ID: "double-match-plain",
83+ Path: "/double-match/",
84+ URI: serverAddr,
85+ },
86+ {
87+ ID: "double-match-rewrite",
88+ Path: "^/double-match/(.*)",
89+ RewriteTarget: "/double-match/rewrite/$1",
90+ URI: serverAddr,
91+ },
92+ }
7593
76− var err error
77− upstreamServer, err = NewProxy(upstreams, sigData, writer)
78− Expect(err).ToNot(HaveOccurred())
79− })
94+ var err error
95+ upstreamServer, err = NewProxy(upstreams, sigData, writer)
96+ Expect(err).ToNot(HaveOccurred())
97+ })
8098
81− type proxyTableInput struct {
82− target string
83− response testHTTPResponse
84− upstream string
85− }
99+ type proxyTableInput struct {
100+ target string
101+ response testHTTPResponse
102+ upstream string
103+ }
86104
87− DescribeTable("Proxy ServeHTTP",
88− func(in *proxyTableInput) {
89− req := middlewareapi.AddRequestScope(
90− httptest.NewRequest("", in.target, nil),
91− &middlewareapi.RequestScope{},
92− )
93− rw := httptest.NewRecorder()
94− // Don't mock the remote Address
95− req.RemoteAddr = ""
105+ DescribeTable("Proxy ServeHTTP",
106+ func(in *proxyTableInput) {
107+ req := middlewareapi.AddRequestScope(
108+ httptest.NewRequest("", in.target, nil),
109+ &middlewareapi.RequestScope{},
110+ )
111+ rw := httptest.NewRecorder()
112+ // Don't mock the remote Address
113+ req.RemoteAddr = ""
96114
97− upstreamServer.ServeHTTP(rw, req)
115+ upstreamServer.ServeHTTP(rw, req)
98116
99− scope := middlewareapi.GetRequestScope(req)
100− Expect(scope.Upstream).To(Equal(in.upstream))
117+ scope := middlewareapi.GetRequestScope(req)
118+ Expect(scope.Upstream).To(Equal(in.upstream))
101119
102− Expect(rw.Code).To(Equal(in.response.code))
120+ Expect(rw.Code).To(Equal(in.response.code))
103121
104− // Delete extra headers that aren't relevant to tests
105− testSanitizeResponseHeader(rw.Header())
106− Expect(rw.Header()).To(Equal(in.response.header))
122+ // Delete extra headers that aren't relevant to tests
123+ testSanitizeResponseHeader(rw.Header())
124+ Expect(rw.Header()).To(Equal(in.response.header))
107125
108− body := rw.Body.Bytes()
109− // If the raw body is set, check that, else check the Request object
110− if in.response.raw != "" {
111− Expect(string(body)).To(Equal(in.response.raw))
112− return
113− }
126+ body := rw.Body.Bytes()
127+ // If the raw body is set, check that, else check the Request object
128+ if in.response.raw != "" {
129+ Expect(string(body)).To(Equal(in.response.raw))
130+ return
131+ }
114132
115− // Compare the reflected request to the upstream
116− request := testHTTPRequest{}
117− Expect(json.Unmarshal(body, &request)).To(Succeed())
118− testSanitizeRequestHeader(request.Header)
119− Expect(request).To(Equal(in.response.request))
120− },
121− Entry("with a request to the HTTP service", &proxyTableInput{
122− target: "http://example.localhost/http/1234",
123− response: testHTTPResponse{
124− code: 200,
125− header: map[string][]string{
126− contentType: {applicationJSON},
133+ // Compare the reflected request to the upstream
134+ request := testHTTPRequest{}
135+ Expect(json.Unmarshal(body, &request)).To(Succeed())
136+ testSanitizeRequestHeader(request.Header)
137+ Expect(request).To(Equal(in.response.request))
138+ },
139+ Entry("with a request to the HTTP service", &proxyTableInput{
140+ target: "http://example.localhost/http/1234",
141+ response: testHTTPResponse{
142+ code: 200,
143+ header: map[string][]string{
144+ contentType: {applicationJSON},
145+ },
146+ request: testHTTPRequest{
147+ Method: "GET",
148+ URL: "http://example.localhost/http/1234",
149+ Header: map[string][]string{
150+ "Gap-Auth": {""},
151+ "Gap-Signature": {"sha256 ofB1u6+FhEUbFLc3/uGbJVkl7GaN4egFqVvyO3+2I1w="},
152+ },
153+ Body: []byte{},
154+ Host: "example.localhost",
155+ RequestURI: "http://example.localhost/http/1234",
156+ },
127157 },
128− request: testHTTPRequest{
129− Method: "GET",
130− URL: "http://example.localhost/http/1234",
131− Header: map[string][]string{
132− "Gap-Auth": {""},
133− "Gap-Signature": {"sha256 ofB1u6+FhEUbFLc3/uGbJVkl7GaN4egFqVvyO3+2I1w="},
158+ upstream: "http-backend",
159+ }),
160+ Entry("with a request to the File backend", &proxyTableInput{
161+ target: "http://example.localhost/files/foo",
162+ response: testHTTPResponse{
163+ code: 200,
164+ header: map[string][]string{
165+ contentType: {textPlainUTF8},
134166 },
135− Body: []byte{},
136− Host: "example.localhost",
137− RequestURI: "http://example.localhost/http/1234",
167+ raw: "foo",
138168 },
139− },
140− upstream: "http-backend",
141− }),
142− Entry("with a request to the File backend", &proxyTableInput{
143− target: "http://example.localhost/files/foo",
144− response: testHTTPResponse{
145− code: 200,
146− header: map[string][]string{
147− contentType: {textPlainUTF8},
169+ upstream: "file-backend",
170+ }),
171+ Entry("with a request to the Static backend", &proxyTableInput{
172+ target: "http://example.localhost/static/bar",
173+ response: testHTTPResponse{
174+ code: 200,
175+ header: map[string][]string{},
176+ raw: "Authenticated",
148177 },
149− raw: "foo",
150− },
151− upstream: "file-backend",
152− }),
153− Entry("with a request to the Static backend", &proxyTableInput{
154− target: "http://example.localhost/static/bar",
155− response: testHTTPResponse{
156− code: 200,
157− header: map[string][]string{},
158− raw: "Authenticated",
159− },
160− upstream: "static-backend",
161− }),
162− Entry("with a request to the bad HTTP backend", &proxyTableInput{
163− target: "http://example.localhost/bad-http/bad",
164− response: testHTTPResponse{
165− code: 502,
166− header: map[string][]string{},
167− // This tests the error handler
168− raw: "Proxy Error",
169− },
170− upstream: "bad-http-backend",
171− }),
172− Entry("with a request to the to an unregistered path", &proxyTableInput{
173− target: "http://example.localhost/unregistered",
174− response: testHTTPResponse{
175− code: 404,
176− header: map[string][]string{
177− "X-Content-Type-Options": {"nosniff"},
178− contentType: {textPlainUTF8},
178+ upstream: "static-backend",
179+ }),
180+ Entry("with a request to the bad HTTP backend", &proxyTableInput{
181+ target: "http://example.localhost/bad-http/bad",
182+ response: testHTTPResponse{
183+ code: 502,
184+ header: map[string][]string{},
185+ // This tests the error handler
186+ raw: "Proxy Error",
179187 },
180− raw: "404 page not found\n",
181− },
182− }),
183− Entry("with a request to the to backend registered to a single path", &proxyTableInput{
184− target: "http://example.localhost/single-path",
185− response: testHTTPResponse{
186− code: 200,
187− header: map[string][]string{},
188− raw: "Authenticated",
189− },
190− upstream: "single-path-backend",
191− }),
192− Entry("with a request to the to a subpath of a backend registered to a single path", &proxyTableInput{
193− target: "http://example.localhost/single-path/unregistered",
194− response: testHTTPResponse{
195− code: 404,
196− header: map[string][]string{
197− "X-Content-Type-Options": {"nosniff"},
198− contentType: {textPlainUTF8},
188+ upstream: "bad-http-backend",
189+ }),
190+ Entry("with a request to the to an unregistered path", &proxyTableInput{
191+ target: "http://example.localhost/unregistered",
192+ response: testHTTPResponse{
193+ code: 404,
194+ header: map[string][]string{
195+ "X-Content-Type-Options": {"nosniff"},
196+ contentType: {textPlainUTF8},
197+ },
198+ raw: "404 page not found\n",
199199 },
200− raw: "404 page not found\n",
201− },
202− }),
203− Entry("with a request to the rewrite prefix server", &proxyTableInput{
204− target: "http://example.localhost/rewrite-prefix/1234",
205− response: testHTTPResponse{
206− code: 200,
207− header: map[string][]string{
208− contentType: {applicationJSON},
200+ }),
201+ Entry("with a request to the to backend registered to a single path", &proxyTableInput{
202+ target: "http://example.localhost/single-path",
203+ response: testHTTPResponse{
204+ code: 200,
205+ header: map[string][]string{},
206+ raw: "Authenticated",
209207 },
210− request: testHTTPRequest{
211− Method: "GET",
212− URL: "http://example.localhost/different/backend/path/1234",
213− Header: map[string][]string{
214− "Gap-Auth": {""},
215− "Gap-Signature": {"sha256 jeAeM7wHSj2ab/l9YPvtTJ9l/8q1tpY2V/iwXF48bgw="},
208+ upstream: "single-path-backend",
209+ }),
210+ Entry("with a request to the to a subpath of a backend registered to a single path", &proxyTableInput{
211+ target: "http://example.localhost/single-path/unregistered",
212+ response: testHTTPResponse{
213+ code: 404,
214+ header: map[string][]string{
215+ "X-Content-Type-Options": {"nosniff"},
216+ contentType: {textPlainUTF8},
216217 },
217− Body: []byte{},
218− Host: "example.localhost",
219− RequestURI: "http://example.localhost/different/backend/path/1234",
218+ raw: "404 page not found\n",
220219 },
221− },
222− upstream: "backend-with-rewrite-prefix",
223− }),
224− Entry("with a request to a subpath of the rewrite prefix server", &proxyTableInput{
225− target: "http://example.localhost/rewrite-prefix/1234/abc",
226− response: testHTTPResponse{
227− code: 200,
228− header: map[string][]string{
229− contentType: {applicationJSON},
220+ }),
221+ Entry("with a request to the rewrite prefix server", &proxyTableInput{
222+ target: "http://example.localhost/rewrite-prefix/1234",
223+ response: testHTTPResponse{
224+ code: 200,
225+ header: map[string][]string{
226+ contentType: {applicationJSON},
227+ },
228+ request: testHTTPRequest{
229+ Method: "GET",
230+ URL: "http://example.localhost/different/backend/path/1234",
231+ Header: map[string][]string{
232+ "Gap-Auth": {""},
233+ "Gap-Signature": {"sha256 jeAeM7wHSj2ab/l9YPvtTJ9l/8q1tpY2V/iwXF48bgw="},
234+ },
235+ Body: []byte{},
236+ Host: "example.localhost",
237+ RequestURI: "http://example.localhost/different/backend/path/1234",
238+ },
230239 },
231− request: testHTTPRequest{
232− Method: "GET",
233− URL: "http://example.localhost/different/backend/path/1234/abc",
234− Header: map[string][]string{
235− "Gap-Auth": {""},
236− "Gap-Signature": {"sha256 rAkAc9gp7EndoOppJuvbuPnYuBcqrTkBnQx6iPS8xTA="},
240+ upstream: "backend-with-rewrite-prefix",
241+ }),
242+ Entry("with a request to a subpath of the rewrite prefix server", &proxyTableInput{
243+ target: "http://example.localhost/rewrite-prefix/1234/abc",
244+ response: testHTTPResponse{
245+ code: 200,
246+ header: map[string][]string{
247+ contentType: {applicationJSON},
248+ },
249+ request: testHTTPRequest{
250+ Method: "GET",
251+ URL: "http://example.localhost/different/backend/path/1234/abc",
252+ Header: map[string][]string{
253+ "Gap-Auth": {""},
254+ "Gap-Signature": {"sha256 rAkAc9gp7EndoOppJuvbuPnYuBcqrTkBnQx6iPS8xTA="},
255+ },
256+ Body: []byte{},
257+ Host: "example.localhost",
258+ RequestURI: "http://example.localhost/different/backend/path/1234/abc",
237259 },
238− Body: []byte{},
239− Host: "example.localhost",
240− RequestURI: "http://example.localhost/different/backend/path/1234/abc",
241260 },
242− },
243− upstream: "backend-with-rewrite-prefix",
244− }),
245− Entry("with a request to a path, missing the trailing slash", &proxyTableInput{
246− target: "http://example.localhost/http",
247− response: testHTTPResponse{
248− code: 301,
249− header: map[string][]string{
250− contentType: {textHTMLUTF8},
251− "Location": {"http://example.localhost/http/"},
261+ upstream: "backend-with-rewrite-prefix",
262+ }),
263+ Entry("with a request to a path, missing the trailing slash", &proxyTableInput{
264+ target: "http://example.localhost/http",
265+ response: testHTTPResponse{
266+ code: 301,
267+ header: map[string][]string{
268+ contentType: {textHTMLUTF8},
269+ "Location": {"http://example.localhost/http/"},
270+ },
271+ raw: "<a href=\"http://example.localhost/http/\">Moved Permanently</a>.\n\n",
252272 },
253− raw: "<a href=\"http://example.localhost/http/\">Moved Permanently</a>.\n\n",
254− },
255− }),
256− Entry("with a request to a path, missing the trailing slash, but registered separately", &proxyTableInput{
257− target: "http://example.localhost/static",
258− response: testHTTPResponse{
259− code: 202,
260− header: map[string][]string{},
261− raw: "Authenticated",
273+ }),
274+ Entry("with a request to a path, missing the trailing slash, but registered separately", &proxyTableInput{
275+ target: "http://example.localhost/static",
276+ response: testHTTPResponse{
277+ code: 202,
278+ header: map[string][]string{},
279+ raw: "Authenticated",
280+ },
281+ upstream: "static-backend-no-trailing-slash",
282+ }),
283+ Entry("should match longest path first", &proxyTableInput{
284+ target: "http://example.localhost/static/long",
285+ response: testHTTPResponse{
286+ code: 202,
287+ header: map[string][]string{},
288+ raw: "Authenticated",
289+ },
290+ upstream: "static-backend-long",
291+ }),
292+ Entry("should match rewrite path first", &proxyTableInput{
293+ target: "http://example.localhost/double-match/foo",
294+ response: testHTTPResponse{
295+ code: 200,
296+ header: map[string][]string{
297+ contentType: {applicationJSON},
298+ },
299+ request: testHTTPRequest{
300+ Method: "GET",
301+ URL: "http://example.localhost/double-match/rewrite/foo",
302+ Header: map[string][]string{
303+ "Gap-Auth": {""},
304+ "Gap-Signature": {"sha256 eYyUNdsrTmnvFpavpP8AdHGUGzqJ39QEjqn0/3fQPHA="},
305+ },
306+ Body: []byte{},
307+ Host: "example.localhost",
308+ RequestURI: "http://example.localhost/double-match/rewrite/foo",
309+ },
310+ },
311+ upstream: "double-match-rewrite",
312+ }),
313+ )
314+ })
315+
316+ Context("sortByPathLongest", func() {
317+ type sortByPathLongestTableInput struct {
318+ input options.Upstreams
319+ expectedOutput options.Upstreams
320+ }
321+
322+ var httpPath = options.Upstream{
323+ Path: "/http/",
324+ }
325+
326+ var httpSubPath = options.Upstream{
327+ Path: "/http/subpath/",
328+ }
329+
330+ var longerPath = options.Upstream{
331+ Path: "/longer-than-http",
332+ }
333+
334+ var shortPathWithRewrite = options.Upstream{
335+ Path: "^/h/(.*)",
336+ RewriteTarget: "/$1",
337+ }
338+
339+ var shortSubPathWithRewrite = options.Upstream{
340+ Path: "^/h/bar/(.*)",
341+ RewriteTarget: "/$1",
342+ }
343+
344+ DescribeTable("short sort into the correct order",
345+ func(in sortByPathLongestTableInput) {
346+ Expect(sortByPathLongest(in.input)).To(Equal(in.expectedOutput))
262347 },
263− upstream: "static-backend-no-trailing-slash",
264− }),
265− )
348+ Entry("with a mix of paths registered", sortByPathLongestTableInput{
349+ input: options.Upstreams{httpPath, httpSubPath, shortSubPathWithRewrite, longerPath, shortPathWithRewrite},
350+ expectedOutput: options.Upstreams{shortSubPathWithRewrite, shortPathWithRewrite, longerPath, httpSubPath, httpPath},
351+ }),
352+ Entry("when a subpath is registered (in order)", sortByPathLongestTableInput{
353+ input: options.Upstreams{httpSubPath, httpPath},
354+ expectedOutput: options.Upstreams{httpSubPath, httpPath},
355+ }),
356+ Entry("when a subpath is registered (out of order)", sortByPathLongestTableInput{
357+ input: options.Upstreams{httpPath, httpSubPath},
358+ expectedOutput: options.Upstreams{httpSubPath, httpPath},
359+ }),
360+ Entry("when longer paths are registered (in order)", sortByPathLongestTableInput{
361+ input: options.Upstreams{longerPath, httpPath},
362+ expectedOutput: options.Upstreams{longerPath, httpPath},
363+ }),
364+ Entry("when longer paths are registered (out of order)", sortByPathLongestTableInput{
365+ input: options.Upstreams{httpPath, longerPath},
366+ expectedOutput: options.Upstreams{longerPath, httpPath},
367+ }),
368+ Entry("when a rewrite target is registered (in order)", sortByPathLongestTableInput{
369+ input: options.Upstreams{shortPathWithRewrite, longerPath},
370+ expectedOutput: options.Upstreams{shortPathWithRewrite, longerPath},
371+ }),
372+ Entry("when a rewrite target is registered (out of order)", sortByPathLongestTableInput{
373+ input: options.Upstreams{longerPath, shortPathWithRewrite},
374+ expectedOutput: options.Upstreams{shortPathWithRewrite, longerPath},
375+ }),
376+ Entry("with multiple rewrite targets registered (in order)", sortByPathLongestTableInput{
377+ input: options.Upstreams{shortSubPathWithRewrite, shortPathWithRewrite},
378+ expectedOutput: options.Upstreams{shortSubPathWithRewrite, shortPathWithRewrite},
379+ }),
380+ Entry("with multiple rewrite targets registered (out of order)", sortByPathLongestTableInput{
381+ input: options.Upstreams{shortPathWithRewrite, shortSubPathWithRewrite},
382+ expectedOutput: options.Upstreams{shortSubPathWithRewrite, shortPathWithRewrite},
383+ }),
384+ )
385+ })
266386})