cdn.install | 7 ++ cdn.routing.yml | 11 ++ cdn.services.yml | 2 +- cdn_ui/js/summaries.js | 9 ++ cdn_ui/src/Form/CdnSettingsForm.php | 113 +++++++++++++++++++++ config/install/cdn.settings.yml | 3 + config/schema/cdn.data_types.schema.yml | 5 + config/schema/cdn.schema.yml | 5 + src/CdnFarfutureController.php | 100 ++++++++++++++++-- src/CdnSettings.php | 58 +++++++++++ src/File/FileUrlGenerator.php | 77 +++++++------- src/PathProcessor/CdnFarfuturePathProcessor.php | 56 ++++++++-- .../Constraint/CdnStreamWrapperConstraint.php | 21 ++++ .../CdnStreamWrapperConstraintValidator.php | 50 +++++++++ tests/src/Functional/CdnIntegrationTest.php | 30 +++++- .../CdnStreamWrapperConstraintValidatorTest.php | 64 ++++++++++++ tests/src/Unit/File/FileUrlGeneratorTest.php | 52 +++++++--- 17 files changed, 591 insertions(+), 72 deletions(-) diff --git a/cdn.install b/cdn.install index c788522..fc16613 100644 --- a/cdn.install +++ b/cdn.install @@ -20,3 +20,10 @@ function cdn_update_8001() { $cdn_settings->save(); } } + +/** + * Add the new "stream wrappers" setting, set it to its default initial value. + */ +function cdn_update_8002() { + \Drupal::configFactory()->getEditable('cdn.settings')->set('stream_wrappers', ['public'])->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 f7b19ed..8e86ad9 100644 --- a/cdn.services.yml +++ b/cdn.services.yml @@ -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 9dfac05..d2b4e73 100644 --- a/cdn_ui/src/Form/CdnSettingsForm.php +++ b/cdn_ui/src/Form/CdnSettingsForm.php @@ -2,14 +2,46 @@ namespace Drupal\cdn_ui\Form; +use Drupal\Component\Utility\UrlHelper; use Drupal\Core\Config\Config; +use Drupal\Core\Config\ConfigFactoryInterface; +use Drupal\Core\Config\TypedConfigManagerInterface; use Drupal\Core\Form\FormStateInterface; +use Drupal\Core\StreamWrapper\StreamWrapperInterface; +use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface; +use Symfony\Component\DependencyInjection\ContainerInterface; /** * Configure CDN settings for this site. */ class CdnSettingsForm extends ValidatableConfigFormBase { + /** + * The stream wrapper manager. + * + * @var \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface + */ + protected $streamWrapperManager; + + /** + * {@inheritdoc} + */ + public function __construct(ConfigFactoryInterface $config_factory, TypedConfigManagerInterface $typed_config_manager, StreamWrapperManagerInterface $streamWrapperManager) { + parent::__construct($config_factory, $typed_config_manager); + $this->streamWrapperManager = $streamWrapperManager; + } + + /** + * {@inheritdoc} + */ + public static function create(ContainerInterface $container) { + return new static( + $container->get('config.factory'), + $container->get('config.typed'), + $container->get('stream_wrapper_manager') + ); + } + /** * {@inheritdoc} */ @@ -152,9 +184,84 @@ class CdnSettingsForm extends ValidatableConfigFormBase { '#default_value' => $config->get('farfuture.status'), ]; + $visible_stream_wrappers = $this->streamWrapperManager->getWrappers(StreamWrapperInterface::VISIBLE); + $non_core_visible_stream_wrappers = array_filter($visible_stream_wrappers, function (array $metadata) { + return strpos($metadata['class'], 'Drupal\Core') !== 0; + }); + $form['wrappers'] = [ + '#type' => 'details', + '#title' => $this->t('Stream wrappers'), + '#group' => 'cdn_settings', + '#tree' => TRUE, + '#access' => !empty($non_core_visible_stream_wrappers), + ]; + $checkboxes = $this->buildStreamWrapperCheckboxes(array_keys($visible_stream_wrappers)); + $form['wrappers']['stream_wrappers'] = [ + '#type' => 'checkboxes', + '#options' => array_combine(array_keys($checkboxes), array_keys($checkboxes)), + '#default_value' => $config->get('stream_wrappers'), + '#description' => $this->t('Stream wrappers whose files to serve from CDN. public:// is always enabled, any other stream wrapper generating local file URLs is eligible.'), + ]; + $form['wrappers']['stream_wrappers'] += $checkboxes; + // Special cases: public:// and private://. + $form['wrappers']['stream_wrappers']['public']['#disabled'] = TRUE; + if (!empty($form['wrappers']['stream_wrappers']['private'])) { + $form['wrappers']['stream_wrappers']['private']['#disabled'] = TRUE; + $form['wrappers']['stream_wrappers']['private']['#title'] = '' . $form['wrappers']['stream_wrappers']['private']['#title'] . ''; + $form['wrappers']['stream_wrappers']['private']['#description'] = $this->t('Private files require authentication and hence cannot be served from a CDN.'); + } + return parent::buildForm($form, $form_state); } + /** + * Determines whether the stream wrapper generates external URLs. + * + * @param string $stream_wrapper_scheme + * A valid stream wrapper scheme. + * @param \Drupal\Core\StreamWrapper\StreamWrapperInterface $stream_wrapper + * A stream wrapper instance. + * + * @return bool + */ + protected function streamWrapperGeneratesExternalUrls($stream_wrapper_scheme, StreamWrapperInterface $stream_wrapper) { + // Generate URL to imaginary file 'cdn.test'. Most stream wrappers don't + // check file existence, just concatenate strings. + $stream_wrapper->setUri($stream_wrapper_scheme . '://cdn.test'); + try { + $absolute_url = $stream_wrapper->getExternalUrl(); + $base_url = $this->getRequest()->getSchemeAndHttpHost() . $this->getRequest()->getBasePath(); + $relative_url = str_replace($base_url, '', $absolute_url); + return UrlHelper::isExternal($relative_url); + } + catch (\Exception $e) { + // In case of failure, assume this would have resulted in an external URL. + return TRUE; + } + } + + /** + * Builds the stream wrapper checkboxes form array. + * + * @param string[] $stream_wrapper_schemes + * The stream wrapper schemes for which to generate form checkboxes. + * + * @return array + */ + protected function buildStreamWrapperCheckboxes(array $stream_wrapper_schemes) { + $checkboxes = []; + foreach ($stream_wrapper_schemes as $stream_wrapper_scheme) { + $wrapper = $this->streamWrapperManager->getViaScheme($stream_wrapper_scheme); + $generates_external_urls = static::streamWrapperGeneratesExternalUrls($stream_wrapper_scheme, $wrapper); + $checkboxes[$stream_wrapper_scheme] = [ + '#title' => $this->t('@name → @scheme://', ['@scheme' => $stream_wrapper_scheme, '@name' => $wrapper->getName()]), + '#disabled' => $generates_external_urls, + '#description' => !$generates_external_urls ? NULL : $this->t('This stream wrapper generates external URLs, and hence cannot be served from a CDN.'), + ]; + } + return $checkboxes; + } + /** * {@inheritdoc} */ @@ -162,6 +269,12 @@ class CdnSettingsForm extends ValidatableConfigFormBase { // Vertical tab: 'Status'. $config->set('status', (bool) $form_state->getValue('status')); + // Vertical tab: 'Stream wrappers'. + $stream_wrappers = array_values(array_filter($form_state->getValue(['wrappers', 'stream_wrappers']))); + // Ensure 'public://' is always enabled, and ensure it's always first. + $stream_wrappers = array_merge(['public'], $stream_wrappers); + $config->set('stream_wrappers', $stream_wrappers); + // Vertical tab: 'Mapping'. if ($form_state->getValue(['mapping', 'type']) === 'simple') { $simple_mapping = $form_state->getValue(['mapping', 'simple']); diff --git a/config/install/cdn.settings.yml b/config/install/cdn.settings.yml index 0ed3fc6..0b12ad9 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 default, additional local stream wrappers can be added. +stream_wrappers: + - public diff --git a/config/schema/cdn.data_types.schema.yml b/config/schema/cdn.data_types.schema.yml index b109173..9f2006d 100644 --- a/config/schema/cdn.data_types.schema.yml +++ b/config/schema/cdn.data_types.schema.yml @@ -7,3 +7,8 @@ cdn.domain: label: 'Domain' constraints: CdnDomain: [] +cdn.stream_wrapper_scheme: + type: string + label: 'Stream wrapper scheme' + constraints: + CdnStreamWrapper: [] diff --git a/config/schema/cdn.schema.yml b/config/schema/cdn.schema.yml index b0d9272..a741365 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: 'CDN-enabled stream wrappers' + type: sequence + sequence: + type: cdn.stream_wrapper_scheme diff --git a/src/CdnFarfutureController.php b/src/CdnFarfutureController.php index e1f7bea..ec7ec75 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,76 @@ 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. + * @param \Drupal\Core\File\FileSystemInterface $file_system + * The file system service. */ - public function __construct(PrivateKey $private_key) { + public function __construct(PrivateKey $private_key, FileSystemInterface $file_system) { $this->privateKey = $private_key; + $this->fileSystem = $file_system; + } + + /** + * Serves the requested file with optimal far future expiration headers. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The current request. $request->query must have 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. + * + * @return \Symfony\Component\HttpFoundation\BinaryFileResponse + * The response that will efficiently send the requested file. + * + * @throws \Symfony\Component\HttpKernel\Exception\BadRequestHttpException + * Thrown when the '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 + // Path comes with a leading slash from the URL. + : $scheme . ':/' . $path; + // 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; } /** @@ -40,7 +106,7 @@ class CdnFarfutureController { * @param int $mtime * The file's mtime. * - * @returns \Symfony\Component\HttpFoundation\BinaryFileResponse + * @return \Symfony\Component\HttpFoundation\BinaryFileResponse * The response that will efficiently send the requested file. * * @throws \Symfony\Component\HttpKernel\Exception\BadRequestHttpException @@ -49,6 +115,11 @@ 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. Will be removed + * + * @deprecated in 3.3 and will be removed before CDN 4.0. Use + * ::downloadByScheme() instead. */ public function download(Request $request, $security_token, $mtime) { // Ensure \Drupal\cdn\PathProcessor\CdnFarfuturePathProcessor did its job. @@ -63,7 +134,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 +183,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 6219eb2..c5e011a 100644 --- a/src/CdnSettings.php +++ b/src/CdnSettings.php @@ -2,6 +2,8 @@ namespace Drupal\cdn; +use Drupal\Component\Assertion\Inspector; +use Drupal\Component\Utility\Unicode; use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Config\ConfigValueException; @@ -77,6 +79,19 @@ class CdnSettings { return $unique_domains; } + /** + * Returns CDN-eligible stream wrappers. + * + * @return string[] + * The allowed stream wrapper scheme names. + */ + public function getStreamWrappers() { + $stream_wrappers = $this->rawSettings->get('stream_wrappers'); + // @see cdn_update_8002() + assert(Inspector::assertAllStrings($stream_wrappers), 'Please run update.php!'); + return $stream_wrappers; + } + /** * Builds a lookup table: file extension to CDN domain(s). * @@ -149,4 +164,47 @@ class CdnSettings { return $lookup_table; } + /** + * 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..8a7314b 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; } @@ -121,25 +122,32 @@ class FileUrlGenerator { return FALSE; } - // 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; - } + $file_path = $relative_url = '/' . $uri; + $real_file = $this->root . rawurldecode($relative_url);; + } + else { + $file_uri = $real_file = $uri; + $file_path = '/' . substr($file_uri, 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($real_file)) { + // 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($real_file); + + // 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($file_path), $this->privateKey->get() . Settings::getHashSalt()); + return '//' . $cdn_domain . $this->getBasePath() . '/cdn/ff/' . $calculated_token . '/' . $mtime . '/' . $scheme . $file_path; } return '//' . $cdn_domain . $this->getBasePath() . $relative_url; @@ -190,40 +198,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. + $allowed_stream_wrappers = $this->settings->getStreamWrappers(); // 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 allowed stream wrapper. + if ($scheme && !in_array($scheme, $allowed_stream_wrappers, TRUE)) { 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..a56dd55 100644 --- a/src/PathProcessor/CdnFarfuturePathProcessor.php +++ b/src/PathProcessor/CdnFarfuturePathProcessor.php @@ -12,6 +12,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 +23,60 @@ class CdnFarfuturePathProcessor implements InboundPathProcessorInterface { * {@inheritdoc} */ public function processInbound($path, Request $request) { - if (strpos($path, '/cdn/farfuture/') !== 0) { - return $path; + // @todo Remove before CDN 4.0. + if (strpos($path, '/cdn/farfuture/') === 0) { + return $this->processLegacyFarFuture($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. + * + * @deprecated in 3.3 and will be removed before CDN 4.0. Use + * ::processFarFuture() instead. + */ + protected function processLegacyFarFuture($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/src/Plugin/Validation/Constraint/CdnStreamWrapperConstraint.php b/src/Plugin/Validation/Constraint/CdnStreamWrapperConstraint.php new file mode 100644 index 0000000..2c26a0a --- /dev/null +++ b/src/Plugin/Validation/Constraint/CdnStreamWrapperConstraint.php @@ -0,0 +1,21 @@ +context->buildViolation($constraint->message) + ->setParameter('%stream_wrapper', $stream_wrapper) + ->setInvalidValue($stream_wrapper) + ->addViolation(); + } + } + + /** + * Validates the given stream wrapper, with an exception for "private". + * + * @param string $stream_wrapper + * A stream wrapper configured for use in Drupal. + * + * @return bool + */ + protected static function isValidCdnStreamWrapper($stream_wrapper) { + /** @var \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface $stream_wrapper_manager */ + $stream_wrapper_manager = \Drupal::service('stream_wrapper_manager'); + $forbidden_wrappers = ['private']; + return !in_array($stream_wrapper, $forbidden_wrappers, TRUE) + && $stream_wrapper_manager->getClass($stream_wrapper) !== FALSE; + } + +} diff --git a/tests/src/Functional/CdnIntegrationTest.php b/tests/src/Functional/CdnIntegrationTest.php index 009230e..37f7f0f 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)); } @@ -177,9 +177,12 @@ class CdnIntegrationTest extends BrowserTestBase { } /** - * Tests that the cdn.farfuture.download route/controller work as expected. + * Tests the legacy far future path. + * + * @group legacy + * @todo Remove before CDN 4.0. */ - public function testFarfuture() { + public function testOldFarfuture() { $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()); @@ -195,4 +198,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/Kernel/CdnStreamWrapperConstraintValidatorTest.php b/tests/src/Kernel/CdnStreamWrapperConstraintValidatorTest.php new file mode 100644 index 0000000..17e3ebb --- /dev/null +++ b/tests/src/Kernel/CdnStreamWrapperConstraintValidatorTest.php @@ -0,0 +1,64 @@ +prophesize(ConstraintViolationBuilderInterface::class); + $constraint_violation_builder->setParameter('%stream_wrapper', $value) + ->willReturn($constraint_violation_builder->reveal()); + $constraint_violation_builder->setInvalidValue($value) + ->willReturn($constraint_violation_builder->reveal()); + $constraint_violation_builder->addViolation() + ->willReturn($constraint_violation_builder->reveal()); + if ($valid) { + $constraint_violation_builder->addViolation()->shouldNotBeCalled(); + } + else { + $constraint_violation_builder->addViolation()->shouldBeCalled(); + } + $context = $this->prophesize(ExecutionContextInterface::class); + $context->buildViolation(Argument::type('string')) + ->willReturn($constraint_violation_builder->reveal()); + + $constraint = new CdnStreamWrapperConstraint(); + + $validate = new CdnStreamWrapperConstraintValidator(); + $validate->initialize($context->reveal()); + $validate->validate($value, $constraint); + } + + public function provideTestValidate() { + $data = []; + + $data['public, registered by default'] = ['public', TRUE]; + $data['private, prohibited by rule'] = ['private', FALSE]; + $data['unregistered'] = ['unregistered', FALSE]; + $data['custom, provided by test module'] = ['dummy-remote', TRUE]; + + return $data; + } + +} diff --git a/tests/src/Unit/File/FileUrlGeneratorTest.php b/tests/src/Unit/File/FileUrlGeneratorTest.php index 40a085d..a505a54 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,12 @@ class FileUrlGeneratorTest extends UnitTestCase { 'farfuture' => [ 'status' => TRUE, ], + // File is used here generically to test a stream wrapper that is not + // shipped with Drupal, but is natively supported by PHP. + // @see \Drupal\cdn\File\FileUrlGenerator::generate(), which uses + // file_exists() and would require actually configuring the stream + // wrapper in the context of the unit test. + 'stream_wrappers' => ['public', 'file'], ]; // Generate file for testing managed file. @@ -146,16 +154,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 + // file:// 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('file://' . $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('file://' . $llama_jpg_filepath)); unlink($llama_jpg_filepath); } @@ -191,9 +204,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 file:// 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 +222,14 @@ class FileUrlGeneratorTest extends UnitTestCase { $current_uri = $args[0]; return $s; }); + $stream_wrapper_manager->getViaUri(Argument::that(function ($uri) { + return substr($uri, 0, 7) === 'file://'; + })) + ->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 +244,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()) ); }