From f292595d32c364f7d3e07b966fd42e424d0b37c7 Mon Sep 17 00:00:00 2001 From: salilg-eng Date: Tue, 5 May 2026 13:24:44 +0000 Subject: [PATCH 1/2] Hash validation on uploads --- Storage/src/Bucket.php | 10 ++ Storage/src/Connection/Rest.php | 21 ++++ Storage/tests/System/UploadObjectsTest.php | 53 ++++++++ Storage/tests/Unit/Connection/RestTest.php | 134 +++++++++++++++++++++ 4 files changed, 218 insertions(+) diff --git a/Storage/src/Bucket.php b/Storage/src/Bucket.php index f005b19bebc2..436e9c0d21e5 100644 --- a/Storage/src/Bucket.php +++ b/Storage/src/Bucket.php @@ -247,6 +247,11 @@ public function exists(array $options = []) * validation hash will be sent. Choose either `md5` or `crc32` to * force a hash method regardless of performance implications. * **Defaults to** `true`. + * @type string $crc32c The base64 encoded CRC32C checksum of the object + * data. If provided, this hash will be used for server-side + * validation. + * @type string $md5 The base64 encoded MD5 hash of the object data. If + * provided, this hash will be used for server-side validation. * @type int $chunkSize If provided the upload will be done in chunks. * The size must be in multiples of 262144 bytes. With chunking * you have increased reliability at the risk of higher overhead. @@ -364,6 +369,11 @@ public function upload($data, array $options = []) * validation hash will be sent. Choose either `md5` or `crc32` to * force a hash method regardless of performance implications. * **Defaults to** `true`. + * @type string $crc32c The base64 encoded CRC32C checksum of the object + * data. If provided, this hash will be used for server-side + * validation. + * @type string $md5 The base64 encoded MD5 hash of the object data. If + * provided, this hash will be used for server-side validation. * @type string $predefinedAcl Predefined ACL to apply to the object. * Acceptable values include, `"authenticatedRead"`, * `"bucketOwnerFullControl"`, `"bucketOwnerRead"`, `"private"`, diff --git a/Storage/src/Connection/Rest.php b/Storage/src/Connection/Rest.php index afa1dde86e7d..8083127f378a 100644 --- a/Storage/src/Connection/Rest.php +++ b/Storage/src/Connection/Rest.php @@ -499,6 +499,27 @@ private function resolveUploadOptions(array $args) $args['name'] = basename($args['data']->getMetadata('uri')); } + if (isset($args['crc32c'])) { + $args['metadata']['crc32c'] = $args['crc32c']; + $userCrc32c = $args['crc32c']; + unset($args['crc32c']); + } + if (isset($args['md5'])) { + $args['metadata']['md5Hash'] = $args['md5']; + $userMd5 = $args['md5']; + unset($args['md5']); + } + if ((isset($userCrc32c) || isset($userMd5)) && !isset($args['headers']['X-Goog-Hash'])) { + $xGoogHash = []; + if (isset($userMd5)) { + $xGoogHash[] = 'md5=' . $userMd5; + } + if (isset($userCrc32c)) { + $xGoogHash[] = 'crc32c=' . $userCrc32c; + } + $args['headers']['X-Goog-Hash'] = implode(',', $xGoogHash); + } + $validate = $this->chooseValidationMethod($args); $xGoogHashHeader = ''; if ($validate !== false) { diff --git a/Storage/tests/System/UploadObjectsTest.php b/Storage/tests/System/UploadObjectsTest.php index 247724424ce1..745a665ad625 100644 --- a/Storage/tests/System/UploadObjectsTest.php +++ b/Storage/tests/System/UploadObjectsTest.php @@ -142,4 +142,57 @@ public function testCrc32cChecksumFails() ] ]); } + + public function testCrc32cChecksumFailsWithTopLevelOption() + { + $this->expectException(BadRequestException::class); + + $data = 'somedata'; + $badChecksum = base64_encode(hash('crc32c', 'bad-data', true)); + + self::$bucket->upload($data, [ + 'name' => uniqid(self::TESTING_PREFIX), + 'crc32c' => $badChecksum + ]); + } + + public function testCrc32cChecksumSucceedsWithTopLevelOption() + { + $data = 'somedata'; + $goodChecksum = base64_encode(hash('crc32c', $data, true)); + + $object = self::$bucket->upload($data, [ + 'name' => uniqid(self::TESTING_PREFIX), + 'crc32c' => $goodChecksum + ]); + $this->assertEquals(strlen($data), $object->info()['size']); + $object->delete(); + } + + public function testMd5ChecksumFailsWithTopLevelOption() + { + $this->expectException(BadRequestException::class); + + $data = 'somedata'; + $badChecksum = base64_encode(hash('md5', 'bad-data', true)); + + self::$bucket->upload($data, [ + 'name' => uniqid(self::TESTING_PREFIX), + 'md5' => $badChecksum + ]); + } + + public function testMd5ChecksumSucceedsWithTopLevelOption() + { + $data = 'somedata'; + $goodChecksum = base64_encode(hash('md5', $data, true)); + + $object = self::$bucket->upload($data, [ + 'name' => uniqid(self::TESTING_PREFIX), + 'md5' => $goodChecksum + ]); + + $this->assertEquals(strlen($data), $object->info()['size']); + $object->delete(); + } } diff --git a/Storage/tests/Unit/Connection/RestTest.php b/Storage/tests/Unit/Connection/RestTest.php index 5f0d2bf4e076..96c96c134e68 100644 --- a/Storage/tests/Unit/Connection/RestTest.php +++ b/Storage/tests/Unit/Connection/RestTest.php @@ -581,6 +581,140 @@ function ($args) use (&$actualRequest, $response) { $this->assertArrayNotHasKey('crc32c', $metadata); } + public function testInsertObjectWithUserProvidedHashes() + { + $rest = new Rest(); + $testData = 'some test data'; + $testStream = Utils::streamFor($testData); + $userCrc32c = 'user-crc'; + $userMd5 = 'user-md5'; + $expectedHashHeader = 'md5=' . $userMd5 . ',crc32c=' . $userCrc32c; + + $actualRequest = null; + $response = new Response(200, ['Location' => 'http://www.mordor.com'], $this->successBody); + + $this->requestWrapper->send( + Argument::type(RequestInterface::class), + Argument::type('array') + )->will( + function ($args) use (&$actualRequest, $response) { + $actualRequest = $args[0]; + return $response; + } + ); + + $rest->setRequestWrapper($this->requestWrapper->reveal()); + + $options = [ + 'bucket' => 'my-test-bucket', + 'name' => 'test-user-hash-file.txt', + 'data' => $testStream, + 'crc32c' => $userCrc32c, + 'md5' => $userMd5, + 'validate' => true + ]; + + $uploader = $rest->insertObject($options); + $this->assertInstanceOf(MultipartUploader::class, $uploader); + $uploader->upload(); + + $this->assertNotNull($actualRequest); + $this->assertTrue($actualRequest->hasHeader('X-Goog-Hash')); + $this->assertEquals([$expectedHashHeader], $actualRequest->getHeader('X-Goog-Hash')); + + list($contentType, $metadata) = $this->getContentTypeAndMetadata($actualRequest); + $this->assertEquals($userMd5, $metadata['md5Hash']); + $this->assertEquals($userCrc32c, $metadata['crc32c']); + } + + public function testInsertObjectWithUserProvidedCrc32cOnly() + { + $rest = new Rest(); + $testData = 'some test data'; + $testStream = Utils::streamFor($testData); + $userCrc32c = 'user-crc'; + $expectedHashHeader = 'crc32c=' . $userCrc32c; + + $actualRequest = null; + $response = new Response(200, ['Location' => 'http://www.mordor.com'], $this->successBody); + + $this->requestWrapper->send( + Argument::type(RequestInterface::class), + Argument::type('array') + )->will( + function ($args) use (&$actualRequest, $response) { + $actualRequest = $args[0]; + return $response; + } + ); + + $rest->setRequestWrapper($this->requestWrapper->reveal()); + + $options = [ + 'bucket' => 'my-test-bucket', + 'name' => 'test-user-hash-file.txt', + 'data' => $testStream, + 'crc32c' => $userCrc32c, + 'validate' => true + ]; + + $uploader = $rest->insertObject($options); + $this->assertInstanceOf(MultipartUploader::class, $uploader); + $uploader->upload(); + + $this->assertNotNull($actualRequest); + $this->assertTrue($actualRequest->hasHeader('X-Goog-Hash')); + $this->assertEquals([$expectedHashHeader], $actualRequest->getHeader('X-Goog-Hash')); + + list($contentType, $metadata) = $this->getContentTypeAndMetadata($actualRequest); + $this->assertEquals($userCrc32c, $metadata['crc32c']); + $this->assertArrayNotHasKey('md5Hash', $metadata); + } + + public function testInsertObjectWithUserProvidedMd5Only() + { + $rest = new Rest(); + $testData = 'some test data'; + $testStream = Utils::streamFor($testData); + $userMd5 = 'user-md5'; + $expectedHashHeader = 'md5=' . $userMd5; + + $actualRequest = null; + $response = new Response(200, ['Location' => 'http://www.mordor.com'], $this->successBody); + + $this->requestWrapper->send( + Argument::type(RequestInterface::class), + Argument::type('array') + )->will( + function ($args) use (&$actualRequest, $response) { + $actualRequest = $args[0]; + return $response; + } + ); + + $rest->setRequestWrapper($this->requestWrapper->reveal()); + + $options = [ + 'bucket' => 'my-test-bucket', + 'name' => 'test-user-hash-file.txt', + 'data' => $testStream, + 'md5' => $userMd5, + 'validate' => true + ]; + + $uploader = $rest->insertObject($options); + $this->assertInstanceOf(MultipartUploader::class, $uploader); + $uploader->upload(); + + $this->assertNotNull($actualRequest); + $this->assertTrue($actualRequest->hasHeader('X-Goog-Hash')); + $this->assertEquals([$expectedHashHeader], $actualRequest->getHeader('X-Goog-Hash')); + + list($contentType, $metadata) = $this->getContentTypeAndMetadata($actualRequest); + $this->assertEquals($userMd5, $metadata['md5Hash']); + $this->assertArrayNotHasKey('crc32c', $metadata); + } + /** * @dataProvider validationMethod */ From f1ac74050bdc240856b954f5c5925b892e8c810e Mon Sep 17 00:00:00 2001 From: salilg-eng Date: Tue, 5 May 2026 14:07:13 +0000 Subject: [PATCH 2/2] Gemini review sorted --- Storage/src/Connection/Rest.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/Storage/src/Connection/Rest.php b/Storage/src/Connection/Rest.php index 8083127f378a..f1667027cf74 100644 --- a/Storage/src/Connection/Rest.php +++ b/Storage/src/Connection/Rest.php @@ -509,7 +509,10 @@ private function resolveUploadOptions(array $args) $userMd5 = $args['md5']; unset($args['md5']); } - if ((isset($userCrc32c) || isset($userMd5)) && !isset($args['headers']['X-Goog-Hash'])) { + if (isset($userCrc32c) || isset($userMd5)) { + // Disable auto-validation to prevent redundant calculations + $args['validate'] = false; + $xGoogHash = []; if (isset($userMd5)) { $xGoogHash[] = 'md5=' . $userMd5; @@ -517,7 +520,13 @@ private function resolveUploadOptions(array $args) if (isset($userCrc32c)) { $xGoogHash[] = 'crc32c=' . $userCrc32c; } - $args['headers']['X-Goog-Hash'] = implode(',', $xGoogHash); + + // Append to existing X-Goog-Hash if present + if (isset($args['headers']['X-Goog-Hash'])) { + $args['headers']['X-Goog-Hash'] .= ',' . implode(',', $xGoogHash); + } else { + $args['headers']['X-Goog-Hash'] = implode(',', $xGoogHash); + } } $validate = $this->chooseValidationMethod($args);