yesodweb / wai

Haskell Web Application Interface
MIT License
834 stars 263 forks source link

wai-extra Network.Wai.Parse doesn't consider quoted multipart form boundary markers. #699

Closed christian-marie closed 6 years ago

christian-marie commented 6 years ago

In trying to use servant-multipart, which uses Network.Wai.Parse for multipart form handling, I found that a .net client (C#, HttpClient) was getting errors from our webservice. Upon digging into the problem, I found the issue to be a double quoted form boundary.

The following patch corrected the issue, but I don't like it as it duplicates the dq function from a where binding later in the file and it wouldn't handle escaped quotes like "\\"" (should some sadistic maniac want to use random data as a separator). It looks like this parser is in need of context.

diff -ur wai-extra-3.0.22.0/Network/Wai/Parse.hs wai-extra-3.0.22.0-dq/Network/
--- wai-extra-3.0.22.0/Network/Wai/Parse.hs 2017-12-20 15:58:21.000000000 +
+++ wai-extra-3.0.22.0-dq/Network/Wai/Parse.hs  2018-07-06 10:36:21.498590701 +
@@ -331,6 +331,9 @@
     semicolon = 59
     equals = 61
     space = 32
+    dq s = if S.length s > 2 && S.head s == 34 && S.last s == 34 -- quote
+                then S.tail $ S.init s
+                else s
     goAttrs front bs
         | S.null bs = front []
         | otherwise =
@@ -339,7 +342,7 @@
     goAttr bs =
         let (k, v') = S.break (== equals) bs
             v = S.drop 1 v'
-         in (strip k, strip v)
+         in (strip k, dq $ strip v)
     strip = S.dropWhile (== space) . fst . S.breakEnd (/= space)

 -- | Parse the body of an HTTP request.
diff -ur wai-extra-3.0.22.0/test/Network/Wai/ParseSpec.hs wai-extra-3.0.22.0-dqc.hs
--- wai-extra-3.0.22.0/test/Network/Wai/ParseSpec.hs    2017-06-01 22:48:32.000
+++ wai-extra-3.0.22.0-dq/test/Network/Wai/ParseSpec.hs 2018-07-06 10:36:21.498
@@ -30,6 +30,7 @@
             [ ("text/plain", "text/plain", [])
             , ("text/plain; charset=UTF-8 ", "text/plain", [("charset", "UTF-8
             , ("text/plain; charset=UTF-8 ; boundary = foo", "text/plain", [("dary", "foo")])
+            , ("text/plain; charset=UTF-8 ; boundary = \"quoted\"", "text/plai ("boundary", "quoted")])
             ]
     it "parseHttpAccept" caseParseHttpAccept
     describe "parseRequestBody" $ do

This issue is basically identical to: https://github.com/fdintino/nginx-upload-module/issues/50

Maybe this is helpful too: https://greenbytes.de/tech/webdav/draft-ietf-httpbis-p1-messaging-16.html#rfc.section.3.2.3

I would have thought this would have affected someone else by now. What does yesod use for multipart form handling? Should servant-multipart be using some other more robust library? Perhaps it is rather rare to use .net multipart form submissions with Haskell web services.

snoyberg commented 6 years ago

Yesod simply uses wai-extra's code, nothing special. I've never run into this myself, but I'm happy to accept a PR that addresses it for everyone. Bonus points for an added test case.