Files limit v2 #203

[ocramz]

second attempt at #203

Summary: form bodies (both URL-encoded and Multipart) are only parsed if/when needed by the files/filesOpts/formParam(s) `function.

Previously, Web.Scotty.Route.mkEnv used to parse request bodies to extract form parameters and files. Now we just use it to configure a body parsing function. A the usage site, ActionT combinators that need to access files or form parameters do the actual traversal/parsing work. This also means that the size of the request body is not known until files, formParams etc. is called.

• files parses the form body and produce temp files, which are then read back into memory to preserve backward API compatibility.
• there is a new function filesOpts that offers a continuation for lexical scoping of the temp files, as well as options to limit request size parameters (number and size of files etc.). I think users should be nudged towards using this one

BREAKING CHANGES:

• some ActionT API functions have now a MonadIO or MonadUnliftIO constraint rather than Monad reflecting that there is where request reading takes place.
• the File type has now a type parameter to reflect whether it carries file contents or just a filepath pointing to the temp file.

NEW MODULE:

• Web/Scotty/Internal/WaiParseSafe.hs , which will be removed once https://github.com/yesodweb/wai/pull/964 gets merged and published upstream now using wai-extra-3.1.14 since https://github.com/yesodweb/wai/pull/972

TODO:

• [x] handler for exception types thrown from WaiParseSafe : InvalidRequest https://hackage.haskell.org/package/warp-3.3.31/docs/Network-Wai-Handler-Warp.html#t:InvalidRequest and RequestParseException . I think these should be simply wrapped into ScottyException and enrich the handler with new cases.
• [x] expose filesOpts from Scotty and .Trans
• [x] add tests to ensure that files works for both URL-encoded and multipart form uploads
  • I have a PR for hspec-wai : https://github.com/hspec/hspec-wai/pull/77
• [x] add test to ensure the body containing files does not disappear after calling next (remember #308 ?).
• [x] update Changelog

closes #203 , closes #183 , closes #370

[ocramz]

@fumieval I would love your feedback on the general design of this PR.

[ocramz]

I'm finding out that Flask (python web framework) has a similar internal design for mulipart form handling (dump into temp files and load those on demand)

[ocramz]

I think the new tests fail because I generate an incorrect 'multipart' form body. Zero files are parsed, without indication of what goes wrong. The plot thickens..

Fixed, will need to upstream the generation of multipart form data to hspec-wai

[ocramz]

This PR was a real trek. @fumieval please take a look, thanks!

[ocramz]

Hi @fumieval I would really like to move this patch forward. I think it's a decent design but need your feedback as you're the only other maintainer at the moment. If there's anything you'd like me to change, or need more time for the review, please let me know.