From cf4fa2fd520fce8bd04ff320103bf2e2786c3401 Mon Sep 17 00:00:00 2001 From: Tony Arnold Date: Wed, 4 Sep 2019 14:24:09 +1000 Subject: [PATCH 1/4] =?UTF-8?q?Allow=20Abort.redirect(=E2=80=A6)=20to=20pa?= =?UTF-8?q?ss=20through=20the=20middleware?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../LeafErrorMiddleware.swift | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/Sources/LeafErrorMiddleware/LeafErrorMiddleware.swift b/Sources/LeafErrorMiddleware/LeafErrorMiddleware.swift index 3989ea6..e3239ba 100644 --- a/Sources/LeafErrorMiddleware/LeafErrorMiddleware.swift +++ b/Sources/LeafErrorMiddleware/LeafErrorMiddleware.swift @@ -22,9 +22,17 @@ public final class LeafErrorMiddleware: Middleware, Service { }.catchFlatMap { error in switch (error) { case let abort as AbortError: - return try self.handleError(for: req, status: abort.status) + guard + abort.status.isRedirect, + let redirectURI = abort.headers[HTTPHeaderName.location.description].first + else { + return try self.handleError(for: req, status: abort.status) + } + + return req.future(req.redirect(to: redirectURI)) + default: - return try self.handleError(for: req, status: .internalServerError) + return try self.handleError(for: req, status: .internalServerError) } } } catch { @@ -82,3 +90,9 @@ extension HTTPStatus { } } } + +private extension HTTPResponseStatus { + var isRedirect: Bool { + return (HTTPResponseStatus.multiStatus.code ... HTTPResponseStatus.permanentRedirect.code) ~= code + } +} From 7b71bcf729e8a54a9434c6d2b52e0b0e5645ca3e Mon Sep 17 00:00:00 2001 From: Tony Arnold Date: Tue, 10 Sep 2019 23:18:56 +1000 Subject: [PATCH 2/4] Add test to verify that redirects are not caught --- .../LeafErrorMiddlewareTests.swift | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Tests/LeafErrorMiddlewareTests/LeafErrorMiddlewareTests.swift b/Tests/LeafErrorMiddlewareTests/LeafErrorMiddlewareTests.swift index 58c6eb6..9a1e93f 100644 --- a/Tests/LeafErrorMiddlewareTests/LeafErrorMiddlewareTests.swift +++ b/Tests/LeafErrorMiddlewareTests/LeafErrorMiddlewareTests.swift @@ -18,6 +18,7 @@ class LeafErrorMiddlewareTests: XCTestCase { ("testThatUnauthorisedIsPassedThroughToServerErrorPage", testThatUnauthorisedIsPassedThroughToServerErrorPage), ("testThatFuture404IsCaughtCorrectly", testThatFuture404IsCaughtCorrectly), ("testThatFuture403IsCaughtCorrectly", testThatFuture403IsCaughtCorrectly), + ("testThatRedirectIsNotCaught", testThatRedirectIsNotCaught) ] // MARK: - Properties @@ -69,6 +70,10 @@ class LeafErrorMiddlewareTests: XCTestCase { router.get("future403") { req -> Future in return req.future(error: Abort(.forbidden)) } + + router.get("future303") { req -> Future in + return req.future(error: Abort.redirect(to: "ok")) + } } let router = EngineRouter.default() @@ -169,6 +174,12 @@ class LeafErrorMiddlewareTests: XCTestCase { XCTAssertEqual(response.http.status, .forbidden) XCTAssertEqual(viewRenderer.leafPath, "serverError") } + + func testThatRedirectIsNotCaught() throws { + let response = try app.getResponse(to: "/future303") + XCTAssertEqual(response.http.status, .seeOther) + XCTAssertEqual(response.http.body.string, "Test") + } } extension HTTPBody { From a98c7c14d4241f74540479663b3c84bf735eee02 Mon Sep 17 00:00:00 2001 From: Tony Arnold Date: Tue, 10 Sep 2019 23:23:59 +1000 Subject: [PATCH 3/4] Check that the status code is an error, rather than a redirect --- Sources/LeafErrorMiddleware/LeafErrorMiddleware.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/LeafErrorMiddleware/LeafErrorMiddleware.swift b/Sources/LeafErrorMiddleware/LeafErrorMiddleware.swift index e3239ba..a52c572 100644 --- a/Sources/LeafErrorMiddleware/LeafErrorMiddleware.swift +++ b/Sources/LeafErrorMiddleware/LeafErrorMiddleware.swift @@ -23,7 +23,7 @@ public final class LeafErrorMiddleware: Middleware, Service { switch (error) { case let abort as AbortError: guard - abort.status.isRedirect, + abort.status.representsError, let redirectURI = abort.headers[HTTPHeaderName.location.description].first else { return try self.handleError(for: req, status: abort.status) @@ -92,7 +92,7 @@ extension HTTPStatus { } private extension HTTPResponseStatus { - var isRedirect: Bool { - return (HTTPResponseStatus.multiStatus.code ... HTTPResponseStatus.permanentRedirect.code) ~= code + var representsError: Bool { + return (HTTPResponseStatus.badRequest.code ... HTTPResponseStatus.networkAuthenticationRequired.code) ~= code } } From f8c2249d252fcc8050d8e9ec41de8bc43db78cc8 Mon Sep 17 00:00:00 2001 From: Tim <0xtimc@gmail.com> Date: Wed, 11 Sep 2019 12:23:59 +0100 Subject: [PATCH 4/4] Fix Abort redirects --- .../LeafErrorMiddleware/LeafErrorMiddleware.swift | 13 +++++++------ .../LeafErrorMiddlewareTests.swift | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Sources/LeafErrorMiddleware/LeafErrorMiddleware.swift b/Sources/LeafErrorMiddleware/LeafErrorMiddleware.swift index a52c572..d3957ec 100644 --- a/Sources/LeafErrorMiddleware/LeafErrorMiddleware.swift +++ b/Sources/LeafErrorMiddleware/LeafErrorMiddleware.swift @@ -23,14 +23,15 @@ public final class LeafErrorMiddleware: Middleware, Service { switch (error) { case let abort as AbortError: guard - abort.status.representsError, - let redirectURI = abort.headers[HTTPHeaderName.location.description].first + abort.status.representsError else { - return try self.handleError(for: req, status: abort.status) + if let location = abort.headers[.location].first { + return req.future(req.redirect(to: location)) + } else { + return try self.handleError(for: req, status: abort.status) + } } - - return req.future(req.redirect(to: redirectURI)) - + return try self.handleError(for: req, status: abort.status) default: return try self.handleError(for: req, status: .internalServerError) } diff --git a/Tests/LeafErrorMiddlewareTests/LeafErrorMiddlewareTests.swift b/Tests/LeafErrorMiddlewareTests/LeafErrorMiddlewareTests.swift index 9a1e93f..5f44a2e 100644 --- a/Tests/LeafErrorMiddlewareTests/LeafErrorMiddlewareTests.swift +++ b/Tests/LeafErrorMiddlewareTests/LeafErrorMiddlewareTests.swift @@ -178,7 +178,7 @@ class LeafErrorMiddlewareTests: XCTestCase { func testThatRedirectIsNotCaught() throws { let response = try app.getResponse(to: "/future303") XCTAssertEqual(response.http.status, .seeOther) - XCTAssertEqual(response.http.body.string, "Test") + XCTAssertEqual(response.http.headers[.location].first, "ok") } }