Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Files limit v2 #203 #369

Merged
merged 25 commits into from
Mar 9, 2024
Merged

Files limit v2 #203 #369

merged 25 commits into from
Mar 9, 2024

Conversation

ocramz
Copy link
Collaborator

@ocramz ocramz commented Dec 29, 2023

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:

TODO:

closes #203 , closes #183 , closes #370

@ocramz ocramz changed the title Files limit v2 #203 WIP Files limit v2 #203 Dec 29, 2023
@ocramz
Copy link
Collaborator Author

ocramz commented Dec 29, 2023

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

@ocramz ocramz marked this pull request as draft January 1, 2024 14:28
@ocramz ocramz marked this pull request as ready for review January 1, 2024 21:47
@ocramz
Copy link
Collaborator Author

ocramz commented Jan 5, 2024

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)

Web/Scotty/Trans.hs Outdated Show resolved Hide resolved
changelog.md Show resolved Hide resolved
@ocramz ocramz marked this pull request as draft February 3, 2024 14:06
@ocramz
Copy link
Collaborator Author

ocramz commented Feb 29, 2024

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 ocramz changed the title WIP Files limit v2 #203 Files limit v2 #203 Feb 29, 2024
@ocramz ocramz marked this pull request as ready for review February 29, 2024 08:27
@ocramz
Copy link
Collaborator Author

ocramz commented Feb 29, 2024

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

@ocramz ocramz requested review from fumieval and removed request for fumieval March 1, 2024 06:00
Web/Scotty/Internal/Types.hs Outdated Show resolved Hide resolved
test/Web/ScottySpec.hs Outdated Show resolved Hide resolved
@ocramz
Copy link
Collaborator Author

ocramz commented Mar 3, 2024

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.

@ocramz ocramz merged commit d376f43 into scotty-web:master Mar 9, 2024
6 checks passed
@ocramz ocramz deleted the files-limit-v2-#203 branch March 9, 2024 16:44
ocramz added a commit that referenced this pull request Mar 9, 2024
ocramz added a commit that referenced this pull request Mar 9, 2024
@ocramz ocramz added this to the Release 0.22 milestone Mar 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant