From b0db6c114a3876e703e4f1c9ab93ad41439eab31 Mon Sep 17 00:00:00 2001 From: xlanor Date: Tue, 15 Sep 2020 16:39:49 +0800 Subject: [PATCH 1/5] Allows errors:handlers:redirect:config:to to be a relative URL instead of an absolute URL --- .schema/config.schema.json | 7 ++++++- pipeline/errors/error_redirect_test.go | 26 ++++++++++++++++++++++---- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/.schema/config.schema.json b/.schema/config.schema.json index ddc886ae45..db394db58e 100644 --- a/.schema/config.schema.json +++ b/.schema/config.schema.json @@ -248,7 +248,12 @@ "title": "Redirect to", "description": "Set the redirect target. Must be a http/https URL.", "type": "string", - "format": "uri" + "format": "uri-relative", + "examples": [ + "https://my-app.com/dashboard", + "https://my-app.com/dashboard", + "/dashboard" + ] }, "code": { "title": "HTTP Redirect Status Code", diff --git a/pipeline/errors/error_redirect_test.go b/pipeline/errors/error_redirect_test.go index b7ec27c5a4..819d72055b 100644 --- a/pipeline/errors/error_redirect_test.go +++ b/pipeline/errors/error_redirect_test.go @@ -33,7 +33,7 @@ func TestErrorRedirect(t *testing.T) { assert func(t *testing.T, recorder *httptest.ResponseRecorder) }{ { - d: "should redirect with 302", + d: "should redirect with 302 - absolute", givenError: &herodot.ErrNotFound, config: `{"to":"http://test/test"}`, assert: func(t *testing.T, rw *httptest.ResponseRecorder) { @@ -42,12 +42,30 @@ func TestErrorRedirect(t *testing.T) { }, }, { - d: "should redirect with 301", + d: "should redirect with 302 - relative", givenError: &herodot.ErrNotFound, - config: `{"to":"http://test/test","code":301}`, + config: `{"to":"/test"}`, + assert: func(t *testing.T, rw *httptest.ResponseRecorder) { + assert.Equal(t, 302, rw.Code) + assert.Equal(t, "/test", rw.Header().Get("Location")) + }, + }, + { + d: "should redirect with 301 - absolute", + givenError: &herodot.ErrNotFound, + config: `{"to":"/test","code":301}`, assert: func(t *testing.T, rw *httptest.ResponseRecorder) { assert.Equal(t, 301, rw.Code) - assert.Equal(t, "http://test/test", rw.Header().Get("Location")) + assert.Equal(t, "/test", rw.Header().Get("Location")) + }, + }, + { + d: "should redirect with 301 - relative", + givenError: &herodot.ErrNotFound, + config: `{"to":"/test"}`, + assert: func(t *testing.T, rw *httptest.ResponseRecorder) { + assert.Equal(t, 302, rw.Code) + assert.Equal(t, "/test", rw.Header().Get("Location")) }, }, } { From 429af6b4ca889ff33b60d3c49ca62d5261112871 Mon Sep 17 00:00:00 2001 From: xlanor Date: Tue, 15 Sep 2020 16:45:12 +0800 Subject: [PATCH 2/5] Updated desc --- .schema/config.schema.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.schema/config.schema.json b/.schema/config.schema.json index db394db58e..29bd7d5c56 100644 --- a/.schema/config.schema.json +++ b/.schema/config.schema.json @@ -246,9 +246,9 @@ "properties": { "to": { "title": "Redirect to", - "description": "Set the redirect target. Must be a http/https URL.", + "description": "Set the redirect target. Can either be a http/https URL, or a relative URL.", "type": "string", - "format": "uri-relative", + "format": "uri-reference", "examples": [ "https://my-app.com/dashboard", "https://my-app.com/dashboard", From 9b3b9c219a3e9cfc621cae81e32fde69726297a1 Mon Sep 17 00:00:00 2001 From: xlanor Date: Tue, 15 Sep 2020 17:03:04 +0800 Subject: [PATCH 3/5] Fixed 301 relative test code --- pipeline/errors/error_redirect_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pipeline/errors/error_redirect_test.go b/pipeline/errors/error_redirect_test.go index 819d72055b..6a11b7f675 100644 --- a/pipeline/errors/error_redirect_test.go +++ b/pipeline/errors/error_redirect_test.go @@ -62,9 +62,9 @@ func TestErrorRedirect(t *testing.T) { { d: "should redirect with 301 - relative", givenError: &herodot.ErrNotFound, - config: `{"to":"/test"}`, + config: `{"to":"/test", "code":301}`, assert: func(t *testing.T, rw *httptest.ResponseRecorder) { - assert.Equal(t, 302, rw.Code) + assert.Equal(t, 301, rw.Code) assert.Equal(t, "/test", rw.Header().Get("Location")) }, }, From a67bbe014bdde14b7f5bc1cacbbf78510c13cb89 Mon Sep 17 00:00:00 2001 From: xlanor Date: Tue, 15 Sep 2020 17:05:43 +0800 Subject: [PATCH 4/5] Revert accidentally removed absolute test for 301 --- pipeline/errors/error_redirect_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pipeline/errors/error_redirect_test.go b/pipeline/errors/error_redirect_test.go index 6a11b7f675..6a524b93f4 100644 --- a/pipeline/errors/error_redirect_test.go +++ b/pipeline/errors/error_redirect_test.go @@ -53,10 +53,10 @@ func TestErrorRedirect(t *testing.T) { { d: "should redirect with 301 - absolute", givenError: &herodot.ErrNotFound, - config: `{"to":"/test","code":301}`, + config: `{"to":"http://test/test","code":301}`, assert: func(t *testing.T, rw *httptest.ResponseRecorder) { assert.Equal(t, 301, rw.Code) - assert.Equal(t, "/test", rw.Header().Get("Location")) + assert.Equal(t, "http://test/test", rw.Header().Get("Location")) }, }, { From 0e2fca86829c5c9ccc3aebb99e64341f5707ab6a Mon Sep 17 00:00:00 2001 From: xlanor Date: Tue, 15 Sep 2020 17:18:55 +0800 Subject: [PATCH 5/5] Replaced schema example with http, and explicitly specifed both http and https in tests for both 301 and 302 --- .schema/config.schema.json | 2 +- pipeline/errors/error_redirect_test.go | 22 ++++++++++++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/.schema/config.schema.json b/.schema/config.schema.json index 29bd7d5c56..80a9a4377f 100644 --- a/.schema/config.schema.json +++ b/.schema/config.schema.json @@ -250,7 +250,7 @@ "type": "string", "format": "uri-reference", "examples": [ - "https://my-app.com/dashboard", + "http://my-app.com/dashboard", "https://my-app.com/dashboard", "/dashboard" ] diff --git a/pipeline/errors/error_redirect_test.go b/pipeline/errors/error_redirect_test.go index 6a524b93f4..66b1e239a9 100644 --- a/pipeline/errors/error_redirect_test.go +++ b/pipeline/errors/error_redirect_test.go @@ -33,7 +33,7 @@ func TestErrorRedirect(t *testing.T) { assert func(t *testing.T, recorder *httptest.ResponseRecorder) }{ { - d: "should redirect with 302 - absolute", + d: "should redirect with 302 - absolute (HTTP)", givenError: &herodot.ErrNotFound, config: `{"to":"http://test/test"}`, assert: func(t *testing.T, rw *httptest.ResponseRecorder) { @@ -41,6 +41,15 @@ func TestErrorRedirect(t *testing.T) { assert.Equal(t, "http://test/test", rw.Header().Get("Location")) }, }, + { + d: "should redirect with 302 - absolute (HTTPS)", + givenError: &herodot.ErrNotFound, + config: `{"to":"https://test/test"}`, + assert: func(t *testing.T, rw *httptest.ResponseRecorder) { + assert.Equal(t, 302, rw.Code) + assert.Equal(t, "https://test/test", rw.Header().Get("Location")) + }, + }, { d: "should redirect with 302 - relative", givenError: &herodot.ErrNotFound, @@ -51,7 +60,7 @@ func TestErrorRedirect(t *testing.T) { }, }, { - d: "should redirect with 301 - absolute", + d: "should redirect with 301 - absolute (HTTP)", givenError: &herodot.ErrNotFound, config: `{"to":"http://test/test","code":301}`, assert: func(t *testing.T, rw *httptest.ResponseRecorder) { @@ -59,6 +68,15 @@ func TestErrorRedirect(t *testing.T) { assert.Equal(t, "http://test/test", rw.Header().Get("Location")) }, }, + { + d: "should redirect with 301 - absolute (HTTPS)", + givenError: &herodot.ErrNotFound, + config: `{"to":"https://test/test","code":301}`, + assert: func(t *testing.T, rw *httptest.ResponseRecorder) { + assert.Equal(t, 301, rw.Code) + assert.Equal(t, "https://test/test", rw.Header().Get("Location")) + }, + }, { d: "should redirect with 301 - relative", givenError: &herodot.ErrNotFound,