Fix: Upload Endpoints

[1yam]

Issue

Issue Description:
The current file upload endpoint reads the entire file before checking if it exceeds the maximum allowed size. The size limits are 25 MB for unauthenticated uploads (MAX_UNAUTHENTICATED_UPLOAD_FILE_SIZE) and 1000 MB for authenticated uploads (MAX_UPLOAD_FILE_SIZE).

Proposed Solutions:

• Read the file in chunks and check if the size exceeds the maximum allowed size.
• Use a temporary file to handle the data, which helps avoid using too much memory.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.67%. Comparing base (555c166) to head (526de08).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #565      +/-   ##
==========================================
+ Coverage   94.60%   94.67%   +0.07%     
==========================================
  Files          89       89              
  Lines        4724     4770      +46     
  Branches      648      652       +4     
==========================================
+ Hits         4469     4516      +47     
+ Misses        233      232       -1     
  Partials       22       22              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[hoh]

Thanks. Would it make sense to add unit tests on the behavior specific to MultipartUploadedFile and RawUploadedFile ?

[hoh]

@philogicae this should fix the file upload issue you reported.

[1yam]

pyaleph/src/aleph/web/controllers/storage.py

Line 272 in 885c59c

post = await request.post()

Based on aiohttp docs post method looks like it's made for MultipartForm:

If method is not POST, PUT, PATCH, TRACE or DELETE or content_type is not empty or application/x-www-form-urlencoded or multipart/form-data returns empty multidict.

@hoh Maybe should i add a check and use read() for RawUploadedFile ?

[aliel]

We should use multipart method instead of post

You might have noticed a big warning in the example above. The general issue is that aiohttp.web.BaseRequest.post() reads the whole payload in memory, resulting in possible OOM errors. To avoid this, for multipart uploads, you should use aiohttp.web.BaseRequest.multipart() which returns a multipart reader:

=>
https://docs.aiohttp.org/en/stable/web_quickstart.html#file-uploads

@hoh

[Psycojoker]

I did a quick general review because I don't know the business code very well and some of them are more questions/suggestions, don't hesitate if you have questions :)

[1yam]

@Psycojoker I did some updates:

• Refactored UploadFile to avoid code duplication (u was right, with the change made was possible to do way easier)
• Used aiofiles
• Fixing most of you'r comment

The main thing I didn't change is that we're still reading and writing the file to a temporary file first to avoid launching the workflow if the file is too large and we need to calculate the hash early for safety check.

I wasn't using the manager correctly, that should be fixed now.

[Psycojoker]

Hugo suggested that we could do a call at some point if you want me to explain some of my comments.

[Psycojoker]

We did a pair programming session and that should be good on my side regard good python practices.

Lyam told me he needed to do one last test with the frontend before confirming it's good.

[1yam]

@hoh @Psycojoker Just finish my last test, everything looks working as intended, I would say ready for merge