Memory Leak: asynchttpserver reads entire request body to memory

[jivank]

The following line of code results in reading the entire request body which excessive memory consumption on large file uploads for any dependent web frameworks like prologue and jester:

Nim/lib/pure/asynchttpserver.nim

Line 286 in 9819fb2

request.body = await client.recv(contentLength)

Should an RFC be opened to further discuss the design and implementation? Getting this corrected will break existing frameworks since form parsing will have to be done at the asynchttpserver level.

I believe the simplest approach is just have asynchttpserver's Request type have a form and files field where form will contain html form input (form items without filename and content-type) while files will contain the path name of any files that were uploaded.

That covers the common way of how files will be uploaded.
I would also think adding an additional parameter to compliment the max body size / content length limit. Something like inMemoryLimit which defines at what byte size should the request body be saved to disk, if for example there is a content-type other than multipart such as JSON.

[Araq]

Should an RFC be opened to further discuss the design and implementation?

Yes but I consider it so severe that I'm willing to do what it takes, even it means breaking somebody's code.

[dom96]

Please check how other libraries deal with this before rushing ahead to change the API in ways similar to the FD problem (that I still don't believe was resolved correctly).

Just because this is a serious problem don't forget that it's been like this for years without any big problems, so yes hurry towards a solution but let's discuss it properly.

[arnetheduck]

chronos exposes a stream reader to handle this issue: https://github.com/status-im/nim-chronos/blob/0b78606e4142affbdc0e0e94bc1b8c39a8705737/chronos/apps/http/httpserver.nim#L335

There's also an option to get the full body instead which incidentally demonstrates how to use the stream reader: https://github.com/status-im/nim-chronos/blob/0b78606e4142affbdc0e0e94bc1b8c39a8705737/chronos/apps/http/httpserver.nim#L369

[ITwrx]

@dom96 I agree that the solution should be well considered, but this issue, manifested for me as planety/prologue#103,
has, at least temporarily, stopped me from being able to build some planned client-facing web/browser-based apps in Nim.

thanks to @arnetheduck 's post i see Scorper uses Chronos, and presumably doesn't have this issue, but Scorper's young, so i may only get so far with that in the short term.