disambiguate parameters (#204) (#303)

* disambiguate parameters (#204)

* query and form parameters don't call next, add tests

* fix comments and add test to clarify behavior of captureParam

* update haddocks

---------

Co-authored-by: Marco Zocca <[email protected]>

Web/Scotty.hs

@@ -17,7 +17,11 @@ module Web.Scotty
       -- ** Route Patterns
     , capture, regex, function, literal
       -- ** Accessing the Request, Captures, and Query Parameters
-    , request, header, headers, body, bodyReader, param, params, jsonData, files
+    , request, header, headers, body, bodyReader
+    , param, params
+    , captureParam, formParam, queryParam
+    , captureParams, formParams, queryParams
+    , jsonData, files
       -- ** Modifying the Response and Redirecting
     , status, addHeader, setHeader, redirect
       -- ** Setting Response Body
@@ -208,10 +212,47 @@ jsonData = Trans.jsonData
 --   capture cannot be parsed.
 param :: Trans.Parsable a => Text -> ActionM a
 param = Trans.param
+{-# DEPRECATED param "(#204) Not a good idea to treat all parameters identically. Use captureParam, formParam and queryParam instead. "#-}
+
+-- | Get a capture parameter.
+--
+-- * Raises an exception which can be caught by 'rescue' if parameter is not found. If the exception is not caught, scotty will return a HTTP error code 500 ("Internal Server Error") to the client.
+--
+-- * If the parameter is found, but 'parseParam' fails to parse to the correct type, 'next' is called.
+captureParam :: Trans.Parsable a => Text -> ActionM a
+captureParam = Trans.captureParam
+
+-- | Get a form parameter.
+--
+-- * Raises an exception which can be caught by 'rescue' if parameter is not found. If the exception is not caught, scotty will return a HTTP error code 400 ("Bad Request") to the client.
+--
+-- * This function raises a code 400 also if the parameter is found, but 'parseParam' fails to parse to the correct type.
+formParam :: Trans.Parsable a => Text -> ActionM a
+formParam = Trans.formParam
+
+-- | Get a query parameter.
+--
+-- * Raises an exception which can be caught by 'rescue' if parameter is not found. If the exception is not caught, scotty will return a HTTP error code 400 ("Bad Request") to the client.
+--
+-- * This function raises a code 400 also if the parameter is found, but 'parseParam' fails to parse to the correct type.
+queryParam :: Trans.Parsable a => Text -> ActionM a
+queryParam = Trans.queryParam
 
 -- | Get all parameters from capture, form and query (in that order).
 params :: ActionM [Param]
 params = Trans.params
+{-# DEPRECATED params "(#204) Not a good idea to treat all parameters identically. Use captureParams, formParams and queryParams instead. "#-}
+
+-- | Get capture parameters
+captureParams :: ActionM [Param]
+captureParams = Trans.captureParams
+-- | Get form parameters
+formParams :: ActionM [Param]
+formParams = Trans.formParams
+-- | Get query parameters
+queryParams :: ActionM [Param]
+queryParams = Trans.queryParams
+
 
 -- | Set the HTTP response status. Default is 200.
 status :: Status -> ActionM ()

Web/Scotty/Action.hs

@@ -1,6 +1,7 @@
 {-# LANGUAGE CPP               #-}
 {-# LANGUAGE OverloadedStrings #-}
 {-# LANGUAGE RankNTypes        #-}
+{-# LANGUAGE LambdaCase #-}
 module Web.Scotty.Action
     ( addHeader
     , body
@@ -17,7 +18,13 @@ module Web.Scotty.Action
     , jsonData
     , next
     , param
+    , captureParam
+    , formParam
+    , queryParam
     , params
+    , captureParams
+    , formParams
+    , queryParams
     , raise
     , raiseStatus
     , raw
@@ -226,7 +233,7 @@ jsonData = do
 --
 -- * Raises an exception which can be caught by 'rescue' if parameter is not found.
 --
--- * If parameter is found, but 'read' fails to parse to the correct type, 'next' is called.
+-- * If parameter is found, but 'parseParam' fails to parse to the correct type, 'next' is called.
 --   This means captures are somewhat typed, in that a route won't match if a correctly typed
 --   capture cannot be parsed.
 param :: (Parsable a, ScottyError e, Monad m) => T.Text -> ActionT e m a
@@ -235,10 +242,78 @@ param k = do
     case val of
         Nothing -> raise $ stringError $ "Param: " ++ T.unpack k ++ " not found!"
         Just v  -> either (const next) return $ parseParam v
+{-# DEPRECATED param "(#204) Not a good idea to treat all parameters identically. Use captureParam, formParam and queryParam instead. "#-}
+
+-- | Get a capture parameter.
+--
+-- * Raises an exception which can be caught by 'rescue' if parameter is not found. If the exception is not caught, scotty will return a HTTP error code 500 ("Internal Server Error") to the client.
+--
+-- * If the parameter is found, but 'parseParam' fails to parse to the correct type, 'next' is called.
+captureParam :: (Parsable a, ScottyError e, Monad m) => T.Text -> ActionT e m a
+captureParam = paramWith CaptureParam getCaptureParams status500
+
+-- | Get a form parameter.
+--
+-- * Raises an exception which can be caught by 'rescue' if parameter is not found. If the exception is not caught, scotty will return a HTTP error code 400 ("Bad Request") to the client.
+--
+-- * This function raises a code 400 also if the parameter is found, but 'parseParam' fails to parse to the correct type.
+formParam :: (Parsable a, ScottyError e, Monad m) => T.Text -> ActionT e m a
+formParam = paramWith FormParam getFormParams status400
+
+-- | Get a query parameter.
+--
+-- * Raises an exception which can be caught by 'rescue' if parameter is not found. If the exception is not caught, scotty will return a HTTP error code 400 ("Bad Request") to the client.
+--
+-- * This function raises a code 400 also if the parameter is found, but 'parseParam' fails to parse to the correct type.
+queryParam :: (Parsable a, ScottyError e, Monad m) => T.Text -> ActionT e m a
+queryParam = paramWith QueryParam getQueryParams status400
+
+data ParamType = CaptureParam
+               | FormParam
+               | QueryParam
+instance Show ParamType where
+  show = \case
+    CaptureParam -> "capture"
+    FormParam -> "form"
+    QueryParam -> "query"
+
+paramWith :: (ScottyError e, Monad m, Parsable b) =>
+             ParamType
+          -> (ActionEnv -> [Param])
+          -> Status -- ^ HTTP status to return if parameter is not found
+          -> T.Text -- ^ parameter name
+          -> ActionT e m b
+paramWith ty f err k = do
+    val <- ActionT $ liftM (lookup k . f) ask
+    case val of
+      Nothing -> raiseStatus err $ stringError (unwords [show ty, "parameter:", T.unpack k, "not found!"])
+      Just v ->
+        let handleParseError = \case
+              CaptureParam -> next
+              _ -> raiseStatus err $ stringError (unwords ["Cannot parse", T.unpack v, "as a", show ty, "parameter"])
+        in either (const $ handleParseError ty) return $ parseParam v
 
 -- | Get all parameters from capture, form and query (in that order).
 params :: Monad m => ActionT e m [Param]
-params = ActionT $ liftM getParams ask
+params = paramsWith getParams
+{-# DEPRECATED params "(#204) Not a good idea to treat all parameters identically. Use captureParams, formParams and queryParams instead. "#-}
+
+-- | Get capture parameters
+captureParams :: Monad m => ActionT e m [Param]
+captureParams = paramsWith getCaptureParams
+-- | Get form parameters
+formParams :: Monad m => ActionT e m [Param]
+formParams = paramsWith getFormParams
+-- | Get query parameters
+queryParams :: Monad m => ActionT e m [Param]
+queryParams = paramsWith getQueryParams
+
+paramsWith :: Monad m => (ActionEnv -> a) -> ActionT e m a
+paramsWith f = ActionT (f <$> ask)
+
+{-# DEPRECATED getParams "(#204) Not a good idea to treat all parameters identically" #-}
+getParams :: ActionEnv -> [Param]
+getParams e = getCaptureParams e <> getFormParams e <> getQueryParams e
 
 -- | Minimum implemention: 'parseParam'
 class Parsable a where

Web/Scotty/Internal/Types.hs

@@ -134,7 +134,9 @@ type Param = (Text, Text)
 type File = (Text, FileInfo ByteString)
 
 data ActionEnv = Env { getReq       :: Request
-                     , getParams    :: [Param]
+                     , getCaptureParams :: [Param]
+                     , getFormParams    :: [Param]
+                     , getQueryParams :: [Param]
                      , getBody      :: IO ByteString
                      , getBodyChunk :: IO BS.ByteString
                      , getFiles     :: [File]

Web/Scotty/Route.hs

@@ -196,10 +196,10 @@ mkEnv req captures opts = do
 
     let
         convert (k, v) = (strictByteStringToLazyText k, strictByteStringToLazyText v)
-        parameters =  captures ++ map convert formparams ++ queryparams
+        formparams' = map convert formparams
         queryparams = parseEncodedParams $ rawQueryString req
 
-    return $ Env req parameters bs safeBodyReader [ (strictByteStringToLazyText k, fi) | (k,fi) <- fs ]
+    return $ Env req captures formparams' queryparams bs safeBodyReader [ (strictByteStringToLazyText k, fi) | (k,fi) <- fs ]
 
 parseEncodedParams :: B.ByteString -> [Param]
 parseEncodedParams bs = [ (T.fromStrict k, T.fromStrict $ fromMaybe "" v) | (k,v) <- parseQueryText bs ]

Web/Scotty/Trans.hs

@@ -21,7 +21,11 @@ module Web.Scotty.Trans
       -- ** Route Patterns
     , capture, regex, function, literal
       -- ** Accessing the Request, Captures, and Query Parameters
-    , request, header, headers, body, bodyReader, param, params, jsonData, files
+    , request, header, headers, body, bodyReader
+    , param, params
+    , captureParam, formParam, queryParam
+    , captureParams, formParams, queryParams
+    , jsonData, files
       -- ** Modifying the Response and Redirecting
     , status, addHeader, setHeader, redirect
       -- ** Setting Response Body

changelog.md

@@ -1,5 +1,6 @@
 ## next [????.??.??]
 * Adds a new `nested` handler that allows you to place an entire WAI Application under a Scotty route
+* Disambiguate request parameters (#204). Adjust the `Env` type to have three [Param] fields instead of one, add `captureParam`, `formParam`, `queryParam` and the associated `captureParams`, `formParams`, `queryParams`. Add deprecation notices to `param` and `params`.
 
 ## 0.12.1 [2022.11.17]
 * Fix CPP bug that prevented tests from building on Windows.

examples/basic.hs

@@ -32,7 +32,7 @@ main = scotty 3000 $ do
     -- Using a parameter in the query string. If it has
     -- not been given, a 500 page is generated.
     get "/foo" $ do
-        v <- param "fooparam"
+        v <- captureParam "fooparam"
         html $ mconcat ["<h1>", v, "</h1>"]
 
     -- An uncaught error becomes a 500 page.
@@ -58,7 +58,7 @@ main = scotty 3000 $ do
     -- any string, and capture that value as a parameter.
     -- URL captures take precedence over query string parameters.
     get "/foo/:bar/required" $ do
-        v <- param "bar"
+        v <- captureParam "bar"
         html $ mconcat ["<h1>", v, "</h1>"]
 
     -- Files are streamed directly to the client.
@@ -75,7 +75,7 @@ main = scotty 3000 $ do
         json $ take 20 $ randomRs (1::Int,100) g
 
     get "/ints/:is" $ do
-        is <- param "is"
+        is <- captureParam "is"
         json $ [(1::Int)..10] ++ is
 
     get "/setbody" $ do

examples/cookies.hs

@@ -33,7 +33,7 @@ main = scotty 3000 $ do
                 H.input H.! type_ "submit" H.! value "set a cookie"
 
     post "/set-a-cookie" $ do
-        name'  <- param "name"
-        value' <- param "value"
+        name'  <- captureParam "name"
+        value' <- captureParam "value"
         setSimpleCookie name' value'
         redirect "/"

examples/exceptions.hs

@@ -53,7 +53,7 @@ main = scottyT 3000 id $ do -- note, we aren't using any additional transformer
                        ]
 
     get "/switch/:val" $ do
-        v <- param "val"
+        v <- captureParam "val"
         _ <- if even v then raise Forbidden else raise (NotFound v)
         text "this will never be reached"
 

examples/urlshortener.hs

@@ -42,17 +42,17 @@ main = do
                         H.input H.! type_ "submit"
 
     post "/shorten" $ do
-        url <- param "url"
+        url <- captureParam "url"
         liftIO $ modifyMVar_ m $ \(i,db) -> return (i+1, M.insert i (T.pack url) db)
         redirect "/list"
 
     -- We have to be careful here, because this route can match pretty much anything.
     -- Thankfully, the type system knows that 'hash' must be an Int, so this route
-    -- only matches if 'read' can successfully parse the hash capture as an Int.
+    -- only matches if 'parseParam' can successfully parse the hash capture as an Int.
     -- Otherwise, the pattern match will fail and Scotty will continue matching
     -- subsequent routes.
     get "/:hash" $ do
-        hash <- param "hash"
+        hash <- captureParam "hash"
         (_,db) <- liftIO $ readMVar m
         case M.lookup hash db of
             Nothing -> raise $ mconcat ["URL hash #", T.pack $ show $ hash, " not found in database!"]

test/Web/ScottySpec.hs

@@ -1,4 +1,4 @@
-{-# LANGUAGE OverloadedStrings, CPP #-}
+{-# LANGUAGE OverloadedStrings, CPP, ScopedTypeVariables #-}
 module Web.ScottySpec (main, spec) where
 
 import           Test.Hspec
@@ -102,23 +102,67 @@ spec = do
       it "has a MonadBaseControl instance" $ do
         get "/" `shouldRespondWith` 200
 
-    withApp (Scotty.get "/dictionary" $ empty <|> param "word1" <|> empty <|> param "word2" >>= text) $
+    withApp (Scotty.get "/dictionary" $ empty <|> queryParam "word1" <|> empty <|> queryParam "word2" >>= text) $
       it "has an Alternative instance" $ do
         get "/dictionary?word1=haskell"   `shouldRespondWith` "haskell"
         get "/dictionary?word2=scotty"    `shouldRespondWith` "scotty"
         get "/dictionary?word1=a&word2=b" `shouldRespondWith` "a"
 
-    describe "param" $ do
-      withApp (Scotty.matchAny "/search" $ param "query" >>= text) $ do
+    describe "captureParam" $ do
+      withApp (
+        do
+          Scotty.matchAny "/search/:q" $ do
+            _ :: Int <- captureParam "q"
+            text "int"
+          Scotty.matchAny "/search/:q" $ do
+            _ :: String <- captureParam "q"
+            text "string"
+              ) $ do
+        it "responds with 200 OK iff at least one route matches at the right type" $ do
+          get "/search/42" `shouldRespondWith` 200 { matchBody = "int" }
+          get "/search/potato" `shouldRespondWith` 200 { matchBody = "string" }
+      withApp (
+        do
+          Scotty.matchAny "/search/:q" $ do
+            v <- captureParam "q"
+            json (v :: Int)
+              ) $ do
+        it "responds with 404 Not Found if no route matches at the right type" $ do
+          get "/search/potato" `shouldRespondWith` 404
+      withApp (
+        do
+          Scotty.matchAny "/search/:q" $ do
+            v <- captureParam "zzz"
+            json (v :: Int)
+              ) $ do
+        it "responds with 500 Server Error if the parameter cannot be found in the capture" $ do
+          get "/search/potato" `shouldRespondWith` 500
+
+    describe "queryParam" $ do
+      withApp (Scotty.matchAny "/search" $ queryParam "query" >>= text) $ do
         it "returns query parameter with given name" $ do
           get "/search?query=haskell" `shouldRespondWith` "haskell"
-
-        context "when used with application/x-www-form-urlencoded data" $ do
-          it "returns POST parameter with given name" $ do
-            request "POST" "/search" [("Content-Type","application/x-www-form-urlencoded")] "query=haskell" `shouldRespondWith` "haskell"
-
-          it "replaces non UTF-8 bytes with Unicode replacement character" $ do
-            request "POST" "/search" [("Content-Type","application/x-www-form-urlencoded")] "query=\xe9" `shouldRespondWith` "\xfffd"
+      withApp (Scotty.matchAny "/search" (do
+                                             v <- queryParam "query"
+                                             json (v :: Int) )) $ do
+        it "responds with 200 OK if the query parameter can be parsed at the right type" $ do
+          get "/search?query=42" `shouldRespondWith` 200
+        it "responds with 400 Bad Request if the query parameter cannot be parsed at the right type" $ do
+          get "/search?query=potato" `shouldRespondWith` 400
+
+    describe "formParam" $ do
+      withApp (Scotty.matchAny "/search" $ formParam "query" >>= text) $ do
+        it "returns form parameter with given name" $ do
+          request "POST" "/search" [("Content-Type","application/x-www-form-urlencoded")] "query=haskell" `shouldRespondWith` "haskell"
+        it "replaces non UTF-8 bytes with Unicode replacement character" $ do
+          request "POST" "/search" [("Content-Type","application/x-www-form-urlencoded")] "query=\xe9" `shouldRespondWith` "\xfffd"
+      withApp (Scotty.matchAny "/search" (do
+                                             v <- formParam "query"
+                                             json (v :: Int))) $ do
+        it "responds with 200 OK if the form parameter can be parsed at the right type" $ do
+          request "POST" "/search" [("Content-Type","application/x-www-form-urlencoded")] "query=42" `shouldRespondWith` 200
+        it "responds with 400 Bad Request if the form parameter cannot be parsed at the right type" $ do
+          request "POST" "/search" [("Content-Type","application/x-www-form-urlencoded")] "query=potato" `shouldRespondWith` 400
 
 
     describe "requestLimit" $ do