Skip to content
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
20 changes: 18 additions & 2 deletions apps/dav/lib/Connector/Sabre/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,24 @@ public function put($data) {
}
}

$lengthHeader = $this->request->getHeader('content-length');
$expected = $lengthHeader !== '' ? (int)$lengthHeader : null;
// Only the Content-Length of a PUT body describes the data that is about to be
// written. put() is also reached by the chunked upload assembly step, a MOVE or
// COPY of <upload>/.file, where the data is the AssemblyStream of the chunks
// and the request itself carries no body at all - Directory::moveInto() and
// ::copyInto() only short-circuit File and Directory sources, so a FutureFile
// falls through to Tree::copyNode() -> createFile() -> put(). Clients that send
// "Content-Length: 0" on that request (Safari, for instance) would otherwise
// hand a 0 down to writeStream(), and backends which only determine the length
// themselves when none was given - such as ObjectStoreStorage - then store an
// empty file while still reporting success. The size comparison further down is
// already restricted to PUT for the same reason.
$expected = null;
if ($this->request->getMethod() === 'PUT') {
$lengthHeader = $this->request->getHeader('content-length');
if ($lengthHeader !== '') {
$expected = (int)$lengthHeader;
}
}

if ($partStorage->instanceOfStorage(IWriteStreamStorage::class)) {
$isEOF = false;
Expand Down
73 changes: 73 additions & 0 deletions apps/dav/tests/unit/Connector/Sabre/FileTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,79 @@ function ($path) use ($storage) {
$this->assertEmpty($this->listPartFiles($view, ''), 'No stray part files');
}

public static function expectedSizeProvider(): array {
return [
'PUT with a length passes it through' => ['PUT', ['CONTENT_LENGTH' => '9'], 9],
'PUT of an empty body still expects zero' => ['PUT', ['CONTENT_LENGTH' => '0'], 0],
'PUT without the header expects nothing' => ['PUT', [], null],
// A MOVE is the chunked upload assembly step and carries no body, so whatever
// Content-Length it happens to send must not be taken as the size of the
// assembled stream. Safari sends 0 here, which used to be written verbatim.
'MOVE ignores a zero length' => ['MOVE', ['CONTENT_LENGTH' => '0'], null],
'MOVE ignores a non-zero length' => ['MOVE', ['CONTENT_LENGTH' => '9'], null],
// COPY reaches put() the same way: Directory::copyInto() only short-circuits
// File and Directory sources at storage level, so a FutureFile falls through
// to Tree::copyNode() -> createFile() -> put().
'COPY ignores a zero length' => ['COPY', ['CONTENT_LENGTH' => '0'], null],
'COPY ignores a non-zero length' => ['COPY', ['CONTENT_LENGTH' => '9'], null],
];
}

/**
* The expected size handed to IWriteStreamStorage::writeStream() may only come from
* the Content-Length of a PUT body. Passing it on for other methods makes storages
* that only measure the stream when given no size - ObjectStoreStorage among them -
* write a truncated or empty file.
*/
#[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'expectedSizeProvider')]
public function testPutExpectedSizeOnlyComesFromPutContentLength(string $method, array $server, ?int $expectedSize): void {
$storage = $this->getMockBuilder(Local::class)
->onlyMethods(['writeStream'])
->setConstructorArgs([['datadir' => Server::get(ITempManager::class)->getTemporaryFolder()]])
->getMock();
Filesystem::mount($storage, [], $this->user . '/');

/** @var View&MockObject $view */
$view = $this->getMockBuilder(View::class)
->onlyMethods(['getRelativePath', 'resolvePath'])
->getMock();
$view->expects($this->atLeastOnce())
->method('resolvePath')
->willReturnCallback(fn ($path) => [$storage, $path]);
$view->expects($this->any())
->method('getRelativePath')
->willReturnArgument(0);

$receivedSize = false;
$storage->expects($this->once())
->method('writeStream')
->willReturnCallback(function (string $path, $stream, ?int $size = null) use (&$receivedSize): int {
$receivedSize = $size;
return (int)stream_copy_to_stream($stream, fopen('php://temp', 'r+'));
});

$info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, [
'permissions' => Constants::PERMISSION_ALL,
'type' => FileInfo::TYPE_FOLDER,
], null);

$request = new Request([
'server' => $server,
'method' => $method,
], $this->requestId, $this->config, null);

$file = new File($view, $info, null, $request);

try {
$file->put($this->getStream('test data'));
} catch (\Exception $e) {
// Whatever happens after the write - size checks, renaming the part file - is
// not what this test is about.
}

$this->assertSame($expectedSize, $receivedSize);
}

/**
* Simulate putting a file to the given path.
*
Expand Down