Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[HttpCache] purge both http and https from http cache #21582

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[HttpCache] purge both http and https from http cache #21582

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
purge both http and https from http cache store
  • Loading branch information
@dbu
dbu committed Feb 18, 2017
commit 0249e87b44bb3fcdf6773c97e990319e96da4e54
20 changes: 18 additions & 2 deletions 20 src/Symfony/Component/HttpKernel/HttpCache/Store.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,14 +317,30 @@ private function getMetadata($key)
/**
* Purges data for the given URL.
*
* This method purges both the HTTP and the HTTPS version of the cache entry.
*
* @param string $url A URL
*
* @return bool true if the URL exists and has been purged, false otherwise
* @return bool true if the URL exists with either HTTP or HTTPS scheme and has been purged, false otherwise
*/
public function purge($url)
{
$key = $this->getCacheKey(Request::create($url));
$http = preg_replace('#^https#', 'http', $url);
Copy link
Contributor Author

@dbu dbu Feb 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if $url is already http://, nothing changes.

this was previously

        $url2 = 0 === strpos($url, 'https://')
            ? preg_replace('#^https#', 'http', $url, 1)
            : preg_replace('#^http#', 'https', $url, 1);

        return $this->doPurge($url) || $this->doPurge($url2);

but i find the new code more readable. we add one preg_replace instead of a strpos. given that we do multiple filesystem operations, the speed difference should not matter.

$https = preg_replace('#^http#', 'https', $url);

return $this->doPurge($http) || $this->doPurge($https);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the title of this PR, I would have expected this to be an && instead of an ||

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my thinking was that when either http or https was purged we report that something was purged. the chances that both http and https were in cache are small.

i think the most sane use case for this is when doing cache invalidation, e.g. with FOSHttpCache: you need to send a request to the specific host by ip, rather than the domain name, to target each server instance. but if you do that over https, you get certificate mismatches and thus want to use http.

apart from such special cases, your webserver should probably redirect requests from http to https rather than allow both.

}

/**
* Purges data for the given URL.
*
* @param string $url A URL
*
* @return bool true if the URL exists and has been purged, false otherwise
*/
private function doPurge($url)
{
$key = $this->getCacheKey(Request::create($url));
if (isset($this->locks[$key])) {
flock($this->locks[$key], LOCK_UN);
fclose($this->locks[$key]);
Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.