From 7a86e943b63be7f56f1bfd256a5d40200bc1a990 Mon Sep 17 00:00:00 2001 From: Christopher Joe Date: Wed, 18 Oct 2017 13:10:46 +1300 Subject: [PATCH] Feature add a default server config behaviour BUG adjusted file permissions to be slightly more lenient BUG adjusted setting visibility only when it is needed BUG Fix issue when running with ErrorPage module installed --- src/Flysystem/AssetAdapter.php | 24 +++++++++-- src/Shortcodes/FileShortcodeProvider.php | 9 +++-- src/Shortcodes/ImageShortcodeProvider.php | 9 +++-- tests/php/Flysystem/AssetAdapterTest.php | 49 +++++------------------ 4 files changed, 40 insertions(+), 51 deletions(-) diff --git a/src/Flysystem/AssetAdapter.php b/src/Flysystem/AssetAdapter.php index f8c530a9..5af0c213 100644 --- a/src/Flysystem/AssetAdapter.php +++ b/src/Flysystem/AssetAdapter.php @@ -28,6 +28,14 @@ class AssetAdapter extends Local */ private static $server_configuration = array(); + /** + * Default server configuration to use if the server type defined by the environment is not found + * + * @config + * @var string + */ + private static $default_server = 'apache'; + /** * Config compatible permissions configuration * @@ -127,13 +135,18 @@ protected function configureServer($forceOverwrite = false) // Determine configurations to write $rules = $this->config()->get('server_configuration'); if (empty($rules[$type])) { - return; + $type = $this->config()->get('default_server'); + if (!$type || empty($rules[$type])) { + return; + } } $configurations = $rules[$type]; + $visibility = 'public'; + // Apply each configuration $config = new FlysystemConfig(); - $config->set('visibility', 'private'); + $config->set('visibility', $visibility); foreach ($configurations as $file => $template) { // Ensure file contents if ($forceOverwrite || !$this->has($file)) { @@ -144,8 +157,11 @@ protected function configureServer($forceOverwrite = false) throw new Exception("Error writing server configuration file \"{$file}\""); } } - // Ensure correct permissions - $this->setVisibility($file, 'private'); + $perms = $this->getVisibility($file); + if ($perms['visibility'] !== $visibility) { + // Ensure correct permissions + $this->setVisibility($file, $visibility); + } } } diff --git a/src/Shortcodes/FileShortcodeProvider.php b/src/Shortcodes/FileShortcodeProvider.php index de7662be..d8c7bb75 100644 --- a/src/Shortcodes/FileShortcodeProvider.php +++ b/src/Shortcodes/FileShortcodeProvider.php @@ -68,7 +68,9 @@ public static function handle_shortcode($arguments, $content, $parser, $shortcod if ($item) { /** @var AssetStore $store */ $store = Injector::inst()->get(AssetStore::class); - $store->grant($item['filename'], $item['hash']); + if (!empty($item['filename'])) { + $store->grant($item['filename'], $item['hash']); + } return $item['markup']; } @@ -100,11 +102,10 @@ public static function handle_shortcode($arguments, $content, $parser, $shortcod // cache it for future reference $cache->set($cacheKey, [ 'markup' => $markup, - 'filename' => $record->getFilename(), - 'hash' => $record->getHash(), + 'filename' => $record instanceof File ? $record->getFilename() : null, + 'hash' => $record instanceof File ? $record->getHash() : null, ]); - return $markup; } diff --git a/src/Shortcodes/ImageShortcodeProvider.php b/src/Shortcodes/ImageShortcodeProvider.php index d164042f..70592b62 100644 --- a/src/Shortcodes/ImageShortcodeProvider.php +++ b/src/Shortcodes/ImageShortcodeProvider.php @@ -3,6 +3,7 @@ namespace SilverStripe\Assets\Shortcodes; use Psr\SimpleCache\CacheInterface; +use SilverStripe\Assets\File; use SilverStripe\Assets\Image; use SilverStripe\Assets\Storage\AssetStore; use SilverStripe\Core\Convert; @@ -50,7 +51,9 @@ public static function handle_shortcode($args, $content, $parser, $shortcode, $e if ($item) { /** @var AssetStore $store */ $store = Injector::inst()->get(AssetStore::class); - $store->grant($item['filename'], $item['hash']); + if (!empty($item['filename'])) { + $store->grant($item['filename'], $item['hash']); + } return $item['markup']; } @@ -98,8 +101,8 @@ public static function handle_shortcode($args, $content, $parser, $shortcode, $e // cache it for future reference $cache->set($cacheKey, [ 'markup' => $markup, - 'filename' => $record->getFilename(), - 'hash' => $record->getHash(), + 'filename' => $record instanceof File ? $record->getFilename() : null, + 'hash' => $record instanceof File ? $record->getHash() : null, ]); return $markup; diff --git a/tests/php/Flysystem/AssetAdapterTest.php b/tests/php/Flysystem/AssetAdapterTest.php index 2d2007b3..f9dc58ae 100644 --- a/tests/php/Flysystem/AssetAdapterTest.php +++ b/tests/php/Flysystem/AssetAdapterTest.php @@ -98,16 +98,17 @@ public function testPermissions() } $_SERVER['SERVER_SOFTWARE'] = 'Apache/2.2.22 (Win64) PHP/5.3.13'; - // Public asset adapter writes .htaccess with private perms + // Public asset adapter writes .htaccess with public perms $adapter = new PublicAssetAdapter($this->rootDir); $adapter->flush(); $this->assertFileExists($this->rootDir . '/.htaccess'); $publicPerm = fileperms($this->rootDir . '/.htaccess'); - // No public read / write + + // Public read $this->assertEquals( - 0000, - $publicPerm & 0066, - $this->readablePerm($publicPerm) . ' has no public read / write' + 0044, + $publicPerm & 0044, + $this->readablePerm($publicPerm) . ' has public read' ); // Same as protected adapter @@ -115,43 +116,11 @@ public function testPermissions() $adapter->flush(); $this->assertFileExists($this->rootDir . '/.protected/.htaccess'); $protectedPerm = fileperms($this->rootDir . '/.protected/.htaccess'); - // No public read / write - $this->assertEquals( - 0000, - $protectedPerm & 0066, - $this->readablePerm($protectedPerm) . ' has no public read / write' - ); - - // Test more permissive check - AssetAdapter::config()->merge('file_permissions', [ - 'file' => [ - 'private' => 0644, - ] - ]); - $adapter = new ProtectedAssetAdapter($this->rootDir . '/.protected'); - $adapter->flush(); - $protectedPerm = fileperms($this->rootDir . '/.protected/.htaccess'); - // Public read, no public write + // Public read $this->assertEquals( 0044, - $protectedPerm & 0066, - $this->readablePerm($protectedPerm) . ' has public read only' - ); - - // Test more permissive check (public write) - AssetAdapter::config()->merge('file_permissions', [ - 'file' => [ - 'private' => 0666, - ] - ]); - $adapter = new ProtectedAssetAdapter($this->rootDir . '/.protected'); - $adapter->flush(); - $protectedPerm = fileperms($this->rootDir . '/.protected/.htaccess'); - // Public read / write - $this->assertEquals( - 0066, - $protectedPerm & 0066, - $this->readablePerm($protectedPerm) . ' has public read / write' + $protectedPerm & 0044, + $this->readablePerm($protectedPerm) . ' has public read' ); }