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

Save large request bodies to temporary directory #3

Open
jivank opened this issue Aug 11, 2020 · 5 comments
Open

Save large request bodies to temporary directory #3

jivank opened this issue Aug 11, 2020 · 5 comments

Comments

@jivank
Copy link

jivank commented Aug 11, 2020

To be able to handle large uploads, it would be helpful to have a setting such that sets the limit of a request body to a certain Content-Length (lets say 8MB). Any Content-Length above 8MB will be streamed to disk rather than parsed in memory.

I don't think there are any implementations of this for any of the Nim projects yet. But you can see how it's done with Python.
See Werkzeug:
https://github.com/pallets/werkzeug/blob/ef545f0d0bf28cbad02066b4cb7471bea50a93ee/src/werkzeug/formparser.py#L59

@jivank jivank changed the title Save large request bodies to disk Save large request bodies to temporary directory Aug 11, 2020
@ringabout
Copy link
Owner

Hi @jivank
Is it a httpx issue? I think Nim should have a better tempfile library. Then I can add supports for streaming to temp file. Anyway pr is welcome.

nim-lang/Nim#9372

@jivank
Copy link
Author

jivank commented Aug 11, 2020

I think it is an httpx issue since the request will read the entire body to the data string. I just tested on an app that have has no POST method to / using curl -d @pop-os_20.04_amd64_intel_10.iso localhost:8080. The file size is 2.1GB and it appeared to use up 6GB of memory and then returned with 404.

I have used https://github.com/OpenSystemsLab/tempfile.nim and it seems to work well. I think using this in combination with https://nim-lang.org/docs/streams.html which contains a FileStream and StringStream may be in the right direction (giving a common interface for the procs used, since body is used as a string). But yeah, it doesn't seem like there is an easy way to solve this. We would need to write to disk, parse the request body from disk (split into files / move into upload directory)

@ringabout
Copy link
Owner

May be related with this, here were changes in asynchttpserver(which were reverted unfortunately)

nim-lang/Nim#13147

nim-lang/Nim#13394

@jivank
Copy link
Author

jivank commented Aug 12, 2020

Yes, but I think instead of a callback (which sounds nice, but all frameworks would have to accommodate for it), we can just add something like files: seq[FilePath] to the Request data type. By the time it reaches Prologue it will already be accessible. The problem is usually we leave the framework like Jester and Prologue to do the form parsing. But I think it would be simpler to do any parsing for file attachments upfront and perhaps leave data as an empty string once parsed.

@ringabout
Copy link
Owner

I think netkit is very suitable for this task, it has a better interface based on buffer and stream.

termermc added a commit to termermc/httpx that referenced this issue Apr 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants