From f7db8e4d04ab14e94a68cfa284a4bf0afb273dc2 Mon Sep 17 00:00:00 2001 From: Petar Jakopec Date: Mon, 2 Feb 2026 17:35:45 +0100 Subject: [PATCH 1/6] SPLAT-4180 if pdf is protected set resource type to raw --- bundle/Controller/Resource/Upload.php | 46 ++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/bundle/Controller/Resource/Upload.php b/bundle/Controller/Resource/Upload.php index 38efe578..3bd3e14f 100644 --- a/bundle/Controller/Resource/Upload.php +++ b/bundle/Controller/Resource/Upload.php @@ -18,9 +18,19 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Contracts\Translation\TranslatorInterface; +use function fclose; +use function fopen; +use function fread; +use function fseek; use function implode; use function in_array; use function is_array; +use function is_file; +use function is_readable; +use function strpos; +use function strtolower; + +use const SEEK_END; final class Upload extends AbstractController { @@ -88,9 +98,11 @@ public function __invoke(Request $request): Response $md5 = $this->fileHashFactory->createHash($file->getRealPath()); $fileStruct = FileStruct::fromUploadedFile($file); + + $resourceType = $this->isEncryptedPdf($file) ? 'raw' : 'auto'; $resourceStruct = new ResourceStruct( $fileStruct, - 'auto', + $resourceType, $folder, $visibility, $request->request->get('filename'), @@ -113,4 +125,36 @@ public function __invoke(Request $request): Response return new JsonResponse($this->formatResource($resource), $httpCode); } + + private function isEncryptedPdf(UploadedFile $file): bool + { + if (strtolower((string) $file->getClientOriginalExtension()) !== 'pdf') { + return false; + } + + $path = (string) $file->getRealPath(); + if ($path === '' || !is_file($path) || !is_readable($path)) { + return false; + } + + $fp = @fopen($path, 'r'); + if ($fp === false) { + return false; + } + + $head = (string) fread($fp, 4096); + + // Encryption marker may be located anywhere; read also from the end. + // Suppress fseek errors (e.g. very small files). + @fseek($fp, -16384, SEEK_END); + $tail = (string) fread($fp, 16384); + + fclose($fp); + + if (strpos($head, '%PDF-') !== 0) { + return false; + } + + return strpos($head . $tail, '/Encrypt') !== false; + } } From d84235083fe6b9de2fe5ea2af2924b2fca5d0a68 Mon Sep 17 00:00:00 2001 From: Petar Jakopec Date: Mon, 2 Feb 2026 17:36:10 +0100 Subject: [PATCH 2/6] SPLAT-4180 update tests --- .../bundle/Controller/Resource/UploadTest.php | 99 ++++++++++++++++++- 1 file changed, 95 insertions(+), 4 deletions(-) diff --git a/tests/bundle/Controller/Resource/UploadTest.php b/tests/bundle/Controller/Resource/UploadTest.php index 50a2ed59..c786651f 100644 --- a/tests/bundle/Controller/Resource/UploadTest.php +++ b/tests/bundle/Controller/Resource/UploadTest.php @@ -27,7 +27,11 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Contracts\Translation\TranslatorInterface; +use function file_put_contents; use function json_encode; +use function sys_get_temp_dir; +use function tempnam; +use function unlink; #[CoversClass(UploadController::class)] #[CoversClass(AbstractController::class)] @@ -78,7 +82,7 @@ public function testUpload(): void ->willReturn('sample_image'); $uploadedFileMock - ->expects(self::exactly(2)) + ->expects(self::exactly(3)) ->method('getClientOriginalExtension') ->willReturn('jpg'); @@ -179,6 +183,93 @@ public function testUpload(): void ); } + public function testUploadEncryptedPdfIsRaw(): void + { + $tmpPdfPath = (string) tempnam(sys_get_temp_dir(), 'ngrm_encrypted_pdf_'); + file_put_contents($tmpPdfPath, "%PDF-1.7\n1 0 obj\n<< /Encrypt 2 0 R >>\nendobj\n"); + + $request = new Request(); + $request->request->add([ + 'folder' => 'media/document', + ]); + + $uploadedFileMock = $this->createMock(UploadedFile::class); + + $uploadedFileMock + ->expects(self::once()) + ->method('isFile') + ->willReturn(true); + + // getRealPath is used for md5 hash + FileStruct + encryption detector + $uploadedFileMock + ->expects(self::exactly(4)) + ->method('getRealPath') + ->willReturn($tmpPdfPath); + + $uploadedFileMock + ->expects(self::exactly(2)) + ->method('getClientOriginalName') + ->willReturn('protected.pdf'); + + // getClientOriginalExtension is used for FileStruct + encryption detector + $uploadedFileMock + ->expects(self::exactly(3)) + ->method('getClientOriginalExtension') + ->willReturn('pdf'); + + $request->files->add([ + 'file' => $uploadedFileMock, + ]); + + $this->fileHashFactoryMock + ->expects(self::once()) + ->method('createHash') + ->with($tmpPdfPath) + ->willReturn('md5hash'); + + $fileStruct = FileStruct::fromUploadedFile($uploadedFileMock); + + $resourceStruct = new ResourceStruct( + $fileStruct, + 'raw', + Folder::fromPath('media/document'), + 'public', + $request->request->get('filename'), + ); + + $resource = new RemoteResource( + remoteId: 'upload|raw|media/document/protected.pdf', + type: 'raw', + url: 'https://cloudinary.com/test/upload/raw/media/document/protected.pdf', + md5: 'md5hash', + name: 'protected.pdf', + folder: Folder::fromPath('media/document'), + size: 123, + ); + + $this->providerMock + ->expects(self::once()) + ->method('upload') + ->with($resourceStruct) + ->willReturn($resource); + + $this->providerMock + ->expects(self::exactly(0)) + ->method('buildVariation') + ->willReturnCallback( + static fn () => new RemoteResourceVariation( + $resource, + 'https://cloudinary.com/test/variation/url', + ), + ); + + $response = $this->controller->__invoke($request); + + self::assertInstanceOf(JsonResponse::class, $response); + + unlink($tmpPdfPath); + } + public function testUploadProtectedWithContext(): void { $uploadContext = [ @@ -211,7 +302,7 @@ public function testUploadProtectedWithContext(): void ->willReturn('sample_image'); $uploadedFileMock - ->expects(self::exactly(2)) + ->expects(self::exactly(3)) ->method('getClientOriginalExtension') ->willReturn('jpg'); @@ -396,7 +487,7 @@ public function testUploadExistingFile(): void ->willReturn('sample_image'); $uploadedFileMock - ->expects(self::exactly(2)) + ->expects(self::exactly(3)) ->method('getClientOriginalExtension') ->willReturn('jpg'); @@ -555,7 +646,7 @@ public function testUploadExistingFileName(): void ->willReturn('sample_image'); $uploadedFileMock - ->expects(self::exactly(2)) + ->expects(self::exactly(3)) ->method('getClientOriginalExtension') ->willReturn('jpg'); From 24eb4e7462b0eff24c50013f5086d1603eee04a7 Mon Sep 17 00:00:00 2001 From: Petar Jakopec Date: Tue, 3 Feb 2026 09:59:41 +0100 Subject: [PATCH 3/6] SPLAT-4180 phpstan fix --- bundle/Controller/Resource/Upload.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bundle/Controller/Resource/Upload.php b/bundle/Controller/Resource/Upload.php index 3bd3e14f..a26dc89a 100644 --- a/bundle/Controller/Resource/Upload.php +++ b/bundle/Controller/Resource/Upload.php @@ -128,7 +128,7 @@ public function __invoke(Request $request): Response private function isEncryptedPdf(UploadedFile $file): bool { - if (strtolower((string) $file->getClientOriginalExtension()) !== 'pdf') { + if (strtolower($file->getClientOriginalExtension()) !== 'pdf') { return false; } @@ -144,8 +144,6 @@ private function isEncryptedPdf(UploadedFile $file): bool $head = (string) fread($fp, 4096); - // Encryption marker may be located anywhere; read also from the end. - // Suppress fseek errors (e.g. very small files). @fseek($fp, -16384, SEEK_END); $tail = (string) fread($fp, 16384); From 0af396fb6e9a42ccb60c00b94f61991cf8a4ede4 Mon Sep 17 00:00:00 2001 From: Petar Jakopec Date: Thu, 12 Feb 2026 09:31:41 +0100 Subject: [PATCH 4/6] SPLAT-4180 additional checks for encrypted pdfs --- bundle/Controller/Resource/Upload.php | 33 +++- .../bundle/Controller/Resource/UploadTest.php | 172 ++++++++++++++++++ 2 files changed, 200 insertions(+), 5 deletions(-) diff --git a/bundle/Controller/Resource/Upload.php b/bundle/Controller/Resource/Upload.php index a26dc89a..b01b2aad 100644 --- a/bundle/Controller/Resource/Upload.php +++ b/bundle/Controller/Resource/Upload.php @@ -27,8 +27,12 @@ use function is_array; use function is_file; use function is_readable; +use function filesize; +use function preg_match; +use function strrpos; use function strpos; use function strtolower; +use function substr; use const SEEK_END; @@ -142,17 +146,36 @@ private function isEncryptedPdf(UploadedFile $file): bool return false; } - $head = (string) fread($fp, 4096); + $fileSize = filesize($path); - @fseek($fp, -16384, SEEK_END); - $tail = (string) fread($fp, 16384); + // For small PDFs, `head` and `tail` reads can overlap and even fully duplicate the file contents. + // This can re-introduce false positives (e.g. `/Encrypt` in a trailing comment after `%%EOF`). + // In that case, scan the full content once. + if ($fileSize !== false && $fileSize <= 20480) { + $content = (string) fread($fp, $fileSize); + } else { + $head = (string) fread($fp, 4096); + + @fseek($fp, -16384, SEEK_END); + $tail = (string) fread($fp, 16384); + + $content = $head . $tail; + } fclose($fp); - if (strpos($head, '%PDF-') !== 0) { + if (strpos($content, '%PDF-') !== 0) { return false; } - return strpos($head . $tail, '/Encrypt') !== false; + // Ignore anything after the last EOF marker (some tools append non-PDF comments/metadata). + $eofPos = strrpos($content, '%%EOF'); + if ($eofPos !== false) { + $content = substr($content, 0, $eofPos + 5); + } + + // Detect presence of encryption dictionary reference in PDF object context. + // This avoids false positives where `/Encrypt` appears in metadata or trailing comments. + return (bool) preg_match('/\/(?:Encrypt)\s+(\d+|<<)/m', $content); } } diff --git a/tests/bundle/Controller/Resource/UploadTest.php b/tests/bundle/Controller/Resource/UploadTest.php index c786651f..e72db1e3 100644 --- a/tests/bundle/Controller/Resource/UploadTest.php +++ b/tests/bundle/Controller/Resource/UploadTest.php @@ -270,6 +270,178 @@ public function testUploadEncryptedPdfIsRaw(): void unlink($tmpPdfPath); } + public function testUploadPdfWithEncryptInMetadataIsAuto(): void + { + $tmpPdfPath = (string) tempnam(sys_get_temp_dir(), 'ngrm_unencrypted_pdf_metadata_'); + file_put_contents( + $tmpPdfPath, + "%PDF-1.7\n1 0 obj\n<< /Type /Catalog >>\nendobj\n" . + "/Title (/Encrypt)\n" . + "%%EOF\n", + ); + + $request = new Request(); + $request->request->add([ + 'folder' => 'media/document', + ]); + + $uploadedFileMock = $this->createMock(UploadedFile::class); + + $uploadedFileMock + ->expects(self::once()) + ->method('isFile') + ->willReturn(true); + + // getRealPath is used for md5 hash + FileStruct + encryption detector + $uploadedFileMock + ->expects(self::exactly(4)) + ->method('getRealPath') + ->willReturn($tmpPdfPath); + + $uploadedFileMock + ->expects(self::exactly(2)) + ->method('getClientOriginalName') + ->willReturn('unencrypted.pdf'); + + // getClientOriginalExtension is used for FileStruct + encryption detector + $uploadedFileMock + ->expects(self::exactly(3)) + ->method('getClientOriginalExtension') + ->willReturn('pdf'); + + $request->files->add([ + 'file' => $uploadedFileMock, + ]); + + $this->fileHashFactoryMock + ->expects(self::once()) + ->method('createHash') + ->with($tmpPdfPath) + ->willReturn('md5hash'); + + $fileStruct = FileStruct::fromUploadedFile($uploadedFileMock); + + $resourceStruct = new ResourceStruct( + $fileStruct, + 'auto', + Folder::fromPath('media/document'), + 'public', + $request->request->get('filename'), + ); + + $resource = new RemoteResource( + remoteId: 'upload|auto|media/document/unencrypted.pdf', + type: 'auto', + url: 'https://cloudinary.com/test/upload/auto/media/document/unencrypted.pdf', + md5: 'md5hash', + name: 'unencrypted.pdf', + folder: Folder::fromPath('media/document'), + size: 123, + ); + + $this->providerMock + ->expects(self::once()) + ->method('upload') + ->with($resourceStruct) + ->willReturn($resource); + + $this->providerMock + ->expects(self::exactly(0)) + ->method('buildVariation'); + + $response = $this->controller->__invoke($request); + + self::assertInstanceOf(JsonResponse::class, $response); + + unlink($tmpPdfPath); + } + + public function testUploadPdfWithEncryptInTrailingCommentAfterEofIsAuto(): void + { + $tmpPdfPath = (string) tempnam(sys_get_temp_dir(), 'ngrm_unencrypted_pdf_comment_'); + file_put_contents( + $tmpPdfPath, + "%PDF-1.7\n1 0 obj\n<< /Type /Catalog >>\nendobj\n" . + "%%EOF\n" . + "% /Encrypt 2 0 R\n", + ); + + $request = new Request(); + $request->request->add([ + 'folder' => 'media/document', + ]); + + $uploadedFileMock = $this->createMock(UploadedFile::class); + + $uploadedFileMock + ->expects(self::once()) + ->method('isFile') + ->willReturn(true); + + // getRealPath is used for md5 hash + FileStruct + encryption detector + $uploadedFileMock + ->expects(self::exactly(4)) + ->method('getRealPath') + ->willReturn($tmpPdfPath); + + $uploadedFileMock + ->expects(self::exactly(2)) + ->method('getClientOriginalName') + ->willReturn('unencrypted.pdf'); + + // getClientOriginalExtension is used for FileStruct + encryption detector + $uploadedFileMock + ->expects(self::exactly(3)) + ->method('getClientOriginalExtension') + ->willReturn('pdf'); + + $request->files->add([ + 'file' => $uploadedFileMock, + ]); + + $this->fileHashFactoryMock + ->expects(self::once()) + ->method('createHash') + ->with($tmpPdfPath) + ->willReturn('md5hash'); + + $fileStruct = FileStruct::fromUploadedFile($uploadedFileMock); + + $resourceStruct = new ResourceStruct( + $fileStruct, + 'auto', + Folder::fromPath('media/document'), + 'public', + $request->request->get('filename'), + ); + + $resource = new RemoteResource( + remoteId: 'upload|auto|media/document/unencrypted.pdf', + type: 'auto', + url: 'https://cloudinary.com/test/upload/auto/media/document/unencrypted.pdf', + md5: 'md5hash', + name: 'unencrypted.pdf', + folder: Folder::fromPath('media/document'), + size: 123, + ); + + $this->providerMock + ->expects(self::once()) + ->method('upload') + ->with($resourceStruct) + ->willReturn($resource); + + $this->providerMock + ->expects(self::exactly(0)) + ->method('buildVariation'); + + $response = $this->controller->__invoke($request); + + self::assertInstanceOf(JsonResponse::class, $response); + + unlink($tmpPdfPath); + } + public function testUploadProtectedWithContext(): void { $uploadContext = [ From d25756e06550864bd15fa7411536cf86f3f2bbc9 Mon Sep 17 00:00:00 2001 From: Petar Jakopec Date: Thu, 12 Feb 2026 10:28:15 +0100 Subject: [PATCH 5/6] SPLAT-4180 remove comments --- bundle/Controller/Resource/Upload.php | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/bundle/Controller/Resource/Upload.php b/bundle/Controller/Resource/Upload.php index b01b2aad..052dbb51 100644 --- a/bundle/Controller/Resource/Upload.php +++ b/bundle/Controller/Resource/Upload.php @@ -19,6 +19,7 @@ use Symfony\Contracts\Translation\TranslatorInterface; use function fclose; +use function filesize; use function fopen; use function fread; use function fseek; @@ -27,10 +28,9 @@ use function is_array; use function is_file; use function is_readable; -use function filesize; use function preg_match; -use function strrpos; use function strpos; +use function strrpos; use function strtolower; use function substr; @@ -148,9 +148,6 @@ private function isEncryptedPdf(UploadedFile $file): bool $fileSize = filesize($path); - // For small PDFs, `head` and `tail` reads can overlap and even fully duplicate the file contents. - // This can re-introduce false positives (e.g. `/Encrypt` in a trailing comment after `%%EOF`). - // In that case, scan the full content once. if ($fileSize !== false && $fileSize <= 20480) { $content = (string) fread($fp, $fileSize); } else { @@ -168,14 +165,11 @@ private function isEncryptedPdf(UploadedFile $file): bool return false; } - // Ignore anything after the last EOF marker (some tools append non-PDF comments/metadata). $eofPos = strrpos($content, '%%EOF'); if ($eofPos !== false) { $content = substr($content, 0, $eofPos + 5); } - // Detect presence of encryption dictionary reference in PDF object context. - // This avoids false positives where `/Encrypt` appears in metadata or trailing comments. return (bool) preg_match('/\/(?:Encrypt)\s+(\d+|<<)/m', $content); } } From a4021da6c633434c0b44a1f72dbcdcb3bc6f618a Mon Sep 17 00:00:00 2001 From: Petar Jakopec Date: Thu, 12 Feb 2026 10:31:17 +0100 Subject: [PATCH 6/6] SPLAT-4180 run php-cs-fixer for tests --- tests/bundle/Controller/Resource/UploadTest.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/bundle/Controller/Resource/UploadTest.php b/tests/bundle/Controller/Resource/UploadTest.php index e72db1e3..1d406cb5 100644 --- a/tests/bundle/Controller/Resource/UploadTest.php +++ b/tests/bundle/Controller/Resource/UploadTest.php @@ -275,9 +275,9 @@ public function testUploadPdfWithEncryptInMetadataIsAuto(): void $tmpPdfPath = (string) tempnam(sys_get_temp_dir(), 'ngrm_unencrypted_pdf_metadata_'); file_put_contents( $tmpPdfPath, - "%PDF-1.7\n1 0 obj\n<< /Type /Catalog >>\nendobj\n" . - "/Title (/Encrypt)\n" . - "%%EOF\n", + "%PDF-1.7\n1 0 obj\n<< /Type /Catalog >>\nendobj\n" + . "/Title (/Encrypt)\n" + . "%%EOF\n", ); $request = new Request(); @@ -361,9 +361,9 @@ public function testUploadPdfWithEncryptInTrailingCommentAfterEofIsAuto(): void $tmpPdfPath = (string) tempnam(sys_get_temp_dir(), 'ngrm_unencrypted_pdf_comment_'); file_put_contents( $tmpPdfPath, - "%PDF-1.7\n1 0 obj\n<< /Type /Catalog >>\nendobj\n" . - "%%EOF\n" . - "% /Encrypt 2 0 R\n", + "%PDF-1.7\n1 0 obj\n<< /Type /Catalog >>\nendobj\n" + . "%%EOF\n" + . "% /Encrypt 2 0 R\n", ); $request = new Request();