feat: Support OC-Checksum in bulk upload

To align with the rest of Nextcloud.

Signed-off-by: Louis Chemineau <louis@chmn.me>

apps/dav/lib/BulkUpload/MultipartRequestParser.php

@@ -134,7 +134,10 @@ class MultipartRequestParser {
 
 		$headers = $this->readPartHeaders();
 
-		$content = $this->readPartContent((int)$headers['content-length'], $headers['x-file-md5']);
+		$length = (int)$headers['content-length'];
+
+		$this->validateHash($length, $headers['x-file-md5'] ?? '', $headers['oc-checksum'] ?? '');
+		$content = $this->readPartContent($length);
 
 		return [$headers, $content];
 	}
@@ -184,8 +187,9 @@ class MultipartRequestParser {
 			throw new LengthRequired('The Content-Length header must not be null.');
 		}
 
-		if (!isset($headers['x-file-md5'])) {
-			throw new BadRequest('The X-File-MD5 header must not be null.');
+		// TODO: Drop $md5 condition when the latest desktop client that uses it is no longer supported.
+		if (!isset($headers['x-file-md5']) && !isset($headers['oc-checksum'])) {
+			throw new BadRequest('The hash headers must not be null.');
 		}
 
 		return $headers;
@@ -197,13 +201,7 @@ class MultipartRequestParser {
 	 * @throws Exception
 	 * @throws BadRequest
 	 */
-	private function readPartContent(int $length, string $md5): string {
-		$computedMd5 = $this->computeMd5Hash($length);
-
-		if ($md5 !== $computedMd5) {
-			throw new BadRequest('Computed md5 hash is incorrect.');
-		}
-
+	private function readPartContent(int $length): string {
 		if ($length === 0) {
 			$content = '';
 		} else {
@@ -225,12 +223,25 @@ class MultipartRequestParser {
 	}
 
 	/**
-	 * Compute the MD5 hash of the next x bytes.
+	 * Compute the MD5 or checksum hash of the next x bytes.
+	 * TODO: Drop $md5 argument when the latest desktop client that uses it is no longer supported.
 	 */
-	private function computeMd5Hash(int $length): string {
-		$context = hash_init('md5');
+	private function validateHash(int $length, string $fileMd5Header, string $checksumHeader): void {
+		if ($checksumHeader !== '') {
+			[$algorithm, $hash] = explode(':', $checksumHeader, 2);
+		} elseif ($fileMd5Header !== '') {
+			$algorithm = 'md5';
+			$hash = $fileMd5Header;
+		} else {
+			throw new BadRequest('No hash provided.');
+		}
+
+		$context = hash_init($algorithm);
 		hash_update_stream($context, $this->stream, $length);
 		fseek($this->stream, -$length, SEEK_CUR);
-		return hash_final($context);
+		$computedHash = hash_final($context);
+		if ($hash !== $computedHash) {
+			throw new BadRequest("Computed $algorithm hash is incorrect ($computedHash).");
+		}
 	}
 }

apps/dav/tests/unit/Files/MultipartRequestParserTest.php

@@ -7,12 +7,14 @@
 namespace OCA\DAV\Tests\unit\DAV;
 
 use OCA\DAV\BulkUpload\MultipartRequestParser;
+use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Log\LoggerInterface;
+use Sabre\HTTP\RequestInterface;
 use Test\TestCase;
 
 class MultipartRequestParserTest extends TestCase {
 
-	protected LoggerInterface $logger;
+	protected LoggerInterface&MockObject $logger;
 
 	protected function setUp(): void {
 		$this->logger = $this->createMock(LoggerInterface::class);
@@ -24,6 +26,7 @@ class MultipartRequestParserTest extends TestCase {
 				'headers' => [
 					'Content-Length' => 7,
 					'X-File-MD5' => '4f2377b4d911f7ec46325fe603c3af03',
+					'OC-Checksum' => 'md5:4f2377b4d911f7ec46325fe603c3af03',
 					'X-File-Path' => '/coucou.txt'
 				],
 				'content' => "Coucou\n"
@@ -32,7 +35,8 @@ class MultipartRequestParserTest extends TestCase {
 	}
 
 	private function getMultipartParser(array $parts, array $headers = [], string $boundary = 'boundary_azertyuiop'): MultipartRequestParser {
-		$request = $this->getMockBuilder('Sabre\HTTP\RequestInterface')
+		/** @var RequestInterface&MockObject $request */
+		$request = $this->getMockBuilder(RequestInterface::class)
 			->disableOriginalConstructor()
 			->getMock();
 
@@ -74,7 +78,8 @@ class MultipartRequestParserTest extends TestCase {
 	 */
 	public function testBodyTypeValidation(): void {
 		$bodyStream = 'I am not a stream, but pretend to be';
-		$request = $this->getMockBuilder('Sabre\HTTP\RequestInterface')
+		/** @var RequestInterface&MockObject $request */
+		$request = $this->getMockBuilder(RequestInterface::class)
 			->disableOriginalConstructor()
 			->getMock();
 		$request->expects($this->any())
@@ -88,15 +93,39 @@ class MultipartRequestParserTest extends TestCase {
 	/**
 	 * Test with valid request.
 	 * - valid boundary
-	 * - valid md5 hash
+	 * - valid hash
 	 * - valid content-length
 	 * - valid file content
 	 * - valid file path
 	 */
 	public function testValidRequest(): void {
-		$multipartParser = $this->getMultipartParser(
-			$this->getValidBodyObject()
-		);
+		$bodyObject = $this->getValidBodyObject();
+		unset($bodyObject['0']['headers']['X-File-MD5']);
+
+		$multipartParser = $this->getMultipartParser($bodyObject);
+
+		[$headers, $content] = $multipartParser->parseNextPart();
+
+		$this->assertSame((int)$headers['content-length'], 7, 'Content-Length header should be the same as provided.');
+		$this->assertSame($headers['oc-checksum'], 'md5:4f2377b4d911f7ec46325fe603c3af03', 'OC-Checksum header should be the same as provided.');
+		$this->assertSame($headers['x-file-path'], '/coucou.txt', 'X-File-Path header should be the same as provided.');
+
+		$this->assertSame($content, "Coucou\n", 'Content should be the same');
+	}
+
+	/**
+	 * Test with valid request.
+	 * - valid boundary
+	 * - valid md5 hash
+	 * - valid content-length
+	 * - valid file content
+	 * - valid file path
+	 */
+	public function testValidRequestWithMd5(): void {
+		$bodyObject = $this->getValidBodyObject();
+		unset($bodyObject['0']['headers']['OC-Checksum']);
+
+		$multipartParser = $this->getMultipartParser($bodyObject);
 
 		[$headers, $content] = $multipartParser->parseNextPart();
 
@@ -107,31 +136,48 @@ class MultipartRequestParserTest extends TestCase {
 		$this->assertSame($content, "Coucou\n", 'Content should be the same');
 	}
 
+	/**
+	 * Test with invalid hash.
+	 */
+	public function testInvalidHash(): void {
+		$bodyObject = $this->getValidBodyObject();
+		$bodyObject['0']['headers']['OC-Checksum'] = 'md5:f2377b4d911f7ec46325fe603c3af03';
+		unset($bodyObject['0']['headers']['X-File-MD5']);
+		$multipartParser = $this->getMultipartParser(
+			$bodyObject
+		);
+
+		$this->expectExceptionMessage('Computed md5 hash is incorrect (4f2377b4d911f7ec46325fe603c3af03).');
+		$multipartParser->parseNextPart();
+	}
+
 	/**
 	 * Test with invalid md5 hash.
 	 */
 	public function testInvalidMd5Hash(): void {
 		$bodyObject = $this->getValidBodyObject();
+		unset($bodyObject['0']['headers']['OC-Checksum']);
 		$bodyObject['0']['headers']['X-File-MD5'] = 'f2377b4d911f7ec46325fe603c3af03';
 		$multipartParser = $this->getMultipartParser(
 			$bodyObject
 		);
 
-		$this->expectExceptionMessage('Computed md5 hash is incorrect.');
+		$this->expectExceptionMessage('Computed md5 hash is incorrect (4f2377b4d911f7ec46325fe603c3af03).');
 		$multipartParser->parseNextPart();
 	}
 
 	/**
-	 * Test with a null md5 hash.
+	 * Test with a null hash headers.
 	 */
-	public function testNullMd5Hash(): void {
+	public function testNullHash(): void {
 		$bodyObject = $this->getValidBodyObject();
+		unset($bodyObject['0']['headers']['OC-Checksum']);
 		unset($bodyObject['0']['headers']['X-File-MD5']);
 		$multipartParser = $this->getMultipartParser(
 			$bodyObject
 		);
 
-		$this->expectExceptionMessage('The X-File-MD5 header must not be null.');
+		$this->expectExceptionMessage('The hash headers must not be null.');
 		$multipartParser->parseNextPart();
 	}
 
@@ -159,7 +205,7 @@ class MultipartRequestParserTest extends TestCase {
 			$bodyObject
 		);
 
-		$this->expectExceptionMessage('Computed md5 hash is incorrect.');
+		$this->expectExceptionMessage('Computed md5 hash is incorrect (41060d3ddfdf63e68fc2bf196f652ee9).');
 		$multipartParser->parseNextPart();
 	}
 
@@ -173,7 +219,7 @@ class MultipartRequestParserTest extends TestCase {
 			$bodyObject
 		);
 
-		$this->expectExceptionMessage('Computed md5 hash is incorrect.');
+		$this->expectExceptionMessage('Computed md5 hash is incorrect (0161002bbee6a744f18741b8a914e413).');
 		$multipartParser->parseNextPart();
 	}