[SR-78] - fix request with extension not html and content-type text/html

holadev · Dec 17, 2020 · 92a78b8 · 92a78b8
1 parent 7bfcfb1
commit 92a78b8
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 5 deletions.
diff --git a/EventSubscriber/AmpOptimizerSubscriber.php b/EventSubscriber/AmpOptimizerSubscriber.php
@@ -7,6 +7,7 @@
 use Psr\Log\LoggerInterface;
 use Sunra\PhpSimple\HtmlDomParser;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Event\ResponseEvent;
 use Symfony\Component\HttpKernel\KernelEvents;
@@ -78,7 +79,7 @@ public function onKernelResponse(ResponseEvent $event): void
             return;
         }
 
-        if (!$this->isAmpHtml($event->getResponse())) {
+        if (!$this->isAmpHtml($event->getResponse(), $event->getRequest())) {
             return;
         }
 
@@ -94,10 +95,16 @@ public function onKernelResponse(ResponseEvent $event): void
 
     /**
      * @param Response $response
+     * @param Request $request
      * @return bool
      */
-    private function isAmpHtml(Response $response): bool
+    private function isAmpHtml(Response $response, Request $request): bool
     {
+        $pathInfo = pathInfo($request->getUri());
+        if (isset($pathInfo['extension']) && $pathInfo['extension'] !== 'html') {
+            return false;
+        }
+
         $contentType = $response->headers->get('Content-type');
         if (strpos($contentType, 'text/html') === false) {
             return false;

diff --git a/Tests/Unit/EventSubscriber/AmpOptimizerSubscriberTest.php b/Tests/Unit/EventSubscriber/AmpOptimizerSubscriberTest.php
@@ -12,6 +12,7 @@
 use Psr\Log\LoggerInterface;
 use ReflectionException;
 use Symfony\Component\HttpFoundation\ParameterBag;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Event\ResponseEvent;
 use Symfony\Component\HttpKernel\KernelEvents;
@@ -80,11 +81,17 @@ private function getEventNotMasterRequestMocked(): ResponseEvent
     public function testNotAmpRequest()
     {
         $instance = $this->getInstanceNotAmpRequest();
+
+        $event = $this->getEventNotAmpRequestMocked('text/html', '', '/segment/image.webp');
+        $instance->onKernelResponse($event);
+
         $event = $this->getEventNotAmpRequestMocked('image/jpeg');
         $instance->onKernelResponse($event);
 
         $event = $this->getEventNotAmpRequestMocked('text/html', '<html></html>');
         $instance->onKernelResponse($event);
+
+
     }
 
     /**
@@ -110,15 +117,20 @@ private function getInstanceNotAmpRequest(): AmpOptimizerSubscriber
      * Provide response event to test with not html amp request and test calls
      * @param string $contentType
      * @param string $content
+     * @param string $uri
      * @return ResponseEvent
      */
-    private function getEventNotAmpRequestMocked($contentType = 'text/html', $content = '<html ⚡></html>'): ResponseEvent
+    private function getEventNotAmpRequestMocked(
+        $contentType = 'text/html',
+        $content = '<html ⚡></html>',
+        $uri = '/segment/doc'
+    ): ResponseEvent
     {
         $headers = $this->prophesize(ParameterBag::class);
         $headers->get(Argument::exact('Content-type'))->willReturn($contentType);
 
         $response = $this->prophesize(Response::class);
-        if ($contentType === 'text/html') {
+        if ($contentType === 'text/html' && $uri === '/segment/doc') {
             $response->getContent()->shouldBeCalled()->willReturn($content);
         }
         if ($contentType === 'image/jpeg') {
@@ -128,9 +140,14 @@ private function getEventNotAmpRequestMocked($contentType = 'text/html', $conten
         $response->headers = $headers;
         $response = $response->reveal();
 
+        $request = $this->prophesize(Request::class);
+        $request->getUri()->willReturn($uri);
+        $request = $request->reveal();
+
         $event = $this->prophesize(ResponseEvent::class);
         $event->isMasterRequest()->shouldBeCalled()->willReturn(true);
         $event->getResponse()->shouldBeCalled()->willReturn($response);
+        $event->getRequest()->shouldBeCalled()->willReturn($request);
         return $event->reveal();
     }
 
@@ -248,9 +265,10 @@ private function getInstanceWithErrorLog(): AmpOptimizerSubscriber
 
     /**
      * Provide response event to test normal operation log and test calls
+     * @param string $uri
      * @return ResponseEvent
      */
-    private function getEventMasterRequestMocked(): ResponseEvent
+    private function getEventMasterRequestMocked($uri = '/segment/doc'): ResponseEvent
     {
         $headers = $this->prophesize(ParameterBag::class);
         $headers->get(Argument::exact('Content-type'))->willReturn('text/html');
@@ -261,9 +279,14 @@ private function getEventMasterRequestMocked(): ResponseEvent
         $response->headers = $headers;
         $response = $response->reveal();
 
+        $request = $this->prophesize(Request::class);
+        $request->getUri()->willReturn($uri);
+        $request = $request->reveal();
+
         $event = $this->prophesize(ResponseEvent::class);
         $event->isMasterRequest()->shouldBeCalled()->willReturn(true);
         $event->getResponse()->shouldBeCalled()->willReturn($response);
+        $event->getRequest()->shouldBeCalled()->willReturn($request);
         return $event->reveal();
     }