Chunked file uploads

[distantnative]

Description

Summary of changes

• New Api\Upload class
  • Takes over all the nested code from Api\Api::upload() to make it more easily to read and test
  • Adds support for uploads in chunks

Reasoning

• https://feedback.getkirby.com/256
• Cleaner class structure and better to test

Changelog

Features

• Panel uploads can now exceed the upload_max_filesize limit as they're uploaded in chunks. If you want to restrict the upload size, please use the file blueprint accept maxsize option: https://getkirby.com/docs/reference/panel/blueprints/file#accept

Enhancements

• New uploadAsChunks JS helper function

Fixed

• Canceling the file upload dialog now also cancels ongoing uploads

Refactored

• New Kirby\\Api\Upload class to handle file uploads via the REST API

Ready?

• In-code documentation (wherever needed)
• Unit tests for fixed bug/feature
• Tests and CI checks all pass

For review team

• Add changes & docs to release notes draft in Notion

[afbora]

I've tried to upload a EXE file. Kirby 4.2.0 throws a error The extension "exe" is not allowed but nothing happens for this PR. Seems something broken.

[afbora]

Also I realized that file is still remains when I cancel the upload. Should be deleted. But should be careful when file replacing.

[distantnative]

@afbora We cannot delete the tmp file on cancel. But we clean up the files with a garbage collection.

[lukasbestle]

Adding the "two-person review" label as I realized we need to be super careful about the security implications here. There's a lot that could go horribly wrong if we miss anything. I think we are heading in a great direction though 👍

[distantnative]

@lukasbestle makes sense. And as written in the initial post, I think we should have a few review rounds as well. Now getting the concept right, later adding unit tests etc. before final review.

[afbora]

We cannot delete the tmp file on cancel. But we clean up the files with a garbage collection.

@distantnative What if we upload to the temp folder, does it make sense? If upload successful, we can move the file to the model root, if not garbage collector take care.

[distantnative]

@afbora That's what we are doing here. Just not the system temp folder as that doesn't persist across requests. So the first parts aren't there anymore when the last chunk is received.

[tobimori]

Is it theoretically possible to add a config option that will limit the file size?

[afbora]

I've found few issues, could you confirm that?

• File not uploaded without error or log (dialog closed and no file uploaded,) with big files. For me, ~100 MB success, ~300 MB fails.
• I'm getting A file with the name "filename.zip" already exists error when big files upload from textarea field

[distantnative]

@afbora the PR currently is not functional as it's in the middle of implementing changes based on Lukas' feedback and some troubles I encountered (see our comments discussion). Sorry that you tried, but I think any upload fails currently that needs more than one chunk.

[afbora]

@distantnative No problem. I'll test again after PR is ready.

[lukasbestle]

Is it theoretically possible to add a config option that will limit the file size?

Good point. I think it would make sense to make this an option for the file blueprint.

[distantnative]

@lukasbestle @tobimori are you thinking of something different than the existing accept maxsize option?

[tobimori]

I didn't know there was an option already. Consider my request as solved 😀

[lukasbestle]

Also forgot about maxsize. 😆

[distantnative]

@lukasbestle added some more validations – skipping MIME types (will still checked for the full file), but at least basic ones as allowed extensions.

[afbora]

Works great for me. I couldn't see any issue 🎉

[distantnative]

I think I have encountered an issue: Uploading a large file and canceling while it's uploading, it still shows up in the file list after a reload.

[distantnative]

Turns out: When one cancels the upload dialog, all running uploads still run and finish in the background. This is not new in this PR but a lot more visible/likely to happen with a different expectation by the user.

[distantnative]

@lukasbestle @afbora Added a way to abort ongoing upload requests. Could you please review the last commit and test it out?

[afbora]

@distantnative Still works great for me 👍

[lukasbestle]

Works great for me as well 👍