From 8f89f0cbbd213cd650400219f94d61b8f4394abe Mon Sep 17 00:00:00 2001
From: Matt Haggard <haggardii@gmail.com>
Date: Tue, 15 Sep 2020 16:38:49 -0400
Subject: [PATCH] Add halt option to redirect; default to halting (#265)

* Fix #264 - add halt option to redirect; default to halting

* Doc umentation fix

* Add breaking change note to changelog
---
 changelog.markdown |  4 ++++
 jester.nim         | 10 ++++++++--
 tests/alltest.nim  | 16 ++++++++++++++++
 tests/tester.nim   | 16 ++++++++++++++++
 4 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/changelog.markdown b/changelog.markdown
index 060c8e6..b5a4db0 100644
--- a/changelog.markdown
+++ b/changelog.markdown
@@ -1,5 +1,9 @@
 # Jester changelog
 
+## x.x.x - xx/xx/xxxx
+
+- **Breaking change:** By default `redirect` now skips future handlers, including when used in a `before` route.  To retain the old behavior, set the parameter `halt=false` (e.g. `redirect("/somewhere", halt=false)`)
+
 ## 0.4.3 - 12/08/2019
 
 Minor release correcting a few packaging issues and includes some other
diff --git a/jester.nim b/jester.nim
index 536335a..46b32ce 100644
--- a/jester.nim
+++ b/jester.nim
@@ -576,8 +576,11 @@ template resp*(code: HttpCode): typed =
   result.matched = true
   break route
 
-template redirect*(url: string): typed =
+template redirect*(url: string, halt = true): typed =
   ## Redirects to ``url``. Returns from this request handler immediately.
+  ##
+  ## If ``halt`` is true, skips executing future handlers, too.
+  ##
   ## Any set response headers are preserved for this request.
   bind TCActionSend, newHttpHeaders
   result[0] = TCActionSend
@@ -585,7 +588,10 @@ template redirect*(url: string): typed =
   setHeader(result[2], "Location", url)
   result[3] = ""
   result.matched = true
-  break route
+  if halt:
+    break allRoutes
+  else:
+    break route
 
 template pass*(): typed =
   ## Skips this request handler.
diff --git a/tests/alltest.nim b/tests/alltest.nim
index 026615c..2b41f33 100644
--- a/tests/alltest.nim
+++ b/tests/alltest.nim
@@ -45,6 +45,12 @@ routes:
 
   get "/halt":
     resp "<h1>Not halted!</h1>"
+  
+  before re"/halt-before/.*?":
+    halt Http502, "Halted!"
+  
+  get "/halt-before/@something":
+    resp "Should never reach this"
 
   get "/guess/@who":
     if @"who" != "Frank": pass()
@@ -55,6 +61,16 @@ routes:
 
   get "/redirect/@url/?":
     redirect(uri(@"url"))
+  
+  get "/redirect-halt/@url/?":
+    redirect(uri(@"url"))
+    resp "ok"
+  
+  before re"/redirect-before/.*?":
+    redirect(uri("/nowhere"))
+  
+  get "/redirect-before/@url/?":
+    resp "should not get here"
 
   get "/win":
     cond rand(5) < 3
diff --git a/tests/tester.nim b/tests/tester.nim
index 240a2d7..03c5c98 100644
--- a/tests/tester.nim
+++ b/tests/tester.nim
@@ -82,6 +82,11 @@ proc allTest(useStdLib: bool) =
     let resp = waitFor client.get(address & "/foo/halt")
     check resp.status.startsWith("502")
     check (waitFor resp.body) == "I'm sorry, this page has been halted."
+  
+  test "/halt-before":
+    let resp = waitFor client.request(address & "/foo/halt-before/something", HttpGet)
+    let body = waitFor resp.body
+    check body == "Halted!"
 
   test "/guess":
     let resp = waitFor client.get(address & "/foo/guess/foo")
@@ -92,6 +97,17 @@ proc allTest(useStdLib: bool) =
   test "/redirect":
     let resp = waitFor client.request(address & "/foo/redirect/halt", HttpGet)
     check resp.headers["location"] == "http://localhost:5454/foo/halt"
+  
+  test "/redirect-halt":
+    let resp = waitFor client.request(address & "/foo/redirect-halt/halt", HttpGet)
+    check resp.headers["location"] == "http://localhost:5454/foo/halt"
+    check (waitFor resp.body) == ""
+  
+  test "/redirect-before":
+    let resp = waitFor client.request(address & "/foo/redirect-before/anywhere", HttpGet)
+    check resp.headers["location"] == "http://localhost:5454/foo/nowhere"
+    let body = waitFor resp.body
+    check body == ""
 
   test "regex":
     let resp = waitFor client.get(address & "/foo/02.html")