Page MenuHomePhabricator
Files F15402432

D15775.diff
No OneTemporary
Actions

D15775.diff
View Options

diff --git a/src/applications/celerity/CelerityResourceMap.php b/src/applications/celerity/CelerityResourceMap.php
--- a/src/applications/celerity/CelerityResourceMap.php
+++ b/src/applications/celerity/CelerityResourceMap.php
@@ -208,6 +208,11 @@
}
+ public function getHashForName($name) {
+ return idx($this->nameMap, $name);
+ }
+
+
/**
* Return the absolute URI for a resource, identified by hash.
* This method is fairly low-level and ignores packaging.
diff --git a/src/applications/celerity/controller/CelerityPhabricatorResourceController.php b/src/applications/celerity/controller/CelerityPhabricatorResourceController.php
--- a/src/applications/celerity/controller/CelerityPhabricatorResourceController.php
+++ b/src/applications/celerity/controller/CelerityPhabricatorResourceController.php
@@ -31,7 +31,11 @@
return new Aphront400Response();
}
- return $this->serveResource($this->path);
+ return $this->serveResource(
+ array(
+ 'path' => $this->path,
+ 'hash' => $this->hash,
+ ));
}
protected function buildResourceTransformer() {
diff --git a/src/applications/celerity/controller/CelerityResourceController.php b/src/applications/celerity/controller/CelerityResourceController.php
--- a/src/applications/celerity/controller/CelerityResourceController.php
+++ b/src/applications/celerity/controller/CelerityResourceController.php
@@ -24,7 +24,10 @@
abstract public function getCelerityResourceMap();
- protected function serveResource($path, $package_hash = null) {
+ protected function serveResource(array $spec) {
+ $path = $spec['path'];
+ $hash = idx($spec, 'hash');
+
// Sanity checking to keep this from exposing anything sensitive, since it
// ultimately boils down to disk reads.
if (preg_match('@(//|\.\.)@', $path)) {
@@ -40,18 +43,24 @@
$dev_mode = PhabricatorEnv::getEnvConfig('phabricator.developer-mode');
- if (AphrontRequest::getHTTPHeader('If-Modified-Since') && !$dev_mode) {
+ $map = $this->getCelerityResourceMap();
+ $expect_hash = $map->getHashForName($path);
+
+ // Test if the URI hash is correct for our current resource map. If it
+ // is not, refuse to cache this resource. This avoids poisoning caches
+ // and CDNs if we're getting a request for a new resource to an old node
+ // shortly after a push.
+ $is_cacheable = ($hash === $expect_hash) &&
+ $this->isCacheableResourceType($type);
+ if (AphrontRequest::getHTTPHeader('If-Modified-Since') && $is_cacheable) {
// Return a "304 Not Modified". We don't care about the value of this
// field since we never change what resource is served by a given URI.
return $this->makeResponseCacheable(new Aphront304Response());
}
- $is_cacheable = (!$dev_mode) &&
- $this->isCacheableResourceType($type);
-
$cache = null;
$data = null;
- if ($is_cacheable) {
+ if ($is_cacheable && !$dev_mode) {
$cache = PhabricatorCaches::getImmutableCache();
$request_path = $this->getRequest()->getPath();
@@ -61,8 +70,6 @@
}
if ($data === null) {
- $map = $this->getCelerityResourceMap();
-
if ($map->isPackageResource($path)) {
$resource_names = $map->getResourceNamesForPackageName($path);
if (!$resource_names) {
@@ -117,7 +124,11 @@
$response->addAllowOrigin('*');
}
- return $this->makeResponseCacheable($response);
+ if ($is_cacheable) {
+ $response = $this->makeResponseCacheable($response);
+ }
+
+ return $response;
}
public static function getSupportedResourceTypes() {

File Metadata

Mime Type
text/plain
Expires
Tue, Mar 18, 10:31 PM (4 d, 8 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7706195
Default Alt Text
D15775.diff (3 KB)

Event Timeline

Phabricator · Built by Phacility