From a3fa418aa5bf34a4fcc4104bc5c8bd8d944c60fb Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Thu, 9 Dec 2021 13:30:52 +0545 Subject: [PATCH 1/4] update chunks uploaded when all chunks are uploaded --- app/controllers/api/storage.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/storage.php b/app/controllers/api/storage.php index 4b37e9c3ce..53a6ff8e7e 100644 --- a/app/controllers/api/storage.php +++ b/app/controllers/api/storage.php @@ -729,7 +729,8 @@ App::post('/v1/storage/buckets/:bucketId/files') ->setAttribute('openSSLVersion', $openSSLVersion) ->setAttribute('openSSLCipher', $openSSLCipher) ->setAttribute('openSSLTag', $openSSLTag) - ->setAttribute('openSSLIV', $openSSLIV); + ->setAttribute('openSSLIV', $openSSLIV) + ->setAttribute('chunksUploaded', $chunksUploaded); if($permissionBucket) { $file = Authorization::skip(function() use ($dbForInternal, $bucketId, $fileId, $file) { From 80a89877c14efbfdb1edf1c1d62b1c717e74d6c9 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Thu, 9 Dec 2021 14:11:30 +0545 Subject: [PATCH 2/4] fix variable --- app/controllers/api/storage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/storage.php b/app/controllers/api/storage.php index 7c597615cd..a26b8141a3 100644 --- a/app/controllers/api/storage.php +++ b/app/controllers/api/storage.php @@ -585,7 +585,7 @@ App::post('/v1/storage/buckets/:bucketId/files') $size = (\is_array($file['size']) && isset($file['size'][0])) ? $file['size'][0] : $file['size']; $contentRange = $request->getHeader('content-range'); - $fileId = $fileId === 'unique()' ? $$dbForExternal->getId() : $fileId; + $fileId = $fileId === 'unique()' ? $dbForExternal->getId() : $fileId; $chunk = 1; $chunks = 1; From 63d3b8e200a71d0a3d4c831f9aadab24729d9cef Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Thu, 9 Dec 2021 16:29:44 +0545 Subject: [PATCH 3/4] more review suggestion fixes --- app/controllers/api/storage.php | 88 +++++++++++++++++---------------- 1 file changed, 45 insertions(+), 43 deletions(-) diff --git a/app/controllers/api/storage.php b/app/controllers/api/storage.php index a26b8141a3..7c35df3f90 100644 --- a/app/controllers/api/storage.php +++ b/app/controllers/api/storage.php @@ -482,7 +482,7 @@ App::delete('/v1/storage/buckets/:bucketId') throw new Exception('Bucket not found', 404); } - if(!$dbForInternal->deleteDocument('buckets', $bucketId)) { + if (!$dbForInternal->deleteDocument('buckets', $bucketId)) { throw new Exception('Failed to remove project from DB', 500); } @@ -545,7 +545,7 @@ App::post('/v1/storage/buckets/:bucketId/files') $bucket = $dbForInternal->getDocument('buckets', $bucketId); - if($bucket->isEmpty() + if ($bucket->isEmpty() || (!$bucket->getAttribute('enabled') && $mode !== APP_MODE_ADMIN )) { throw new Exception('Bucket not found', 404); } @@ -594,7 +594,7 @@ App::post('/v1/storage/buckets/:bucketId/files') $end = $request->getContentRangeEnd(); $size = $request->getContentRangeSize(); $fileId = $request->getHeader('x-appwrite-id', $fileId); - if(is_null($start) || is_null($end) || is_null($size)) { + if (is_null($start) || is_null($end) || is_null($size)) { throw new Exception('Invalid content-range header', 400); } @@ -665,7 +665,7 @@ App::post('/v1/storage/buckets/:bucketId/files') } if ($bucket->getAttribute('encryption', true) && $size <= APP_STORAGE_READ_BUFFER) { - if(empty($data)) { + if (empty($data)) { $data = $device->read($path); } $key = App::getEnv('_APP_OPENSSL_KEY_V1'); @@ -673,7 +673,7 @@ App::post('/v1/storage/buckets/:bucketId/files') $data = OpenSSL::encrypt($data, OpenSSL::CIPHER_AES_128_GCM, $key, 0, $iv, $tag); } - if(!empty($data)) { + if (!empty($data)) { if (!$device->write($path, $data, $mimeType)) { throw new Exception('Failed to save file', 500); } @@ -715,7 +715,7 @@ App::post('/v1/storage/buckets/:bucketId/files') 'openSSLIV' => $openSSLIV, 'search' => implode(' ', [$fileId, $fileName,]), ]); - if($permissionBucket) { + if ($permissionBucket) { $file = Authorization::skip(function() use ($dbForExternal, $bucketId, $doc) { return $dbForExternal->createDocument('bucket_' . $bucketId, $doc); }); @@ -736,7 +736,7 @@ App::post('/v1/storage/buckets/:bucketId/files') ->setAttribute('openSSLIV', $openSSLIV) ->setAttribute('chunksUploaded', $chunksUploaded); - if($permissionBucket) { + if ($permissionBucket) { $file = Authorization::skip(function() use ($dbForExternal, $bucketId, $fileId, $file) { return $dbForExternal->updateDocument('bucket_' . $bucketId, $fileId, $file); }); @@ -773,7 +773,7 @@ App::post('/v1/storage/buckets/:bucketId/files') 'chunksUploaded' => $chunksUploaded, 'search' => implode(' ', [$fileId, $fileName,]), ]); - if($permissionBucket) { + if ($permissionBucket) { $file = Authorization::skip(function() use ($dbForExternal, $bucketId, $doc) { return $dbForExternal->createDocument('bucket_' . $bucketId, $doc); }); @@ -784,7 +784,7 @@ App::post('/v1/storage/buckets/:bucketId/files') $file = $file ->setAttribute('chunksUploaded', $chunksUploaded); - if($permissionBucket) { + if ($permissionBucket) { $file = Authorization::skip(function() use ($dbForExternal, $bucketId, $fileId, $file) { return $dbForExternal->updateDocument('bucket_' . $bucketId, $fileId, $file); }); @@ -849,7 +849,7 @@ App::get('/v1/storage/buckets/:bucketId/files') $bucket = $dbForInternal->getDocument('buckets', $bucketId); - if($bucket->isEmpty() + if ($bucket->isEmpty() || (!$bucket->getAttribute('enabled') && $mode !== APP_MODE_ADMIN )) { throw new Exception('Bucket not found', 404); } @@ -869,7 +869,7 @@ App::get('/v1/storage/buckets/:bucketId/files') } if (!empty($cursor)) { - if($bucket->getAttribute('permission') ==='bucket') { + if ($bucket->getAttribute('permission') ==='bucket') { $cursorFile = Authorization::skip(function() use ($dbForExternal, $bucket, $cursor) { return $dbForExternal->getDocument('bucket_' . $bucket->getId(), $cursor); }); @@ -888,7 +888,7 @@ App::get('/v1/storage/buckets/:bucketId/files') $queries[] = new Query('search', Query::TYPE_SEARCH, [$search]); } - if($bucket->getAttribute('permission') === 'bucket') { + if ($bucket->getAttribute('permission') === 'bucket') { $files = Authorization::skip(function() use ($dbForExternal, $bucketId, $queries, $limit, $offset, $cursor, $cursorDirection, $orderType) { return $dbForExternal->find('bucket_' . $bucketId, $queries, $limit, $offset, [], [$orderType], $cursorFile ?? null, $cursorDirection); }); @@ -934,7 +934,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId') $bucket = $dbForInternal->getDocument('buckets', $bucketId); - if($bucket->isEmpty() + if ($bucket->isEmpty() || (!$bucket->getAttribute('enabled') && $mode !== APP_MODE_ADMIN )) { throw new Exception('Bucket not found', 404); } @@ -947,7 +947,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId') } } - if($bucket->getAttribute('permission') === 'bucket') { + if ($bucket->getAttribute('permission') === 'bucket') { $file = Authorization::skip(function() use ($dbForExternal, $bucketId, $fileId) { return $dbForExternal->getDocument('bucket_' . $bucketId, $fileId); }); @@ -1016,7 +1016,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/preview') } $bucket = $dbForInternal->getDocument('buckets', $bucketId); - if($bucket->isEmpty() + if ($bucket->isEmpty() || (!$bucket->getAttribute('enabled') && $mode !== APP_MODE_ADMIN )) { throw new Exception('Bucket not found', 404); } @@ -1040,7 +1040,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/preview') $date = \date('D, d M Y H:i:s', \time() + (60 * 60 * 24 * 45)).' GMT'; // 45 days cache $key = \md5($fileId.$width.$height.$gravity.$quality.$borderWidth.$borderColor.$borderRadius.$opacity.$rotation.$background.$storage.$output); - if($bucket->getAttribute('permission')==='bucket') { + if ($bucket->getAttribute('permission')==='bucket') { // skip authorization $file = Authorization::skip(function () use ($dbForExternal, $bucketId, $fileId) { return $dbForExternal->getDocument('bucket_' . $bucketId, $fileId); @@ -1180,7 +1180,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/download') $bucket = $dbForInternal->getDocument('buckets', $bucketId); - if($bucket->isEmpty() + if ($bucket->isEmpty() || (!$bucket->getAttribute('enabled') && $mode !== APP_MODE_ADMIN )) { throw new Exception('Bucket not found', 404); } @@ -1193,7 +1193,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/download') } } - if($bucket->getAttribute('permission') === 'bucket') { + if ($bucket->getAttribute('permission') === 'bucket') { $file = Authorization::skip(function() use ($dbForExternal, $fileId, $bucketId) { return $dbForExternal->getDocument('bucket_' . $bucketId, $fileId); }); @@ -1228,16 +1228,16 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/download') $size = $file->getAttribute('sizeOriginal', 0); $rangeHeader = $request->getHeader('range'); - if(!empty($rangeHeader)) { + if (!empty($rangeHeader)) { $start = $request->getRangeStart(); $end = $request->getRangeEnd(); $unit = $request->getRangeUnit(); - if($end == null) { + if ($end === null) { $end = min(($start + 2000000-1), ($size - 1)); } - if($unit != 'bytes' || $start >= $end || $end >= $size) { + if ($unit !== 'bytes' || $start >= $end || $end >= $size) { throw new Exception('Invalid range', 416); } @@ -1262,21 +1262,21 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/download') } if (!empty($file->getAttribute('algorithm', ''))) { - if(empty($source)) { + if (empty($source)) { $source = $device->read($path); } $compressor = new GZIP(); $source = $compressor->decompress($source); } - if(!empty($source)) { - if(!empty($rangeHeader)) { + if (!empty($source)) { + if (!empty($rangeHeader)) { $response->send(substr($source, $start, ($end - $start + 1))); } $response->send($source); } - if(!empty($rangeHeader)) { + if (!empty($rangeHeader)) { $response->send($device->read($path, $start, ($end - $start + 1))); } @@ -1318,7 +1318,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/view') $bucket = $dbForInternal->getDocument('buckets', $bucketId); - if($bucket->isEmpty() + if ($bucket->isEmpty() || (!$bucket->getAttribute('enabled') && $mode !== APP_MODE_ADMIN )) { throw new Exception('Bucket not found', 404); } @@ -1331,7 +1331,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/view') } } - if($bucket->getAttribute('permission') === 'bucket') { + if ($bucket->getAttribute('permission') === 'bucket') { $file = Authorization::skip(function() use ($dbForExternal, $fileId, $bucketId) { return $dbForExternal->getDocument('bucket_' . $bucketId, $fileId); }); @@ -1372,22 +1372,22 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/view') $size = $file->getAttribute('sizeOriginal', 0); $rangeHeader = $request->getHeader('range'); - if(!empty($rangeHeader)) { + if (!empty($rangeHeader)) { $start = $request->getRangeStart(); $end = $request->getRangeEnd(); $unit = $request->getRangeUnit(); - if($end == null) { + if ($end === null) { $end = min(($start + 2000000-1), ($size - 1)); } - if($unit != 'bytes' || $start >= $end || $end >= $size) { + if ($unit != 'bytes' || $start >= $end || $end >= $size) { throw new Exception('Invalid range', 416); } $response ->addHeader('Accept-Ranges', 'bytes') - ->addHeader('Content-Range', 'bytes ' . $start . '-' . $end . '/' . $size) + ->addHeader('Content-Range', "bytes $start-$end/$size") ->addHeader('Content-Length', $end - $start + 1) ->setStatusCode(Response::STATUS_CODE_PARTIALCONTENT); } @@ -1406,7 +1406,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/view') } if (!empty($file->getAttribute('algorithm', ''))) { - if(empty($source)) { + if (empty($source)) { $source = $device->read($path); } $compressor = new GZIP(); @@ -1418,14 +1418,14 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/view') ->setParam('bucketId', $bucketId) ; - if(!empty($source)) { - if(!empty($rangeHeader)) { + if (!empty($source)) { + if (!empty($rangeHeader)) { $response->send(substr($source, $start, ($end - $start + 1))); } $response->send($source); } - if(!empty($rangeHeader)) { + if (!empty($rangeHeader)) { $response->send($device->read($path, $start, ($end - $start + 1))); } @@ -1434,7 +1434,9 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/view') $response->addHeader('Content-Length', $device->getFileSize($path)); $chunk = 2000000; // Max chunk of 2 mb for ($i=0; $i < ceil($size / $chunk); $i++) { - $response->chunk($device->read($path, ($i * $chunk), min($chunk, $size - ($i * $chunk))), (($i + 1) * $chunk) >= $size); + $response->chunk( + $device->read($path, ($i * $chunk), + min($chunk, $size - ($i * $chunk))), (($i + 1) * $chunk) >= $size); } } else { $response->send($device->read($path)); @@ -1472,7 +1474,7 @@ App::put('/v1/storage/buckets/:bucketId/files/:fileId') $bucket = $dbForInternal->getDocument('buckets', $bucketId); - if($bucket->isEmpty() + if ($bucket->isEmpty() || (!$bucket->getAttribute('enabled') && $mode !== APP_MODE_ADMIN )) { throw new Exception('Bucket not found', 404); } @@ -1485,7 +1487,7 @@ App::put('/v1/storage/buckets/:bucketId/files/:fileId') } } - if($bucket->getAttribute('permission') === 'bucket') { + if ($bucket->getAttribute('permission') === 'bucket') { $file = Authorization::skip(function() use ($dbForExternal, $fileId, $bucketId) { return $dbForExternal->getDocument('bucket_' . $bucketId, $fileId); }); @@ -1497,7 +1499,7 @@ App::put('/v1/storage/buckets/:bucketId/files/:fileId') throw new Exception('File not found', 404); } - if($bucket->getAttribute('permission') === 'bucket') { + if ($bucket->getAttribute('permission') === 'bucket') { $file = Authorization::skip(function() use ($dbForExternal, $fileId, $bucketId, $file, $read, $write) { return $dbForExternal->updateDocument('bucket_' . $bucketId, $fileId, $file ->setAttribute('$read', $read) @@ -1555,7 +1557,7 @@ App::delete('/v1/storage/buckets/:bucketId/files/:fileId') $bucket = $dbForInternal->getDocument('buckets', $bucketId); - if($bucket->isEmpty() + if ($bucket->isEmpty() || (!$bucket->getAttribute('enabled') && $mode !== APP_MODE_ADMIN )) { throw new Exception('Bucket not found', 404); } @@ -1568,7 +1570,7 @@ App::delete('/v1/storage/buckets/:bucketId/files/:fileId') } } - if($bucket->getAttribute('permission') === 'bucket') { + if ($bucket->getAttribute('permission') === 'bucket') { $file = Authorization::skip(function() use ($dbForExternal, $fileId, $bucketId) { return $dbForExternal->getDocument('bucket_' . $bucketId, $fileId); }); @@ -1583,7 +1585,7 @@ App::delete('/v1/storage/buckets/:bucketId/files/:fileId') $device = Storage::getDevice('files'); if ($device->delete($file->getAttribute('path', ''))) { - if($bucket->getAttribute('permission') === 'bucket') { + if ($bucket->getAttribute('permission') === 'bucket') { $deleted = Authorization::skip(function() use ($dbForExternal, $fileId, $bucketId) { return $dbForExternal->deleteDocument('bucket_' . $bucketId, $fileId); }); @@ -1746,7 +1748,7 @@ App::get('/v1/storage/:bucketId/usage') $bucket = $dbForInternal->getDocument('buckets', $bucketId); - if($bucket->isEmpty()) { + if ($bucket->isEmpty()) { throw new Exception('Bucket not found', 404); } From 6bc7825089288e06b6131981de1cbd4a21933ed8 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Fri, 10 Dec 2021 16:14:59 +0545 Subject: [PATCH 4/4] fix hardcoded chunk size with a const --- app/controllers/api/storage.php | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/app/controllers/api/storage.php b/app/controllers/api/storage.php index 7c35df3f90..5743109f11 100644 --- a/app/controllers/api/storage.php +++ b/app/controllers/api/storage.php @@ -1234,7 +1234,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/download') $unit = $request->getRangeUnit(); if ($end === null) { - $end = min(($start + 2000000-1), ($size - 1)); + $end = min(($start + MAX_OUTPUT_CHUNK_SIZE-1), ($size - 1)); } if ($unit !== 'bytes' || $start >= $end || $end >= $size) { @@ -1283,7 +1283,14 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/download') if ($size > APP_STORAGE_READ_BUFFER) { $response->addHeader('Content-Length', $device->getFileSize($path)); for ($i=0; $i < ceil($size / MAX_OUTPUT_CHUNK_SIZE); $i++) { - $response->chunk($device->read($path, ($i * MAX_OUTPUT_CHUNK_SIZE), min(MAX_OUTPUT_CHUNK_SIZE, $size - ($i * MAX_OUTPUT_CHUNK_SIZE))), (($i + 1) * MAX_OUTPUT_CHUNK_SIZE) >= $size); + $response->chunk( + $device->read( + $path, + ($i * MAX_OUTPUT_CHUNK_SIZE), + min(MAX_OUTPUT_CHUNK_SIZE, $size - ($i * MAX_OUTPUT_CHUNK_SIZE)) + ), + (($i + 1) * MAX_OUTPUT_CHUNK_SIZE) >= $size + ); } } else { $response->send($device->read($path)); @@ -1432,11 +1439,15 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/view') $size = $device->getFileSize($path); if ($size > APP_STORAGE_READ_BUFFER) { $response->addHeader('Content-Length', $device->getFileSize($path)); - $chunk = 2000000; // Max chunk of 2 mb - for ($i=0; $i < ceil($size / $chunk); $i++) { + for ($i=0; $i < ceil($size / MAX_OUTPUT_CHUNK_SIZE); $i++) { $response->chunk( - $device->read($path, ($i * $chunk), - min($chunk, $size - ($i * $chunk))), (($i + 1) * $chunk) >= $size); + $device->read( + $path, + ($i * MAX_OUTPUT_CHUNK_SIZE), + min(MAX_OUTPUT_CHUNK_SIZE, $size - ($i * MAX_OUTPUT_CHUNK_SIZE)) + ), + (($i + 1) * MAX_OUTPUT_CHUNK_SIZE) >= $size + ); } } else { $response->send($device->read($path));