Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable29] fix: make sure we process mime extensions as string #50703

Open
wants to merge 2 commits into
base: stable29
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ class GenerateMimetypeFileBuilder {
public function generateFile(array $aliases): string {
// Remove comments
$aliases = array_filter($aliases, static function ($key) {
// Single digit extensions will be treated as integers
// Let's make sure they are strings
// https://github.com/nextcloud/server/issues/42902
$key = (string)$key;
return !($key === '' || $key[0] === '_');
}, ARRAY_FILTER_USE_KEY);

Expand Down
4 changes: 4 additions & 0 deletions core/Command/Maintenance/Mimetype/UpdateDB.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$totalNewMimetypes = 0;

foreach ($mappings as $ext => $mimetypes) {
// Single digit extensions will be treated as integers
// Let's make sure they are strings
// https://github.com/nextcloud/server/issues/42902
$ext = (string)$ext;
if ($ext[0] === '_') {
// comment
continue;
Expand Down
14 changes: 10 additions & 4 deletions lib/private/Files/Type/Detection.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ public function __construct(IURLGenerator $urlGenerator,
public function registerType(string $extension,
string $mimetype,
?string $secureMimeType = null): void {
// Make sure the extension is a string
// https://github.com/nextcloud/server/issues/42902
$this->mimetypes[$extension] = [$mimetype, $secureMimeType];
$this->secureMimeTypes[$mimetype] = $secureMimeType ?: $mimetype;
}
Expand All @@ -112,13 +114,17 @@ public function registerType(string $extension,
* @param array $types
*/
public function registerTypeArray(array $types): void {
$this->mimetypes = array_merge($this->mimetypes, $types);
// Register the types,
foreach ($types as $extension => $mimeType) {
$this->registerType((string)$extension, $mimeType[0], $mimeType[1] ?? null);
}

// Update the alternative mimetypes to avoid having to look them up each time.
foreach ($this->mimetypes as $extension => $mimeType) {
if (str_starts_with($extension, '_comment')) {
if (str_starts_with((string)$extension, '_comment')) {
continue;
}

$this->secureMimeTypes[$mimeType[0]] = $mimeType[1] ?? $mimeType[0];
if (isset($mimeType[1])) {
$this->secureMimeTypes[$mimeType[1]] = $mimeType[1];
Expand Down Expand Up @@ -179,7 +185,7 @@ private function loadMappings(): void {
}

/**
* @return array
* @return array<string, list{string, string|null}>
*/
public function getAllMappings(): array {
$this->loadMappings();
Expand Down Expand Up @@ -209,7 +215,7 @@ public function detectPath($path): string {
$extension = strrchr($fileName, '.');
if ($extension !== false) {
$extension = strtolower($extension);
$extension = substr($extension, 1); //remove leading .
$extension = substr($extension, 1); // remove leading .
return $this->mimetypes[$extension][0] ?? 'application/octet-stream';
}
}
Expand Down
6 changes: 6 additions & 0 deletions lib/public/Files/IMimeTypeDetector.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,10 @@ public function mimeTypeIcon($mimeType);
* @since 28.0.0
*/
public function getAllAliases(): array;

/**
* @return array<string, list{string, string|null}>
* @since 32.0.0
*/
public function getAllMappings(): array;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it needed to make this public also in backports?
If yes then the since is missleading and should be 29.0.13, otherwise drop it from the public interface

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it's not, I'll adjust later then 👍

}
10 changes: 10 additions & 0 deletions lib/public/Files/IMimeTypeLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,14 @@ public function exists(string $mimetype): bool;
* @since 8.2.0
*/
public function reset(): void;

/**
* Update filecache mimetype based on file extension
*
* @param string $ext
* @param int $mimeTypeId
* @return int
* @since 32.0.0
*/
public function updateFilecache(string $ext, int $mimeTypeId): int;
}
35 changes: 35 additions & 0 deletions tests/lib/Files/Type/DetectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,41 @@ public function testDetectString(): void {
$this->assertEquals($expected, $result);
}

public function dataMimeTypeCustom(): array {
return [
['123', 'foobar/123'],
['a123', 'foobar/123'],
['bar', 'foobar/bar'],
];
}

/**
* @dataProvider dataMimeTypeCustom
*
* @param string $ext
* @param string $mime
*/
public function testDetectMimeTypeCustom(string $ext, string $mime): void {
$confDir = sys_get_temp_dir();
file_put_contents($confDir . '/mimetypemapping.dist.json', json_encode([]));

/** @var IURLGenerator $urlGenerator */
$urlGenerator = $this->getMockBuilder(IURLGenerator::class)
->disableOriginalConstructor()
->getMock();

/** @var LoggerInterface $logger */
$logger = $this->createMock(LoggerInterface::class);

// Create new mapping file
file_put_contents($confDir . '/mimetypemapping.dist.json', json_encode([$ext => [$mime]]));

$detection = new Detection($urlGenerator, $logger, $confDir, $confDir);
$mappings = $detection->getAllMappings();
$this->assertArrayHasKey($ext, $mappings);
$this->assertEquals($mime, $detection->detectPath('foo.' . $ext));
}

public function dataGetSecureMimeType(): array {
return [
['image/svg+xml', 'text/plain'],
Expand Down