Skip to content

Commit a44e74f

Browse files
AhsanIsEpicAhsanIsEpic
committed
Fix: Implement better filename sanitization utility for Google Drive and Photos services using IFilenameValidator
Signed-off-by: Ahsan Ahmed <ahmedah05@outlook.com>
1 parent d3a81f2 commit a44e74f

3 files changed

Lines changed: 140 additions & 4 deletions

File tree

‎lib/Service/GoogleDriveAPIService.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use OC\User\NoUserException;
1818
use OCA\Google\AppInfo\Application;
1919
use OCA\Google\BackgroundJob\ImportDriveJob;
20+
use OCA\Google\Service\Utils\FileUtils;
2021
use OCP\BackgroundJob\IJobList;
2122
use OCP\Files\File;
2223
use OCP\Files\Folder;
@@ -540,7 +541,7 @@ private function createDirsUnder(array &$directoriesById, Folder $currentFolder,
540541
// create dir if we are on top OR if its parent is current dir
541542
if (($currentFolderId === '' && !array_key_exists($parentId, $directoriesById))
542543
|| $parentId === $currentFolderId) {
543-
$name = preg_replace('/[\/?<>\\:*|"]/', '-', trim((string)($dir['name'] ?? 'Untitled')));
544+
$name = FileUtils::sanitizeFilename((string)($dir['name']), $id, $this->logger);
544545
if (!$currentFolder->nodeExists($name)) {
545546
$newDir = $currentFolder->newFolder($name);
546547
} else {
@@ -623,7 +624,7 @@ private function downloadAndSaveFile(
623624
* @return string name of the file to be saved
624625
*/
625626
private function getFileName(array $fileItem, string $userId, bool $hasNameConflict): string {
626-
$fileName = preg_replace('/[\/?<>\\:*|"]/', '-', trim((string)($fileItem['name'] ?? 'Untitled')));
627+
$fileName = FileUtils::sanitizeFilename((string)($fileItem['name']), $fileItem['id'], $this->logger);
627628

628629
if (in_array($fileItem['mimeType'], array_values(self::DOCUMENT_MIME_TYPES))) {
629630
$documentFormat = $this->getUserDocumentFormat($userId);

‎lib/Service/GooglePhotosAPIService.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Exception;
1717
use OCA\Google\AppInfo\Application;
1818
use OCA\Google\BackgroundJob\ImportPhotosJob;
19+
use OCA\Google\Service\Utils\FileUtils;
1920
use OCP\BackgroundJob\IJobList;
2021
use OCP\Files\FileInfo;
2122
use OCP\Files\Folder;
@@ -277,7 +278,7 @@ public function importPhotos(
277278
$seenIds = [];
278279
foreach ($albums as $album) {
279280
$albumId = $album['id'];
280-
$albumName = preg_replace('/[\/?<>\\:*|"]/', '-', trim((string)($album['title'] ?? 'Untitled')));
281+
$albumName = FileUtils::sanitizeFilename((string)($album['title']), $album['id'], $this->logger);
281282
if (!$folder->nodeExists($albumName)) {
282283
$albumFolder = $folder->newFolder($albumName);
283284
} else {
@@ -372,7 +373,7 @@ public function importPhotos(
372373
* @throws \OCP\Files\NotPermittedException
373374
*/
374375
private function getPhoto(string $userId, array $photo, Folder $albumFolder): ?int {
375-
$photoName = preg_replace('/[\/?<>\\:*|"]/', '-', trim((string)($photo['filename'] ?? 'Untitled')));
376+
$photoName = FileUtils::sanitizeFilename((string)($photo['filename']), $photo['id'], $this->logger);
376377
if ($albumFolder->nodeExists($photoName)) {
377378
$photoName = $photo['id'] . '_' . $photoName;
378379
}

‎lib/Service/Utils/FileUtils.php‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
<?php
2+
3+
namespace OCA\Google\Service\Utils;
4+
5+
use OCP\Files\FileNameTooLongException;
6+
use OCP\Files\EmptyFileNameException;
7+
use OCP\Files\InvalidCharacterInPathException;
8+
use OCP\Files\InvalidDirectoryException;
9+
use OCP\Files\ReservedWordException;
10+
use OCP\Files\InvalidPathException;
11+
use Psr\Log\LoggerInterface;
12+
use OC;
13+
14+
class FileUtils {
15+
16+
/**
17+
* Sanitize the filename to ensure it is valid, does not exceed length limits.
18+
*
19+
* @param string $filename The original filename to sanitize.
20+
* @param string $id A unique ID to append if necessary to ensure uniqueness.
21+
* @param int $recursionDepth The current recursion depth (used to prevent infinite loops).
22+
* @return string The sanitized and validated filename.
23+
*/
24+
public static function sanitizeFilename(
25+
string $filename,
26+
string $id,
27+
LoggerInterface $logger,
28+
int $recursionDepth = 0,
29+
string $originalFilename = null
30+
): string {
31+
// Prevent infinite recursion by limiting the depth.
32+
if ($recursionDepth > 15) {
33+
$filename = 'Untitled_' . $id;
34+
$logger->warning('Maximum recursion depth reached while sanitizing filename: ' . $originalFilename . ' renaming to ' . $filename);
35+
return $filename;
36+
}
37+
38+
// If the original filename is not provided, use the current filename.
39+
if ($originalFilename === null) {
40+
$originalFilename = $filename;
41+
}
42+
43+
// Trim leading/trailing whitespace and trailing dots.
44+
$filename = rtrim(trim($filename), '.');
45+
46+
// Check if trimming altered the filename.
47+
$trimmed = ($originalFilename !== $filename);
48+
49+
// Helper function to append the ID before the file extension.
50+
$appendIdBeforeExtension = function ($filename, $id) {
51+
$pathInfo = pathinfo($filename);
52+
if (isset($pathInfo['extension'])) {
53+
return $pathInfo['filename'] . '_' . $id . '.' . $pathInfo['extension'];
54+
} else {
55+
return $filename . '_' . $id;
56+
}
57+
};
58+
59+
// Append the ID if trimming occurred and the ID is not already present.
60+
if ($trimmed && !str_contains($filename, $id)) {
61+
$filename = $appendIdBeforeExtension($filename, $id);
62+
}
63+
64+
// Ensure the filename length does not exceed the maximum allowed length.
65+
$maxLength = 254;
66+
if (mb_strlen($filename) > $maxLength) {
67+
$pathInfo = pathinfo($filename);
68+
$baseLength = $maxLength - mb_strlen($id) - 2; // Account for '_' and '.'.
69+
if (isset($pathInfo['extension'])) {
70+
$baseLength -= mb_strlen($pathInfo['extension']);
71+
$filename = mb_substr($pathInfo['filename'], 0, $baseLength) . '_' . $id . '.' . $pathInfo['extension'];
72+
} else {
73+
$filename = mb_substr($filename, 0, $baseLength) . '_' . $id;
74+
}
75+
}
76+
77+
try {
78+
// Validate the filename using the Nextcloud filename validator.
79+
\OC::$server->get(\OCP\Files\IFilenameValidator::class)->validateFilename($filename);
80+
81+
// if recursion depth is greater than 0, log the change.
82+
if ($recursionDepth > 0) {
83+
$logger->info('Filename sanitized successfully: "' . $filename . '" (original: "' . $originalFilename . '")');
84+
}
85+
86+
return $filename;
87+
} catch (InvalidPathException $exception) {
88+
$logger->warning('Invalid filename detected during sanitization: ' . $filename, ['exception' => $exception]);
89+
}
90+
91+
// Handle specific exceptions and adjust the filename accordingly.
92+
switch (true) {
93+
case $exception instanceof FileNameTooLongException:
94+
$filename = mb_substr($filename, 0, $maxLength - mb_strlen($id) - 2);
95+
break;
96+
97+
case $exception instanceof EmptyFileNameException:
98+
$filename = 'Untitled';
99+
break;
100+
101+
case $exception instanceof InvalidCharacterInPathException:
102+
if (preg_match('/"(.*?)"/', $exception->getMessage(), $matches)) {
103+
$invalidChars = array_merge(str_split($matches[1]), ['"']);
104+
$filename = str_replace($invalidChars, '-', $filename);
105+
}
106+
break;
107+
108+
case $exception instanceof InvalidDirectoryException:
109+
$logger->error('Invalid directory detected in filename: ' . $exception->getMessage());
110+
$filename = 'Untitled';
111+
break;
112+
113+
case $exception instanceof ReservedWordException:
114+
if (preg_match('/"(.*?)"/', $exception->getMessage(), $matches)) {
115+
$reservedWord = $matches[1];
116+
$filename = str_ireplace($reservedWord, '-' . $reservedWord . '-', $filename);
117+
}
118+
break;
119+
120+
default:
121+
$logger->error('Unknown exception encountered during filename sanitization: ' . $filename);
122+
$filename = 'Untitled';
123+
break;
124+
}
125+
126+
// Append the ID if the filename was modified and does not already contain the ID.
127+
if (!str_contains($filename, $id)) {
128+
$filename = $appendIdBeforeExtension($filename, $id);
129+
}
130+
131+
// Recursively validate the adjusted filename.
132+
return self::sanitizeFilename($filename, $id, $logger, $recursionDepth + 1, $originalFilename);
133+
}
134+
}

0 commit comments

Comments
 (0)