Merge pull request #3497 from fmata/post-location

Prevent header Location on POST when status code is not 201 or 30x

Author: soyuka

src/EventListener/RespondListener.php

@@ -63,14 +63,6 @@ public function onKernelView(ViewEvent $event): void
             'X-Frame-Options' => 'deny',
         ];
 
-        if ($request->attributes->has('_api_write_item_iri')) {
-            $headers['Content-Location'] = $request->attributes->get('_api_write_item_iri');
-
-            if ($request->isMethod('POST')) {
-                $headers['Location'] = $request->attributes->get('_api_write_item_iri');
-            }
-        }
-
         $status = null;
         if ($this->resourceMetadataFactory && $attributes) {
             $resourceMetadata = $this->resourceMetadataFactory->create($attributes['resource_class']);
@@ -83,9 +75,19 @@ public function onKernelView(ViewEvent $event): void
             $status = $resourceMetadata->getOperationAttribute($attributes, 'status');
         }
 
+        $status = $status ?? self::METHOD_TO_CODE[$request->getMethod()] ?? Response::HTTP_OK;
+
+        if ($request->attributes->has('_api_write_item_iri')) {
+            $headers['Content-Location'] = $request->attributes->get('_api_write_item_iri');
+
+            if ((Response::HTTP_CREATED === $status || (300 <= $status && $status < 400)) && $request->isMethod('POST')) {
+                $headers['Location'] = $request->attributes->get('_api_write_item_iri');
+            }
+        }
+
         $event->setResponse(new Response(
             $controllerResult,
-            $status ?? self::METHOD_TO_CODE[$request->getMethod()] ?? Response::HTTP_OK,
+            $status,
             $headers
         ));
     }

tests/EventListener/RespondListenerTest.php

@@ -80,6 +80,55 @@ public function testCreate200Response()
         $this->assertEquals('deny', $response->headers->get('X-Frame-Options'));
     }
 
+    public function testPost200WithoutLocation()
+    {
+        $kernelProphecy = $this->prophesize(HttpKernelInterface::class);
+
+        $request = new Request([], [], ['_api_resource_class' => Dummy::class, '_api_item_operation_name' => 'get', '_api_respond' => true, '_api_write_item_iri' => '/dummy_entities/1']);
+        $request->setMethod('POST');
+
+        $event = new ViewEvent(
+            $kernelProphecy->reveal(),
+            $request,
+            HttpKernelInterface::MASTER_REQUEST,
+            'bar'
+        );
+        $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class);
+        $resourceMetadataFactoryProphecy->create(Dummy::class)->willReturn(new ResourceMetadata(null, null, null, ['get' => ['status' => Response::HTTP_OK]]));
+
+        $listener = new RespondListener($resourceMetadataFactoryProphecy->reveal());
+        $listener->onKernelView($event);
+
+        $response = $event->getResponse();
+        $this->assertFalse($response->headers->has('Location'));
+        $this->assertSame(Response::HTTP_OK, $event->getResponse()->getStatusCode());
+    }
+
+    public function testPost301WithLocation()
+    {
+        $kernelProphecy = $this->prophesize(HttpKernelInterface::class);
+
+        $request = new Request([], [], ['_api_resource_class' => Dummy::class, '_api_item_operation_name' => 'get', '_api_respond' => true, '_api_write_item_iri' => '/dummy_entities/1']);
+        $request->setMethod('POST');
+
+        $event = new ViewEvent(
+            $kernelProphecy->reveal(),
+            $request,
+            HttpKernelInterface::MASTER_REQUEST,
+            'bar'
+        );
+        $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class);
+        $resourceMetadataFactoryProphecy->create(Dummy::class)->willReturn(new ResourceMetadata(null, null, null, ['get' => ['status' => Response::HTTP_MOVED_PERMANENTLY]]));
+
+        $listener = new RespondListener($resourceMetadataFactoryProphecy->reveal());
+        $listener->onKernelView($event);
+
+        $response = $event->getResponse();
+        $this->assertTrue($response->headers->has('Location'));
+        $this->assertEquals('/dummy_entities/1', $response->headers->get('Location'));
+        $this->assertSame(Response::HTTP_MOVED_PERMANENTLY, $event->getResponse()->getStatusCode());
+    }
+
     public function testCreate201Response()
     {
         $kernelProphecy = $this->prophesize(HttpKernelInterface::class);
@@ -107,6 +156,7 @@ public function testCreate201Response()
         $this->assertEquals('deny', $response->headers->get('X-Frame-Options'));
         $this->assertEquals('/dummy_entities/1', $response->headers->get('Location'));
         $this->assertEquals('/dummy_entities/1', $response->headers->get('Content-Location'));
+        $this->assertTrue($response->headers->has('Location'));
     }
 
     public function testCreate204Response()