diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index ae68770d9a90c..6c099904ed2cb 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -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 /.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; diff --git a/apps/dav/tests/unit/Connector/Sabre/FileTest.php b/apps/dav/tests/unit/Connector/Sabre/FileTest.php index 361359593dd2a..dfa244ab35736 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FileTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FileTest.php @@ -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. *