Sanitise filenames for error pages

fixes #299
silverstripe · Sep 7, 2017 · 2da5828 · 2da5828
1 parent adfa725
commit 2da5828
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 11 deletions.
diff --git a/code/extensions/ErrorPageSubsite.php b/code/extensions/ErrorPageSubsite.php
@@ -2,6 +2,7 @@
 
 namespace SilverStripe\Subsites\Extensions;
 
+use SilverStripe\Assets\FileNameFilter;
 use SilverStripe\CMS\Model\SiteTree;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\ORM\DataExtension;
@@ -52,11 +53,13 @@ public function updateErrorFilename(&$name, &$statusCode)
             && $locale
             && $locale != Translatable::default_locale()
         ) {
-            $filepath = $static_filepath . "/error-{$statusCode}-{$locale}{$subdomainPart}.html";
+            $fileName = "error-{$statusCode}-{$locale}{$subdomainPart}.html";
         } else {
-            $filepath = $static_filepath . "/error-{$statusCode}{$subdomainPart}.html";
+            $fileName=  "error-{$statusCode}{$subdomainPart}.html";
         }
 
-        $name = $filepath;
+        $fileName = FileNameFilter::create()->filter($fileName);
+
+        $name = implode('/', [$static_filepath, $fileName]);
     }
 }
diff --git a/tests/php/FileSubsitesTest.php b/tests/php/FileSubsitesTest.php
@@ -94,6 +94,7 @@ public function testSubsitesFolderDropdown()
             'Test 3',
             'Test Non-SSL',
             'Test SSL',
+            'Test Vagrant VM on port 8080'
         ], array_values($source));
     }
 }
diff --git a/tests/php/SiteTreeSubsitesTest.php b/tests/php/SiteTreeSubsitesTest.php
@@ -3,13 +3,14 @@
 namespace SilverStripe\Subsites\Tests;
 
 use Page;
+use SilverStripe\Assets\FileNameFilter;
 use SilverStripe\CMS\Controllers\CMSMain;
 use SilverStripe\CMS\Controllers\ModelAsController;
-use SilverStripe\ErrorPage\ErrorPage;
 use SilverStripe\CMS\Model\SiteTree;
 use SilverStripe\Control\Director;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Convert;
+use SilverStripe\ErrorPage\ErrorPage;
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Security\Member;
 use SilverStripe\SiteConfig\SiteConfig;
@@ -80,15 +81,27 @@ public function testBasicSanity()
         $this->assertTrue(is_array(singleton(SiteTreeSubsites::class)->extraStatics()));
     }
 
-    public function testErrorPageLocations()
+    public function errorPageLocationsProvider()
     {
-        $subsite1 = $this->objFromFixture(Subsite::class, 'domaintest1');
+        return [
+            ['domaintest1', '/error-500-one.example.org.html'],
+            ['domaintestVagrant', '/error-500-localhost8080.html']
+        ];
+    }
 
-        Subsite::changeSubsite($subsite1->ID);
+    /**
+     * @dataProvider errorPageLocationsProvider
+     */
+    public function testErrorPageLocations($subsiteFixtureName, $expectedFilename)
+    {
+        $static_path = Config::inst()->get(ErrorPage::class, 'static_filepath');
+
+        $subsite = $this->objFromFixture(Subsite::class, $subsiteFixtureName);
+        $expected_path = $static_path . $expectedFilename;
+
+        Subsite::changeSubsite($subsite->ID);
         $path = TestErrorPage::get_error_filename_spy(500);
 
-        $static_path = Config::inst()->get(ErrorPage::class, 'static_filepath');
-        $expected_path = $static_path . '/error-500-' . $subsite1->domain() . '.html';
         $this->assertEquals($expected_path, $path);
     }
 

diff --git a/tests/php/SubsiteTest.php b/tests/php/SubsiteTest.php
@@ -351,7 +351,8 @@ public function testAllSites()
             ['Title' => 'Test 2'],
             ['Title' => 'Test 3'],
             ['Title' => 'Test Non-SSL'],
-            ['Title' => 'Test SSL']
+            ['Title' => 'Test SSL'],
+            ['Title' => 'Test Vagrant VM on port 8080']
         ], $subsites, 'Lists all subsites');
     }
 
@@ -396,7 +397,8 @@ public function testAccessibleSites()
             'Test 2',
             'Test 3',
             'Test Non-SSL',
-            'Test SSL'
+            'Test SSL',
+            'Test Vagrant VM on port 8080'
         ], array_values($adminSiteTitles));
 
         $member2Sites = Subsite::accessible_sites(

diff --git a/tests/php/SubsiteTest.yml b/tests/php/SubsiteTest.yml
@@ -16,6 +16,8 @@ SilverStripe\Subsites\Model\Subsite:
     Title: 'Test SSL'
   domaintest5:
     Title: 'Test Non-SSL'
+  domaintestVagrant:
+    Title: 'Test Vagrant VM on port 8080'
 SilverStripe\Subsites\Model\SubsiteDomain:
   subsite1:
     SubsiteID: =>SilverStripe\Subsites\Model\Subsite.subsite1
@@ -61,6 +63,11 @@ SilverStripe\Subsites\Model\SubsiteDomain:
     Domain: www.tertiary.com
     Protocol: http
     IsPrimary: 1
+  dtVagrant:
+     SubsiteID: =>SilverStripe\Subsites\Model\Subsite.domaintestVagrant
+     Domain: localhost:8080
+     Protocol: http
+     IsPrimary: 1
 Page:
    mainSubsitePage:
       Title: 'MainSubsitePage'