Merge pull request #430 from FriendsOfSymfony/clean-tags-after-tagging

clear response tagger after tagging to not leak tags from one response to the next

CHANGELOG.md

@@ -11,6 +11,12 @@ unreleased
 * Fixed: Remove the xkey header in vcl_deliver if we are not in debug mode
 * Do not cleanup the Vary header and keep the user context hash if we are in debug mode
 
+### Cache Tagging
+
+* Fixed: Clear the ResponseTagger after we tagged a response. Usually PHP uses
+  a new instance for every request. But for example the hash lookup when using
+  Symfony HttpCache does two requests in the same PHP process.
+
 2.5.1
 -----
 

doc/response-tagging.rst

@@ -100,6 +100,11 @@ Before any content is sent out, you need to send the tag header_::
         $responseTagger->getTagsHeaderName(),
         $responseTagger->getTagsHeaderValue()
     ));
+    $responseTagger->clear();
+
+The call to ``clear`` is only relevant if the same PHP process handles multiple
+requests. This happens for example when you :doc:`cache on user context <user-context>`
+with the Symfony HttpCache.
 
 .. tip::
 

src/ResponseTagger.php

@@ -21,6 +21,10 @@
 /**
  * Service for Response cache tagging.
  *
+ * Record tags with this class and then either get the tags header or have the
+ * tagger add the tags to a PSR-7 response.
+ * Recorded tags are cleared after tagging a response.
+ *
  * @author David de Boer <[email protected]>
  * @author David Buchmann <[email protected]>
  * @author André Rømcke <[email protected]>
@@ -129,7 +133,18 @@ public function addTags(array $tags)
     }
 
     /**
-     * Set tags on a response.
+     * Remove all tags that have been recorded.
+     *
+     * This is usually called after adding the tags header to a response. It is
+     * automatically called by the tagResponse method.
+     */
+    public function clear()
+    {
+        $this->tags = [];
+    }
+
+    /**
+     * Set tags on a response and then clear the tags.
      *
      * @param ResponseInterface $response Original response
      * @param bool              $replace  Whether to replace the current tags
@@ -143,10 +158,13 @@ public function tagResponse(ResponseInterface $response, $replace = false)
             return $response;
         }
 
+        $tagsHeaderValue = $this->getTagsHeaderValue();
+        $this->clear();
+
         if ($replace) {
-            return $response->withHeader($this->getTagsHeaderName(), $this->getTagsHeaderValue());
+            return $response->withHeader($this->getTagsHeaderName(), $tagsHeaderValue);
         }
 
-        return $response->withAddedHeader($this->getTagsHeaderName(), $this->getTagsHeaderValue());
+        return $response->withAddedHeader($this->getTagsHeaderName(), $tagsHeaderValue);
     }
 }

tests/Unit/ResponseTaggerTest.php

@@ -43,6 +43,7 @@ public function testGetTagsHeaderValue()
         $tagger->addTags(['post-1', 'test,post']);
         $this->assertTrue($tagger->hasTags());
         $this->assertEquals('post-1,test_post', $tagger->getTagsHeaderValue());
+        $this->assertTrue($tagger->hasTags()); // getting the header does not clear the tags
     }
 
     public function testTagResponseReplace()
@@ -66,6 +67,7 @@ public function testTagResponseReplace()
 
         $tagger->addTags(['tag-1', 'tag-2']);
         $tagger->tagResponse($response, true);
+        $this->assertFalse($tagger->hasTags());
     }
 
     public function testTagResponseAdd()
@@ -89,6 +91,7 @@ public function testTagResponseAdd()
 
         $tagger->addTags(['tag-1', 'tag-2']);
         $tagger->tagResponse($response);
+        $this->assertFalse($tagger->hasTags());
     }
 
     public function testTagResponseNoTags()
@@ -144,4 +147,14 @@ public function testUniqueTags()
 
         $this->assertEquals('post-1,post-2,post-3', $tagHandler->getTagsHeaderValue());
     }
+
+    public function testClear()
+    {
+        $tagger = new ResponseTagger();
+        $this->assertFalse($tagger->hasTags());
+        $tagger->addTags(['foo', 'bar']);
+        $this->assertTrue($tagger->hasTags());
+        $tagger->clear();
+        $this->assertFalse($tagger->hasTags());
+    }
 }