diff --git a/app/code/Magento/Csp/Model/Deploy/Package/Processor/PostProcessor/Integrity.php b/app/code/Magento/Csp/Model/Deploy/Package/Processor/PostProcessor/Integrity.php index ba77e172355d..41e81752632f 100644 --- a/app/code/Magento/Csp/Model/Deploy/Package/Processor/PostProcessor/Integrity.php +++ b/app/code/Magento/Csp/Model/Deploy/Package/Processor/PostProcessor/Integrity.php @@ -1,7 +1,7 @@ filesystem = $filesystem; $this->hashGenerator = $hashGenerator; $this->integrityFactory = $integrityFactory; $this->integrityCollector = $integrityCollector; - $this->logger = $logger ?? ObjectManager::getInstance()->get(LoggerInterface::class); - $this->repositoryPool = $repositoryPool ?? - ObjectManager::getInstance()->get(SubresourceIntegrityRepositoryPool::class); } /** @@ -88,7 +68,7 @@ public function process(Package $package, array $options): bool ); foreach ($package->getFiles() as $file) { - if (strtolower($file->getExtension()) === "js") { + if ($file->getExtension() == "js") { $integrity = $this->integrityFactory->create( [ "data" => [ @@ -104,21 +84,6 @@ public function process(Package $package, array $options): bool } } - // Save collected data directly to repository before process exits - $collectedData = $this->integrityCollector->release(); - if (!empty($collectedData)) { - $area = explode('/', $package->getPath())[0]; - try { - $this->repositoryPool->get($area)->saveBunch($collectedData); - } catch (\Exception $e) { - //phpcs:ignore - $this->logger->error('Integrity PostProcessor: Failed saving to ' . $area . ' repository: ' . $e->getMessage()); - } - - // Clear collector for next package (if any) - $this->integrityCollector->clear(); - } - return true; } } diff --git a/app/code/Magento/Csp/Model/Deploy/Package/Processor/PostProcessor/Map.php b/app/code/Magento/Csp/Model/Deploy/Package/Processor/PostProcessor/Map.php index 0f6a5d66f956..4e97517a165c 100644 --- a/app/code/Magento/Csp/Model/Deploy/Package/Processor/PostProcessor/Map.php +++ b/app/code/Magento/Csp/Model/Deploy/Package/Processor/PostProcessor/Map.php @@ -1,19 +1,17 @@ minification = $minification; $this->integrityFactory = $integrityFactory; @@ -107,10 +89,6 @@ public function __construct( $this->driver = $driver; $this->integrityCollector = $integrityCollector; $this->filesystem = $filesystem; - $this->repositoryPool = $repositoryPool ?? - ObjectManager::getInstance()->get(SubresourceIntegrityRepositoryPool::class); - $this->logger = $logger ?? - ObjectManager::getInstance()->get(LoggerInterface::class); parent::__construct($deployStaticFile, $formatter, $packageFileFactory, $minification); } @@ -140,18 +118,7 @@ public function process(Package $package, array $options): bool ] ] ); - // Save immediately to repository instead of using collector - $area = $package->getArea(); - - if (!empty($area)) { - try { - $this->repositoryPool->get($area)->save($integrity); - $this->logger->info("Map PostProcessor: Saved SRI hash for {$relativePath} in {$area} area"); - } catch (\Exception $e) { - //phpcs:ignore - $this->logger->error("Map PostProcessor: Failed to save SRI hash for {$relativePath} in {$area} area"); - } - } + $this->integrityCollector->collect($integrity); } } return true; diff --git a/app/code/Magento/Csp/Model/SubresourceIntegrityCollector.php b/app/code/Magento/Csp/Model/SubresourceIntegrityCollector.php index 13f2f2273dc3..36b1775b6558 100644 --- a/app/code/Magento/Csp/Model/SubresourceIntegrityCollector.php +++ b/app/code/Magento/Csp/Model/SubresourceIntegrityCollector.php @@ -1,7 +1,7 @@ data; } - - /** - * Clear all collected data. - * - * @return void - */ - public function clear(): void - { - $this->data = []; - } } diff --git a/app/code/Magento/Csp/Plugin/GenerateBundleAssetIntegrity.php b/app/code/Magento/Csp/Plugin/GenerateBundleAssetIntegrity.php deleted file mode 100644 index c219b1874b3c..000000000000 --- a/app/code/Magento/Csp/Plugin/GenerateBundleAssetIntegrity.php +++ /dev/null @@ -1,106 +0,0 @@ -hashGenerator = $hashGenerator; - $this->integrityFactory = $integrityFactory; - $this->integrityCollector = $integrityCollector; - $this->filesystem = $filesystem; - $this->fileIo = $fileIo; - } - - /** - * Generate SRI hashes for JS files in the bundle directory. - * - * @param Bundle $subject - * @param string|null $result - * @param string $area - * @param string $theme - * @param string $locale - * @return void - * @throws FileSystemException - * @SuppressWarnings(PHPMD.UnusedFormalParameter) - */ - public function afterDeploy(Bundle $subject, ?string $result, string $area, string $theme, string $locale) - { - if (PHP_SAPI == 'cli') { - $pubStaticDir = $this->filesystem->getDirectoryRead(DirectoryList::STATIC_VIEW); - $files = $pubStaticDir->search( - $area ."/" . $theme . "/" . $locale . "/" . Bundle::BUNDLE_JS_DIR . "/*.js" - ); - - foreach ($files as $file) { - $bundlePath = $area . '/' . $theme . '/' . $locale . - "/" . Bundle::BUNDLE_JS_DIR . '/' . $this->fileIo->getPathInfo($file)['basename']; - - $integrity = $this->integrityFactory->create( - [ - "data" => [ - 'hash' => $this->hashGenerator->generate( - $pubStaticDir->readFile($file) - ), - 'path' => $bundlePath - ] - ] - ); - - $this->integrityCollector->collect($integrity); - } - } - } -} diff --git a/app/code/Magento/Csp/Plugin/GenerateMergedAssetIntegrity.php b/app/code/Magento/Csp/Plugin/GenerateMergedAssetIntegrity.php deleted file mode 100644 index 6eb6a3c54c44..000000000000 --- a/app/code/Magento/Csp/Plugin/GenerateMergedAssetIntegrity.php +++ /dev/null @@ -1,93 +0,0 @@ -sourceIntegrityRepository = $sourceIntegrityRepositoryPool->get(Area::AREA_FRONTEND); - $this->hashGenerator = $hashGenerator; - $this->integrityFactory = $integrityFactory; - $this->filesystem = $filesystem; - } - - /** - * Generate SRI hash for merged JS files. - * - * @param FileExists $subject - * @param string|null $result - * @param array $assetsToMerge - * @param LocalInterface $resultAsset - * @return string|null - * @throws FileSystemException - * @SuppressWarnings(PHPMD.UnusedFormalParameter) - */ - public function afterMerge(FileExists $subject, ?string $result, array $assetsToMerge, LocalInterface $resultAsset) - { - if ($resultAsset->getContentType() !== 'js') { - return $result; - } - - $integrity = $this->integrityFactory->create( - [ - "data" => [ - 'hash' => $this->hashGenerator->generate($resultAsset->getContent()), - 'path' => $resultAsset->getPath() - ] - ] - ); - - $this->sourceIntegrityRepository->save($integrity); - - return $result; - } -} diff --git a/app/code/Magento/Csp/Plugin/RemoveAllAssetIntegrityHashes.php b/app/code/Magento/Csp/Plugin/RemoveAllAssetIntegrityHashes.php index 0bc7e24a3041..85b97e9091c5 100644 --- a/app/code/Magento/Csp/Plugin/RemoveAllAssetIntegrityHashes.php +++ b/app/code/Magento/Csp/Plugin/RemoveAllAssetIntegrityHashes.php @@ -1,7 +1,7 @@ integrityRepositoryPool = $integrityRepositoryPool; - $this->integrityCollector = $integrityCollector ?? ObjectManager::getInstance() - ->get(SubresourceIntegrityCollector::class); } /** @@ -58,14 +47,10 @@ public function beforeDeploy( array $options ): void { if (PHP_SAPI == 'cli' && !$this->isRefreshContentVersionOnly($options)) { - // Clear stored integrity hashes from all areas foreach ([Package::BASE_AREA, Area::AREA_FRONTEND, Area::AREA_ADMINHTML] as $area) { $this->integrityRepositoryPool->get($area) ->deleteAll(); } - - // Clear any leftover in-memory integrity hashes from previous runs - $this->integrityCollector->clear(); } } diff --git a/app/code/Magento/Csp/Plugin/StoreAssetIntegrityHashes.php b/app/code/Magento/Csp/Plugin/StoreAssetIntegrityHashes.php index 75a9aca67d35..21bfd581449c 100644 --- a/app/code/Magento/Csp/Plugin/StoreAssetIntegrityHashes.php +++ b/app/code/Magento/Csp/Plugin/StoreAssetIntegrityHashes.php @@ -1,7 +1,7 @@ integrityCollector = $integrityCollector; $this->integrityRepositoryPool = $integrityRepositoryPool; - $this->logger = $logger ?? - ObjectManager::getInstance()->get(LoggerInterface::class); } /** @@ -66,19 +55,16 @@ public function afterDeploy( array $options ): void { $bunches = []; - $integrityHashes = $this->integrityCollector->release(); - foreach ($integrityHashes as $integrity) { + foreach ($this->integrityCollector->release() as $integrity) { $area = explode("/", $integrity->getPath())[0]; + $bunches[$area][] = $integrity; } foreach ($bunches as $area => $bunch) { - try { - $this->integrityRepositoryPool->get($area)->saveBunch($bunch); - } catch (\Exception $e) { - $this->logger->error('SRI Store: Failed saving ' . $area . ': ' . $e->getMessage()); - } + $this->integrityRepositoryPool->get($area) + ->saveBunch($bunch); } } } diff --git a/app/code/Magento/Csp/Test/Unit/Model/SubresourceIntegrityCollectorTest.php b/app/code/Magento/Csp/Test/Unit/Model/SubresourceIntegrityCollectorTest.php deleted file mode 100644 index 2c92a8b8879c..000000000000 --- a/app/code/Magento/Csp/Test/Unit/Model/SubresourceIntegrityCollectorTest.php +++ /dev/null @@ -1,169 +0,0 @@ -collector = new SubresourceIntegrityCollector(); - } - - /** - * Test that collect method adds integrity objects to internal storage - */ - public function testCollectAddsIntegrityObject(): void - { - $integrityMock = $this->createMock(SubresourceIntegrity::class); - - $this->collector->collect($integrityMock); - - $result = $this->collector->release(); - $this->assertCount(1, $result); - $this->assertSame($integrityMock, $result[0]); - } - - /** - * Test that multiple collect calls accumulate objects - */ - public function testMultipleCollectCallsAccumulate(): void - { - $integrity1 = $this->createMock(SubresourceIntegrity::class); - $integrity2 = $this->createMock(SubresourceIntegrity::class); - $integrity3 = $this->createMock(SubresourceIntegrity::class); - - $this->collector->collect($integrity1); - $this->collector->collect($integrity2); - $this->collector->collect($integrity3); - - $result = $this->collector->release(); - $this->assertCount(3, $result); - $this->assertSame($integrity1, $result[0]); - $this->assertSame($integrity2, $result[1]); - $this->assertSame($integrity3, $result[2]); - } - - /** - * Test that release returns empty array when no objects collected - */ - public function testReleaseReturnsEmptyArrayWhenEmpty(): void - { - $result = $this->collector->release(); - - $this->assertIsArray($result); - $this->assertEmpty($result); - } - - /** - * Test that release does not clear the internal data - */ - public function testReleaseDoesNotClearData(): void - { - $integrityMock = $this->createMock(SubresourceIntegrity::class); - - $this->collector->collect($integrityMock); - - // Call release multiple times - $result1 = $this->collector->release(); - $result2 = $this->collector->release(); - - $this->assertCount(1, $result1); - $this->assertCount(1, $result2); - $this->assertSame($integrityMock, $result1[0]); - $this->assertSame($integrityMock, $result2[0]); - } - - /** - * Test that clear method empties the internal storage - */ - public function testClearEmptiesInternalStorage(): void - { - $integrity1 = $this->createMock(SubresourceIntegrity::class); - $integrity2 = $this->createMock(SubresourceIntegrity::class); - - // Add some objects - $this->collector->collect($integrity1); - $this->collector->collect($integrity2); - - // Verify they're there - $resultBeforeClear = $this->collector->release(); - $this->assertCount(2, $resultBeforeClear); - - // Clear the data - $this->collector->clear(); - - // Verify it's empty - $resultAfterClear = $this->collector->release(); - $this->assertEmpty($resultAfterClear); - } - - /** - * Test that collector works properly after clear - */ - public function testCollectorWorksAfterClear(): void - { - $integrity1 = $this->createMock(SubresourceIntegrity::class); - $integrity2 = $this->createMock(SubresourceIntegrity::class); - - // Add and clear - $this->collector->collect($integrity1); - $this->collector->clear(); - - // Add new data - $this->collector->collect($integrity2); - - $result = $this->collector->release(); - $this->assertCount(1, $result); - $this->assertSame($integrity2, $result[0]); - } - - /** - * Test collect, release, clear cycle - */ - public function testCompleteCollectReleaseClearCycle(): void - { - $integrity1 = $this->createMock(SubresourceIntegrity::class); - $integrity2 = $this->createMock(SubresourceIntegrity::class); - - // Initially empty - $this->assertEmpty($this->collector->release()); - - // Collect objects - $this->collector->collect($integrity1); - $this->collector->collect($integrity2); - - // Release should return collected objects - $released = $this->collector->release(); - $this->assertCount(2, $released); - $this->assertSame($integrity1, $released[0]); - $this->assertSame($integrity2, $released[1]); - - // Data should still be there after release - $this->assertCount(2, $this->collector->release()); - - // Clear should empty everything - $this->collector->clear(); - $this->assertEmpty($this->collector->release()); - } -} diff --git a/app/code/Magento/Csp/Test/Unit/Plugin/GenerateBundleAssetIntegrityTest.php b/app/code/Magento/Csp/Test/Unit/Plugin/GenerateBundleAssetIntegrityTest.php deleted file mode 100644 index 8637e652f98a..000000000000 --- a/app/code/Magento/Csp/Test/Unit/Plugin/GenerateBundleAssetIntegrityTest.php +++ /dev/null @@ -1,117 +0,0 @@ -hashGenerator = $this->createMock(HashGenerator::class); - $this->integrityFactory = $this->createMock(SubresourceIntegrityFactory::class); - $this->integrityCollector = $this->createMock(SubresourceIntegrityCollector::class); - $this->filesystem = $this->createMock(Filesystem::class); - $this->fileIo = $this->createMock(File::class); - } - - /** - * @return void - * @throws Exception - * @throws FileSystemException - */ - public function testAfterDeploy(): void - { - $subject = $this->createMock(Bundle::class); - $result = null; - $area = 'frontend'; - $theme = 'Magento/blank'; - $locale = 'en_US'; - $file = '/path/to/file.js'; - $hash = 'asdfghjkl'; - $fileContent = 'content'; - - $pubStaticDir = $this->createMock(ReadInterface::class); - $pubStaticDir->expects($this->once())->method('search')->with( - $area ."/" . $theme . "/" . $locale . "/" . Bundle::BUNDLE_JS_DIR . "/*.js" - )->willReturn([$file]); - $pubStaticDir->expects($this->once())->method('readFile')->willReturn($fileContent); - $this->filesystem->expects($this->once())->method('getDirectoryRead')->willReturn($pubStaticDir); - $integrity = $this->createMock(SubresourceIntegrity::class); - $this->hashGenerator->expects($this->once()) - ->method('generate') - ->with($fileContent) - ->willReturn($hash); - $this->fileIo->expects($this->once()) - ->method('getPathInfo') - ->with($file) - ->willReturn(['basename' => 'file.js']); - $this->integrityFactory->expects($this->once()) - ->method('create') - ->with([ - 'data' => [ - 'hash' => $hash, - 'path' => $area . '/' . $theme . '/' . $locale . '/' . Bundle::BUNDLE_JS_DIR . '/file.js' - ] - ]) - ->willReturn($integrity); - $this->integrityCollector->expects($this->once())->method('collect')->with($integrity); - - $plugin = new GenerateBundleAssetIntegrity( - $this->hashGenerator, - $this->integrityFactory, - $this->integrityCollector, - $this->filesystem, - $this->fileIo - ); - $plugin->afterDeploy($subject, $result, $area, $theme, $locale); - } -} diff --git a/app/code/Magento/Csp/Test/Unit/Plugin/GenerateMergedAssetIntegrityTest.php b/app/code/Magento/Csp/Test/Unit/Plugin/GenerateMergedAssetIntegrityTest.php deleted file mode 100644 index f55e4c395415..000000000000 --- a/app/code/Magento/Csp/Test/Unit/Plugin/GenerateMergedAssetIntegrityTest.php +++ /dev/null @@ -1,98 +0,0 @@ -sourceIntegrityRepository = $this->createMock(SubresourceIntegrityRepositoryPool::class); - $this->hashGenerator = $this->createMock(HashGenerator::class); - $this->integrityFactory = $this->createMock(SubresourceIntegrityFactory::class); - } - - /** - * @return void - * @throws Exception|FileSystemException - */ - public function testAfterMerge(): void - { - $subject = $this->createMock(FileExists::class); - $result = null; - $assetsToMerge = []; - $fileExtension = 'js'; - $filePath = 'path/to/file.js'; - $hash = '1234567890abcdef'; - $fileContent = 'some content'; - $resultAsset = $this->createMock(File::class); - $resultAsset->expects($this->once())->method('getContentType')->willReturn($fileExtension); - $resultAsset->expects($this->once())->method('getContent')->willReturn($fileContent); - $resultAsset->expects($this->once()) - ->method('getPath') - ->willReturn($filePath); - $this->hashGenerator->expects($this->once())->method('generate')->with($fileContent)->willReturn($hash); - $integrity = $this->createMock(SubresourceIntegrity::class); - $this->integrityFactory->expects($this->once()) - ->method('create')->with([ - 'data' => [ - 'hash' => $hash, - 'path' => $filePath - ] - ])->willReturn($integrity); - $repository = $this->createMock(SubresourceIntegrityRepository::class); - $this->sourceIntegrityRepository->expects($this->once())->method('get') - ->with(Area::AREA_FRONTEND) - ->willReturn($repository); - $repository->expects($this->once())->method('save')->with($integrity); - - $plugin = new GenerateMergedAssetIntegrity( - $this->sourceIntegrityRepository, - $this->hashGenerator, - $this->integrityFactory, - $this->createMock(Filesystem::class) - ); - $plugin->afterMerge($subject, $result, $assetsToMerge, $resultAsset); - } -} diff --git a/app/code/Magento/Csp/Test/Unit/Plugin/StoreAssetIntegrityHashesTest.php b/app/code/Magento/Csp/Test/Unit/Plugin/StoreAssetIntegrityHashesTest.php index c70585ceb888..7e8752745723 100644 --- a/app/code/Magento/Csp/Test/Unit/Plugin/StoreAssetIntegrityHashesTest.php +++ b/app/code/Magento/Csp/Test/Unit/Plugin/StoreAssetIntegrityHashesTest.php @@ -15,7 +15,6 @@ use Magento\Csp\Plugin\StoreAssetIntegrityHashes; use Magento\Csp\Model\SubresourceIntegrityCollector; use PHPUnit\Framework\TestCase; -use Psr\Log\LoggerInterface; /** * Plugin that removes existing integrity hashes for all assets. @@ -32,11 +31,6 @@ class StoreAssetIntegrityHashesTest extends TestCase */ private MockObject $integrityCollectorMock; - /** - * @var MockObject - */ - private MockObject $loggerMock; - /** * @var StoreAssetIntegrityHashes */ @@ -58,13 +52,9 @@ protected function setUp(): void ->disableOriginalConstructor() ->onlyMethods(['release']) ->getMock(); - $this->loggerMock = $this->getMockBuilder(LoggerInterface::class) - ->disableOriginalConstructor() - ->getMock(); $this->plugin = new StoreAssetIntegrityHashes( $this->integrityCollectorMock, $this->integrityRepositoryPoolMock, - $this->loggerMock ); } diff --git a/app/code/Magento/Csp/etc/di.xml b/app/code/Magento/Csp/etc/di.xml index 06900f8c8df0..0cfb2641cb42 100644 --- a/app/code/Magento/Csp/etc/di.xml +++ b/app/code/Magento/Csp/etc/di.xml @@ -146,10 +146,4 @@ - - - - - - diff --git a/app/code/Magento/PageCache/Model/App/Request/Http/IdentifierForSave.php b/app/code/Magento/PageCache/Model/App/Request/Http/IdentifierForSave.php index cbb3c34db6cb..bcd5291ad6da 100644 --- a/app/code/Magento/PageCache/Model/App/Request/Http/IdentifierForSave.php +++ b/app/code/Magento/PageCache/Model/App/Request/Http/IdentifierForSave.php @@ -49,7 +49,8 @@ public function getValue() $data = [ $this->request->isSecure(), preg_replace($pattern, $replace, (string)$this->request->getUriString()), - $this->context->getVaryString() + $this->request->get(\Magento\Framework\App\Response\Http::COOKIE_VARY_STRING) + ?: $this->context->getVaryString() ]; $data = $this->identifierStoreReader->getPageTagsWithStoreCacheTags($data); diff --git a/dev/tests/integration/testsuite/Magento/PageCache/Model/App/Request/Http/IdentifierForSaveTest.php b/dev/tests/integration/testsuite/Magento/PageCache/Model/App/Request/Http/IdentifierForSaveTest.php new file mode 100644 index 000000000000..def999243b3d --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/PageCache/Model/App/Request/Http/IdentifierForSaveTest.php @@ -0,0 +1,130 @@ +objectManager = Bootstrap::getObjectManager(); + $this->identifierForSave = $this->objectManager->get(IdentifierForSave::class); + $this->fixtures = $this->objectManager->get(DataFixtureStorageManager::class)->getStorage(); + $this->context = $this->objectManager->get(Context::class); + $this->cookieManager = $this->objectManager->get(CookieManagerInterface::class); + $this->cookieMetadataFactory = $this->objectManager->get(CookieMetadataFactory::class); + } + + /** + * Test that cache identifier properly handles logged-in customers + */ + #[ + ConfigFixture('system/full_page_cache/caching_application', '1', 'store'), + ConfigFixture('system/full_page_cache/enabled', '1', 'store'), + DataFixture(CustomerFixture::class, as: 'customer') + ] + public function testAfterGetValueWithLoggedInCustomer() + { + // Get customer and login + $customer = $this->fixtures->get('customer'); + $customerSession = $this->objectManager->get(Session::class); + $customerSession->loginById($customer->getId()); + + // Get cache identifiers + $result = $this->identifierForSave->getValue(); + + // Verify that both cache keys are not empty and contain customer context + $this->assertNotEmpty($result, 'Cache identifier for save should not be empty for logged-in user'); + + // Test scenario: Simulate context vary string being empty but cookie vary string present + // Get the current vary string from context + $originalVaryString = $this->context->getVaryString(); + $this->assertNotEmpty($originalVaryString, 'Context vary string should not be empty for logged-in user'); + + // Set the vary cookie to simulate a previous request + $cookieMetadata = $this->cookieMetadataFactory->createSensitiveCookieMetadata()->setPath('/'); + $this->cookieManager->setSensitiveCookie( + self::COOKIE_VARY_STRING, + $originalVaryString, + $cookieMetadata + ); + + // Clear the context vary string to simulate depersonalization + $this->context->_resetState(); + + // Verify context vary string is now empty + $this->assertEmpty($this->context->getVaryString(), 'Context vary string should be empty after reset'); + + // Get cache identifiers again - should still work due to cookie fallback + $resultWithEmptyContext = $this->identifierForSave->getValue(); + + // Both should still generate valid cache keys due to cookie fallback + $this->assertNotEmpty( + $resultWithEmptyContext, + 'Cache identifier for save should work with empty context due to cookie fallback' + ); + + // Both cache key should be same even after context vary string is empty because it use cookie vary string + $this->assertEquals($result, $resultWithEmptyContext); + + // Clean up + $this->cookieManager->deleteCookie(self::COOKIE_VARY_STRING, $cookieMetadata); + } +}