Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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());
}
}