Only add header in middleware for correct response type

Previously, the middleware was applied for all GET responses, which could cause errors

Author: thomaskeyte

src/Middleware/AddLinkHeader.php

@@ -3,9 +3,21 @@
 namespace TomKeyte\LaravelHttp2Push\Middleware;
 
 use Closure;
+use Illuminate\Support\Facades\Log;
+use Throwable;
 
 class AddLinkHeader
 {
+    /**
+     * The request object
+     */
+    protected $request;
+
+    /**
+     * The response object
+     */
+    protected $response;
+
     /**
      * Handle an incoming request.
      *
@@ -16,15 +28,46 @@ class AddLinkHeader
      */
     public function handle($request, Closure $next)
     {
-        $response = $next($request);
+        $this->request = $request;
+        $this->response = $next($request);
+
+        try {
+            if ($this->isCorrectRequestType() && $this->isCorrectResponseType()) {
+                $this->pushReources();
+            }
+        } catch (Throwable $t) {
+            Log::error($t);
+        }
+
+        return $this->response;
+    }
+
+    /**
+     * Check that the request is of the correct type for a HTTP push
+     */
+    private function isCorrectRequestType(): bool
+    {
+        return $this->request->method() === 'GET' && !$this->request->isJson();
+    }
+
+    /**
+     * Check that the response is of the correct type for a HTTP push
+     */
+    private function isCorrectResponseType(): bool
+    {
+        return ($this->response instanceof \Illuminate\Http\Response && !$this->response->isRedirection());
+    }
 
+    /**
+     * Add the Link header to the response
+     */
+    private function pushReources(): void
+    {
         $http2push = resolve('http2push');
 
-        if ($request->method() === 'GET' && $http2push->isNotEmpty()) {
-            $response->header('Link', $http2push->buildLinkHeader(), false);
-            $http2push->setPushCookies($response);
+        if ($http2push->isNotEmpty()) {
+            $this->response->header('Link', $http2push->buildLinkHeader(), false);
+            $http2push->setPushCookies($this->response);
         }
-
-        return $response;
     }
 }

tests/MiddlewareTest.php

@@ -2,11 +2,15 @@
 
 namespace TomKeyte\LaravelHttp2Push\Tests;
 
+use Illuminate\Foundation\Testing\TestResponse;
 use Illuminate\Http\Request;
+use Illuminate\Http\Response;
+use Illuminate\Support\Facades\Log;
 use TomKeyte\LaravelHttp2Push\Http2PushServiceProvider;
 use TomKeyte\LaravelHttp2Push\Middleware\AddLinkHeader;
 use TomKeyte\LaravelHttp2Push\PushCookie;
 use Orchestra\Testbench\TestCase;
+use stdClass;
 
 class MiddlewareTest extends TestCase
 {
@@ -26,40 +30,129 @@ protected function setUp(): void
         // have to empty cookie array between runs
         $_COOKIE = [];
 
-        $this->response = $this->get('/');
-        $this->request = Request::create('/', 'GET');
+        $this->response = new Response();
+        $this->request = new Request();
         $this->push = $this->app->get('http2push');
         $this->push->add('/js/app.js');
     }
 
+    /**
+     * Converts a base response class into a test response
+     */
+    private function toTestResponse($response)
+    {
+        return TestResponse::fromBaseResponse($response);
+    }
+
+    /**
+     * @test
+     */
+    public function a_response_has_the_link_header()
+    {
+        $this->toTestResponse(
+            (new AddLinkHeader)
+                ->handle(
+                    $this->request,
+                    function () {
+                        return $this->response;
+                    }
+                )
+        )
+            ->assertHeader('Link', $this->push->buildLinkHeader());
+    }
 
     /**
      * @test
      */
     public function a_response_has_the_push_cookie()
     {
-        (new AddLinkHeader)
-            ->handle(
-                $this->request,
-                function () {
-                    return $this->response;
-                }
-            )
+        $this->toTestResponse(
+            (new AddLinkHeader)
+                ->handle(
+                    $this->request,
+                    function () {
+                        return $this->response;
+                    }
+                )
+        )
             ->assertCookie((new PushCookie('/js/app.js'))->getName());
     }
 
     /**
      * @test
      */
-    public function a_response_has_the_link_header()
+    public function it_does_not_set_link_header_for_json_request()
     {
+        $jsonRequest = (new Request());
+        $jsonRequest->headers->set('Content-Type', 'application/json', true);
+
+        $this->toTestResponse(
+            (new AddLinkHeader)
+                ->handle(
+                    $jsonRequest,
+                    function () {
+                        return $this->response;
+                    }
+                )
+        )
+            ->assertHeaderMissing('Link');
+    }
+
+    /**
+     * @test
+     */
+    public function it_does_not_set_link_header_for_redirect_response()
+    {
+        $redirectResponse = new Response('', 302);
+
+        $this->toTestResponse(
+            (new AddLinkHeader)
+                ->handle(
+                    $this->request,
+                    function () use ($redirectResponse) {
+                        return $redirectResponse;
+                    }
+                )
+        )
+            ->assertHeaderMissing('Link');
+    }
+
+    /**
+     * @test
+     */
+    public function it_does_not_set_link_header_for_a_binary_response()
+    {
+        $binaryResponse = response()->streamDownload(
+            function () {
+                return ['data'];
+            }
+        );
+
+        $this->toTestResponse(
+            (new AddLinkHeader)
+                ->handle(
+                    $this->request,
+                    function () use ($binaryResponse) {
+                        return $binaryResponse;
+                    }
+                )
+        )
+            ->assertHeaderMissing('Link');
+    }
+
+    /**
+     * @test
+     */
+    public function it_logs_an_error_if_middleware_throws_exception()
+    {
+        Log::shouldReceive('error');
+
         (new AddLinkHeader)
             ->handle(
-                $this->request,
+                (new stdClass),
                 function () {
                     return $this->response;
                 }
-            )
-            ->assertHeader('Link', $this->push->buildLinkHeader());
+            );
     }
 }