diff --git a/config/api/routes/files.php b/config/api/routes/files.php index 7cf1073611..c433aff570 100644 --- a/config/api/routes/files.php +++ b/config/api/routes/files.php @@ -1,7 +1,9 @@ upload(function ($source, $filename) use ($path) { - $props = [ + // check if upload is sent in chunks and handle them + $source = Upload::chunk($this, $source, $filename); + + // complete files return an absolute path; + // if file is not yet complete, end here + if ($source === null) { + return; + } + + // move the source file to the content folder + return $this->parent($path)->createFile([ 'content' => [ 'sort' => $this->requestBody('sort') ], 'source' => $source, 'template' => $this->requestBody('template'), 'filename' => $filename - ]; - - // move the source file from the temp dir - return $this->parent($path)->createFile($props, true); + ], true); }); // @codeCoverageIgnoreEnd } diff --git a/panel/src/helpers/upload.js b/panel/src/helpers/upload.js index 515d21ed30..0c890c89bc 100644 --- a/panel/src/helpers/upload.js +++ b/panel/src/helpers/upload.js @@ -1,3 +1,45 @@ +import { random } from "./string.js"; + +/** + * Uploads a file in chunks + * @param {File} file - file to upload + * @param {Object} params - upload options (see `upload` method for details) + * @param {number} size - chunk size in bytes + */ +export async function chunked(file, params, size = 5242880) { + const parts = Math.ceil(file.size / size); + const id = random(4).toLowerCase(); + let response; + + for (let i = 0; i < parts; i++) { + const start = i * size; + const end = Math.min(start + size, file.size); + const chunk = parts > 1 ? file.slice(start, end, file.type) : file; + + // when more than one part, add flag to + // recognize chunked upload and its last chunk + if (parts > 1) { + params.headers = { + ...params.headers, + "Upload-Length": file.size, + "Upload-Offset": start, + "Upload-Id": id + }; + } + + response = await upload(chunk, { + ...params, + // calculate the total progress based on chunk progress + progress: (xhr, chunk, percent) => { + const total = (i + percent / 100) / parts; + params.progress(xhr, file, Math.round(total * 100)); + } + }); + } + + return response; +} + /** * Uploads a file using XMLHttpRequest. * @@ -13,7 +55,7 @@ * @param {Function} params.success - callback when upload succeeded * @param {Function} params.error - callback when upload failed */ -export default async (file, params) => { +export async function upload(file, params) { return new Promise((resolve, reject) => { const defaults = { url: "/", @@ -96,4 +138,6 @@ export default async (file, params) => { xhr.send(data); }); -}; +} + +export default upload; diff --git a/panel/src/panel/upload.js b/panel/src/panel/upload.js index c55b016b25..39673343eb 100644 --- a/panel/src/panel/upload.js +++ b/panel/src/panel/upload.js @@ -2,7 +2,7 @@ import { uuid } from "@/helpers/string"; import State from "./state.js"; import listeners from "./listeners.js"; import queue from "@/helpers/queue.js"; -import upload from "@/helpers/upload.js"; +import { chunked as upload } from "@/helpers/upload.js"; import { extension, name, niceSize } from "@/helpers/file.js"; export const defaults = () => { @@ -312,15 +312,19 @@ export default (panel) => { }, async upload(file, attributes) { try { - const response = await upload(file.src, { - attributes: attributes, - headers: { "x-csrf": panel.system.csrf }, - filename: file.name + "." + file.extension, - url: this.url, - progress: (xhr, src, progress) => { - file.progress = progress; - } - }); + const response = await upload( + file.src, + { + attributes: attributes, + headers: { "x-csrf": panel.system.csrf }, + filename: file.name + "." + file.extension, + url: this.url, + progress: (xhr, src, progress) => { + file.progress = progress; + } + }, + panel.config.upload + ); file.completed = true; file.model = response.data; diff --git a/src/Api/Api.php b/src/Api/Api.php index 75b5d1d6f0..c2d7e96a83 100644 --- a/src/Api/Api.php +++ b/src/Api/Api.php @@ -605,6 +605,8 @@ protected function setRequestMethod( * Added debug parameter for testing purposes as we did in the Email class * * @throws \Exception If request has no files or there was an error with the upload + * + * @todo Move most of the logic to `Api\Upload` class */ public function upload( Closure $callback, diff --git a/src/Api/Upload.php b/src/Api/Upload.php new file mode 100644 index 0000000000..b84e44f652 --- /dev/null +++ b/src/Api/Upload.php @@ -0,0 +1,214 @@ + + * @link https://getkirby.com + * @copyright Bastian Allgeier + * @license https://getkirby.com/license + * @since 4.3.0 + * @internal + */ +class Upload +{ + /** + * Handle chunked uploads by merging all chunks + * in the tmp directory and only returning the new + * $source path to the tmp file once complete + * + * @throws \Kirby\Exception\DuplicateException Duplicate first chunk (same filename and id) + * @throws \Kirby\Exception\Exception Chunk offset does not match existing tmp file + * @throws \Kirby\Exception\NotFoundException Subsequent chunk has no existing tmp file + */ + public static function chunk( + Api $api, + string $source, + string $filename + ): string|null { + // if the file is uploaded in chunks… + if ($total = (int)$api->requestHeaders('Upload-Length')) { + // ensure the tmp upload directory exists + Dir::make($dir = static::tmp()); + + // create path for file in tmp upload directory; + // prefix with id while file isn't completely uploaded yet + $id = static::chunkId($api->requestHeaders('Upload-Id')); + $filename = basename($filename); + $tmpRoot = $dir . '/' . $id . '-' . $filename; + + // validate various aspects of the request + // to ensure the chunk isn't trying to do malicious actions + static::validateChunk( + source: $source, + tmp: $tmpRoot, + total: $total, + offset: $api->requestHeaders('Upload-Offset'), + template: $api->requestBody('template'), + ); + + // stream chunk content and append it to partial file + stream_copy_to_stream( + fopen($source, 'r'), + fopen($tmpRoot, 'a') + ); + + // clear file stat cache so the following call to `F::size` + // really returns the updated file size + clearstatcache(); + + // if file isn't complete yet, return early + if (F::size($tmpRoot) < $total) { + return null; + } + + // remove id from partial filename now the file is complete, + // so we can pass the path from the tmp upload directory + // as new source path for the file back to the API upload method + rename( + $tmpRoot, + $newRoot = $dir . '/' . $filename + ); + + return $newRoot; + } + + return $source; + } + + /** + * Ensures a clean chunk ID by stripping forbidden characters + */ + public static function chunkId(string $id): string + { + return Str::slug($id, '', 'a-z0-9'); + } + + /** + * Returns the ideal size for a file chunk + */ + public static function chunkSize(): int + { + $max = [ + Str::toBytes(ini_get('upload_max_filesize')), + Str::toBytes(ini_get('post_max_size')) + ]; + + // consider cloudflare proxy limit, if detected + if (isset($_SERVER['HTTP_CF_CONNECTING_IP']) === true) { + $max[] = Str::toBytes('100M'); + } + + // to be sure, only use 95% of the max possible upload size + return (int)floor(min($max) * 0.95); + } + + /** + * Clean up tmp directory of stale files + */ + public static function clean(): void + { + foreach (Dir::files($dir = static::tmp(), [], true) as $file) { + // remove any file that hasn't been altered + // in the last 24 hours + if (F::modified($file) < time() - 86400) { + F::remove($file); + } + } + + // remove tmp directory if completely empty + if (Dir::isEmpty($dir) === true) { + Dir::remove($dir); + } + } + + /** + * Returns root of directory used for + * temporarily storing (incomplete) uploads + * @codeCoverageIgnore + */ + protected static function tmp(): string + { + return App::instance()->root('cache') . '/.uploads'; + } + + /** + * Ensures the sent chunk is valid + * + * @throws \Kirby\Exception\DuplicateException Duplicate first chunk (same filename and id) + * @throws \Kirby\Exception\Exception Chunk offset does not match existing tmp file + * @throws \Kirby\Exception\InvalidArgumentException The maximum file size for this blueprint was exceeded + * @throws \Kirby\Exception\NotFoundException Subsequent chunk has no existing tmp file + */ + protected static function validateChunk( + string $source, + string $tmp, + int $total, + int $offset, + string|null $template = null + ): void { + $file = new File([ + 'parent' => new Page(['slug' => 'tmp']), + 'filename' => $filename = basename($tmp), + 'template' => $template + ]); + + // if the blueprint `maxsize` option is set, + // ensure that the total size communicated in the header + // as well as the current tmp size after adding this chunk + // does not exceed the max limit + if ( + ($max = $file->blueprint()->accept()['maxsize'] ?? null) && + ( + $total > $max || + (F::size($source) + F::size($tmp)) > $max + ) + ) { + throw new InvalidArgumentException(['key' => 'file.maxsize']); + } + + // validate the first chunk + if ($offset === 0) { + // sent chunk is expected to be the first part, + // but tmp file already exists + if (F::exists($tmp) === true) { + throw new DuplicateException('A tmp file upload with the same filename and upload id already exists: ' . $filename); + } + + // validate file (extension, name) for first chunk; + // will also be validate again by `$model->createFile()` + // when completely uploaded + FileRules::validFile($file, false); + + // first chunk is valid + return; + } + + // validate subsequent chunks: + // no tmp in place + if (F::exists($tmp) === false) { + throw new NotFoundException('Chunk offset ' . $offset . ' for non-existing tmp file: ' . $filename); + } + + // sent chunk's offset is not the continuation of the tmp file + if ($offset !== F::size($tmp)) { + throw new Exception('Chunk offset ' . $offset . ' does not match the existing tmp upload file size of ' . F::size($tmp)); + } + } +} diff --git a/src/Cms/FileRules.php b/src/Cms/FileRules.php index 33f25e36a5..84a93ec2a4 100644 --- a/src/Cms/FileRules.php +++ b/src/Cms/FileRules.php @@ -315,7 +315,7 @@ public static function validFilename(File $file, string $filename): bool public static function validMime(File $file, string $mime = null): bool { // make it easier to compare the mime - $mime = strtolower($mime); + $mime = strtolower($mime ?? ''); if (empty($mime)) { throw new InvalidArgumentException([ diff --git a/src/Panel/Panel.php b/src/Panel/Panel.php index e6f9ff67a5..22f407d19a 100644 --- a/src/Panel/Panel.php +++ b/src/Panel/Panel.php @@ -3,6 +3,7 @@ namespace Kirby\Panel; use Closure; +use Kirby\Api\Upload; use Kirby\Cms\App; use Kirby\Cms\Url as CmsUrl; use Kirby\Cms\User; @@ -137,6 +138,21 @@ public static function firewall( return true; } + /** + * Garbage collection which runs with a probability + * of 10% on each Panel request + * + * @since 4.3.0 + * @codeCoverageIgnore + */ + protected static function garbage(): void + { + // run garbage collection with a chance of 10%; + if (mt_rand(1, 10000) <= 0.1 * 10000) { + // clean up leftover upload chunks + Upload::clean(); + } + } /** * Redirect to a Panel url @@ -262,6 +278,9 @@ public static function router(string|null $path = null): Response|null return null; } + // run garbage collection + static::garbage(); + // set the translation for Panel UI before // gathering areas and routes, so that the // `t()` helper can already be used diff --git a/src/Panel/View.php b/src/Panel/View.php index a2fcebcff1..d5bfe613b3 100644 --- a/src/Panel/View.php +++ b/src/Panel/View.php @@ -2,6 +2,7 @@ namespace Kirby\Panel; +use Kirby\Api\Upload; use Kirby\Cms\App; use Kirby\Exception\Exception; use Kirby\Http\Response; @@ -278,6 +279,7 @@ public static function globals(): array 'debug' => $kirby->option('debug', false), 'kirbytext' => $kirby->option('panel.kirbytext', true), 'translation' => $kirby->option('panel.language', 'en'), + 'upload' => Upload::chunkSize(), ], '$system' => function () use ($kirby) { $locales = []; diff --git a/tests/Api/UploadTest.php b/tests/Api/UploadTest.php new file mode 100644 index 0000000000..f64aa3fa3b --- /dev/null +++ b/tests/Api/UploadTest.php @@ -0,0 +1,428 @@ +app = new App([ + 'roots' => [ + 'index' => static::TMP + ], + ]); + + $this->app->impersonate('kirby'); + } + + public function tearDown(): void + { + Blueprint::$loaded = []; + ini_restore('upload_max_filesize'); + ini_restore('post_max_size'); + unset($_SERVER['HTTP_CF_CONNECTING_IP']); + Dir::remove(static::TMP); + App::destroy(); + } + + /** + * @covers ::chunk + */ + public function testChunkNoChunks() + { + $source = static::TMP . '/test.md'; + $api = new Api([]); + $this->assertSame($source, Upload::chunk($api, $source, 'a.md')); + } + + /** + * @covers ::chunk + */ + public function testChunkFirstChunkFullLength() + { + $source = static::TMP . '/test.md'; + F::write($source, 'abcdef'); + $size = F::size($source); + + $api = new Api([ + 'requestData' => [ + 'headers' => [ + 'Upload-Length' => $size, + 'Upload-Offset' => 0, + 'Upload-Id' => 'abcd' + ] + ] + ]); + + $this->assertSame( + $file = static::TMP . '/site/cache/.uploads/test.md', + Upload::chunk($api, $source, basename($source)) + ); + $this->assertFileExists($file); + $this->assertFileDoesNotExist('abcd-' . $file); + } + + /** + * @covers ::chunk + */ + public function testChunkFirstChunkPartialLength() + { + $source = static::TMP . '/test.md'; + F::write($source, 'abcdef'); + + $api = new Api([ + 'requestData' => [ + 'headers' => [ + 'Upload-Length' => 3000, + 'Upload-Offset' => 0, + 'Upload-Id' => 'abcd' + ] + ] + ]); + + $dir = static::TMP . '/site/cache/.uploads'; + $this->assertNull(Upload::chunk($api, $source, basename($source))); + $this->assertFileDoesNotExist($dir . '/test.md'); + $this->assertFileExists($dir . '/abcd-test.md'); + } + + /** + * @covers ::chunk + */ + public function testChunkIdRemoveUnallowedCharacters() + { + $source = static::TMP . '/test.md'; + F::write($source, 'abcdef'); + + $api = new Api([ + 'requestData' => [ + 'headers' => [ + 'Upload-Length' => 3000, + 'Upload-Offset' => 0, + 'Upload-Id' => '../a/b!!cd' + ] + ] + ]); + + $dir = static::TMP . '/site/cache/.uploads'; + $this->assertNull(Upload::chunk($api, $source, basename($source))); + $this->assertFileDoesNotExist($dir . '/test.md'); + $this->assertFileExists($dir . '/abcd-test.md'); + } + + /** + * @covers ::chunk + */ + public function testChunkFilenameNoDirectoryTraversal() + { + $source = static::TMP . '/test.md'; + F::write($source, 'abcdef'); + + $api = new Api([ + 'requestData' => [ + 'headers' => [ + 'Upload-Length' => 3000, + 'Upload-Offset' => 0, + 'Upload-Id' => 'abcd' + ] + ] + ]); + + $dir = static::TMP . '/site/cache/.uploads'; + $this->assertNull(Upload::chunk($api, $source, '../../test.md')); + $this->assertFileDoesNotExist($dir . '/test.md'); + $this->assertFileExists($dir . '/abcd-test.md'); + } + + /** + * @covers ::chunk + */ + public function testChunkSuccesfulAll() + { + $source = static::TMP . '/test.md'; + F::write($source, 'abc'); + $api = new Api([ + 'requestData' => [ + 'headers' => [ + 'Upload-Length' => 6, + 'Upload-Offset' => 0, + 'Upload-Id' => 'abcd' + ] + ] + ]); + + $this->assertNull(Upload::chunk($api, $source, basename($source))); + + $source = static::TMP . '/test.md'; + F::write($source, 'def'); + $api = new Api([ + 'requestData' => [ + 'headers' => [ + 'Upload-Length' => 6, + 'Upload-Offset' => 3, + 'Upload-Id' => 'abcd' + ] + ] + ]); + + $file = static::TMP . '/site/cache/.uploads/test.md'; + $this->assertSame($file, Upload::chunk($api, $source, 'test.md')); + $this->assertFileExists($file); + $this->assertFileDoesNotExist('abcd-' . $file); + $this->assertSame('abcdef', F::read($file)); + } + + /** + * @covers ::chunkId + */ + public function testChunkId() + { + $this->assertSame('abcd', Upload::chunkId('abcd')); + $this->assertSame('abcd', Upload::chunkId('ab/cd')); + $this->assertSame('abcd', Upload::chunkId('../../ab/cd')); + $this->assertSame('abcd', Upload::chunkId('a-b/../cd.')); + } + + /** + * @covers ::chunkSize + */ + public function testChunkSize() + { + ini_set('upload_max_filesize', '10M'); + ini_set('post_max_size', '20M'); + + $this->assertSame( + (int)floor(10 * 1024 * 1024 * 0.95), + Upload::chunkSize() + ); + + // with CloudFlare + ini_set('upload_max_filesize', '200M'); + ini_set('post_max_size', '200M'); + $_SERVER['HTTP_CF_CONNECTING_IP'] = '1.1.1.1'; + + $this->assertSame( + (int)floor(100 * 1024 * 1024 * 0.95), + Upload::chunkSize() + ); + } + + /** + * @covers ::clean + */ + public function testClean() + { + $dir = static::TMP . '/site/cache/.uploads'; + F::write($a = $dir . '/abcd-a.md', ''); + F::write($b = $dir . '/efgh-b.md', ''); + touch($b, time() - 86400 - 1); + + $this->assertDirectoryExists($dir); + $this->assertFileExists($a); + $this->assertFileExists($b); + + Upload::clean(); + + $this->assertDirectoryExists($dir); + $this->assertFileExists($a); + $this->assertFileDoesNotExist($b); + + touch($a, time() - 86400 - 1); + + Upload::clean(); + + $this->assertDirectoryDoesNotExist($dir); + $this->assertFileDoesNotExist($a); + $this->assertFileDoesNotExist($b); + } + + /** + * @covers ::validateChunk + */ + public function testValidateChunkDuplicate() + { + $source = static::TMP . '/test.md'; + F::write($source, 'abcdef'); + + $api = new Api([ + 'requestData' => [ + 'headers' => [ + 'Upload-Length' => 3000, + 'Upload-Offset' => 0, + 'Upload-Id' => 'abcd' + ] + ] + ]); + + $this->assertNull(Upload::chunk($api, $source, basename($source))); + $this->expectException(DuplicateException::class); + $this->expectExceptionMessage('A tmp file upload with the same filename and upload id already exists: abcd-test.md'); + Upload::chunk($api, $source, basename($source)); + } + + /** + * @covers ::validateChunk + */ + public function testValidateChunkSubsequentInvalidOffset() + { + $source = static::TMP . '/a.md'; + F::write($source, 'abcdef'); + + $api = new Api([ + 'requestData' => [ + 'headers' => [ + 'Upload-Length' => 3000, + 'Upload-Offset' => 0, + 'Upload-Id' => 'abcd' + ] + ] + ]); + + $this->assertNull(Upload::chunk($api, $source, basename($source))); + + $api = new Api([ + 'requestData' => [ + 'headers' => [ + 'Upload-Length' => 3000, + 'Upload-Offset' => 1500, + 'Upload-Id' => 'abcd' + ] + ] + ]); + + $this->expectException(Exception::class); + $this->expectExceptionMessage('Chunk offset 1500 does not match the existing tmp upload file size of 6'); + Upload::chunk($api, $source, basename($source)); + } + + /** + * @covers ::validateChunk + */ + public function testValidateChunkSubsequentNoFirst() + { + $source = static::TMP . '/a.md'; + F::write($source, 'abcdef'); + + $api = new Api([ + 'requestData' => [ + 'headers' => [ + 'Upload-Length' => 3000, + 'Upload-Offset' => 10, + 'Upload-Id' => 'abcd' + ] + ] + ]); + + $this->expectException(NotFoundException::class); + $this->expectExceptionMessage('Chunk offset 10 for non-existing tmp file: abcd-a.md'); + Upload::chunk($api, $source, basename($source)); + } + + /** + * @covers ::validateChunk + */ + public function testValidateChunkInvalidExtension() + { + $source = static::TMP . '/a.php'; + $api = new Api([ + 'requestData' => [ + 'headers' => [ + 'Upload-Length' => 3000, + 'Upload-Offset' => 0, + 'Upload-Id' => 'abcd' + ] + ] + ]); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('You are not allowed to upload PHP files'); + Upload::chunk($api, $source, basename($source)); + } + + /** + * @covers ::validateChunk + */ + public function testValidateChunkTooLargeTotal() + { + $source = static::TMP . '/a.md'; + $app = $this->app->clone([ + 'blueprints' => [ + 'files/test' => [ + 'name' => 'test', + 'accept' => ['maxsize' => 100] + ] + ] + ]); + $api = new Api([ + 'requestData' => [ + 'body' => [ + 'template' => 'test' + ], + 'headers' => [ + 'Upload-Length' => 120, + 'Upload-Offset' => 0, + 'Upload-Id' => 'abcd' + ] + ] + ]); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionCode('error.file.maxsize'); + Upload::chunk($api, $source, basename($source)); + } + + /** + * @covers ::validateChunk + */ + public function testValidateChunkTooLargeCurrentChunk() + { + $dir = static::TMP . '/site/cache/.uploads'; + $source = static::TMP . '/a.md'; + F::write($dir . '/abcd-a.md', 'abcdef'); + F::write($source, 'ghijkl'); + + $app = $this->app->clone([ + 'blueprints' => [ + 'files/test' => [ + 'name' => 'test', + 'accept' => ['maxsize' => 10] + ] + ] + ]); + $api = new Api([ + 'requestData' => [ + 'body' => [ + 'template' => 'test' + ], + 'headers' => [ + 'Upload-Length' => 10, + 'Upload-Offset' => 6, + 'Upload-Id' => 'abcd' + ] + ] + ]); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionCode('error.file.maxsize'); + Upload::chunk($api, $source, basename($source)); + } +} diff --git a/tests/Api/mocks.php b/tests/Api/mocks.php new file mode 100644 index 0000000000..26dec2b839 --- /dev/null +++ b/tests/Api/mocks.php @@ -0,0 +1,47 @@ +