cdn.install | 8 +-- cdn.module | 3 +- cdn.services.yml | 2 +- cdn_ui/src/Form/CdnSettingsForm.php | 108 ++++++++++++++++++++------------ config/install/cdn.settings.yml | 2 +- config/schema/cdn.data_types.schema.yml | 3 + config/schema/cdn.schema.yml | 4 +- src/CdnSettings.php | 26 +++----- src/File/FileUrlGenerator.php | 2 +- tests/src/Unit/CdnSettingsTest.php | 4 +- 10 files changed, 87 insertions(+), 75 deletions(-) diff --git a/cdn.install b/cdn.install index 5ac2574..fc16613 100644 --- a/cdn.install +++ b/cdn.install @@ -22,12 +22,8 @@ function cdn_update_8001() { } /** - * Update the default settings to include the stream_wrappers schema. + * Add the new "stream wrappers" setting, set it to its default initial value. */ 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(); - } + \Drupal::configFactory()->getEditable('cdn.settings')->set('stream_wrappers', ['public'])->save(); } diff --git a/cdn.module b/cdn.module index ebdd366..cdf075b 100644 --- a/cdn.module +++ b/cdn.module @@ -34,7 +34,8 @@ function cdn_help($route_name, RouteMatchInterface $route_match) { */ function cdn_file_url_alter(&$uri) { // Don't alter file URLs when running update.php. - if (defined('MAINTENANCE_MODE')) { + // @todo Drupal 8 no longer defines the MAINTENANCE_MODE constant when running update.php; once that is fixed, we can stop using $_SERVER here. + if (stripos($_SERVER['PHP_SELF'], 'update.php') !== FALSE) { return; } diff --git a/cdn.services.yml b/cdn.services.yml index 244a5b2..5ab54ef 100644 --- a/cdn.services.yml +++ b/cdn.services.yml @@ -1,7 +1,7 @@ services: cdn.settings: class: Drupal\cdn\CdnSettings - arguments: ['@config.factory', '@stream_wrapper_manager'] + arguments: ['@config.factory'] cdn.file_url_generator: class: Drupal\cdn\File\FileUrlGenerator diff --git a/cdn_ui/src/Form/CdnSettingsForm.php b/cdn_ui/src/Form/CdnSettingsForm.php index 969d978..31f985f 100644 --- a/cdn_ui/src/Form/CdnSettingsForm.php +++ b/cdn_ui/src/Form/CdnSettingsForm.php @@ -3,6 +3,7 @@ namespace Drupal\cdn_ui\Form; use Drupal\cdn\CdnSettings; +use Drupal\Component\Render\MarkupInterface; use Drupal\Component\Utility\UrlHelper; use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Form\ConfigFormBase; @@ -173,50 +174,79 @@ 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'); + $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' => count($wrappers) > count($localWrappers), + '#access' => count($non_core_visible_stream_wrappers), ]; + $checkboxes = $this->buildStreamWrapperCheckboxes(array_keys($visible_stream_wrappers)); $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.'), + '#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.'), ]; - 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.'); - } + $form['wrappers']['stream_wrappers'] += $checkboxes; + // Special cases: public:// and private://. + $form['wrappers']['stream_wrappers']['public']['#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); } - foreach ($localWrappers as $localWrapper) { - $form['wrappers']['stream_wrappers'][$localWrapper]['#disabled'] = TRUE; + catch (\Exception $e) { + // In case of failure, assume this would have resulted in an external URL. + return TRUE; } - return parent::buildForm($form, $form_state); + } + + /** + * 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; } /** @@ -243,7 +273,10 @@ class CdnSettingsForm extends ConfigFormBase { $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'])))); + $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') { @@ -282,11 +315,4 @@ 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 79e0859..0b12ad9 100644 --- a/config/install/cdn.settings.yml +++ b/config/install/cdn.settings.yml @@ -69,6 +69,6 @@ mapping: farfuture: status: true -# Public is enabled by rule, additional wrappers can be added. +# 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 58213e2..d90508f 100644 --- a/config/schema/cdn.data_types.schema.yml +++ b/config/schema/cdn.data_types.schema.yml @@ -5,3 +5,6 @@ cdn.domain: type: string label: 'Domain' +stream_wrapper_scheme: + type: string + label: 'Stream wrapper scheme' diff --git a/config/schema/cdn.schema.yml b/config/schema/cdn.schema.yml index e543e5b..ec8c562 100644 --- a/config/schema/cdn.schema.yml +++ b/config/schema/cdn.schema.yml @@ -21,7 +21,7 @@ cdn.settings: label: 'Forever cacheable files — status' type: boolean stream_wrappers: - label: 'Stream wrappers for CDN' + label: 'CDN-enabled stream wrappers' type: sequence sequence: - type: string + type: stream_wrapper_scheme diff --git a/src/CdnSettings.php b/src/CdnSettings.php index acc189b..9fff585 100644 --- a/src/CdnSettings.php +++ b/src/CdnSettings.php @@ -2,11 +2,10 @@ namespace Drupal\cdn; +use Drupal\Component\Assertion\Inspector; 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. @@ -30,24 +29,14 @@ 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, StreamWrapperManagerInterface $streamWrapperManager) { + public function __construct(ConfigFactoryInterface $config_factory) { $this->rawSettings = $config_factory->get('cdn.settings'); $this->lookupTable = NULL; - $this->streamWrapperManager = $streamWrapperManager; } /** @@ -95,12 +84,11 @@ class CdnSettings { * * @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']); + 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; } /** diff --git a/src/File/FileUrlGenerator.php b/src/File/FileUrlGenerator.php index 65153c2..92bc654 100644 --- a/src/File/FileUrlGenerator.php +++ b/src/File/FileUrlGenerator.php @@ -214,7 +214,7 @@ class FileUrlGenerator { $scheme = $this->fileSystem->uriScheme($uri); // Allow additional stream wrappers to be served via CDN. - $streamWrapperTypes = $this->settings->streamWrappers(); + $streamWrapperTypes = $this->settings->getStreamWrappers(); // If the URI is absolute — HTTP(S) or otherwise — return early, except if // it's an absolute URI using an approved stream wrapper type. if ($scheme && !in_array($scheme, $streamWrapperTypes)) { diff --git a/tests/src/Unit/CdnSettingsTest.php b/tests/src/Unit/CdnSettingsTest.php index 2ba20ec..c247403 100644 --- a/tests/src/Unit/CdnSettingsTest.php +++ b/tests/src/Unit/CdnSettingsTest.php @@ -4,7 +4,6 @@ namespace Drupal\Tests\cdn\Unit; use Drupal\cdn\CdnSettings; use Drupal\Core\Config\ConfigValueException; -use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface; use Drupal\Tests\UnitTestCase; /** @@ -445,8 +444,7 @@ class CdnSettingsTest extends UnitTestCase { * The CdnSettings object to test. */ protected function createCdnSettings(array $raw_config) { - $stream_wrapper_manager = $this->prophesize(StreamWrapperManagerInterface::class); - return new CdnSettings($this->getConfigFactoryStub(['cdn.settings' => $raw_config]), $stream_wrapper_manager->reveal()); + return new CdnSettings($this->getConfigFactoryStub(['cdn.settings' => $raw_config])); } }