dom96 / jester

A sinatra-like web framework for Nim.
MIT License
1.57k stars 120 forks source link

redirect with before block should halt #264

Closed iffy closed 4 years ago

iffy commented 4 years ago

Here's my program:

# example.nim
import jester

settings:
  port=Port(5000)
  bindAddr="127.0.0.1"

routes:
  before "/a":
    redirect(uri("/ok"))
  get "/a":
    resp "a\l"
  get "/b":
    redirect(uri("/ok"))
    resp "b\l"

  get "/ok":
    resp "ok\l"

I start that with nim c -r example.nim, then do the following:

$ curl -L http://127.0.0.1:5000/a
a
$ curl -L http://127.0.0.1:5000/b
ok

I would expect to see this instead:

$ curl -L http://127.0.0.1:5000/a
ok
$ curl -L http://127.0.0.1:5000/b
ok

I can make a PR to add a failing test case (and maybe figure out how to fix it) if you agree that it should behave as described above.

iffy commented 4 years ago

Here's a potential fix, which could be made backward compatible or not depending on the default value of halt:

diff --git a/jester.nim b/jester.nim
index d7a7e64..0f24490 100644
--- a/jester.nim
+++ b/jester.nim
@@ -578,8 +578,9 @@ 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
@@ -587,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