From 324e946858bf4739dce99eed1b7c7e6682e977d9 Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Fri, 19 Dec 2025 09:08:38 +0000 Subject: [PATCH 1/3] Feat(Storage): Enable full object checksum validation on JSON path --- Storage/src/Connection/Rest.php | 44 +++++++++++-- Storage/tests/Unit/BucketTest.php | 60 ++++++++++++++++++ Storage/tests/Unit/Connection/RestTest.php | 73 ++++++++++++++++++++++ 3 files changed, 172 insertions(+), 5 deletions(-) diff --git a/Storage/src/Connection/Rest.php b/Storage/src/Connection/Rest.php index 4766c0ac2ee8..dc6a348e7fe8 100644 --- a/Storage/src/Connection/Rest.php +++ b/Storage/src/Connection/Rest.php @@ -500,11 +500,29 @@ private function resolveUploadOptions(array $args) } $validate = $this->chooseValidationMethod($args); - if ($validate === 'md5') { - $args['metadata']['md5Hash'] = base64_encode(Utils::hash($args['data'], 'md5', true)); - } elseif ($validate === 'crc32') { - $args['metadata']['crc32c'] = $this->crcFromStream($args['data']); + $md5Hash = null; + $crc32c = null; + + if ($validate !== false) { + $md5Hash = base64_encode(Utils::hash($args['data'], 'md5', true)); + $crc32c = $this->crcFromStream($args['data']); + + if ($validate === 'md5') { + $args['metadata']['md5Hash'] = $md5Hash; + } elseif ($validate === 'crc32') { + $args['metadata']['crc32c'] = $crc32c; + } + } + + // Prepare the X-Goog-Hash header string + $xGoogHash = []; + if ($crc32c) { + $xGoogHash[] = 'crc32c=' . $crc32c; + } + if ($md5Hash) { + $xGoogHash[] = 'md5=' . $md5Hash; } + $xGoogHashHeader = implode(',', $xGoogHash); $args['metadata']['name'] = $args['name']; if (isset($args['retention'])) { @@ -532,6 +550,22 @@ private function resolveUploadOptions(array $args) $args['uploaderOptions'] = array_intersect_key($args, array_flip($uploaderOptionKeys)); $args = array_diff_key($args, array_flip($uploaderOptionKeys)); + $args['uploaderOptions']['restOptions'] ??= []; + $args['uploaderOptions']['restOptions']['headers'] ??= []; + + // Add the X-Goog-Hash header only if there are hashes to include + if (!empty($xGoogHashHeader)) { + $args['uploaderOptions']['restOptions']['headers']['x-goog-hash'] = $xGoogHashHeader; + } + + if (!empty($args['headers'])) { + $args['uploaderOptions']['restOptions']['headers'] = array_merge( + $args['uploaderOptions']['restOptions']['headers'] ?? [], + $args['headers'] + ); + } + unset($args['headers']); + // Passing on custom retry function to $args['uploaderOptions'] $retryFunc = $this->getRestRetryFunction( 'objects', @@ -714,7 +748,7 @@ private function buildDownloadObjectParams(array $args) private function chooseValidationMethod(array $args) { // If the user provided a hash, skip hashing. - if (isset($args['metadata']['md5Hash']) || isset($args['metadata']['crc32c'])) { + if (isset($args['metadata']['md5Hash']) || isset($args['metadata']['crc32c']) || isset($args['headers']['x-goog-hash'])) { return false; } diff --git a/Storage/tests/Unit/BucketTest.php b/Storage/tests/Unit/BucketTest.php index 5ac38e70763f..27bfe91dc40d 100644 --- a/Storage/tests/Unit/BucketTest.php +++ b/Storage/tests/Unit/BucketTest.php @@ -183,6 +183,66 @@ public function testGetResumableUploaderWithStringWithNoName() $bucket->getResumableUploader('some more data'); } + /** + * Verifies that a resumable upload triggered through a Bucket + * only sends the X-Goog-Hash on the final chunk. + */ + public function testUploadResumableFinalChunkHashes() + { + $data = 'chunk1chunk2'; // 12 bytes + $name = 'test-resumable.txt'; + $resumeUri = 'http://example.com/resumable/123'; + $hash = 'crc32c=mb+64g==,md5=ecA+ttxFBv4gFVBh52kiWA=='; + + $rw = $this->prophesize(RequestWrapper::class); + + // Handshake call (POST) + $rw->send(Argument::that(function ($request) { + return $request->getMethod() === 'POST'; + }), Argument::any())->willReturn(new \GuzzleHttp\Psr7\Response(200, ['Location' => $resumeUri])); + + // Intermediate chunk (PUT) - Should NOT have X-Goog-Hash + $rw->send(Argument::that(function ($request) { + return $request->getMethod() === 'PUT' + && $request->getHeaderLine('Content-Range') === 'bytes 0-5/12' + && !$request->hasHeader('X-Goog-Hash'); + }), Argument::any())->willReturn(new \GuzzleHttp\Psr7\Response(308, ['Range' => 'bytes=0-5'])); + + // FINAL chunk (PUT) - MUST HAVE X-Goog-Hash + $rw->send(Argument::that(function ($request) use ($hash) { + return $request->getMethod() === 'PUT' + && $request->getHeaderLine('Content-Range') === 'bytes 6-11/12' + && $request->getHeaderLine('X-Goog-Hash') === $hash; + }), Argument::any())->willReturn(new \GuzzleHttp\Psr7\Response(200, [], '{"name":"' . $name . '","generation":"1"}')); + + $this->connection->projectId()->willReturn(self::PROJECT_ID); + $this->connection->requestWrapper()->willReturn($rw->reveal()); + + $uploader = new ResumableUploader( + $rw->reveal(), + $data, + 'http://example.com/upload', + [ + 'chunkSize' => 6, + 'contentType' => 'text/plain', + 'restOptions' => ['headers' => ['X-Goog-Hash' => $hash]] + ] + ); + + $this->connection->insertObject(Argument::any()) + ->willReturn($uploader); + + $bucket = $this->getBucket(); + $object = $bucket->upload($data, [ + 'name' => $name, + 'resumable' => true, + 'chunkSize' => 6, + ]); + + $this->assertInstanceOf(StorageObject::class, $object); + $this->assertEquals($name, $object->name()); + } + public function testGetObject() { $bucket = $this->getBucket(); diff --git a/Storage/tests/Unit/Connection/RestTest.php b/Storage/tests/Unit/Connection/RestTest.php index 1a2205307d45..c8405b26ec58 100644 --- a/Storage/tests/Unit/Connection/RestTest.php +++ b/Storage/tests/Unit/Connection/RestTest.php @@ -533,6 +533,57 @@ public function insertObjectProvider() ]; } + public function testInsertObjectWithCalculatedXGoogHashHeader() + { + $rest = new RestCrc32cStub(); + $testData = 'some test data'; + $testStream = Utils::streamFor($testData); + $expectedCrc32c = $rest->getCrcFromStreamForTest($testStream); + $expectedMd5 = base64_encode(Utils::hash($testStream, 'md5', true)); + $expectedHashHeader = 'crc32c=' . $expectedCrc32c . ',md5=' . $expectedMd5; + + $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->extensionLoaded = false; + $rest->supportsBuiltin = true; + + $rest->setRequestWrapper($this->requestWrapper->reveal()); + + $uploadStream = Utils::streamFor($testData); + + $options = [ + 'bucket' => 'my-test-bucket', + 'name' => 'test-calculated-hash-file.txt', + 'data' => $uploadStream, + 'validate' => 'md5' + ]; + + $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($expectedMd5, $metadata['md5Hash']); + + $this->assertEquals($expectedMd5, $metadata['md5Hash']); + $this->assertArrayNotHasKey('crc32c', $metadata); + } + /** * @dataProvider validationMethod */ @@ -593,6 +644,11 @@ public function validationMethod() true, true, false + ], [ + ['validate' => true, 'headers' => ['x-goog-hash' => 'crc32c=abc']], + true, + true, + false ] ]; } @@ -693,4 +749,21 @@ public function chooseValidationMethodProxy(array $args) $call = $chooseValidationMethod->bindTo($this, Rest::class); return $call($args); } + + /** + * Helper proxy to expose crcFromStream for testing. + * * @param StreamInterface $data + * @return string + */ + public function getCrcFromStreamForTest(StreamInterface $data) + { + $data->rewind(); + + $crcFromStream = function (StreamInterface $data) { + return call_user_func_array([$this, 'crcFromStream'], func_get_args()); + }; + + $call = $crcFromStream->bindTo($this, Rest::class); + return $call($data); + } } From 33bb4cb83214c9a4d446ac5b1a42ecb0c4444ff8 Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Fri, 19 Dec 2025 09:35:17 +0000 Subject: [PATCH 2/3] bug fix --- Storage/src/Connection/Rest.php | 8 ++++++-- Storage/tests/Unit/BucketTest.php | 4 +++- Storage/tests/Unit/Connection/RestTest.php | 4 +--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/Storage/src/Connection/Rest.php b/Storage/src/Connection/Rest.php index dc6a348e7fe8..5559abaf8033 100644 --- a/Storage/src/Connection/Rest.php +++ b/Storage/src/Connection/Rest.php @@ -560,7 +560,7 @@ private function resolveUploadOptions(array $args) if (!empty($args['headers'])) { $args['uploaderOptions']['restOptions']['headers'] = array_merge( - $args['uploaderOptions']['restOptions']['headers'] ?? [], + $args['uploaderOptions']['restOptions']['headers'], $args['headers'] ); } @@ -748,7 +748,11 @@ private function buildDownloadObjectParams(array $args) private function chooseValidationMethod(array $args) { // If the user provided a hash, skip hashing. - if (isset($args['metadata']['md5Hash']) || isset($args['metadata']['crc32c']) || isset($args['headers']['x-goog-hash'])) { + if ( + isset($args['metadata']['md5Hash']) + || isset($args['metadata']['crc32c']) + || isset($args['headers']['x-goog-hash']) + ) { return false; } diff --git a/Storage/tests/Unit/BucketTest.php b/Storage/tests/Unit/BucketTest.php index 27bfe91dc40d..7f352a60b6f1 100644 --- a/Storage/tests/Unit/BucketTest.php +++ b/Storage/tests/Unit/BucketTest.php @@ -213,7 +213,9 @@ public function testUploadResumableFinalChunkHashes() return $request->getMethod() === 'PUT' && $request->getHeaderLine('Content-Range') === 'bytes 6-11/12' && $request->getHeaderLine('X-Goog-Hash') === $hash; - }), Argument::any())->willReturn(new \GuzzleHttp\Psr7\Response(200, [], '{"name":"' . $name . '","generation":"1"}')); + }), Argument::any())->willReturn( + new \GuzzleHttp\Psr7\Response(200, [], '{"name":"' . $name . '","generation":"1"}') + ); $this->connection->projectId()->willReturn(self::PROJECT_ID); $this->connection->requestWrapper()->willReturn($rw->reveal()); diff --git a/Storage/tests/Unit/Connection/RestTest.php b/Storage/tests/Unit/Connection/RestTest.php index c8405b26ec58..60bd8b7ac3e9 100644 --- a/Storage/tests/Unit/Connection/RestTest.php +++ b/Storage/tests/Unit/Connection/RestTest.php @@ -578,8 +578,6 @@ function ($args) use (&$actualRequest, $response) { $this->assertEquals([$expectedHashHeader], $actualRequest->getHeader('X-Goog-Hash')); list($contentType, $metadata) = $this->getContentTypeAndMetadata($actualRequest); - $this->assertEquals($expectedMd5, $metadata['md5Hash']); - $this->assertEquals($expectedMd5, $metadata['md5Hash']); $this->assertArrayNotHasKey('crc32c', $metadata); } @@ -752,7 +750,7 @@ public function chooseValidationMethodProxy(array $args) /** * Helper proxy to expose crcFromStream for testing. - * * @param StreamInterface $data + * @param StreamInterface $data * @return string */ public function getCrcFromStreamForTest(StreamInterface $data) From 75e45e7a492299668c733e2dde184e6071ac0b9b Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Fri, 19 Dec 2025 09:39:19 +0000 Subject: [PATCH 3/3] bug fix --- Storage/src/Connection/Rest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Storage/src/Connection/Rest.php b/Storage/src/Connection/Rest.php index 5559abaf8033..3a568ca7b074 100644 --- a/Storage/src/Connection/Rest.php +++ b/Storage/src/Connection/Rest.php @@ -748,8 +748,7 @@ private function buildDownloadObjectParams(array $args) private function chooseValidationMethod(array $args) { // If the user provided a hash, skip hashing. - if ( - isset($args['metadata']['md5Hash']) + if (isset($args['metadata']['md5Hash']) || isset($args['metadata']['crc32c']) || isset($args['headers']['x-goog-hash']) ) {