diff --git a/_config/config.yml b/_config/config.yml index 540b285ab..f7b296cf8 100644 --- a/_config/config.yml +++ b/_config/config.yml @@ -6,6 +6,10 @@ SilverStripe\Core\Injector\Injector: class: SilverStripe\Forms\Schema\FormSchema FixtureFactory: class: SilverStripe\Dev\FixtureFactory + SilverStripe\Core\Manifest\ResourceURLGenerator: + class: SilverStripe\Control\SimpleResourceURLGenerator + properties: + NonceStyle: mtime SilverStripe\Control\HTTP: cache_control: max-age: 0 diff --git a/src/Control/SimpleResourceURLGenerator.php b/src/Control/SimpleResourceURLGenerator.php new file mode 100644 index 000000000..30375a7be --- /dev/null +++ b/src/Control/SimpleResourceURLGenerator.php @@ -0,0 +1,70 @@ +nonceStyle; + } + + /* + * Set the style of nonce-suffixes to use, or null to disable + * Currently only "mtime" is allowed + * + * @param string|null $nonceStyle The style of nonces to apply, or null to disable + */ + public function setNonceStyle($nonceStyle) + { + if ($nonceStyle && $nonceStyle !== 'mtime') { + throw new InvalidArgumentException('The only allowed NonceStyle is mtime'); + } + $this->nonceStyle = $nonceStyle; + } + + /** + * Return the URL for a resource, prefixing with Director::baseURL() and suffixing with a nonce + * + * @param string $relativePath File or directory path relative to BASE_PATH + * @return string Doman-relative URL + * @throws InvalidArgumentException If the resource doesn't exist + */ + public function urlForResource($relativePath) + { + $absolutePath = preg_replace('/\?.*/', '', Director::baseFolder() . '/' . $relativePath); + + if (!file_exists($absolutePath)) { + throw new InvalidArgumentException("File {$relativePath} does not exist"); + } + + $nonce = ''; + if ($this->nonceStyle) { + $nonce = (strpos($relativePath, '?') === false) ? '?' : '&'; + + switch ($this->nonceStyle) { + case 'mtime': + $nonce .= "m=" . filemtime($absolutePath); + break; + } + } + + return Director::baseURL() . $relativePath . $nonce; + } +} diff --git a/src/Core/Manifest/Module.php b/src/Core/Manifest/Module.php index 564984ab3..d59a1dd30 100644 --- a/src/Core/Manifest/Module.php +++ b/src/Core/Manifest/Module.php @@ -4,6 +4,7 @@ namespace SilverStripe\Core\Manifest; use Exception; use Serializable; +use SilverStripe\Core\Injector\Injector; class Module implements Serializable { @@ -161,13 +162,44 @@ class Module implements Serializable * @param string $path File or directory path relative to module directory * @return string Path relative to base directory */ - public function getResourcePath($path) + public function getRelativeResourcePath($path) { $base = trim($this->getRelativePath(), '/\\'); $path = trim($path, '/\\'); return trim("{$base}/{$path}", '/\\'); } + /** + * Gets path to physical file resource relative to base directory. + * Directories included + * + * This method makes no distinction between public / local resources, + * which may change in the near future. + * + * @internal Experimental API and may change + * @param string $path File or directory path relative to module directory + * @return string Path relative to base directory + */ + public function getResourcePath($path) + { + return $this->basePath . '/' . $this->getRelativeResourcePath($path); + } + + /** + * Gets the URL for a given resource. + * Relies on the ModuleURLGenerator Injector service to do the heavy lifting + * + * @internal Experimental API and may change + * @param string $path File or directory path relative to module directory + * @return string URL, either domain-relative (starting with /) or absolute + */ + public function getResourceURL($path) + { + return Injector::inst() + ->get(ResourceURLGenerator::class) + ->urlForResource($this->getRelativeResourcePath($path)); + } + /** * Check if this module has a given resource * @@ -177,7 +209,6 @@ class Module implements Serializable */ public function hasResource($path) { - $resource = $this->getResourcePath($path); - return file_exists($this->basePath . '/' . $resource); + return file_exists($this->getResourcePath($path)); } } diff --git a/src/Core/Manifest/ResourceURLGenerator.php b/src/Core/Manifest/ResourceURLGenerator.php new file mode 100644 index 000000000..2a8ac48d7 --- /dev/null +++ b/src/Core/Manifest/ResourceURLGenerator.php @@ -0,0 +1,23 @@ +getTinyMCEPath() - ); + $tinyMCEBaseURL = $this->getAdminModule()->getResourceURL('thirdparty/tinymce'); $settings['baseURL'] = $tinyMCEBaseURL; // map all plugins to absolute urls for loading @@ -618,7 +615,7 @@ class TinyMCEConfig extends HTMLEditorConfig $editor = array(); // Add standard editor.css - $editor[] = Director::absoluteURL(ltrim($this->getAdminPath() . '/client/dist/styles/editor.css', '/')); + $editor[] = $this->getAdminModule()->getResourceURL('client/dist/styles/editor.css'); // Themed editor.css $themedEditor = ThemeResourceLoader::inst()->findThemedCSS('editor', SSViewer::get_themes()); @@ -635,6 +632,7 @@ class TinyMCEConfig extends HTMLEditorConfig * so that multiple HTTP requests on the client don't need to be made. * * @return string + * @throws Exception */ public function getScriptURL() { @@ -687,18 +685,7 @@ class TinyMCEConfig extends HTMLEditorConfig /** * @return string|false - */ - public function getAdminPath() - { - $module = $this->getAdminModule(); - if ($module) { - return $module->getRelativePath(); - } - return false; - } - - /** - * @return string|false + * @throws Exception */ public function getTinyMCEPath() { @@ -708,7 +695,7 @@ class TinyMCEConfig extends HTMLEditorConfig } if ($admin = $this->getAdminModule()) { - return $admin->getResourcePath('thirdparty/tinymce'); + return $admin->getRelativeResourcePath('thirdparty/tinymce'); } throw new Exception(sprintf( @@ -723,6 +710,6 @@ class TinyMCEConfig extends HTMLEditorConfig */ protected function getAdminModule() { - return ModuleLoader::inst()->getManifest()->getModule('silverstripe/admin'); + return ModuleLoader::getModule('silverstripe/admin'); } } diff --git a/src/View/GenericTemplateGlobalProvider.php b/src/View/GenericTemplateGlobalProvider.php index b16fcb020..382ae3aa7 100644 --- a/src/View/GenericTemplateGlobalProvider.php +++ b/src/View/GenericTemplateGlobalProvider.php @@ -2,7 +2,7 @@ namespace SilverStripe\View; -use InvalidArgumentException; +use SilverStripe\Core\Manifest\ModuleLoader; use SilverStripe\ORM\DataList; class GenericTemplateGlobalProvider implements TemplateGlobalProvider @@ -16,16 +16,6 @@ class GenericTemplateGlobalProvider implements TemplateGlobalProvider ); } - /** - * @var array Module paths - */ - public static $modules = array( - 'framework' => FRAMEWORK_DIR, - 'frameworkadmin' => FRAMEWORK_ADMIN_DIR, - 'thirdparty' => THIRDPARTY_DIR, - 'assets' => ASSETS_DIR - ); - /** * Given some pre-defined modules, return the filesystem path of the module. * @param string $name Name of module to find path of @@ -33,15 +23,17 @@ class GenericTemplateGlobalProvider implements TemplateGlobalProvider */ public static function ModulePath($name) { - if (isset(self::$modules[$name])) { - return self::$modules[$name]; - } else { - throw new InvalidArgumentException(sprintf( - '%s is not a supported argument. Possible values: %s', - $name, - implode(', ', self::$modules) - )); + // BC for a couple fo the key modules in the old syntax. Reduces merge brittleness but can + // be removed before 4.0 stable + $legacyMapping = [ + 'framework' => 'silverstripe/framework', + 'frameworkadmin' => 'silverstripe/admin', + ]; + if (isset($legacyMapping[$name])) { + $name = $legacyMapping[$name]; } + + return ModuleLoader::getModule($name)->getRelativePath(); } /** diff --git a/src/View/Requirements_Backend.php b/src/View/Requirements_Backend.php index d95240246..1eafb79e3 100644 --- a/src/View/Requirements_Backend.php +++ b/src/View/Requirements_Backend.php @@ -11,6 +11,9 @@ use SilverStripe\Control\HTTPResponse; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Convert; use SilverStripe\Core\Injector\Injectable; +use SilverStripe\Core\Injector\Injector; +use SilverStripe\Core\Manifest\ResourceURLGenerator; +use SilverStripe\Core\Manifest\ModuleLoader; use SilverStripe\Dev\Debug; use SilverStripe\Dev\Deprecation; use SilverStripe\i18n\i18n; @@ -389,7 +392,7 @@ class Requirements_Backend /** * Register the given JavaScript file as required. * - * @param string $file Relative to docroot + * @param string $file Either relative to docroot or in the form "vendor/package:resource" * @param array $options List of options. Available options include: * - 'provides' : List of scripts files included in this file * - 'async' : Boolean value to set async attribute to script tag @@ -398,6 +401,8 @@ class Requirements_Backend */ public function javascript($file, $options = array()) { + $file = $this->parseModuleResourceReference($file); + // Get type $type = null; if (isset($this->javascript[$file]['type'])) { @@ -621,11 +626,34 @@ class Requirements_Backend */ public function css($file, $media = null) { + $file = $this->parseModuleResourceReference($file); + $this->css[$file] = array( "media" => $media ); } + /** + * Convert a file of the form "vendor/package:resource" into a BASE_PATH-relative file + * For other files, reutrn original value + * + * @param string $file + * @return string + */ + protected function parseModuleResourceReference($file) + { + // String of the form vendor/package:resource. Excludes "http://bla" as that's an absolute URL + if (preg_match('#([^ ]*/[^ ]*) *: *([^ ]*)#', $file, $matches)) { + list(, $module, $resource) = $matches; + $moduleObj = ModuleLoader::getModule($module); + if (!$moduleObj) { + throw new \InvalidArgumentException("Can't find module '$module'"); + } + return $moduleObj->getRelativeResourcePath($resource); + } + return $file; + } + /** * Remove a css requirement * @@ -1019,27 +1047,8 @@ class Requirements_Backend // Since combined urls could be root relative, treat them as urls here. if (preg_match('{^(//)|(http[s]?:)}', $fileOrUrl) || Director::is_root_relative_url($fileOrUrl)) { return $fileOrUrl; - } elseif (Director::fileExists($fileOrUrl)) { - $filePath = preg_replace('/\?.*/', '', Director::baseFolder() . '/' . $fileOrUrl); - $prefix = Director::baseURL(); - $mtimesuffix = ""; - $suffix = ''; - if ($this->getSuffixRequirements()) { - $mtimesuffix = "?m=" . filemtime($filePath); - $suffix = '&'; - } - if (strpos($fileOrUrl, '?') !== false) { - if (strlen($suffix) == 0) { - $suffix = '?'; - } - $suffix .= substr($fileOrUrl, strpos($fileOrUrl, '?') + 1); - $fileOrUrl = substr($fileOrUrl, 0, strpos($fileOrUrl, '?')); - } else { - $suffix = ''; - } - return "{$prefix}{$fileOrUrl}{$mtimesuffix}{$suffix}"; } else { - throw new InvalidArgumentException("File {$fileOrUrl} does not exist"); + return Injector::inst()->get(ResourceURLGenerator::class)->urlForResource($fileOrUrl); } } diff --git a/tests/php/Core/Manifest/ModuleManifestTest.php b/tests/php/Core/Manifest/ModuleManifestTest.php index 01d36f062..ca7705058 100644 --- a/tests/php/Core/Manifest/ModuleManifestTest.php +++ b/tests/php/Core/Manifest/ModuleManifestTest.php @@ -82,7 +82,7 @@ class ModuleManifestTest extends SapphireTest $this->assertFalse($module->hasResource('package.json')); $this->assertEquals( 'moduleb/composer.json', - $module->getResourcePath('composer.json') + $module->getRelativeResourcePath('composer.json') ); } @@ -96,7 +96,7 @@ class ModuleManifestTest extends SapphireTest $this->assertTrue($module->hasResource('composer.json')); $this->assertEquals( 'composer.json', - $module->getResourcePath('composer.json') + $module->getRelativeResourcePath('composer.json') ); } } diff --git a/tests/php/Forms/HTMLEditor/HTMLEditorConfigTest.php b/tests/php/Forms/HTMLEditor/HTMLEditorConfigTest.php index 8fe6fbeba..b4ea219e4 100644 --- a/tests/php/Forms/HTMLEditor/HTMLEditorConfigTest.php +++ b/tests/php/Forms/HTMLEditor/HTMLEditorConfigTest.php @@ -5,10 +5,13 @@ namespace SilverStripe\Forms\Tests\HTMLEditor; use Exception; use PHPUnit_Framework_MockObject_MockObject; use SilverStripe\Control\Director; +use SilverStripe\Control\SimpleResourceURLGenerator; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Convert; +use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Manifest\ModuleLoader; use SilverStripe\Core\Manifest\ModuleManifest; +use SilverStripe\Core\Manifest\ResourceURLGenerator; use SilverStripe\Dev\SapphireTest; use SilverStripe\Forms\HTMLEditor\HTMLEditorConfig; use SilverStripe\Forms\HTMLEditor\HTMLEditorField; @@ -49,6 +52,10 @@ class HTMLEditorConfigTest extends SapphireTest public function testEnablePluginsByArrayWithPaths() { + // Disable nonces + $urlGenerator = new SimpleResourceURLGenerator(); + Injector::inst()->registerService($urlGenerator, ResourceURLGenerator::class); + Config::modify()->set(Director::class, 'alternate_base_url', 'http://mysite.com/subdir'); $c = new TinyMCEConfig(); $c->setTheme('modern'); @@ -92,7 +99,7 @@ class HTMLEditorConfigTest extends SapphireTest // Plugin specified with standard location $this->assertContains('plugin4', array_keys($plugins)); $this->assertEquals( - 'http://mysite.com/subdir/test/thirdparty/tinymce/plugins/plugin4/plugin.min.js', + '/subdir/silverstripe-admin/thirdparty/tinymce/plugins/plugin4/plugin.min.js', $plugins['plugin4'] ); diff --git a/tests/php/View/RequirementsTest.php b/tests/php/View/RequirementsTest.php index 6c73f2f32..1a43e9353 100644 --- a/tests/php/View/RequirementsTest.php +++ b/tests/php/View/RequirementsTest.php @@ -11,6 +11,8 @@ use SilverStripe\View\Requirements; use SilverStripe\View\ArrayData; use SilverStripe\Assets\Tests\Storage\AssetStoreTest\TestAssetStore; use SilverStripe\View\Requirements_Backend; +use SilverStripe\Core\Manifest\ResourceURLGenerator; +use SilverStripe\Control\SimpleResourceURLGenerator; /** * @todo Test that order of combine_files() is correct @@ -634,14 +636,14 @@ class RequirementsTest extends SapphireTest /* Javascript has correct path */ $this->assertRegExp( - '/src=".*\/RequirementsTest_a\.js\?m=\d\d+&test=1&test=2&test=3/', + '/src=".*\/RequirementsTest_a\.js\?test=1&test=2&test=3&m=\d\d+/', $html, 'javascript has correct path' ); /* CSS has correct path */ $this->assertRegExp( - '/href=".*\/RequirementsTest_a\.css\?m=\d\d+&test=1&test=2&test=3/', + '/href=".*\/RequirementsTest_a\.css\?test=1&test=2&test=3&m=\d\d+/', $html, 'css has correct path' ); @@ -804,6 +806,9 @@ class RequirementsTest extends SapphireTest public function testCommentedOutScriptTagIsIgnored() { + /// Disable nonce + Injector::inst()->registerService(new SimpleResourceURLGenerator(), ResourceURLGenerator::class); + $template = '' . '

more content

'; /** @@ -811,7 +816,7 @@ class RequirementsTest extends SapphireTest */ $backend = Injector::inst()->create(Requirements_Backend::class); $this->setupRequirements($backend); - $backend->setSuffixRequirements(false); + $src = $this->getThemeRoot() . '/javascript/RequirementsTest_a.js'; $urlSrc = Controller::join_links(Director::baseURL(), $src); $backend->javascript($src); @@ -889,6 +894,10 @@ EOS public function testSuffix() { + /// Disable nonce + $urlGenerator = new SimpleResourceURLGenerator(); + Injector::inst()->registerService($urlGenerator, ResourceURLGenerator::class); + $template = '
My header

Body

'; $basePath = $this->getThemeRoot(); @@ -901,20 +910,20 @@ EOS $backend->css($basePath .'/css/RequirementsTest_a.css'); $backend->css($basePath .'/css/RequirementsTest_b.css?foo=bar&bla=blubb'); - $backend->setSuffixRequirements(true); + $urlGenerator->setNonceStyle('mtime'); $html = $backend->includeInHTML($template); $this->assertRegExp('/RequirementsTest_a\.js\?m=[\d]*"/', $html); - $this->assertRegExp('/RequirementsTest_b\.js\?m=[\d]*&foo=bar&bla=blubb"/', $html); + $this->assertRegExp('/RequirementsTest_b\.js\?foo=bar&bla=blubb&m=[\d]*"/', $html); $this->assertRegExp('/RequirementsTest_a\.css\?m=[\d]*"/', $html); - $this->assertRegExp('/RequirementsTest_b\.css\?m=[\d]*&foo=bar&bla=blubb"/', $html); + $this->assertRegExp('/RequirementsTest_b\.css\?foo=bar&bla=blubb&m=[\d]*"/', $html); - $backend->setSuffixRequirements(false); + $urlGenerator->setNonceStyle(null); $html = $backend->includeInHTML($template); $this->assertNotContains('RequirementsTest_a.js=', $html); $this->assertNotRegExp('/RequirementsTest_a\.js\?m=[\d]*"/', $html); - $this->assertNotRegExp('/RequirementsTest_b\.js\?m=[\d]*&foo=bar&bla=blubb"/', $html); + $this->assertNotRegExp('/RequirementsTest_b\.js\?foo=bar&bla=blubb&m=[\d]*"/', $html); $this->assertNotRegExp('/RequirementsTest_a\.css\?m=[\d]*"/', $html); - $this->assertNotRegExp('/RequirementsTest_b\.css\?m=[\d]*&foo=bar&bla=blubb"/', $html); + $this->assertNotRegExp('/RequirementsTest_b\.css\?foo=bar&bla=blubb&m=[\d]*"/', $html); } /**