Merge pull request #1516 from guygastineau/captureall

CaptureAll

Author: domenkozar

changelog.d/CaptureAll

@@ -0,0 +1,14 @@
+synopsis: Bugfix - CaptureAll produces [""] for empty paths due to trailing slash.
+prs: #1516
+issues: #1243
+
+description: {
+
+CaptureAll resulted in `[""]` for empty paths due to trailing slash.  Similar
+oddities occurred around these edge cases like `"/"` resulted in `[]` correctly,
+but `"//"` resulted in `["", ""]`.  This patch simply eliminates the first `""`
+in the pathinfo list as taken from the wai response.  This might break user
+code that relies on personal shims to solve the problem, however simply removing their
+workarounds should fix their code as the behavior is now sane.
+
+}

servant-server/src/Servant/Server/Internal/Router.hs

@@ -213,7 +213,10 @@ runRouterEnv fmt router env request respond  =
           -> let request' = request { pathInfo = rest }
              in  runRouterEnv fmt router' (first, env) request' respond
     CaptureAllRouter _ router' ->
-      let segments = pathInfo request
+      let segments = case pathInfo request of
+            -- this case is to handle trailing slashes.
+            ("":xs) -> xs
+            xs -> xs
           request' = request { pathInfo = [] }
       in runRouterEnv fmt router' (segments, env) request' respond
     RawRouter app ->

servant-server/test/Servant/ServerSpec.hs

@@ -282,45 +282,73 @@ captureSpec = do
 -- * captureAllSpec {{{
 ------------------------------------------------------------------------------
 
-type CaptureAllApi = CaptureAll "legs" Integer :> Get '[JSON] Animal
+type CaptureAllApi =  "legs" :> CaptureAll "legs" Integer :> Get '[JSON] Animal
+                 :<|> "arms" :> CaptureAll "arms" String :> Get '[JSON] [String]
 captureAllApi :: Proxy CaptureAllApi
 captureAllApi = Proxy
-captureAllServer :: [Integer] -> Handler Animal
-captureAllServer legs = case sum legs of
-  4 -> return jerry
-  2 -> return tweety
-  0 -> return beholder
-  _ -> throwError err404
+captureAllServer :: Server CaptureAllApi
+captureAllServer = handleLegs :<|> return
+  where
+    handleLegs [] = return beholder
+    handleLegs legs = case sum legs of
+      4 -> return jerry
+      2 -> return tweety
+      _ -> throwError err404
+
+type RootedCaptureAllApi = CaptureAll "xs" String :> Get '[JSON] [String]
 
 captureAllSpec :: Spec
 captureAllSpec = do
+  let getStringList = decode' @[String] . simpleBody
+
   describe "Servant.API.CaptureAll" $ do
     with (return (serve captureAllApi captureAllServer)) $ do
 
       it "can capture a single element of the 'pathInfo'" $ do
-        response <- get "/2"
+        response <- get "/legs/2"
         liftIO $ decode' (simpleBody response) `shouldBe` Just tweety
 
       it "can capture multiple elements of the 'pathInfo'" $ do
-        response <- get "/2/2"
+        response <- get "/legs/2/2"
         liftIO $ decode' (simpleBody response) `shouldBe` Just jerry
 
       it "can capture arbitrarily many elements of the 'pathInfo'" $ do
-        response <- get "/1/1/0/1/0/1"
+        response <- get "/legs/1/1/0/1/0/1"
         liftIO $ decode' (simpleBody response) `shouldBe` Just jerry
 
       it "can capture when there are no elements in 'pathInfo'" $ do
-        response <- get "/"
+        response <- get "/legs/"
         liftIO $ decode' (simpleBody response) `shouldBe` Just beholder
 
       it "returns 400 if the decoding fails" $ do
-        get "/notAnInt" `shouldRespondWith` 400
+        get "/legs/notAnInt" `shouldRespondWith` 400
 
       it "returns 400 if the decoding fails, regardless of which element" $ do
-        get "/1/0/0/notAnInt/3/" `shouldRespondWith` 400
+        get "/legs/1/0/0/notAnInt/3/" `shouldRespondWith` 400
 
       it "returns 400 if the decoding fails, even when it's multiple elements" $ do
-        get "/1/0/0/notAnInt/3/orange/" `shouldRespondWith` 400
+        get "/legs/1/0/0/notAnInt/3/orange/" `shouldRespondWith` 400
+
+      it "can capture single String" $ do
+        response <- get "/arms/jerry"
+        liftIO $ getStringList response `shouldBe` Just ["jerry"]
+
+      it "can capture when there are no elements in 'pathinfo'" $ do
+        response <- get "/arms/"
+        liftIO $ getStringList response `shouldBe` Just []
+
+      it "can capture empty string from captureall" $ do
+        response <- get "/arms//"
+        liftIO $ getStringList response `shouldBe` Just [""]
+
+    with (return (serve (Proxy :: Proxy RootedCaptureAllApi) return)) $ do
+      it "can capture empty rooted capture all" $ do
+        response <- get "/"
+        liftIO $ getStringList response `shouldBe` Just []
+
+      it "can capture empty string from rooted capture all" $ do
+        response <- get "//"
+        liftIO $ getStringList response `shouldBe` Just [""]
 
     with (return (serve
         (Proxy :: Proxy (CaptureAll "segments" String :> Raw))