From 9566c9df15c6c07f703a8f4d4f8f1613bceed6db Mon Sep 17 00:00:00 2001 From: Sam James Date: Wed, 15 Jul 2026 22:43:07 +0100 Subject: [PATCH] fix: preg_replace backreference corruption in preload head injection getPreloadsHTML() output was passed straight into preg_replace()'s replacement argument. PHP treats $0-$99 in that argument as backreferences regardless of source, so any href containing a literal "$" followed by digits (e.g. a filename like image$1.jpg, or a querystring like ?v=$1) was silently stripped from the injected markup with no error. Switched to preg_replace_callback(), whose return value is used literally. --- Observer/ResponseBefore.php | 4 ++-- Test/Unit/Observer/ResponseBeforeTest.php | 27 +++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/Observer/ResponseBefore.php b/Observer/ResponseBefore.php index a4c7374..803d0e7 100644 --- a/Observer/ResponseBefore.php +++ b/Observer/ResponseBefore.php @@ -21,9 +21,9 @@ public function execute(\Magento\Framework\Event\Observer $observer) } $response = $observer->getEvent()->getData('response'); - $response->setBody(preg_replace( + $response->setBody(preg_replace_callback( '//', - " + fn() => " {$this->getPreloadsHTML()} ", diff --git a/Test/Unit/Observer/ResponseBeforeTest.php b/Test/Unit/Observer/ResponseBeforeTest.php index f87733c..7c5fb38 100644 --- a/Test/Unit/Observer/ResponseBeforeTest.php +++ b/Test/Unit/Observer/ResponseBeforeTest.php @@ -158,6 +158,33 @@ public function testModifiesResponseInFrontendAreaWithLinks(): void $this->subject->execute($this->observerMock); } + /** + * Test that hrefs containing "$" + digits (e.g. "/img$1.jpg") are not corrupted. + * preg_replace() treats $0-$99 in its replacement argument as backreferences + * regardless of source; preg_replace_callback() does not have this problem. + */ + public function testDoesNotCorruptHrefsContainingDollarDigitSequences(): void + { + $linkMock = $this->createMock(LinkInterface::class); + $linkMock->method('getAttrs')->willReturn(['rel' => 'preload', 'href' => '/img$1.jpg']); + + $this->appStateMock->method('getAreaCode')->willReturn(Area::AREA_FRONTEND); + $this->linkStoreMock->method('get')->willReturn([$linkMock]); + + $this->secureHtmlRendererMock->method('renderTag') + ->willReturn(''); + + $this->responseMock->method('getBody')->willReturn(self::SAMPLE_RESPONSE_HTML); + + $this->responseMock->expects($this->once()) + ->method('setBody') + ->with($this->callback(function ($body) { + return str_contains($body, '/img$1.jpg'); + })); + + $this->subject->execute($this->observerMock); + } + /** * Test that only the first tag is replaced. */