diff --git a/cdn.install b/cdn.install index c788522..5ac2574 100644 --- a/cdn.install +++ b/cdn.install @@ -20,3 +20,14 @@ function cdn_update_8001() { $cdn_settings->save(); } } + +/** + * Update the default settings to include the stream_wrappers schema. + */ +function cdn_update_8002() { + $cdn_settings = \Drupal::configFactory()->getEditable('cdn.settings'); + if (is_null($cdn_settings->get('stream_wrappers'))) { + $cdn_settings->set('stream_wrappers', []); + $cdn_settings->save(); + } +} diff --git a/cdn.routing.yml b/cdn.routing.yml index 6a53eb5..6912dbf 100644 --- a/cdn.routing.yml +++ b/cdn.routing.yml @@ -1,7 +1,17 @@ +# The /cdn/farfuture route has been deprecated and is rewritten +# in CdnFarfuturePathProcessor. cdn.farfuture.download: path: '/cdn/farfuture/{security_token}/{mtime}' defaults: _controller: cdn.controller.farfuture:download + _disable_route_normalizer: TRUE + requirements: + _access: 'TRUE' + mtime: \d+ +cdn.farfuture_scheme.download: + path: '/cdn/ff/{security_token}/{mtime}/{scheme}' + defaults: + _controller: cdn.controller.farfuture:downloadByScheme # Ensure the redirect module does not redirect to add a language prefix. # @see \Drupal\redirect\EventSubscriber\RouteNormalizerRequestSubscriber # @todo Update this comment when https://www.drupal.org/project/drupal/issues/2641118 lands. @@ -9,3 +19,4 @@ cdn.farfuture.download: requirements: _access: 'TRUE' mtime: \d+ + scheme: '(:\w+:)|([a-zA-Z0-9+.-]+)' diff --git a/cdn.services.yml b/cdn.services.yml index 61f94d3..244a5b2 100644 --- a/cdn.services.yml +++ b/cdn.services.yml @@ -1,7 +1,7 @@ services: cdn.settings: class: Drupal\cdn\CdnSettings - arguments: ['@config.factory'] + arguments: ['@config.factory', '@stream_wrapper_manager'] cdn.file_url_generator: class: Drupal\cdn\File\FileUrlGenerator @@ -22,7 +22,7 @@ services: # Controllers. cdn.controller.farfuture: class: \Drupal\cdn\CdnFarfutureController - arguments: ['@private_key'] + arguments: ['@private_key', '@file_system'] # Inbound path processor for the cdn.farfuture.download route, since the # Drupal 8/Symfony routing system does not support "menu tail" or "slash in diff --git a/cdn_ui/js/summaries.js b/cdn_ui/js/summaries.js index 64696be..0be8aa3 100644 --- a/cdn_ui/js/summaries.js +++ b/cdn_ui/js/summaries.js @@ -15,6 +15,15 @@ return document.querySelector('input[name="status"]').checked ? Drupal.t('Enabled') : Drupal.t('Disabled'); }); + $('[data-drupal-selector="edit-wrappers"]').drupalSetSummary(function () { + var additional = $('[data-drupal-selector="edit-wrappers-stream-wrappers"] input:checked'); + var wrappers = []; + additional.each(function(index) { + wrappers.push(this.getAttribute('value')); + }); + return wrappers.join(', '); + }); + $('[data-drupal-selector="edit-mapping"]').drupalSetSummary(function () { if (document.querySelector('select[name="mapping[type]"]').value === 'simple') { var domain = document.querySelector('input[name="mapping[simple][domain]"]').value; diff --git a/cdn_ui/src/Form/CdnSettingsForm.php b/cdn_ui/src/Form/CdnSettingsForm.php index bd975a7..969d978 100644 --- a/cdn_ui/src/Form/CdnSettingsForm.php +++ b/cdn_ui/src/Form/CdnSettingsForm.php @@ -3,14 +3,45 @@ namespace Drupal\cdn_ui\Form; use Drupal\cdn\CdnSettings; +use Drupal\Component\Utility\UrlHelper; +use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Form\ConfigFormBase; use Drupal\Core\Form\FormStateInterface; +use Drupal\Core\StreamWrapper\StreamWrapperInterface; +use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface; +use Drupal\Core\Url; +use Symfony\Component\DependencyInjection\ContainerInterface; /** * Configure CDN settings for this site. */ class CdnSettingsForm extends ConfigFormBase { + /** + * The stream wrapper manager. + * + * @var \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface + */ + protected $streamWrapperManager; + + /** + * @inheritDoc + */ + public function __construct(ConfigFactoryInterface $config_factory, StreamWrapperManagerInterface $streamWrapperManager) { + parent::__construct($config_factory); + $this->streamWrapperManager = $streamWrapperManager; + } + + /** + * @inheritDoc + */ + public static function create(ContainerInterface $container) { + return new static( + $container->get('config.factory'), + $container->get('stream_wrapper_manager') + ); + } + /** * {@inheritdoc} */ @@ -74,7 +105,6 @@ class CdnSettingsForm extends ConfigFormBase { '#wrapper_attributes' => ['class' => ['container-inline']], '#attributes' => ['class' => ['container-inline']], '#default_value' => $config->get('mapping.type') === 'simple' ?: 'advanced', - '#attributes' => ['class' => ['container-inline']], ]; $form['mapping']['simple'] = [ '#type' => 'container', @@ -143,6 +173,49 @@ class CdnSettingsForm extends ConfigFormBase { '#default_value' => $config->get('farfuture.status'), ]; + $visibleWrappers = array_keys($this->streamWrapperManager + ->getWrappers(StreamWrapperInterface::VISIBLE)); + $localWrappers = array_keys($this->streamWrapperManager + ->getWrappers(StreamWrapperInterface::LOCAL_NORMAL)); + $wrappers = array_unique(array_merge($localWrappers, $visibleWrappers)); + $existingWrappers = $config->get('stream_wrappers'); + $form['wrappers'] = [ + '#type' => 'details', + '#title' => $this->t('Stream wrappers'), + '#group' => 'cdn_settings', + '#tree' => TRUE, + '#access' => count($wrappers) > count($localWrappers), + ]; + $form['wrappers']['stream_wrappers'] = [ + '#type' => 'checkboxes', + '#options' => array_combine($wrappers, $wrappers), + '#default_value' => array_diff( + array_merge($localWrappers, $existingWrappers), + ['private'] + ), + '#description' => $this->t('Stream wrappers to rewrite for CDN. Non-private "local" stream wrappers are always enabled.'), + ]; + foreach (array_diff($visibleWrappers, $localWrappers) as $wrapper) { + // Determine whether the stream wrapper generates truly external URLs. + // If so, it's likely CDN module will have no effect and we should warn. + try { + $external = $this->streamWrapperManager->getViaUri($wrapper . '://') + ->getExternalUrl(); + } + catch (\Exception $e) { + // Some stream wrappers might not comply with this kind of blind test. + // Give the stream wrapper the benefit of the doubt. + continue; + } + $relative_url = str_replace($this->getRequest()->getSchemeAndHttpHost() . $this->getBasePath(), '', $external); + if (UrlHelper::isExternal($relative_url)) { + $form['wrappers']['stream_wrappers'][$wrapper]['#disabled'] = TRUE; + $form['wrappers']['stream_wrappers'][$wrapper]['#description'] = $this->t('CDN will not rewrite external URLs.'); + } + } + foreach ($localWrappers as $localWrapper) { + $form['wrappers']['stream_wrappers'][$localWrapper]['#disabled'] = TRUE; + } return parent::buildForm($form, $form_state); } @@ -169,6 +242,9 @@ class CdnSettingsForm extends ConfigFormBase { // Vertical tab: 'Status'. $config->set('status', (bool) $form_state->getValue('status')); + // Vertical tab: 'Additional stream wrappers'. + $config->set('stream_wrappers', array_values(array_filter($form_state->getValue(['wrappers', 'stream_wrappers'])))); + // Vertical tab: 'Mapping'. if ($form_state->getValue(['mapping', 'type']) === 'simple') { $simple_mapping = $form_state->getValue(['mapping', 'simple']); @@ -206,4 +282,11 @@ class CdnSettingsForm extends ConfigFormBase { parent::submitForm($form, $form_state); } + /** + * @see \Symfony\Component\HttpFoundation\Request::getBasePath() + */ + protected function getBasePath() { + return $this->getRequest()->getBasePath(); + } + } diff --git a/config/install/cdn.settings.yml b/config/install/cdn.settings.yml index 0ed3fc6..79e0859 100644 --- a/config/install/cdn.settings.yml +++ b/config/install/cdn.settings.yml @@ -69,3 +69,6 @@ mapping: farfuture: status: true +# Public is enabled by rule, additional wrappers can be added. +stream_wrappers: + - public diff --git a/config/schema/cdn.schema.yml b/config/schema/cdn.schema.yml index b0d9272..e543e5b 100644 --- a/config/schema/cdn.schema.yml +++ b/config/schema/cdn.schema.yml @@ -20,3 +20,8 @@ cdn.settings: status: label: 'Forever cacheable files — status' type: boolean + stream_wrappers: + label: 'Stream wrappers for CDN' + type: sequence + sequence: + type: string diff --git a/src/CdnFarfutureController.php b/src/CdnFarfutureController.php index e1f7bea..0ef5fda 100644 --- a/src/CdnFarfutureController.php +++ b/src/CdnFarfutureController.php @@ -2,7 +2,9 @@ namespace Drupal\cdn; +use Drupal\cdn\File\FileUrlGenerator; use Drupal\Component\Utility\Crypt; +use Drupal\Core\File\FileSystemInterface; use Drupal\Core\PrivateKey; use Drupal\Core\Site\Settings; use Symfony\Component\HttpFoundation\BinaryFileResponse; @@ -19,12 +21,73 @@ class CdnFarfutureController { */ protected $privateKey; + /** + * The file system service. + * + * @var \Drupal\Core\File\FileSystemInterface + */ + protected $fileSystem; + /** * @param \Drupal\Core\PrivateKey $private_key * The private key service. */ - public function __construct(PrivateKey $private_key) { + public function __construct(PrivateKey $private_key, FileSystemInterface $fileSystem) { $this->privateKey = $private_key; + $this->fileSystem = $fileSystem; + } + + /** + * Serves the requested file with optimal far future expiration headers. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The current request. $request->query must have root_relative_file_url, + * set by \Drupal\cdn\PathProcessor\CdnFarfuturePathProcessor. + * @param string $security_token + * The security token. Ensures that users can not request any file they want + * by manipulating the URL (they could otherwise request settings.php for + * example). See https://www.drupal.org/node/1441502. + * @param int $mtime + * The file's mtime. + * @param string $scheme + * The file's scheme. + * + * @returns \Symfony\Component\HttpFoundation\BinaryFileResponse + * The response that will efficiently send the requested file. + * + * @throws \Symfony\Component\HttpKernel\Exception\BadRequestHttpException + * Thrown when the 'root_relative_file_url' query argument is not set, which + * can only happen in case of malicious requests or in case of a malfunction + * in \Drupal\cdn\PathProcessor\CdnFarfuturePathProcessor. + * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException + * Thrown when an invalid security token is provided. + */ + public function downloadByScheme(Request $request, $security_token, $mtime, $scheme) { + // Validate the scheme early. + if (!$request->query->has('relative_file_url') || ($scheme != FileUrlGenerator::RELATIVE && !$this->fileSystem->validScheme($scheme))) { + throw new BadRequestHttpException(); + } + + $path = $request->query->get('relative_file_url'); + // A relative URL for a file contains '%20' instead of spaces. A relative + // file path contains spaces. + $uri = $scheme == FileUrlGenerator::RELATIVE + ? $path + : $scheme . ':/' . $path; // Path comes with a leading slash from the URL. + // Validate security token. + $calculated_token = Crypt::hmacBase64($mtime . $scheme . $path, $this->privateKey->get() . Settings::getHashSalt()); + if ($security_token !== $calculated_token) { + throw new AccessDeniedHttpException('Invalid security token.'); + } + + // Strip the leading slash for truly relative paths. + if ($scheme == FileUrlGenerator::RELATIVE) { + $uri = substr($path, 1); + } + + $response = new BinaryFileResponse(rawurldecode($uri), 200, $this->getFarfutureHeaders(), TRUE, NULL, FALSE, FALSE); + $response->isNotModified($request); + return $response; } /** @@ -49,6 +112,8 @@ class CdnFarfutureController { * in \Drupal\cdn\PathProcessor\CdnFarfuturePathProcessor. * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException * Thrown when an invalid security token is provided. + * + * @deprecated This method is deprecated in favor of ::downloadByScheme */ public function download(Request $request, $security_token, $mtime) { // Ensure \Drupal\cdn\PathProcessor\CdnFarfuturePathProcessor did its job. @@ -63,7 +128,22 @@ class CdnFarfutureController { throw new AccessDeniedHttpException('Invalid security token.'); } - $farfuture_headers = [ + // A relative URL for a file contains '%20' instead of spaces. A relative + // file path contains spaces. + $relative_file_path = rawurldecode($root_relative_file_url); + + $response = new BinaryFileResponse(substr($relative_file_path, 1), 200, $this->getFarfutureHeaders(), TRUE, NULL, FALSE, FALSE); + $response->isNotModified($request); + return $response; + } + + /** + * Return the headers to serve with far future responses. + * + * @return string[] + */ + protected function getFarfutureHeaders() { + return [ // Instead of being powered by PHP, tell the world this resource was // powered by the CDN module! 'X-Powered-By' => 'Drupal CDN module (https://www.drupal.org/project/cdn)', @@ -97,14 +177,6 @@ class CdnFarfutureController { // Also see http://code.google.com/speed/page-speed/docs/caching.html. 'Last-Modified' => 'Wed, 20 Jan 1988 04:20:42 GMT', ]; - - // A relative URL for a file contains '%20' instead of spaces. A relative - // file path contains spaces. - $relative_file_path = rawurldecode($root_relative_file_url); - - $response = new BinaryFileResponse(substr($relative_file_path, 1), 200, $farfuture_headers, TRUE, NULL, FALSE, FALSE); - $response->isNotModified($request); - return $response; } } diff --git a/src/CdnSettings.php b/src/CdnSettings.php index 45da071..acc189b 100644 --- a/src/CdnSettings.php +++ b/src/CdnSettings.php @@ -2,8 +2,11 @@ namespace Drupal\cdn; +use Drupal\Component\Utility\Unicode; use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Config\ConfigValueException; +use Drupal\Core\StreamWrapper\StreamWrapperInterface; +use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface; /** * Wraps the CDN settings configuration, contains all parsing. @@ -26,15 +29,25 @@ class CdnSettings { */ protected $lookupTable; + /** + * The stream wrapper manager. + * + * @var \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface + */ + protected $streamWrapperManager; + /** * Constructs a new CdnSettings object. * * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory * The config factory. + * @param \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface $streamWrapperManager + * The stream wrapper manager. */ - public function __construct(ConfigFactoryInterface $config_factory) { + public function __construct(ConfigFactoryInterface $config_factory, StreamWrapperManagerInterface $streamWrapperManager) { $this->rawSettings = $config_factory->get('cdn.settings'); $this->lookupTable = NULL; + $this->streamWrapperManager = $streamWrapperManager; } /** @@ -77,6 +90,19 @@ class CdnSettings { return $unique_domains; } + /** + * Returns CDN-eligible stream wrappers. + * + * @return string[] The allowed stream wrapper scheme names. + */ + public function streamWrappers() { + $configured = $this->rawSettings->get('stream_wrappers'); + $wrappers = array_merge(array_keys($this->streamWrapperManager + ->getWrappers(StreamWrapperInterface::LOCAL_NORMAL)), $configured); + // Private scheme is always excluded. + return array_diff($wrappers, ['private']); + } + /** * Builds a lookup table: file extension to CDN domain(s). * @@ -175,4 +201,47 @@ class CdnSettings { return $components === FALSE ? FALSE : empty(array_intersect($forbidden_components, array_keys($components))); } + /** + * Maps a URI to a CDN domain. + * + * @param string $uri + * The URI to map. + * + * @return string|bool + * The mapped domain, or FALSE if it could not be matched. + */ + public function getCdnDomain($uri) { + // Extension-specific mapping. + $file_extension = Unicode::strtolower(pathinfo($uri, PATHINFO_EXTENSION)); + $lookup_table = $this->getLookupTable(); + if (isset($lookup_table[$file_extension])) { + $key = $file_extension; + } + // Generic or fallback mapping. + elseif (isset($lookup_table['*'])) { + $key = '*'; + } + // No mapping. + else { + return FALSE; + } + + $result = $lookup_table[$key]; + + if ($result === FALSE) { + return FALSE; + } + // If there are multiple results, pick one using consistent hashing: ensure + // the same file is always served from the same CDN domain. + elseif (is_array($result)) { + $filename = basename($uri); + $hash = hexdec(substr(md5($filename), 0, 5)); + $cdn_domain = $result[$hash % count($result)]; + } + else { + $cdn_domain = $result; + } + return $cdn_domain; + } + } diff --git a/src/File/FileUrlGenerator.php b/src/File/FileUrlGenerator.php index 13a8cf3..65153c2 100644 --- a/src/File/FileUrlGenerator.php +++ b/src/File/FileUrlGenerator.php @@ -5,10 +5,10 @@ namespace Drupal\cdn\File; use Drupal\cdn\CdnSettings; use Drupal\Component\Utility\Crypt; use Drupal\Component\Utility\Unicode; +use Drupal\Component\Utility\UrlHelper; use Drupal\Core\File\FileSystemInterface; use Drupal\Core\PrivateKey; use Drupal\Core\Site\Settings; -use Drupal\Core\StreamWrapper\StreamWrapperInterface; use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface; use Symfony\Component\HttpFoundation\RequestStack; @@ -19,6 +19,8 @@ use Symfony\Component\HttpFoundation\RequestStack; */ class FileUrlGenerator { + const RELATIVE = ':relative:'; + /** * The app root. * @@ -111,8 +113,7 @@ class FileUrlGenerator { return FALSE; } - $relative_url = $this->getRelativeUrl($uri); - if ($relative_url === FALSE) { + if (!$this->canServe($uri)) { return FALSE; } @@ -123,23 +124,32 @@ class FileUrlGenerator { // When farfuture is enabled, rewrite the file URL to let Drupal serve the // file with optimal headers. Only possible if the file exists. - if ($this->settings->farfutureIsEnabled()) { + if (!$scheme = $this->fileSystem->uriScheme($uri)) { + $scheme = self::RELATIVE; // A relative URL for a file contains '%20' instead of spaces. A relative // file path contains spaces. - $relative_file_path = rawurldecode($relative_url); - $absolute_file_path = $this->root . $relative_file_path; - if (file_exists($absolute_file_path)) { - // We do the filemtime() call separately, because a failed filemtime() - // will cause a PHP warning to be written to the log, which would remove - // any performance gain achieved by removing the file_exists() call. - $mtime = filemtime($absolute_file_path); - - // Generate a security token. Ensures that users can not request any - // file they want by manipulating the URL (they could otherwise request - // settings.php for example). See https://www.drupal.org/node/1441502. - $calculated_token = Crypt::hmacBase64($mtime . $relative_url, $this->privateKey->get() . Settings::getHashSalt()); - return '//' . $cdn_domain . $this->getBasePath() . '/cdn/farfuture/' . $calculated_token . '/' . $mtime . $relative_url; - } + $filePath = $relative_url = '/' . $uri; + $realFile = $this->root . rawurldecode($relative_url);; + } + else { + $fileUri = $realFile = $uri; + $filePath = '/' . substr($fileUri, strlen($scheme . '://')); + $relative_url = str_replace($this->requestStack->getCurrentRequest()->getSchemeAndHttpHost() . $this->getBasePath(), '', $this->streamWrapperManager->getViaUri($uri)->getExternalUrl()); + } + + // When farfuture is enabled, rewrite the file URL to let Drupal serve the + // file with optimal headers. Only possible if the file exists. + if ($this->settings->farfutureIsEnabled() && file_exists($realFile)) { + // We do the filemtime() call separately, because a failed filemtime() + // will cause a PHP warning to be written to the log, which would remove + // any performance gain achieved by removing the file_exists() call. + $mtime = filemtime($realFile); + + // Generate a security token. Ensures that users can not request any + // file they want by manipulating the URL (they could otherwise request + // settings.php for example). See https://www.drupal.org/node/1441502. + $calculated_token = Crypt::hmacBase64($mtime . $scheme . UrlHelper::encodePath($filePath), $this->privateKey->get() . Settings::getHashSalt()); + return '//' . $cdn_domain . $this->getBasePath() . '/cdn/ff/' . $calculated_token . '/' . $mtime . '/' . $scheme . $filePath; } return '//' . $cdn_domain . $this->getBasePath() . $relative_url; @@ -190,40 +200,31 @@ class FileUrlGenerator { } /** - * Gets the relative URL for files that are shipped or in a local stream. + * Determines if a URI can/should be served by CDN. * * @param string $uri * The URI to a file for which we need a CDN URL, or the path to a shipped * file. * - * @return bool|string - * Returns FALSE if the URI is not for a shipped file or in a local stream. - * Otherwise, returns the relative URL. + * @return bool + * Returns FALSE if the URI is not for a shipped file or in an eligible + * stream. TRUE otherwise. */ - protected function getRelativeUrl($uri) { + protected function canServe($uri) { $scheme = $this->fileSystem->uriScheme($uri); + // Allow additional stream wrappers to be served via CDN. + $streamWrapperTypes = $this->settings->streamWrappers(); // If the URI is absolute — HTTP(S) or otherwise — return early, except if - // it's an absolute URI using a local stream wrapper scheme. - if ($scheme && !isset($this->streamWrapperManager->getWrappers(StreamWrapperInterface::LOCAL)[$scheme])) { + // it's an absolute URI using an approved stream wrapper type. + if ($scheme && !in_array($scheme, $streamWrapperTypes)) { return FALSE; } // If the URI is protocol-relative, return early. elseif (Unicode::substr($uri, 0, 2) === '//') { return FALSE; } - // The private:// stream wrapper is explicitly not supported. - elseif ($scheme === 'private') { - return FALSE; - } - - $request = $this->requestStack->getCurrentRequest(); - - return $scheme - // Local stream wrapper. - ? str_replace($request->getSchemeAndHttpHost() . $this->getBasePath(), '', $this->streamWrapperManager->getViaUri($uri)->getExternalUrl()) - // Shipped file. - : '/' . $uri; + return TRUE; } /** diff --git a/src/PathProcessor/CdnFarfuturePathProcessor.php b/src/PathProcessor/CdnFarfuturePathProcessor.php index d793834..b44702f 100644 --- a/src/PathProcessor/CdnFarfuturePathProcessor.php +++ b/src/PathProcessor/CdnFarfuturePathProcessor.php @@ -2,6 +2,7 @@ namespace Drupal\cdn\PathProcessor; +use Drupal\cdn\File\FileUrlGenerator; use Drupal\Component\Utility\UrlHelper; use Drupal\Core\PathProcessor\InboundPathProcessorInterface; use Symfony\Component\HttpFoundation\Request; @@ -12,6 +13,9 @@ use Symfony\Component\HttpFoundation\Request; * As the route system does not allow arbitrary amount of parameters convert * the file path to a query parameter on the request. * + * Also normalizes legacy far-future URLs generated prior to + * https://www.drupal.org/node/2870435 + * * @see \Drupal\image\PathProcessor\PathProcessorImageStyles */ class CdnFarfuturePathProcessor implements InboundPathProcessorInterface { @@ -20,19 +24,54 @@ class CdnFarfuturePathProcessor implements InboundPathProcessorInterface { * {@inheritdoc} */ public function processInbound($path, Request $request) { - if (strpos($path, '/cdn/farfuture/') !== 0) { - return $path; + if (strpos($path, '/cdn/farfuture/') === 0) { + return $this->processDeprecatedFarFuture($path, $request); + } + if (strpos($path, '/cdn/ff/') === 0) { + return $this->processFarFuture($path, $request); } + return $path; + } - // Parse the security token, mtime and root-relative file URL. + /** + * Process the path for the far future controller. + * + * @param string $path + * The path. + * @param \Symfony\Component\HttpFoundation\Request $request + * The request. + * + * @return string The processed path. + */ + protected function processFarFuture($path, Request $request) { + // Parse the security token, mtime, scheme and root-relative file URL. + $tail = substr($path, strlen('/cdn/ff/')); + list($security_token, $mtime, $scheme, $relative_file_url) = explode('/', $tail, 4); + $returnPath = "/cdn/ff/$security_token/$mtime/$scheme"; + // Set the root-relative file URL as query parameter. + $request->query->set('relative_file_url', '/' . UrlHelper::encodePath($relative_file_url)); + // Return the same path, but without the trailing file. + return $returnPath; + } + + /** + * Process the path for the deprecated far future controller. + * + * @param string $path + * The path. + * @param \Symfony\Component\HttpFoundation\Request $request + * The request. + * + * @return string The processed path. + */ + protected function processDeprecatedFarFuture($path, Request $request) { $tail = substr($path, strlen('/cdn/farfuture/')); list($security_token, $mtime, $root_relative_file_url) = explode('/', $tail, 3); - + $returnPath = "/cdn/farfuture/$security_token/$mtime"; // Set the root-relative file URL as query parameter. $request->query->set('root_relative_file_url', '/' . UrlHelper::encodePath($root_relative_file_url)); - // Return the same path, but without the trailing file. - return "/cdn/farfuture/$security_token/$mtime"; + return $returnPath; } } diff --git a/tests/src/Functional/CdnIntegrationTest.php b/tests/src/Functional/CdnIntegrationTest.php index 4484467..a2008f3 100644 --- a/tests/src/Functional/CdnIntegrationTest.php +++ b/tests/src/Functional/CdnIntegrationTest.php @@ -116,7 +116,7 @@ class CdnIntegrationTest extends BrowserTestBase { $this->drupalGet(''); $this->assertSame('MISS', $session->getResponseHeader('X-Drupal-Cache'), 'Changing CDN settings causes Page Cache miss: setting changes have immediate effect.'); $href = $this->cssSelect('link[rel=stylesheet]')[0]->getAttribute('href'); - $regexp = '#//cdn.example.com' . base_path() . 'cdn/farfuture/[a-zA-Z0-9_-]{43}/[0-9]{10}/' . $this->siteDirectory . '/files/css/css_[a-zA-Z0-9_-]{43}\.css\?[a-z0-9]{6}#'; + $regexp = '#//cdn.example.com' . base_path() . 'cdn/ff/[a-zA-Z0-9_-]{43}/[0-9]{10}/public/css/css_[a-zA-Z0-9_-]{43}\.css\?[a-z0-9]{6}#'; $this->assertSame(1, preg_match($regexp, $href)); $this->assertCssFileUsesRootRelativeUrl($this->baseUrl . str_replace('//cdn.example.com', '', $href)); } @@ -154,9 +154,9 @@ class CdnIntegrationTest extends BrowserTestBase { } /** - * Tests that the cdn.farfuture.download route/controller work as expected. + * Tests the legacy far future path. */ - public function testFarfuture() { + public function testLegacyFarfuture() { $druplicon_png_mtime = filemtime('public://druplicon ❤️.png'); $druplicon_png_security_token = Crypt::hmacBase64($druplicon_png_mtime . '/' . $this->siteDirectory . '/files/' . UrlHelper::encodePath('druplicon ❤️.png'), \Drupal::service('private_key')->get() . Settings::getHashSalt()); @@ -172,4 +172,25 @@ class CdnIntegrationTest extends BrowserTestBase { $this->assertSession()->statusCodeEquals(403); } + /** + * Tests that the cdn.farfuture.download route/controller work as expected. + */ + public function testFarfuture() { + $druplicon_png_mtime = filemtime('public://druplicon ❤️.png'); + $druplicon_png_security_token = Crypt::hmacBase64($druplicon_png_mtime . 'public' . UrlHelper::encodePath('/druplicon ❤️.png'), \Drupal::service('private_key')->get() . Settings::getHashSalt()); + $druplicon_png_relative_security_token = Crypt::hmacBase64($druplicon_png_mtime . ':relative:' . UrlHelper::encodePath('/' . $this->siteDirectory . '/files/druplicon ❤️.png'), \Drupal::service('private_key')->get() . Settings::getHashSalt()); + $this->drupalGet('/cdn/ff/' . $druplicon_png_security_token . '/' . $druplicon_png_mtime . '/public/druplicon ❤️.png'); + $this->assertSession()->statusCodeEquals(200); + $this->drupalGet('/cdn/ff/' . $druplicon_png_relative_security_token . '/' . $druplicon_png_mtime . '/:relative:/' . $this->siteDirectory . '/files/druplicon ❤️.png'); + $this->assertSession()->statusCodeEquals(200); + // Assert presence of headers that \Drupal\cdn\CdnFarfutureController sets. + $this->assertSame('Wed, 20 Jan 1988 04:20:42 GMT', $this->getSession()->getResponseHeader('Last-Modified')); + // Assert presence of headers that Symfony's BinaryFileResponse sets. + $this->assertSame('bytes', $this->getSession()->getResponseHeader('Accept-Ranges')); + + // Any chance to the security token should cause a 403. + $this->drupalGet('/cdn/ff/' . substr($druplicon_png_security_token, 1) . '/' . $druplicon_png_mtime . '/public/druplicon ❤️.png'); + $this->assertSession()->statusCodeEquals(403); + } + } diff --git a/tests/src/Unit/CdnSettingsTest.php b/tests/src/Unit/CdnSettingsTest.php index c247403..2ba20ec 100644 --- a/tests/src/Unit/CdnSettingsTest.php +++ b/tests/src/Unit/CdnSettingsTest.php @@ -4,6 +4,7 @@ namespace Drupal\Tests\cdn\Unit; use Drupal\cdn\CdnSettings; use Drupal\Core\Config\ConfigValueException; +use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface; use Drupal\Tests\UnitTestCase; /** @@ -444,7 +445,8 @@ class CdnSettingsTest extends UnitTestCase { * The CdnSettings object to test. */ protected function createCdnSettings(array $raw_config) { - return new CdnSettings($this->getConfigFactoryStub(['cdn.settings' => $raw_config])); + $stream_wrapper_manager = $this->prophesize(StreamWrapperManagerInterface::class); + return new CdnSettings($this->getConfigFactoryStub(['cdn.settings' => $raw_config]), $stream_wrapper_manager->reveal()); } } diff --git a/tests/src/Unit/File/FileUrlGeneratorTest.php b/tests/src/Unit/File/FileUrlGeneratorTest.php index 40a085d..4d4a7f4 100644 --- a/tests/src/Unit/File/FileUrlGeneratorTest.php +++ b/tests/src/Unit/File/FileUrlGeneratorTest.php @@ -9,6 +9,7 @@ use Drupal\Component\Utility\UrlHelper; use Drupal\Core\File\FileSystem; use Drupal\Core\PrivateKey; use Drupal\Core\Site\Settings; +use Drupal\Core\StreamWrapper\LocalStream; use Drupal\Core\StreamWrapper\PublicStream; use Drupal\Core\StreamWrapper\StreamWrapperInterface; use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface; @@ -67,10 +68,11 @@ class FileUrlGeneratorTest extends UnitTestCase { ], ], ], - 'farfuture' => [ - 'status' => FALSE, - ], ], + 'farfuture' => [ + 'status' => FALSE, + ], + 'stream_wrappers' => ['public'], ]); $this->assertSame($expected_result, $gen->generate($uri)); } @@ -133,6 +135,7 @@ class FileUrlGeneratorTest extends UnitTestCase { 'farfuture' => [ 'status' => TRUE, ], + 'stream_wrappers' => ['public', 'cdn-module-test'], ]; // Generate file for testing managed file. @@ -146,16 +149,21 @@ class FileUrlGeneratorTest extends UnitTestCase { $gen = $this->createFileUrlGenerator('', $config); $this->assertSame('//cdn.example.com/core/misc/does-not-exist.js', $gen->generate('core/misc/does-not-exist.js')); $drupal_js_mtime = filemtime($this->root . '/core/misc/drupal.js'); - $drupal_js_security_token = Crypt::hmacBase64($drupal_js_mtime . '/core/misc/drupal.js', static::$privateKey . Settings::getHashSalt()); - $this->assertSame('//cdn.example.com/cdn/farfuture/' . $drupal_js_security_token . '/' . $drupal_js_mtime . '/core/misc/drupal.js', $gen->generate('core/misc/drupal.js')); - $llama_jpg_security_token = Crypt::hmacBase64($llama_jpg_mtime . '/sites/default/files/' . UrlHelper::encodePath($llama_jpg_filename), static::$privateKey . Settings::getHashSalt()); - $this->assertSame('//cdn.example.com/cdn/farfuture/' . $llama_jpg_security_token . '/' . $llama_jpg_mtime . '/sites/default/files/' . UrlHelper::encodePath($llama_jpg_filename), $gen->generate('public://' . $llama_jpg_filename)); + $drupal_js_security_token = Crypt::hmacBase64($drupal_js_mtime . ':relative:' . UrlHelper::encodePath('/core/misc/drupal.js'), static::$privateKey . Settings::getHashSalt()); + $this->assertSame('//cdn.example.com/cdn/ff/' . $drupal_js_security_token . '/' . $drupal_js_mtime . '/:relative:/core/misc/drupal.js', $gen->generate('core/misc/drupal.js')); + // Since the public stream wrapper is not available in the unit test, + // and we use file_exists() in the target method, we are using the + // cdn-module-test:// scheme that ships with PHP. This does require + // injecting a leading into the path that we compare against, to match + // the method. + $llama_jpg_security_token = Crypt::hmacBase64($llama_jpg_mtime . 'file' . UrlHelper::encodePath('/' . $llama_jpg_filepath), static::$privateKey . Settings::getHashSalt()); + $this->assertSame('//cdn.example.com/cdn/ff/' . $llama_jpg_security_token . '/' . $llama_jpg_mtime . '/file/' . $llama_jpg_filepath, $gen->generate('cdn-module-test://' . $llama_jpg_filepath)); // In subdir: 1) non-existing file, 2) shipped file, 3) managed file. $gen = $this->createFileUrlGenerator('/subdir', $config); $this->assertSame('//cdn.example.com/subdir/core/misc/does-not-exist.js', $gen->generate('core/misc/does-not-exist.js')); - $this->assertSame('//cdn.example.com/subdir/cdn/farfuture/' . $drupal_js_security_token . '/' . $drupal_js_mtime . '/core/misc/drupal.js', $gen->generate('core/misc/drupal.js')); - $this->assertSame('//cdn.example.com/subdir/cdn/farfuture/' . $llama_jpg_security_token . '/' . $llama_jpg_mtime . '/sites/default/files/' . UrlHelper::encodePath($llama_jpg_filename), $gen->generate('public://' . $llama_jpg_filename)); + $this->assertSame('//cdn.example.com/subdir/cdn/ff/' . $drupal_js_security_token . '/' . $drupal_js_mtime . '/:relative:/core/misc/drupal.js', $gen->generate('core/misc/drupal.js')); + $this->assertSame('//cdn.example.com/subdir/cdn/ff/' . $llama_jpg_security_token . '/' . $llama_jpg_mtime . '/file/' . $llama_jpg_filepath, $gen->generate('cdn-module-test://' . $llama_jpg_filepath)); unlink($llama_jpg_filepath); } @@ -191,9 +199,16 @@ class FileUrlGeneratorTest extends UnitTestCase { ->will(function () use ($base_path, &$current_uri) { return 'http://example.com' . $base_path . '/sites/default/files/' . UrlHelper::encodePath(substr($current_uri, 9)); }); + $file_stream_wrapper = $this->prophesize(LocalStream::class); + $root = $this->root; + $file_stream_wrapper->getExternalUrl() + ->will(function () use ($root, $base_path, &$current_uri) { + // The cdn-module-test:// stream wrapper is only used for testing FF. + return 'http://example.com/inaccessible'; + }); $stream_wrapper_manager = $this->prophesize(StreamWrapperManagerInterface::class); - $stream_wrapper_manager->getWrappers(StreamWrapperInterface::LOCAL) - ->willReturn(['public' => TRUE, 'private' => TRUE]); + $stream_wrapper_manager->getWrappers(StreamWrapperInterface::LOCAL_NORMAL) + ->willReturn(['public' => TRUE]); $stream_wrapper_manager->getViaUri(Argument::that(function ($uri) { return substr($uri, 0, 9) === 'public://'; })) @@ -202,6 +217,14 @@ class FileUrlGeneratorTest extends UnitTestCase { $current_uri = $args[0]; return $s; }); + $stream_wrapper_manager->getViaUri(Argument::that(function ($uri) { + return substr($uri, 0, 7) === 'cdn-module-test://'; + })) + ->will(function ($args) use (&$file_stream_wrapper, &$current_uri) { + $s = $file_stream_wrapper->reveal(); + $current_uri = $args[0]; + return $s; + }); $private_key = $this->prophesize(PrivateKey::class); $private_key->get() ->willReturn(static::$privateKey); @@ -216,7 +239,7 @@ class FileUrlGeneratorTest extends UnitTestCase { $stream_wrapper_manager->reveal(), $request_stack->reveal(), $private_key->reveal(), - new CdnSettings($this->getConfigFactoryStub(['cdn.settings' => $raw_config])) + new CdnSettings($this->getConfigFactoryStub(['cdn.settings' => $raw_config]), $stream_wrapper_manager->reveal()) ); }