Skip to content

Commit

Permalink
clear response tagger after tagging to not leak tags from one respons…
Browse files Browse the repository at this point in the history
…e to the next
  • Loading branch information
dbu committed Oct 3, 2018
1 parent f5da3a9 commit bc9c1ff
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 3 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----

Expand Down
5 changes: 5 additions & 0 deletions doc/response-tagging.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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::

Expand Down
24 changes: 21 additions & 3 deletions src/ResponseTagger.php
Original file line number Diff line number Diff line change
Expand Up @@ -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]>
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
}
13 changes: 13 additions & 0 deletions tests/Unit/ResponseTaggerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -66,6 +67,7 @@ public function testTagResponseReplace()

$tagger->addTags(['tag-1', 'tag-2']);
$tagger->tagResponse($response, true);
$this->assertFalse($tagger->hasTags());
}

public function testTagResponseAdd()
Expand All @@ -89,6 +91,7 @@ public function testTagResponseAdd()

$tagger->addTags(['tag-1', 'tag-2']);
$tagger->tagResponse($response);
$this->assertFalse($tagger->hasTags());
}

public function testTagResponseNoTags()
Expand Down Expand Up @@ -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());
}
}

0 comments on commit bc9c1ff

Please sign in to comment.