open-dsi
/
nextcloud-server
mirror of https://github.com/nextcloud/server
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

refactor(TempManager): Simplify and unify implementations and remove legacy behavior

Browse Source 
Signed-off-by: provokateurin <kate@provokateurin.de>
pull/51194/head
provokateurin 2 months ago
parent afae742a2b
commit 8acfc0f0f2
No known key found for this signature in database
4 changed files with 34 additions and 87 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 6
      build/psalm-baseline.xml
  2. 78
      lib/private/TempManager.php
  3. 12
      lib/public/ITempManager.php
  4. 25
      tests/lib/TempManagerTest.php

6
build/psalm-baseline.xml
Unescape View File

@ -2517,12 +2517,6 @@
<code><![CDATA[$tag]]></code>
</MoreSpecificImplementedParamType>
</file>
<file src="lib/private/TempManager.php">
<FalsableReturnStatement>
<code><![CDATA[false]]></code>
<code><![CDATA[false]]></code>
</FalsableReturnStatement>
</file>
<file src="lib/private/Template/CSSResourceLocator.php">
<ParamNameMismatch>
<code><![CDATA[$style]]></code>

78
lib/private/TempManager.php
Unescape View File

@ -10,6 +10,7 @@ namespace OC;
use bantu\IniGetWrapper\IniGetWrapper;
use OCP\IConfig;
use OCP\ITempManager;
use OCP\Security\ISecureRandom;
use Psr\Log\LoggerInterface;
class TempManager implements ITempManager {
@ -34,51 +35,25 @@ class TempManager implements ITempManager {
$this->tmpBaseDir = $this->getTempBaseDir();
}
/**
* Builds the filename with suffix and removes potential dangerous characters
* such as directory separators.
*
* @param string $absolutePath Absolute path to the file / folder
* @param string $postFix Postfix appended to the temporary file name, may be user controlled
* @return string
*/
private function buildFileNameWithSuffix($absolutePath, $postFix = '') {
private function generateTemporaryPath(string $postFix): string {
$secureRandom = \OCP\Server::get(ISecureRandom::class);
$absolutePath = $this->tmpBaseDir . '/' . self::TMP_PREFIX . $secureRandom->generate(32, ISecureRandom::CHAR_ALPHANUMERIC);
if ($postFix !== '') {
$postFix = '.' . ltrim($postFix, '.');
$postFix = str_replace(['\\', '/'], '', $postFix);
$absolutePath .= '-';
}
return $absolutePath . $postFix;
}
/**
* Create a temporary file and return the path
*
* @param string $postFix Postfix appended to the temporary file name
* @return string
*/
public function getTemporaryFile($postFix = '') {
if (is_writable($this->tmpBaseDir)) {
// To create an unique file and prevent the risk of race conditions
// or duplicated temporary files by other means such as collisions
// we need to create the file using `tempnam` and append a possible
// postfix to it later
$file = tempnam($this->tmpBaseDir, self::TMP_PREFIX);
$this->current[] = $file;
public function getTemporaryFile($postFix = ''): string|false {
$path = $this->generateTemporaryPath($postFix);
// If a postfix got specified sanitize it and create a postfixed
// temporary file
if ($postFix !== '') {
$fileNameWithPostfix = $this->buildFileNameWithSuffix($file, $postFix);
touch($fileNameWithPostfix);
chmod($fileNameWithPostfix, 0600);
$this->current[] = $fileNameWithPostfix;
return $fileNameWithPostfix;
}
return $file;
} else {
$old_umask = umask(0077);
$fp = fopen($path, 'x');
umask($old_umask);
if ($fp === false) {
$this->log->warning(
'Can not create a temporary file in directory {dir}. Check it exists and has correct permissions',
[
@ -87,30 +62,16 @@ class TempManager implements ITempManager {
);
return false;
}
}
/**
* Create a temporary folder and return the path
*
* @param string $postFix Postfix appended to the temporary folder name
* @return string
*/
public function getTemporaryFolder($postFix = '') {
if (is_writable($this->tmpBaseDir)) {
// To create an unique directory and prevent the risk of race conditions
// or duplicated temporary files by other means such as collisions
// we need to create the file using `tempnam` and append a possible
// postfix to it later
$uniqueFileName = tempnam($this->tmpBaseDir, self::TMP_PREFIX);
$this->current[] = $uniqueFileName;
fclose($fp);
$this->current[] = $path;
return $path;
}
// Build a name without postfix
$path = $this->buildFileNameWithSuffix($uniqueFileName . '-folder', $postFix);
mkdir($path, 0700);
$this->current[] = $path;
public function getTemporaryFolder($postFix = ''): string|false {
$path = $this->generateTemporaryPath($postFix) . '/';
return $path . '/';
} else {
if (mkdir($path, 0700) === false) {
$this->log->warning(
'Can not create a temporary folder in directory {dir}. Check it exists and has correct permissions',
[
@ -119,6 +80,9 @@ class TempManager implements ITempManager {
);
return false;
}
$this->current[] = $path;
return $path;
}
/**

12
lib/public/ITempManager.php
Unescape View File

@ -16,20 +16,20 @@ interface ITempManager {
/**
* Create a temporary file and return the path
*
* @param string $postFix
* @return string
* @param string $postFix Postfix appended to the temporary file name
*
* @since 8.0.0
*/
public function getTemporaryFile($postFix = '');
public function getTemporaryFile(string $postFix = ''): string|false;
/**
* Create a temporary folder and return the path
*
* @param string $postFix
* @return string
* @param string $postFix Postfix appended to the temporary folder name
*
* @since 8.0.0
*/
public function getTemporaryFolder($postFix = '');
public function getTemporaryFolder(string $postFix = ''): string|false;
/**
* Remove the temporary files and folders generated during this request

25
tests/lib/TempManagerTest.php
Unescape View File

@ -154,34 +154,23 @@ class TempManagerTest extends \Test\TestCase {
$this->assertFalse($manager->getTemporaryFolder());
}
public function testBuildFileNameWithPostfix(): void {
public function testGenerateTemporaryPathWithPostfix(): void {
$logger = $this->createMock(LoggerInterface::class);
$tmpManager = self::invokePrivate(
$this->getManager($logger),
'buildFileNameWithSuffix',
['/tmp/myTemporaryFile', 'postfix']
'generateTemporaryPath',
['postfix']
);
$this->assertEquals('/tmp/myTemporaryFile-.postfix', $tmpManager);
$this->assertStringEndsWith('.postfix', $tmpManager);
}
public function testBuildFileNameWithoutPostfix(): void {
public function testGenerateTemporaryPathTraversal(): void {
$logger = $this->createMock(LoggerInterface::class);
$tmpManager = self::invokePrivate(
$this->getManager($logger),
'buildFileNameWithSuffix',
['/tmp/myTemporaryFile', '']
);
$this->assertEquals('/tmp/myTemporaryFile', $tmpManager);
}
public function testBuildFileNameWithSuffixPathTraversal(): void {
$logger = $this->createMock(LoggerInterface::class);
$tmpManager = self::invokePrivate(
$this->getManager($logger),
'buildFileNameWithSuffix',
['foo', '../Traversal\\../FileName']
'generateTemporaryPath',
['../Traversal\\../FileName']
);
$this->assertStringEndsNotWith('./Traversal\\../FileName', $tmpManager);

Write Preview
Loading…
Cancel
Save