From 8d077203d4d2a6f98b738f9c5f83e3264545677f Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 12 Jan 2018 16:25:02 +1300 Subject: [PATCH] API Implement support for public/ webroot folder (#7741) * API Implement support for public/ webroot folder * Bugfixes and refactor based on feedback --- .travis.yml | 1 + .../01_Installation/03_Windows.md | 4 +- .../Windows_IIS7.md | 2 +- .../How_To/Configure_Lighttpd.md | 13 +- .../01_Installation/How_To/Configure_Nginx.md | 45 +--- .../How_To/Setup_Nginx_and_HHVM.md | 25 +- .../04_Directory_Structure.md | 14 +- docs/en/04_Changelogs/4.0.0.md | 2 + docs/en/04_Changelogs/4.1.0.md | 208 +++++++++++++++ src/Control/Director.php | 64 ++++- src/Control/SimpleResourceURLGenerator.php | 158 +++++++++-- src/Core/Convert.php | 16 ++ src/Core/CoreKernel.php | 9 +- src/Core/Manifest/Module.php | 25 +- src/Core/Manifest/ModuleResource.php | 26 +- src/Core/Path.php | 61 +++++ src/Core/TempFolder.php | 23 +- src/Dev/Install/InstallRequirements.php | 135 ++++++---- src/Dev/Install/Installer.php | 65 +++-- src/Dev/Install/config-form.html | 1 + src/Dev/Install/install5.php | 6 +- src/View/PublicThemes.php | 11 + src/View/Requirements.php | 4 +- src/View/Requirements_Backend.php | 47 ++-- src/View/SSViewer.php | 9 +- src/View/ThemeResourceLoader.php | 20 +- src/includes/constants.php | 32 ++- tests/php/Control/DirectorTest.php | 6 + .../SimpleResourceURLGeneratorTest.php | 33 ++- .../public/basemodule/css/style.css | 2 + .../php/Core/Manifest/ModuleResourceTest.php | 8 +- tests/php/Core/PathTest.php | 118 +++++++++ .../Forms/HTMLEditor/HTMLEditorFieldTest.php | 2 +- .../TinyMCECombinedGeneratorTest.php | 1 + tests/php/View/RequirementsTest.php | 247 +++++++++--------- tests/php/View/SSViewerTest.php | 2 +- .../public/css/RequirementsTest_d.css | 1 + .../public/css/RequirementsTest_e.css | 1 + .../public/css/RequirementsTest_print_d.css | 1 + .../public/css/RequirementsTest_print_e.css | 1 + .../public/javascript/RequirementsTest_d.js | 1 + .../public/javascript/RequirementsTest_e.js | 1 + 42 files changed, 1062 insertions(+), 389 deletions(-) create mode 100644 docs/en/04_Changelogs/4.1.0.md create mode 100644 src/Core/Path.php create mode 100644 src/View/PublicThemes.php create mode 100644 tests/php/Control/SimpleResourceURLGeneratorTest/_fakewebroot/public/basemodule/css/style.css create mode 100644 tests/php/Core/PathTest.php create mode 100644 tests/php/View/SSViewerTest/public/css/RequirementsTest_d.css create mode 100644 tests/php/View/SSViewerTest/public/css/RequirementsTest_e.css create mode 100644 tests/php/View/SSViewerTest/public/css/RequirementsTest_print_d.css create mode 100644 tests/php/View/SSViewerTest/public/css/RequirementsTest_print_e.css create mode 100644 tests/php/View/SSViewerTest/public/javascript/RequirementsTest_d.js create mode 100644 tests/php/View/SSViewerTest/public/javascript/RequirementsTest_e.js diff --git a/.travis.yml b/.travis.yml index 9e1dec634..e98cbf704 100644 --- a/.travis.yml +++ b/.travis.yml @@ -76,6 +76,7 @@ before_script: # Install composer dependencies - export PATH=~/.composer/vendor/bin:$PATH - composer validate + - mkdir ./public - if [[ $DB == PGSQL ]]; then composer require silverstripe/postgresql:2.0.x-dev --no-update; fi - if [[ $DB == SQLITE ]]; then composer require silverstripe/sqlite3:2.0.x-dev --no-update; fi - composer require silverstripe/recipe-core:1.1.x-dev silverstripe/admin:1.1.x-dev silverstripe/versioned:1.1.x-dev --no-update diff --git a/docs/en/00_Getting_Started/01_Installation/03_Windows.md b/docs/en/00_Getting_Started/01_Installation/03_Windows.md index 31493254e..1900d332f 100644 --- a/docs/en/00_Getting_Started/01_Installation/03_Windows.md +++ b/docs/en/00_Getting_Started/01_Installation/03_Windows.md @@ -40,9 +40,9 @@ $ composer create-project silverstripe/installer ./silverstripe ## Install and configure * Option 1: Environment file - Set up a file named `.env` file either in the webroot and setup as per the [Environment Management process](/getting_started/environment_management). -* Option 2: Installer - Visit `http://localhost/silverstripe` - you will see SilverStripe's installation screen. +* Option 2: Installer - Visit `http://localhost/silverstripe/public` - you will see SilverStripe's installation screen. * You should be able to click "Install SilverStripe" and the installer will do its thing. It takes a minute or two. -* Once the installer has finished, visit `http://localhost/silverstripe`. You should see your new SilverStripe site's +* Once the installer has finished, visit `http://localhost/silverstripe/public`. You should see your new SilverStripe site's home page. ## Troubleshooting diff --git a/docs/en/00_Getting_Started/01_Installation/04_Other_installation_Options/Windows_IIS7.md b/docs/en/00_Getting_Started/01_Installation/04_Other_installation_Options/Windows_IIS7.md index 40ba16355..5cd030b12 100644 --- a/docs/en/00_Getting_Started/01_Installation/04_Other_installation_Options/Windows_IIS7.md +++ b/docs/en/00_Getting_Started/01_Installation/04_Other_installation_Options/Windows_IIS7.md @@ -190,7 +190,7 @@ After gettng the code installed, make sure you set the folder permissions proper ## Start SilverStripe installer -Open a browser and point it to `http://localhost/ss` +Open a browser and point it to `http://localhost/ss/public` If an installation screen shows up, congratulations! We're very close now. diff --git a/docs/en/00_Getting_Started/01_Installation/How_To/Configure_Lighttpd.md b/docs/en/00_Getting_Started/01_Installation/How_To/Configure_Lighttpd.md index 77ac9a127..f195b7e1b 100644 --- a/docs/en/00_Getting_Started/01_Installation/How_To/Configure_Lighttpd.md +++ b/docs/en/00_Getting_Started/01_Installation/How_To/Configure_Lighttpd.md @@ -1,11 +1,11 @@ # Lightttpd 1. Lighttpd works fine so long as you provide a custom config. Add the following to lighttpd.conf **BEFORE** installing -Silverstripe. Replace "yoursite.com" and "/home/yoursite/public_html/" below. +Silverstripe. Replace "yoursite.com" and "/home/yoursite/public/" below. $HTTP["host"] == "yoursite.com" { - server.document-root = "/home/yoursite/public_html/" + server.document-root = "/home/yoursite/public/" # Disable directory listings dir-listing.activate = "disable" @@ -14,16 +14,11 @@ Silverstripe. Replace "yoursite.com" and "/home/yoursite/public_html/" below. url.access-deny += ( ".ss" ) static-file.exclude-extensions += ( ".ss" ) - # Deny access to SilverStripe command-line interface - $HTTP["url"] =~ "^/vendor/silverstripe/framework/cli-script.php" { + # Deny access to vendor + $HTTP["url"] =~ "^/vendor" { url.access-deny = ( "" ) } - # Disable FastCGI in assets directory (so that PHP files are not executed) - $HTTP["url"] =~ "^/assets/" { - fastcgi.server = () - } - # Rewrite URLs so they are nicer url.rewrite-once = ( "^/.*\.[A-Za-z0-9]+.*?$" => "$0", diff --git a/docs/en/00_Getting_Started/01_Installation/How_To/Configure_Nginx.md b/docs/en/00_Getting_Started/01_Installation/How_To/Configure_Nginx.md index fdd45c4c3..2eb4e4587 100644 --- a/docs/en/00_Getting_Started/01_Installation/How_To/Configure_Nginx.md +++ b/docs/en/00_Getting_Started/01_Installation/How_To/Configure_Nginx.md @@ -16,11 +16,12 @@ If you don't fully understand the configuration presented here, consult the Especially be aware of [accidental php-execution](https://nealpoole.com/blog/2011/04/setting-up-php-fastcgi-and-nginx-dont-trust-the-tutorials-check-your-configuration/ "Don't trust the tutorials") when extending the configuration. -But enough of the disclaimer, on to the actual configuration — typically in `nginx.conf`: +But enough of the disclaimer, on to the actual configuration — typically in `nginx.conf`. This assumes +you are running your site configuration with a separate `public/` webroot folder. server { listen 80; - root /path/to/ss/folder; + root /var/www/the-website/public; server_name site.com www.site.com; @@ -43,53 +44,15 @@ But enough of the disclaimer, on to the actual configuration — typically in `n sendfile on; try_files $uri index.php?$query_string; } - - location ~ /framework/.*(main|rpc|tiny_mce_gzip)\.php$ { - fastcgi_keep_conn on; - fastcgi_pass 127.0.0.1:9000; - fastcgi_index index.php; - fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name; - include fastcgi_params; - } - - location ~ /(mysite|framework|cms)/.*\.(php|php3|php4|php5|phtml|inc)$ { - deny all; - } - + location ~ /\.. { deny all; } - location ~ \.ss$ { - satisfy any; - allow 127.0.0.1; - deny all; - } - location ~ web\.config$ { deny all; } - location ~ \.ya?ml$ { - deny all; - } - - location ^~ /vendor/ { - deny all; - } - - location ~* /silverstripe-cache/ { - deny all; - } - - location ~* composer\.(json|lock)$ { - deny all; - } - - location ~* /(cms|framework)/silverstripe_version$ { - deny all; - } - location ~ \.php$ { fastcgi_keep_conn on; fastcgi_pass 127.0.0.1:9000; diff --git a/docs/en/00_Getting_Started/01_Installation/How_To/Setup_Nginx_and_HHVM.md b/docs/en/00_Getting_Started/01_Installation/How_To/Setup_Nginx_and_HHVM.md index 19b023a95..4ef41f4c5 100644 --- a/docs/en/00_Getting_Started/01_Installation/How_To/Setup_Nginx_and_HHVM.md +++ b/docs/en/00_Getting_Started/01_Installation/How_To/Setup_Nginx_and_HHVM.md @@ -57,35 +57,12 @@ Create `/etc/nginx/silverstripe.conf` and add this configuration: location ^~ /assets/ { try_files $uri =404; } - location ~ /(mysite|framework|cms)/.*\.(php|php3|php4|php5|phtml|inc)$ { - deny all; - } location ~ /\.. { deny all; } - location ~ \.ss$ { - satisfy any; - allow 127.0.0.1; - deny all; - } location ~ web\.config$ { deny all; } - location ~ \.ya?ml$ { - deny all; - } - location ^~ /vendor/ { - deny all; - } - location ~* /silverstripe-cache/ { - deny all; - } - location ~* composer\.(json|lock)$ { - deny all; - } - location ~* /(cms|framework)/silverstripe_version$ { - deny all; - } The above script passes all non-static file requests to `/index.php` in the webroot which relies on `hhvm.conf` being included prior so that php requests are handled. @@ -97,7 +74,7 @@ e.g. `/etc/nginx/sites-enabled/mysite`: server { listen 80; - root /var/www/mysite; + root /var/www/mysite/public; server_name www.mysite.com; error_log /var/log/nginx/mysite.error.log; diff --git a/docs/en/00_Getting_Started/04_Directory_Structure.md b/docs/en/00_Getting_Started/04_Directory_Structure.md index c9f7ffd41..0a418d64d 100644 --- a/docs/en/00_Getting_Started/04_Directory_Structure.md +++ b/docs/en/00_Getting_Started/04_Directory_Structure.md @@ -7,11 +7,13 @@ directories is meaningful to its logic. ## Core Structure -Directory | Description ---------- | ----------- -`assets/` | Images and other files uploaded via the SilverStripe CMS. You can also place your own content inside it, and link to it from within the content area of the CMS. -`resources/`| Public files from modules (added automatically) -`vendor/` | SilverStripe modules and other supporting libraries (the framework is in `vendor/silverstripe/framework`) +Directory | Description +--------- | ----------- +`public/` | Webserver public webroot +`public/assets/` | Images and other files uploaded via the SilverStripe CMS. You can also place your own content inside it, and link to it from within the content area of the CMS. +`public/resources/` | Exposed public files added from modules. Folders within this parent will match that of the source root location. +`vendor/` | SilverStripe modules and other supporting libraries (the framework is in `vendor/silverstripe/framework`) +`themes/` | Standard theme installation location ## Custom Code Structure @@ -100,4 +102,4 @@ by using a `flush=1` query parameter. See the ["Manifests" documentation](/devel ## Best Practices ### Making /assets readonly -See [Secure coding](/developer_guides/security/secure_coding#filesystem) \ No newline at end of file +See [Secure coding](/developer_guides/security/secure_coding#filesystem) diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md index 2daabc0ce..3b7ade92e 100644 --- a/docs/en/04_Changelogs/4.0.0.md +++ b/docs/en/04_Changelogs/4.0.0.md @@ -6,6 +6,8 @@ This version introduces many breaking changes, which in most projects can be man of automatic upgrade processes as well as manual code review. This document reviews these changes and will guide developers in preparing existing 3.x code for compatibility with 4.0 +For users upgrading to 4.1.0 please see the specific [4.1.0 upgrading guide](4.1.0.md). + ## Overview {#overview} * Minimum version dependencies have increased; PHP 5.5 and Internet Explorer 11 (or other modern browser) diff --git a/docs/en/04_Changelogs/4.1.0.md b/docs/en/04_Changelogs/4.1.0.md new file mode 100644 index 000000000..e5c4098ad --- /dev/null +++ b/docs/en/04_Changelogs/4.1.0.md @@ -0,0 +1,208 @@ +# 4.1.0 + +## Overview {#overview} + + * Support for public webroot folder `public/` + * Better support for cross-platform filesystem path manipulation + +## Upgrading + +### Upgrade `public/` folder + +This release allows the maintenance of a public webroot folder which separates all +web-accessible files from protected project files (like the vendor folder +and all of your PHP files). This makes your web hosting more secure, and +less likely to leak information through accidentally deployed files like +a project README. New projects will default to this behaviour. Existing +projects (updating from 3.x or 4.0) will continue working as-is, but we +strongly recommend switching to the public webroot structure in order to +get the security benefits. + +This folder name is not configurable, but is turned on by creating this folder, and off by ensuring +this folder doesn't exist. + +When separating the public webroot from the BASE_PATH it is necessary to move a few files during migration: + + - Move `.htaccess` from base to `public/` + - Move `index.php` from base to `public/` + - Move `assets` folder (including the nested `assets/.protected` folder) into `public/`. + This is the only folder which needs write permissions. + - Ensure that the `public/resources` folder exists; If this folder already exists in root, you should + delete this, and re-generate it by running `composer vendor-expose` in your root path. + - Any public assets committed directly to your project intended to be served directly to the + webserver. E.g. move `mysite/javascript/script.js` to `public/javascript/script.js`. + You can then use `Requirements::css('javascript/script.js');` / + `<% require css('javascript/script.js') %>` to include this file. + - Ensure that the web-root configured for your server of choice points to the public/ folder instead of the base path. + E.g. an apache virtualhost configuration would look like: + +``` + + ServerName mywebsite.com + ServerAlias *.mywebsite.com + VirtualDocumentRoot "/var/www/Sites/mywebsite/public/" + +``` + +You may also need to add various changes to your code if you reference the BASE_PATH directly: + + - You should use `Director::publicFolder()` instead of `Director::baseFolder()` if referring to the + public folder. + - You can check if a public folder exists with `Director::publicDir()` + +### Example `public/` folder structure + +For example, this is an existing folder structure: + +``` +/var/www/mysite +├── assets/ +│ └── .protected/ +├── mysite/ +│ ├── code/ +│ │ ├── Page.php +│ │ └── PageController.php +│ └── css/ +│ └── projectstyle.css +├── resources/ _(auto-generated by vendor-plugin `composer vendor-expose` command)_ +│ └── silverstripe/ +│ └── blog/ +│ └── css/ _(symlink)_ +│ └── blog.css +├── themes/ +│ └── mytheme/ +│ ├── css/ +│ │ └── theme.css +│ └── templates/ +│ └── BlogPage.ss +├── vendor/ +│ └── silverstripe/ +│ └── blog/ +│ ├── css/ _(exposed in blog composer.json)_ +│ │ └── blog.css +│ └── composer.json +├── .htaccess +├── composer.json +├── favicon.ico +├── index.php +└── install.php +``` + +After migration the folder structure would look like: + +``` +/var/www/mysite +├── mysite/ +│ └── code/ +│ ├── Page.php +│ └── PageController.php +├── public/ +│ ├── assets/ +│ │ └── .protected/ +│ ├── css/ +│ │ └── somestyle.css +│ ├── resources/ _(auto-generated by vendor-plugin `composer vendor-expose` command)_ +│ │ ├── themes/ +│ │ │ └── mytheme/ +│ │ │ └── css/ _(symlink)_ +│ │ │ └── theme.css +│ │ └── vendor/ +│ │ └── silverstripe/ +│ │ └── blog/ +│ │ └── css/ _(symlink)_ +│ │ └── blog.css +│ ├── .htaccess +│ ├── favicon.ico +│ ├── index.php +│ └── install.php +├── themes/ +│ └── mytheme/ +│ ├── css/ _(exposed in root composer.json)_ +│ │ └── theme.css +│ └── templates/ +│ └── BlogPage.ss +├── vendor/ +│ └── silverstripe/ +│ └── blog/ +│ ├── css/ _(exposed in blog composer.json)_ +│ │ └── blog.css +│ └── composer.json +└── composer.php +``` + +### Use new `$public` theme set + +In addition there is a new helper pseudo-theme that you can configure to expose files in the `public/` +folder to the themed css / javascript file lookup. For instance, this is how you can prioritise those +files: + +```yaml +--- +Name: mytheme +--- +SilverStripe\View\SSViewer: + themes: + - '$public' + - 'simple' + - '$default' +``` + +This would allow `<% require themedCSS('style.css') %>` to find a file comitted to `public/css/style.css`. + +Note that `Requirements` calls will look in both the `public` folder (first) and then the base path when +resolving css or javascript files. Any files that aren't in the public folder must be exposed using +the composer.json "expose" mechanism described below. + +### Expose root project files + +If you have files comitted to source control outside of the `public` folder, but you need them to be available +to the web server, you can also use the composer.json `expose` directive to symlink / copy these to `public/resources/`. + +**composer.json** (in project root) + +```json +{ + "extra": { + "expose": [ + "mysite/client" + ] + } +} +``` + +Then run the composer helper `composer vendor-expose` in your project root. This will symlink (or copy) +the `mysite/client` directory to `public/resources/mysite/client`. + +If you are using third party modules which may not have explicit `expose` directives, +you can also expose those assets manually by declaring the full path to the directory to expose. +This works the same for `silverstripe-module` and `silverstripe-vendormodule` types. + +```json +{ + "extra": { + "expose": [ + "vendor/somevendor/somemodule/css", + "anothermodule/css" + ] + } +} +``` + +For more information on how vendor modules work please see the documentation on the +[vendor plugin page](https://github.com/silverstripe/vendor-plugin) or the +[publishing a module](/developer_guides/extending/how_tos/publish_a_module) documentation. + +### Path manipulation helpers + +The following filesystem helpers have been added in order to better support working with cross-platform +path manipulation: + + * `SilverStripe\Core\Convert::slashes()` to convert directory separators to either `/` or `\` + * `SilverStripe\Core\Path::join()` which will join one or more relative or absolute paths. + * `SilverStripe\Core\Path::normalise()` which will normalise and trim directory separators in a relative or absolute path + +For example: normalising `Convert::normalise('/some\\dir/')` will convert to `/some/dir`. +Setting the second arg to true will also trim leading slashes. +E.g. `Convert::normalise('/sub\\dir/', true)` will convert to `sub/dir`. + +It is preferrable to use these helpers in your code instead of assuming `DIRECTORY_SEPARATOR` === `/` diff --git a/src/Control/Director.php b/src/Control/Director.php index 2f42c8c5c..8af295743 100644 --- a/src/Control/Director.php +++ b/src/Control/Director.php @@ -11,6 +11,7 @@ use SilverStripe\Core\Extensible; use SilverStripe\Core\Injector\Injectable; use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Kernel; +use SilverStripe\Core\Path; use SilverStripe\Dev\Deprecation; use SilverStripe\Versioned\Versioned; use SilverStripe\View\Requirements; @@ -76,15 +77,13 @@ class Director implements TemplateGlobalProvider private static $alternate_base_folder; /** - * Force the base_url to a specific value. - * If assigned, default_base_url and the value in the $_SERVER - * global is ignored. - * Supports back-ticked vars; E.g. '`SS_BASE_URL`' + * Override PUBLIC_DIR. Set to a non-null value to override. + * Setting to an empty string will disable public dir. * * @config - * @var string + * @var bool|null */ - private static $alternate_base_url; + private static $alternate_public_dir = null; /** * Base url to populate if cannot be determined otherwise. @@ -589,7 +588,40 @@ class Director implements TemplateGlobalProvider public static function baseFolder() { $alternate = Director::config()->uninherited('alternate_base_folder'); - return ($alternate) ? $alternate : BASE_PATH; + return $alternate ?: BASE_PATH; + } + + /** + * Check if using a seperate public dir, and if so return this directory + * name. + * + * This will be removed in 5.0 and fixed to 'public' + * + * @return string + */ + public static function publicDir() + { + $alternate = self::config()->uninherited('alternate_public_dir'); + if (isset($alternate)) { + return $alternate; + } + return PUBLIC_DIR; + } + + /** + * Gets the webroot of the project, which may be a subfolder of {@see baseFolder()} + * + * @return string + */ + public static function publicFolder() + { + $folder = self::baseFolder(); + $publicDir = self::publicDir(); + if ($publicDir) { + return Path::join($folder, $publicDir); + } + + return $folder; } /** @@ -618,7 +650,7 @@ class Director implements TemplateGlobalProvider } // Remove base folder or url - foreach ([self::baseFolder(), self::baseURL()] as $base) { + foreach ([self::publicFolder(), self::baseFolder(), self::baseURL()] as $base) { // Ensure single / doesn't break comparison (unless it would make base empty) $base = rtrim($base, '\\/') ?: $base; if (stripos($url, $base) === 0) { @@ -746,7 +778,21 @@ class Director implements TemplateGlobalProvider */ public static function getAbsFile($file) { - return self::is_absolute($file) ? $file : Director::baseFolder() . '/' . $file; + // If already absolute + if (self::is_absolute($file)) { + return $file; + } + + // If path is relative to public folder search there first + if (self::publicDir()) { + $path = Path::join(self::publicFolder(), $file); + if (file_exists($path)) { + return $path; + } + } + + // Default to base folder + return Path::join(self::baseFolder(), $file); } /** diff --git a/src/Control/SimpleResourceURLGenerator.php b/src/Control/SimpleResourceURLGenerator.php index 6ad13704c..10c8f476c 100644 --- a/src/Control/SimpleResourceURLGenerator.php +++ b/src/Control/SimpleResourceURLGenerator.php @@ -4,8 +4,11 @@ namespace SilverStripe\Control; use InvalidArgumentException; use SilverStripe\Core\Config\Config; +use SilverStripe\Core\Convert; +use SilverStripe\Core\Manifest\ManifestFileFinder; use SilverStripe\Core\Manifest\ModuleResource; use SilverStripe\Core\Manifest\ResourceURLGenerator; +use SilverStripe\Core\Path; /** * Generate URLs assuming that BASE_PATH is also the webroot @@ -21,9 +24,7 @@ class SimpleResourceURLGenerator implements ResourceURLGenerator * @config * @var array */ - private static $url_rewrites = [ - '#^vendor/#i' => 'resources/', - ]; + private static $url_rewrites = []; /* * @var string @@ -65,40 +66,163 @@ class SimpleResourceURLGenerator implements ResourceURLGenerator */ public function urlForResource($relativePath) { + $query = ''; if ($relativePath instanceof ModuleResource) { - // Load from module resource - $resource = $relativePath; - $relativePath = $resource->getRelativePath(); - $exists = $resource->exists(); - $absolutePath = $resource->getPath(); + list($exists, $absolutePath, $relativePath) = $this->resolveModuleResource($relativePath); } else { - // Use normal string - $absolutePath = preg_replace('/\?.*/', '', Director::baseFolder() . '/' . $relativePath); - $exists = file_exists($absolutePath); + // Save querystring for later + if (strpos($relativePath, '?') !== false) { + list($relativePath, $query) = explode('?', $relativePath); + } + + // Determine lookup mechanism based on existence of public/ folder. + // From 5.0 onwards only resolvePublicResource() will be used. + if (!Director::publicDir()) { + list($exists, $absolutePath, $relativePath) = $this->resolveUnsecuredResource($relativePath); + } else { + list($exists, $absolutePath, $relativePath) = $this->resolvePublicResource($relativePath); + } } if (!$exists) { trigger_error("File {$relativePath} does not exist", E_USER_NOTICE); } + // Switch slashes for URL + $relativeURL = Convert::slashes($relativePath, '/'); + // Apply url rewrites $rules = Config::inst()->get(static::class, 'url_rewrites') ?: []; foreach ($rules as $from => $to) { - $relativePath = preg_replace($from, $to, $relativePath); + $relativeURL = preg_replace($from, $to, $relativeURL); } // Apply nonce - $nonce = ''; // Don't add nonce to directories if ($this->nonceStyle && $exists && is_file($absolutePath)) { - $nonce = (strpos($relativePath, '?') === false) ? '?' : '&'; - switch ($this->nonceStyle) { case 'mtime': - $nonce .= "m=" . filemtime($absolutePath); + if ($query) { + $query .= '&'; + } + $query .= "m=" . filemtime($absolutePath); break; } } - return Director::baseURL() . $relativePath . $nonce; + // Add back querystring + if ($query) { + $relativeURL .= '?' . $query; + } + + return Director::baseURL() . $relativeURL; + } + + /** + * Update relative path for a module resource + * + * @param ModuleResource $resource + * @return array List of [$exists, $absolutePath, $relativePath] + */ + protected function resolveModuleResource(ModuleResource $resource) + { + // Load from module resource + $relativePath = $resource->getRelativePath(); + $exists = $resource->exists(); + $absolutePath = $resource->getPath(); + + // Rewrite to resources with public directory + if (Director::publicDir()) { + // All resources mapped directly to resources/ + $relativePath = Path::join(ManifestFileFinder::RESOURCES_DIR, $relativePath); + } elseif (stripos($relativePath, ManifestFileFinder::VENDOR_DIR . DIRECTORY_SEPARATOR) === 0) { + // @todo Non-public dir support will be removed in 5.0, so remove this block there + // If there is no public folder, map to resources/ but trim leading vendor/ too (4.0 compat) + $relativePath = Path::join( + ManifestFileFinder::RESOURCES_DIR, + substr($relativePath, strlen(ManifestFileFinder::VENDOR_DIR)) + ); + } + return [$exists, $absolutePath, $relativePath]; + } + + /** + * Resolve resource in the absence of a public/ folder + * + * @deprecated 4.1.0...5.0.0 Will be removed in 5.0 when public/ folder becomes mandatory + * @param string $relativePath + * @return array List of [$exists, $absolutePath, $relativePath] + */ + protected function resolveUnsecuredResource($relativePath) + { + // Check if the path requested is public-only, but we have no public folder + $publicOnly = $this->inferPublicResourceRequired($relativePath); + if ($publicOnly) { + trigger_error('Requesting a public resource without a public folder has no effect', E_USER_WARNING); + } + + // Resolve path to base + $absolutePath = Path::join(Director::baseFolder(), $relativePath); + $exists = file_exists($absolutePath); + + // Rewrite vendor/ to resources/ folder + if (stripos($relativePath, ManifestFileFinder::VENDOR_DIR . DIRECTORY_SEPARATOR) === 0) { + $relativePath = Path::join( + ManifestFileFinder::RESOURCES_DIR, + substr($relativePath, strlen(ManifestFileFinder::VENDOR_DIR)) + ); + } + return [$exists, $absolutePath, $relativePath]; + } + + /** + * Determine if the requested $relativePath requires a public-only resource. + * An error will occur if this file isn't immediately available in the public/ assets folder. + * + * @param string $relativePath Requested relative path which may have a public/ prefix. + * This prefix will be removed if exists. This path will also be normalised to match DIRECTORY_SEPARATOR + * @return bool True if the resource must be a public resource + */ + protected function inferPublicResourceRequired(&$relativePath) + { + // Normalise path + $relativePath = Path::normalise($relativePath, true); + + // Detect public-only request + $publicOnly = stripos($relativePath, 'public' . DIRECTORY_SEPARATOR) === 0; + if ($publicOnly) { + $relativePath = substr($relativePath, strlen(Director::publicDir() . DIRECTORY_SEPARATOR)); + } + return $publicOnly; + } + + /** + * Resolve a resource that may either exist in a public/ folder, or be exposed from the base path to + * public/resources/ + * + * @param string $relativePath + * @return array List of [$exists, $absolutePath, $relativePath] + */ + protected function resolvePublicResource($relativePath) + { + // Determine if we should search both public and base resources, or only public + $publicOnly = $this->inferPublicResourceRequired($relativePath); + + // Search public folder first, and unless `public/` is prefixed, also private base path + $publicPath = Path::join(Director::publicFolder(), $relativePath); + if (file_exists($publicPath)) { + // String is a literal url comitted directly to public folder + return [true, $publicPath, $relativePath]; + } + + // Fall back to private path (and assume expose will make this available to resources/) + $privatePath = Path::join(Director::baseFolder(), $relativePath); + if (!$publicOnly && file_exists($privatePath)) { + // String is private but exposed to resources/, so rewrite to the symlinked base + $relativePath = Path::join(ManifestFileFinder::RESOURCES_DIR, $relativePath); + return [true, $privatePath, $relativePath]; + } + + // File doesn't exist, fail + return [false, null, $relativePath]; } } diff --git a/src/Core/Convert.php b/src/Core/Convert.php index 7db3ad03d..277acf4d8 100644 --- a/src/Core/Convert.php +++ b/src/Core/Convert.php @@ -591,4 +591,20 @@ class Convert $num = round($bytes / pow(1024, $scale), $decimal); return $num . $scales[$scale]; } + + /** + * Convert slashes in relative or asolute filesystem path. Defaults to DIRECTORY_SEPARATOR + * + * @param string $path + * @param string $separator + * @param bool $multiple Collapses multiple slashes or not + * @return string + */ + public static function slashes($path, $separator = DIRECTORY_SEPARATOR, $multiple = true) + { + if ($multiple) { + return preg_replace('#[/\\\\]+#', $separator, $path); + } + return str_replace(['/', '\\'], DIRECTORY_SEPARATOR, $path); + } } diff --git a/src/Core/CoreKernel.php b/src/Core/CoreKernel.php index 3f5e4cee1..a37dadc4b 100644 --- a/src/Core/CoreKernel.php +++ b/src/Core/CoreKernel.php @@ -24,6 +24,8 @@ use SilverStripe\Dev\DebugView; use SilverStripe\Dev\Install\DatabaseAdapterRegistry; use SilverStripe\Logging\ErrorHandler; use SilverStripe\ORM\DB; +use SilverStripe\View\PublicThemes; +use SilverStripe\View\SSViewer; use SilverStripe\View\ThemeManifest; use SilverStripe\View\ThemeResourceLoader; use SilverStripe\Dev\Deprecation; @@ -115,7 +117,8 @@ class CoreKernel implements Kernel // Load template manifest $themeResourceLoader = ThemeResourceLoader::inst(); - $themeResourceLoader->addSet('$default', new ThemeManifest( + $themeResourceLoader->addSet(SSViewer::PUBLIC_THEME, new PublicThemes()); + $themeResourceLoader->addSet(SSViewer::DEFAULT_THEME, new ThemeManifest( $basePath, null, // project is defined in config, and this argument is deprecated $manifestCacheFactory @@ -306,9 +309,9 @@ class CoreKernel implements Kernel protected function redirectToInstaller() { // Error if installer not available - if (!file_exists($this->basePath . '/install.php')) { + if (!file_exists(Director::publicFolder() . '/install.php')) { throw new HTTPResponse_Exception( - 'SilverStripe Framework requires a $databaseConfig defined.', + 'SilverStripe Framework requires database configuration defined via .env', 500 ); } diff --git a/src/Core/Manifest/Module.php b/src/Core/Manifest/Module.php index 6084235d0..a78b7988c 100644 --- a/src/Core/Manifest/Module.php +++ b/src/Core/Manifest/Module.php @@ -3,12 +3,17 @@ namespace SilverStripe\Core\Manifest; use Exception; +use InvalidArgumentException; use Serializable; +use SilverStripe\Core\Path; use SilverStripe\Dev\Deprecation; class Module implements Serializable { - const TRIM_CHARS = '/\\'; + /** + * @deprecated 4.1..5.0 Use Path::normalise() instead + */ + const TRIM_CHARS = ' /\\'; /** * Full directory path to this module with no trailing slash @@ -42,12 +47,12 @@ class Module implements Serializable * Construct a module * * @param string $path Absolute filesystem path to this module - * @param string $base base url for the application this module is installed in + * @param string $basePath base path for the application this module is installed in */ - public function __construct($path, $base) + public function __construct($path, $basePath) { - $this->path = rtrim($path, self::TRIM_CHARS); - $this->basePath = rtrim($base, self::TRIM_CHARS); + $this->path = Path::normalise($path); + $this->basePath = Path::normalise($basePath); $this->loadComposer(); } @@ -137,7 +142,10 @@ class Module implements Serializable */ public function getRelativePath() { - return trim(substr($this->path, strlen($this->basePath)), self::TRIM_CHARS); + if ($this->path === $this->basePath) { + return ''; + } + return substr($this->path, strlen($this->basePath) + 1); } public function serialize() @@ -188,7 +196,10 @@ class Module implements Serializable */ public function getResource($path) { - $path = trim($path, self::TRIM_CHARS); + $path = Path::normalise($path, true); + if (empty($path)) { + throw new InvalidArgumentException('$path is required'); + } if (isset($this->resources[$path])) { return $this->resources[$path]; } diff --git a/src/Core/Manifest/ModuleResource.php b/src/Core/Manifest/ModuleResource.php index be8ec9cee..22003d944 100644 --- a/src/Core/Manifest/ModuleResource.php +++ b/src/Core/Manifest/ModuleResource.php @@ -4,6 +4,7 @@ namespace SilverStripe\Core\Manifest; use InvalidArgumentException; use SilverStripe\Core\Injector\Injector; +use SilverStripe\Core\Path; /** * This object represents a single resource file attached to a module, and can be used @@ -39,7 +40,7 @@ class ModuleResource public function __construct(Module $module, $relativePath) { $this->module = $module; - $this->relativePath = ltrim($relativePath, Module::TRIM_CHARS); + $this->relativePath = Path::normalise($relativePath, true); if (empty($this->relativePath)) { throw new InvalidArgumentException("Resource cannot have empty path"); } @@ -55,7 +56,7 @@ class ModuleResource */ public function getPath() { - return $this->module->getPath() . '/' . $this->relativePath; + return Path::join($this->module->getPath(), $this->relativePath); } /** @@ -68,8 +69,12 @@ class ModuleResource */ public function getRelativePath() { - $path = $this->module->getRelativePath() . '/' . $this->relativePath; - return ltrim($path, Module::TRIM_CHARS); + // Root module + $parent = $this->module->getRelativePath(); + if (!$parent) { + return $this->relativePath; + } + return Path::join($parent, $this->relativePath); } /** @@ -135,15 +140,8 @@ class ModuleResource */ public function getRelativeResource($path) { - // Check cache - $path = trim($path, Module::TRIM_CHARS); - if (isset($this->resources[$path])) { - return $this->resources[$path]; - } - - // Build new relative path - $relativeBase = rtrim($this->relativePath, Module::TRIM_CHARS); - $relativePath = "{$relativeBase}/{$path}"; - return $this->resources[$path] = new ModuleResource($this->getModule(), $relativePath); + // Defer to parent module + $relativeToModule = Path::join($this->relativePath, $path); + return $this->getModule()->getResource($relativeToModule); } } diff --git a/src/Core/Path.php b/src/Core/Path.php new file mode 100644 index 000000000..f243f2335 --- /dev/null +++ b/src/Core/Path.php @@ -0,0 +1,61 @@ +baseDir, '/\\') . '/'; + return Path::normalise($this->baseDir) . DIRECTORY_SEPARATOR; + } + + /** + * Get path to public directory + * + * @return string + */ + public function getPublicDir() + { + $base = $this->getBaseDir(); + $public = Path::join($base, 'public') . DIRECTORY_SEPARATOR; + if (file_exists($public)) { + return $public; + } + return $base; } /** @@ -297,16 +318,30 @@ class InstallRequirements )); } + // Check public folder exists + $this->requireFile( + 'public', + [ + 'File permissions', + 'Is there a public/ directory?', + 'It is recommended to have a separate public/ web directory', + ], + false, + false + ); // Ensure root assets dir is writable - $this->requireWriteable('assets', array("File permissions", "Is the assets/ directory writeable?", null)); + $this->requireWriteable(ASSETS_PATH, array("File permissions", "Is the assets/ directory writeable?", null), true); // Ensure all assets files are writable - $assetsDir = $this->getBaseDir() . 'assets'; - $innerIterator = new RecursiveDirectoryIterator($assetsDir, RecursiveDirectoryIterator::SKIP_DOTS); + $innerIterator = new RecursiveDirectoryIterator(ASSETS_PATH, RecursiveDirectoryIterator::SKIP_DOTS); $iterator = new RecursiveIteratorIterator($innerIterator, RecursiveIteratorIterator::SELF_FIRST); /** @var SplFileInfo $file */ foreach ($iterator as $file) { + // Only report file as error if not writable + if ($file->isWritable()) { + continue; + } $relativePath = substr($file->getPathname(), strlen($this->getBaseDir())); $message = $file->isDir() ? "Is the {$relativePath} directory writeable?" @@ -838,13 +873,23 @@ class InstallRequirements } } - public function requireFile($filename, $testDetails) + public function requireFile($filename, $testDetails, $absolute = false, $error = true) { $this->testing($testDetails); - $filename = $this->getBaseDir() . $filename; - if (!file_exists($filename)) { - $testDetails[2] .= " (file '$filename' not found)"; + + if ($absolute) { + $filename = Path::normalise($filename); + } else { + $filename = Path::join($this->getBaseDir(), $filename); + } + if (file_exists($filename)) { + return; + } + $testDetails[2] .= " (file '$filename' not found)"; + if ($error) { $this->error($testDetails); + } else { + $this->warning($testDetails); } } @@ -853,9 +898,9 @@ class InstallRequirements $this->testing($testDetails); if ($absolute) { - $filename = str_replace('/', DIRECTORY_SEPARATOR, $filename); + $filename = Path::normalise($filename); } else { - $filename = $this->getBaseDir() . str_replace('/', DIRECTORY_SEPARATOR, $filename); + $filename = Path::join($this->getBaseDir(), $filename); } if (file_exists($filename)) { @@ -864,47 +909,49 @@ class InstallRequirements $isWriteable = is_writeable(dirname($filename)); } - if (!$isWriteable) { - if (function_exists('posix_getgroups')) { - $userID = posix_geteuid(); - $user = posix_getpwuid($userID); + if ($isWriteable) { + return; + } - $currentOwnerID = fileowner(file_exists($filename) ? $filename : dirname($filename)); - $currentOwner = posix_getpwuid($currentOwnerID); + if (function_exists('posix_getgroups')) { + $userID = posix_geteuid(); + $user = posix_getpwuid($userID); - $testDetails[2] .= "User '$user[name]' needs to be able to write to this file:\n$filename\n\nThe " - . "file is currently owned by '$currentOwner[name]'. "; + $currentOwnerID = fileowner(file_exists($filename) ? $filename : dirname($filename)); + $currentOwner = posix_getpwuid($currentOwnerID); - if ($user['name'] == $currentOwner['name']) { - $testDetails[2] .= "We recommend that you make the file writeable."; - } else { - $groups = posix_getgroups(); - $groupList = array(); - foreach ($groups as $group) { - $groupInfo = posix_getgrgid($group); - if (in_array($currentOwner['name'], $groupInfo['members'])) { - $groupList[] = $groupInfo['name']; - } - } - if ($groupList) { - $testDetails[2] .= " We recommend that you make the file group-writeable " - . "and change the group to one of these groups:\n - " . implode("\n - ", $groupList) - . "\n\nFor example:\nchmod g+w $filename\nchgrp " . $groupList[0] . " $filename"; - } else { - $testDetails[2] .= " There is no user-group that contains both the web-server user and the " - . "owner of this file. Change the ownership of the file, create a new group, or " - . "temporarily make the file writeable by everyone during the install process."; + $testDetails[2] .= "User '$user[name]' needs to be able to write to this file:\n$filename\n\nThe " + . "file is currently owned by '$currentOwner[name]'. "; + + if ($user['name'] == $currentOwner['name']) { + $testDetails[2] .= "We recommend that you make the file writeable."; + } else { + $groups = posix_getgroups(); + $groupList = array(); + foreach ($groups as $group) { + $groupInfo = posix_getgrgid($group); + if (in_array($currentOwner['name'], $groupInfo['members'])) { + $groupList[] = $groupInfo['name']; } } - } else { - $testDetails[2] .= "The webserver user needs to be able to write to this file:\n$filename"; + if ($groupList) { + $testDetails[2] .= " We recommend that you make the file group-writeable " + . "and change the group to one of these groups:\n - " . implode("\n - ", $groupList) + . "\n\nFor example:\nchmod g+w $filename\nchgrp " . $groupList[0] . " $filename"; + } else { + $testDetails[2] .= " There is no user-group that contains both the web-server user and the " + . "owner of this file. Change the ownership of the file, create a new group, or " + . "temporarily make the file writeable by everyone during the install process."; + } } + } else { + $testDetails[2] .= "The webserver user needs to be able to write to this file:\n$filename"; + } - if ($error) { - $this->error($testDetails); - } else { - $this->warning($testDetails); - } + if ($error) { + $this->error($testDetails); + } else { + $this->warning($testDetails); } } diff --git a/src/Dev/Install/Installer.php b/src/Dev/Install/Installer.php index 55e61adeb..70cdc496a 100644 --- a/src/Dev/Install/Installer.php +++ b/src/Dev/Install/Installer.php @@ -7,6 +7,7 @@ use SilverStripe\Control\Cookie; use SilverStripe\Control\HTTPApplication; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPRequestBuilder; +use SilverStripe\Core\Convert; use SilverStripe\Core\CoreKernel; use SilverStripe\Core\EnvironmentLoader; use SilverStripe\Core\Kernel; @@ -27,13 +28,15 @@ class Installer extends InstallRequirements protected function installHeader() { + $clientPath = PUBLIC_DIR + ? 'resources/vendor/silverstripe/framework/src/Dev/Install/client' + : 'resources/silverstripe/framework/src/Dev/Install/client'; ?> Installing SilverStripe... - + @@ -209,7 +212,15 @@ use SilverStripe\Control\HTTPRequestBuilder; use SilverStripe\Core\CoreKernel; use SilverStripe\Core\Startup\ErrorControlChainMiddleware; -require __DIR__ . '/vendor/autoload.php'; +// Find autoload.php +if (file_exists(__DIR__ . '/vendor/autoload.php')) { + require __DIR__ . '/vendor/autoload.php'; +} elseif (file_exists(__DIR__ . '/../vendor/autoload.php')) { + require __DIR__ . '/../vendor/autoload.php'; +} else { + echo "autoload.php not found"; + die; +} // Build request and detect flush $request = HTTPRequestBuilder::createFromEnvironment(); @@ -221,7 +232,8 @@ $app->addMiddleware(new ErrorControlChainMiddleware($app)); $response = $app->handle($request); $response->output(); PHP; - $this->writeToFile('index.php', $content); + $path = $this->getPublicDir() . 'index.php'; + $this->writeToFile($path, $content, true); } /** @@ -340,11 +352,13 @@ PHP if ($config['theme'] && $config['theme'] !== 'tutorial') { $theme = $this->ymlString($config['theme']); $themeYML = <<getBaseDir(); - $this->statusMessage("Setting up $base$filename"); + $path = $absolute + ? $filename + : $this->getBaseDir() . $filename; + $this->statusMessage("Setting up $path"); - if ((@$fh = fopen($base . $filename, 'wb')) && fwrite($fh, $content) && fclose($fh)) { + if ((@$fh = fopen($path, 'wb')) && fwrite($fh, $content) && fclose($fh)) { // Set permissions to writable - @chmod($base . $filename, 0775); + @chmod($path, 0775); return true; } - $this->error("Couldn't write to file $base$filename"); + $this->error("Couldn't write to file $path"); return false; } @@ -405,11 +422,7 @@ YML $end = "\n### SILVERSTRIPE END ###"; $base = dirname($_SERVER['SCRIPT_NAME']); - if (defined('DIRECTORY_SEPARATOR')) { - $base = str_replace(DIRECTORY_SEPARATOR, '/', $base); - } else { - $base = str_replace("\\", '/', $base); - } + $base = Convert::slashes($base, '/'); if ($base != '.') { $baseClause = "RewriteBase '$base'\n"; @@ -474,8 +487,9 @@ ErrorDocument 500 /assets/error-500.html TEXT; - if (file_exists('.htaccess')) { - $htaccess = file_get_contents('.htaccess'); + $htaccessPath = $this->getPublicDir() . '.htaccess'; + if (file_exists($htaccessPath)) { + $htaccess = file_get_contents($htaccessPath); if (strpos($htaccess, '### SILVERSTRIPE START ###') === false && strpos($htaccess, '### SILVERSTRIPE END ###') === false @@ -492,7 +506,7 @@ TEXT; } } - $this->writeToFile('.htaccess', $start . $rewrite . $end); + $this->writeToFile($htaccessPath, $start . $rewrite . $end, true); } /** @@ -509,7 +523,6 @@ TEXT; - @@ -534,7 +547,8 @@ TEXT; TEXT; - $this->writeToFile('web.config', $content); + $path = $this->getPublicDir() . 'web.config'; + $this->writeToFile($path, $content, true); } public function checkRewrite() @@ -542,8 +556,11 @@ TEXT; $token = new ParameterConfirmationToken('flush', new HTTPRequest('GET', '/')); $params = http_build_query($token->params()); - $destinationURL = str_replace('install.php', '', $_SERVER['SCRIPT_NAME']) . - ($this->checkModuleExists('cms') ? "home/successfullyinstalled?$params" : "?$params"); + $destinationURL = BASE_URL . '/' . ( + $this->checkModuleExists('cms') + ? "home/successfullyinstalled?$params" + : "?$params" + ); echo <<Testing... diff --git a/src/Dev/Install/config-form.html b/src/Dev/Install/config-form.html index 5a8149dbd..29667f72f 100644 --- a/src/Dev/Install/config-form.html +++ b/src/Dev/Install/config-form.html @@ -5,6 +5,7 @@ SilverStripe CMS / Framework Installation + diff --git a/src/Dev/Install/install5.php b/src/Dev/Install/install5.php index 6bf86d49d..b50ff0116 100755 --- a/src/Dev/Install/install5.php +++ b/src/Dev/Install/install5.php @@ -105,7 +105,10 @@ if ($installFromCli && ($req->hasErrors() || $dbReq->hasErrors())) { } // Path to client resources (copied through silverstripe/vendor-plugin) -$clientPath = 'resources/silverstripe/framework/src/Dev/Install/client'; +$base = BASE_URL; +$clientPath = PUBLIC_DIR + ? 'resources/vendor/silverstripe/framework/src/Dev/Install/client' + : 'resources/silverstripe/framework/src/Dev/Install/client'; // If already installed, ensure the user clicked "reinstall" $expectedArg = $alreadyInstalled ? 'reinstall' : 'go'; @@ -135,6 +138,7 @@ $adminConfig = $config->getAdminConfig($_REQUEST, false); // config-form.html vars (placeholder to prevent deletion) [ + $base, $theme, $clientPath, $adminConfig, diff --git a/src/View/PublicThemes.php b/src/View/PublicThemes.php new file mode 100644 index 000000000..2305e1145 --- /dev/null +++ b/src/View/PublicThemes.php @@ -0,0 +1,11 @@ +add_i18n_javascript($langDir, $return, $langOnly); + return self::backend()->add_i18n_javascript($langDir, $return); } /** diff --git a/src/View/Requirements_Backend.php b/src/View/Requirements_Backend.php index 16e841e22..fe5bfb8a9 100644 --- a/src/View/Requirements_Backend.php +++ b/src/View/Requirements_Backend.php @@ -14,6 +14,7 @@ use SilverStripe\Core\Injector\Injectable; use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Manifest\ModuleResourceLoader; use SilverStripe\Core\Manifest\ResourceURLGenerator; +use SilverStripe\Core\Path; use SilverStripe\Dev\Debug; use SilverStripe\Dev\Deprecation; use SilverStripe\i18n\i18n; @@ -603,7 +604,12 @@ class Requirements_Backend public function javascriptTemplate($file, $vars, $uniquenessID = null) { $file = ModuleResourceLoader::singleton()->resolvePath($file); - $script = file_get_contents(Director::getAbsFile($file)); + $absolutePath = Director::getAbsFile($file); + if (!file_exists($absolutePath)) { + throw new InvalidArgumentException("Javascript template file {$file} does not exist"); + } + + $script = file_get_contents($absolutePath); $search = array(); $replace = array(); @@ -629,9 +635,9 @@ class Requirements_Backend { $file = ModuleResourceLoader::singleton()->resolvePath($file); - $this->css[$file] = array( + $this->css[$file] = [ "media" => $media - ); + ]; } /** @@ -997,12 +1003,6 @@ class Requirements_Backend $langDir = ModuleResourceLoader::singleton()->resolvePath($langDir); $files = array(); - $base = Director::baseFolder() . '/'; - - if (substr($langDir, -1) != '/') { - $langDir .= '/'; - } - $candidates = array( 'en.js', 'en_US.js', @@ -1012,19 +1012,21 @@ class Requirements_Backend i18n::get_locale() . '.js', ); foreach ($candidates as $candidate) { - if (file_exists($base . DIRECTORY_SEPARATOR . $langDir . $candidate)) { - $files[] = $langDir . $candidate; + $relativePath = Path::join($langDir, $candidate); + $absolutePath = Director::getAbsFile($relativePath); + if (file_exists($absolutePath)) { + $files[] = $relativePath; } } if ($return) { return $files; - } else { - foreach ($files as $file) { - $this->javascript($file); - } - return null; } + + foreach ($files as $file) { + $this->javascript($file); + } + return null; } /** @@ -1345,9 +1347,12 @@ MESSAGE function () use ($fileList, $minify, $type) { // Physically combine all file content $combinedData = ''; - $base = Director::baseFolder() . '/'; foreach ($fileList as $file) { - $fileContent = file_get_contents($base . $file); + $filePath = Director::getAbsFile($file); + if (!file_exists($filePath)) { + throw new InvalidArgumentException("Combined file {$file} does not exist"); + } + $fileContent = file_get_contents($filePath); // Use configured minifier if ($minify) { $fileContent = $this->minifier->minify($fileContent, $type, $file); @@ -1419,11 +1424,11 @@ MESSAGE protected function hashOfFiles($fileList) { // Get hash based on hash of each file - $base = Director::baseFolder() . '/'; $hash = ''; foreach ($fileList as $file) { - if (file_exists($base . $file)) { - $hash .= sha1_file($base . $file); + $absolutePath = Director::getAbsFile($file); + if (file_exists($absolutePath)) { + $hash .= sha1_file($absolutePath); } else { throw new InvalidArgumentException("Combined file {$file} does not exist"); } diff --git a/src/View/SSViewer.php b/src/View/SSViewer.php index 30a5664de..1ae35cbaf 100644 --- a/src/View/SSViewer.php +++ b/src/View/SSViewer.php @@ -49,6 +49,11 @@ class SSViewer implements Flushable */ const DEFAULT_THEME = '$default'; + /** + * Identifier for the public theme + */ + const PUBLIC_THEME = '$public'; + /** * A list (highest priority first) of themes to use * Only used when {@link $theme_enabled} is set to TRUE. @@ -267,7 +272,7 @@ class SSViewer implements Flushable */ public static function get_themes() { - $default = [self::DEFAULT_THEME]; + $default = [self::PUBLIC_THEME, self::DEFAULT_THEME]; if (!SSViewer::config()->uninherited('theme_enabled')) { return $default; @@ -284,7 +289,7 @@ class SSViewer implements Flushable // Support legacy behaviour if ($theme = SSViewer::config()->uninherited('theme')) { - return [$theme, self::DEFAULT_THEME]; + return [self::PUBLIC_THEME, $theme, self::DEFAULT_THEME]; } return $default; diff --git a/src/View/ThemeResourceLoader.php b/src/View/ThemeResourceLoader.php index ca06e2ba1..59b0d5e0c 100644 --- a/src/View/ThemeResourceLoader.php +++ b/src/View/ThemeResourceLoader.php @@ -4,6 +4,7 @@ namespace SilverStripe\View; use InvalidArgumentException; use SilverStripe\Core\Manifest\ModuleLoader; +use SilverStripe\Core\Path; /** * Handles finding templates from a stack of template manifest objects. @@ -103,12 +104,12 @@ class ThemeResourceLoader if (count($parts) > 1) { throw new InvalidArgumentException("Invalid theme identifier {$identifier}"); } - return ltrim($identifier, '/'); + return Path::normalise($identifier, true); } // If there is no slash / colon it's a legacy theme if ($slashPos === false && count($parts) === 1) { - return THEMES_DIR.'/'.$identifier; + return Path::join(THEMES_DIR, $identifier); } // Extract from /: format. @@ -148,7 +149,7 @@ class ThemeResourceLoader } // Join module with subpath - return ltrim($modulePath . $subpath, '/'); + return Path::normalise($modulePath . $subpath, true); } /** @@ -214,7 +215,7 @@ class ThemeResourceLoader foreach ($themePaths as $themePath) { // Join path $pathParts = [ $this->base, $themePath, 'templates', $head, $type, $tail ]; - $path = implode('/', array_filter($pathParts)) . '.ss'; + $path = Path::join($pathParts) . '.ss'; if (file_exists($path)) { return $path; } @@ -282,16 +283,13 @@ class ThemeResourceLoader */ public function findThemedResource($resource, $themes) { - if ($resource[0] !== '/') { - $resource = '/' . $resource; - } - $paths = $this->getThemePaths($themes); foreach ($paths as $themePath) { - $abspath = $this->base . '/' . $themePath; - if (file_exists($abspath . $resource)) { - return $themePath . $resource; + $relativePath = Path::join($themePath, $resource); + $absolutePath = Path::join($this->base, $relativePath); + if (file_exists($absolutePath)) { + return $relativePath; } } diff --git a/src/includes/constants.php b/src/includes/constants.php index a53ee8c0d..303835f43 100644 --- a/src/includes/constants.php +++ b/src/includes/constants.php @@ -1,5 +1,6 @@ set( + 'alternate_public_dir', + 'public' + ); } public function testAddMTime() @@ -30,7 +34,7 @@ class SimpleResourceURLGeneratorTest extends SapphireTest $generator = Injector::inst()->get(ResourceURLGenerator::class); $mtime = filemtime(__DIR__ .'/SimpleResourceURLGeneratorTest/_fakewebroot/basemodule/client/file.js'); $this->assertEquals( - '/basemodule/client/file.js?m='.$mtime, + '/resources/basemodule/client/file.js?m='.$mtime, $generator->urlForResource('basemodule/client/file.js') ); } @@ -43,11 +47,34 @@ class SimpleResourceURLGeneratorTest extends SapphireTest __DIR__ .'/SimpleResourceURLGeneratorTest/_fakewebroot/vendor/silverstripe/mymodule/client/style.css' ); $this->assertEquals( - '/resources/silverstripe/mymodule/client/style.css?m='.$mtime, + '/resources/vendor/silverstripe/mymodule/client/style.css?m='.$mtime, $generator->urlForResource('vendor/silverstripe/mymodule/client/style.css') ); } + public function testPublicDirResource() + { + /** @var SimpleResourceURLGenerator $generator */ + $generator = Injector::inst()->get(ResourceURLGenerator::class); + $mtime = filemtime( + __DIR__ .'/SimpleResourceURLGeneratorTest/_fakewebroot/public/basemodule/css/style.css' + ); + + $this->assertEquals( + '/basemodule/css/style.css?m='.$mtime, + $generator->urlForResource('public/basemodule/css/style.css') + ); + + $mtime = filemtime( + __DIR__ .'/SimpleResourceURLGeneratorTest/_fakewebroot/basemodule/client/file.js' + ); + + $this->assertEquals( + '/resources/basemodule/client/file.js?m='.$mtime, + $generator->urlForResource('basemodule/client/file.js') + ); + } + public function testModuleResource() { /** @var SimpleResourceURLGenerator $generator */ @@ -60,7 +87,7 @@ class SimpleResourceURLGeneratorTest extends SapphireTest __DIR__ .'/SimpleResourceURLGeneratorTest/_fakewebroot/vendor/silverstripe/mymodule/client/style.css' ); $this->assertEquals( - '/resources/silverstripe/mymodule/client/style.css?m='.$mtime, + '/resources/vendor/silverstripe/mymodule/client/style.css?m='.$mtime, $generator->urlForResource($module->getResource('client/style.css')) ); } diff --git a/tests/php/Control/SimpleResourceURLGeneratorTest/_fakewebroot/public/basemodule/css/style.css b/tests/php/Control/SimpleResourceURLGeneratorTest/_fakewebroot/public/basemodule/css/style.css new file mode 100644 index 000000000..45fc078e8 --- /dev/null +++ b/tests/php/Control/SimpleResourceURLGeneratorTest/_fakewebroot/public/basemodule/css/style.css @@ -0,0 +1,2 @@ +/* mymodule/style.css */ +body {} diff --git a/tests/php/Core/Manifest/ModuleResourceTest.php b/tests/php/Core/Manifest/ModuleResourceTest.php index 0fae87433..74b241a69 100644 --- a/tests/php/Core/Manifest/ModuleResourceTest.php +++ b/tests/php/Core/Manifest/ModuleResourceTest.php @@ -3,7 +3,6 @@ namespace SilverStripe\Core\Tests\Manifest; use SilverStripe\Control\Director; -use SilverStripe\Core\Manifest\ModuleLoader; use SilverStripe\Core\Manifest\ModuleManifest; use SilverStripe\Dev\SapphireTest; @@ -27,6 +26,7 @@ class ModuleResourceTest extends SapphireTest $this->manifest = new ModuleManifest($this->base); $this->manifest->init(); Director::config()->set('alternate_base_url', 'http://www.mysite.com/basefolder/'); + Director::config()->set('alternate_public_dir', 'public'); } public function testBaseModuleResource() @@ -42,7 +42,7 @@ class ModuleResourceTest extends SapphireTest $resource->getPath() ); $this->assertStringStartsWith( - '/basefolder/module/client/script.js?m=', + '/basefolder/resources/module/client/script.js?m=', $resource->getURL() ); } @@ -60,7 +60,7 @@ class ModuleResourceTest extends SapphireTest $resource->getPath() ); $this->assertStringStartsWith( - '/basefolder/resources/silverstripe/modulec/client/script.js?m=', + '/basefolder/resources/vendor/silverstripe/modulec/client/script.js?m=', $resource->getURL() ); } @@ -80,7 +80,7 @@ class ModuleResourceTest extends SapphireTest $resource->getPath() ); $this->assertStringStartsWith( - '/basefolder/resources/silverstripe/modulec/client/script.js?m=', + '/basefolder/resources/vendor/silverstripe/modulec/client/script.js?m=', $resource->getURL() ); } diff --git a/tests/php/Core/PathTest.php b/tests/php/Core/PathTest.php new file mode 100644 index 000000000..3808b7f91 --- /dev/null +++ b/tests/php/Core/PathTest.php @@ -0,0 +1,118 @@ +assertEquals($expected, $joined); + } + + /** + * List of tests for testJoinPaths + * + * @return array + */ + public function providerTestJoinPaths() + { + $tests = [ + // Single arg + [['/'], '/'], + [['\\'], '/'], + [['base'], 'base'], + [['c:/base\\'], 'c:/base'], + // Windows paths + [['c:/', 'bob'], 'c:/bob'], + [['c:/', '\\bob/'], 'c:/bob'], + [['c:\\basedir', '/bob\\'], 'c:/basedir/bob'], + // Empty-ish paths to clear out + [['/root/dir', '/', ' ', 'next/', '\\'], '/root/dir/next'], + [['/', '/', ' ', '/', '\\'], '/'], + [['/', '', '',], '/'], + [['/root', '/', ' ', '/', '\\'], '/root'], + [['', '/root', '/', ' ', '/', '\\'], '/root'], + [['', 'root', '/', ' ', '/', '\\'], 'root'], + [['\\', '', '/root', '/', ' ', '/', '\\'], '/root'], + // join blocks of paths + [['/root/dir', 'another/path\\to/join'], '/root/dir/another/path/to/join'], + ]; + + // Rewrite tests for other filesystems (output arg only) + if (DIRECTORY_SEPARATOR !== '/') { + foreach ($tests as $index => $test) { + $tests[$index][1] = str_replace('/', DIRECTORY_SEPARATOR, $tests[$index][1]); + } + } + return $tests; + } + + /** + * Test that joinPaths give the appropriate error + * + * @dataProvider providerTestJoinPathsErrors + * @param array $args Arguments to pass to Filesystem::joinPath() + * @param string $error Expected path + */ + public function testJoinPathsErrors($args, $error) + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage($error); + Path::join($args); + } + + public function providerTestJoinPathsErrors() + { + return [ + [['/base', '../passwd'], 'Can not collapse relative folders'], + [['/base/../', 'passwd/path'], 'Can not collapse relative folders'], + [['../', 'passwd/path'], 'Can not collapse relative folders'], + ]; + } + + /** + * @dataProvider providerTestNormalise + * @param string $input + * @param string $expected + */ + public function testNormalise($input, $expected) + { + $output = Path::normalise($input); + $this->assertEquals($expected, $output, "Expected $input to be normalised to $expected"); + } + + public function providerTestNormalise() + { + $tests = [ + // Windows paths + ["c:/bob", "c:/bob"], + ["c://bob", "c:/bob"], + ["/root/dir/", "/root/dir"], + ["/root\\dir\\\\sub/", "/root/dir/sub"], + [" /some/dir/ ", "/some/dir"], + ["", ""], + ["/", ""], + ["\\", ""], + ]; + + // Rewrite tests for other filesystems (output arg only) + if (DIRECTORY_SEPARATOR !== '/') { + foreach ($tests as $index => $test) { + $tests[$index][1] = str_replace('/', DIRECTORY_SEPARATOR, $tests[$index][1]); + } + } + return $tests; + } +} diff --git a/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php b/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php index 3b15ae1fb..9a12e1a64 100644 --- a/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php +++ b/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php @@ -140,7 +140,7 @@ class HTMLEditorFieldTest extends FunctionalTest = '/assets/HTMLEditorFieldTest/f5c7c2f814/example__ResizedImageWzEwLDIwXQ.jpg'; $this->assertEquals($neededFilename, (string)$xml[0]['src'], 'Correct URL of resized image is set.'); - $this->assertTrue(file_exists(BASE_PATH.DIRECTORY_SEPARATOR.$neededFilename), 'File for resized image exists'); + $this->assertTrue(file_exists(PUBLIC_PATH.DIRECTORY_SEPARATOR.$neededFilename), 'File for resized image exists'); $this->assertEquals(false, $obj->HasBrokenFile, 'Referenced image file exists.'); } diff --git a/tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest.php b/tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest.php index e1512b799..6cbbc916d 100644 --- a/tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest.php +++ b/tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest.php @@ -20,6 +20,7 @@ class TinyMCECombinedGeneratorTest extends SapphireTest // Set custom base_path for tinymce Director::config()->set('alternate_base_folder', __DIR__ . '/TinyMCECombinedGeneratorTest'); Director::config()->set('alternate_base_url', 'http://www.mysite.com/basedir/'); + Director::config()->set('alternate_public_dir', ''); // Disable public dir SSViewer::config()->set('themes', [SSViewer::DEFAULT_THEME]); TinyMCEConfig::config() ->set('base_dir', 'tinymce') diff --git a/tests/php/View/RequirementsTest.php b/tests/php/View/RequirementsTest.php index a620817fd..90405834e 100644 --- a/tests/php/View/RequirementsTest.php +++ b/tests/php/View/RequirementsTest.php @@ -3,6 +3,7 @@ namespace SilverStripe\View\Tests; use InvalidArgumentException; +use SilverStripe\Control\Director; use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\SapphireTest; use SilverStripe\View\Requirements; @@ -11,6 +12,8 @@ use SilverStripe\Assets\Tests\Storage\AssetStoreTest\TestAssetStore; use SilverStripe\View\Requirements_Backend; use SilverStripe\Core\Manifest\ResourceURLGenerator; use SilverStripe\Control\SimpleResourceURLGenerator; +use SilverStripe\View\SSViewer; +use SilverStripe\View\ThemeResourceLoader; /** * @todo Test that order of combine_files() is correct @@ -20,16 +23,28 @@ use SilverStripe\Control\SimpleResourceURLGenerator; class RequirementsTest extends SapphireTest { + /** + * @var ThemeResourceLoader + */ + protected $oldThemeResourceLoader = null; + static $html_template = ''; protected function setUp() { parent::setUp(); + Director::config()->set('alternate_base_folder', __DIR__ . '/SSViewerTest'); + Director::config()->set('alternate_base_url', 'http://www.mysite.com/basedir/'); + Director::config()->set('alternate_public_dir', 'public'); // Enforce public dir + // Add public as a theme in itself + SSViewer::set_themes([SSViewer::PUBLIC_THEME, SSViewer::DEFAULT_THEME]); TestAssetStore::activate('RequirementsTest'); // Set backend root to /RequirementsTest + $this->oldThemeResourceLoader = ThemeResourceLoader::inst(); } protected function tearDown() { + ThemeResourceLoader::set_instance($this->oldThemeResourceLoader); TestAssetStore::reset(); parent::tearDown(); } @@ -84,20 +99,23 @@ class RequirementsTest extends SapphireTest */ protected function setupCombinedRequirements($backend) { - $basePath = $this->getThemeRoot(); $this->setupRequirements($backend); // require files normally (e.g. called from a FormField instance) - $backend->javascript($basePath . '/javascript/RequirementsTest_a.js'); - $backend->javascript($basePath . '/javascript/RequirementsTest_b.js'); - $backend->javascript($basePath . '/javascript/RequirementsTest_c.js'); + $backend->javascript('javascript/RequirementsTest_a.js'); + $backend->javascript('javascript/RequirementsTest_b.js'); + $backend->javascript('javascript/RequirementsTest_c.js'); + + // Public resources may or may not be specified with `public/` prefix + $backend->javascript('javascript/RequirementsTest_d.js'); + $backend->javascript('public/javascript/RequirementsTest_e.js'); // require two of those files as combined includes $backend->combineFiles( 'RequirementsTest_bc.js', array( - $basePath . '/javascript/RequirementsTest_b.js', - $basePath . '/javascript/RequirementsTest_c.js' + 'javascript/RequirementsTest_b.js', + 'javascript/RequirementsTest_c.js' ) ); } @@ -109,15 +127,14 @@ class RequirementsTest extends SapphireTest */ protected function setupCombinedNonrequiredRequirements($backend) { - $basePath = $this->getThemeRoot(); $this->setupRequirements($backend); // require files as combined includes $backend->combineFiles( 'RequirementsTest_bc.js', array( - $basePath . '/javascript/RequirementsTest_b.js', - $basePath . '/javascript/RequirementsTest_c.js' + 'javascript/RequirementsTest_b.js', + 'javascript/RequirementsTest_c.js' ) ); } @@ -129,25 +146,24 @@ class RequirementsTest extends SapphireTest */ protected function setupCombinedRequirementsJavascriptAsyncDefer($backend, $async, $defer) { - $basePath = $this->getThemeRoot(); $this->setupRequirements($backend); // require files normally (e.g. called from a FormField instance) - $backend->javascript($basePath . '/javascript/RequirementsTest_a.js'); - $backend->javascript($basePath . '/javascript/RequirementsTest_b.js'); - $backend->javascript($basePath . '/javascript/RequirementsTest_c.js'); + $backend->javascript('javascript/RequirementsTest_a.js'); + $backend->javascript('javascript/RequirementsTest_b.js'); + $backend->javascript('javascript/RequirementsTest_c.js'); // require two of those files as combined includes $backend->combineFiles( 'RequirementsTest_bc.js', - array( - $basePath . '/javascript/RequirementsTest_b.js', - $basePath . '/javascript/RequirementsTest_c.js' - ), - array( + [ + 'javascript/RequirementsTest_b.js', + 'javascript/RequirementsTest_c.js' + ], + [ 'async' => $async, 'defer' => $defer, - ) + ] ); } @@ -155,17 +171,14 @@ class RequirementsTest extends SapphireTest { /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create(Requirements_Backend::class); - $basePath = $this->getThemeRoot(); $this->setupRequirements($backend); // require files normally (e.g. called from a FormField instance) $backend->javascript( - $basePath . '/javascript/RequirementsTest_a.js', - [ - 'type' => 'application/json' - ] + 'javascript/RequirementsTest_a.js', + [ 'type' => 'application/json' ] ); - $backend->javascript($basePath . '/javascript/RequirementsTest_b.js'); + $backend->javascript('javascript/RequirementsTest_b.js'); $result = $backend->includeInHTML(self::$html_template); $this->assertRegExp( '#