Merge remote-tracking branch 'origin/1.x'

* origin/1.x: Fixed cache permissions, resolves #275
goaop · Apr 26, 2016 · fcdefc2 · fcdefc2
2 parents 16f5bfb + 3079dca
commit fcdefc2
Show file tree

Hide file tree

Showing 8 changed files with 63 additions and 33 deletions.
diff --git a/src/Core/AspectKernel.php b/src/Core/AspectKernel.php
@@ -179,7 +179,7 @@ protected function getDefaultOptions()
             'debug'                  => false,
             'appDir'                 => __DIR__ . '/../../../../../',
             'cacheDir'               => null,
-            'cacheFileMode'          => null,
+            'cacheFileMode'          => 0770 & ~umask(), // Respect user umask() policy
             'features'               => 0,
 
             'includePaths'           => [],
@@ -206,8 +206,9 @@ protected function normalizeOptions(array $options)
 
         $options['appDir']   = PathResolver::realpath($options['appDir']);
         $options['cacheDir'] = PathResolver::realpath($options['cacheDir']);
-        $options['includePaths'] = PathResolver::realpath($options['includePaths']);
-        $options['excludePaths'] = PathResolver::realpath($options['excludePaths']);
+        $options['cacheFileMode'] = (int) $options['cacheFileMode'];
+        $options['includePaths']  = PathResolver::realpath($options['includePaths']);
+        $options['excludePaths']  = PathResolver::realpath($options['excludePaths']);
 
         return $options;
     }

diff --git a/src/Core/CachedAspectLoader.php b/src/Core/CachedAspectLoader.php
@@ -30,6 +30,13 @@ class CachedAspectLoader extends AspectLoader
      */
     protected $cacheDir;
 
+    /**
+     * File mode for the cache files
+     *
+     * @var integer
+     */
+    protected $cacheFileMode;
+
     /**
      * Identifier of original loader
      *
@@ -46,9 +53,10 @@ class CachedAspectLoader extends AspectLoader
      */
     public function __construct(AspectContainer $container, $loaderId, array $options = [])
     {
-        $this->cacheDir  = isset($options['cacheDir']) ? $options['cacheDir'] : null;
-        $this->loaderId  = $loaderId;
-        $this->container = $container;
+        $this->cacheDir      = isset($options['cacheDir']) ? $options['cacheDir'] : null;
+        $this->cacheFileMode = $options['cacheFileMode'];
+        $this->loaderId      = $loaderId;
+        $this->container     = $container;
     }
 
     /**
@@ -115,10 +123,13 @@ protected function loadFromCache($fileName)
      */
     protected function saveToCache($items, $fileName)
     {
-        $content = serialize($items);
-        if (!is_dir(dirname($fileName))) {
-            mkdir(dirname($fileName));
+        $content       = serialize($items);
+        $directoryName = dirname($fileName);
+        if (!is_dir($directoryName)) {
+            mkdir($directoryName, $this->cacheFileMode, true);
         }
-        file_put_contents($fileName, $content);
+        file_put_contents($fileName, $content, LOCK_EX);
+        // For cache files we don't want executable bits by default
+        chmod($fileName, $this->cacheFileMode & (~0111));
     }
 }
diff --git a/src/Core/GoAspectContainer.php b/src/Core/GoAspectContainer.php
@@ -90,7 +90,8 @@ public function __construct()
                 $reader = new FileCacheReader(
                     $reader,
                     $options['cacheDir'] . DIRECTORY_SEPARATOR . '_annotations' . DIRECTORY_SEPARATOR,
-                    $options['debug']
+                    $options['debug'],
+                    0777 & (~$options['cacheFileMode'])
                 );
             }
 

diff --git a/src/Instrument/ClassLoading/CachePathManager.php b/src/Instrument/ClassLoading/CachePathManager.php
@@ -38,6 +38,13 @@ class CachePathManager
      */
     protected $cacheDir;
 
+    /**
+     * File mode
+     *
+     * @var integer
+     */
+    protected $fileMode;
+
     /**
      * @var string|null
      */
@@ -61,8 +68,9 @@ public function __construct(AspectKernel $kernel)
     {
         $this->kernel   = $kernel;
         $this->options  = $kernel->getOptions();
-        $this->cacheDir = $this->options['cacheDir'];
         $this->appDir   = $this->options['appDir'];
+        $this->cacheDir = $this->options['cacheDir'];
+        $this->fileMode = $this->options['cacheFileMode'];
 
         if ($this->cacheDir) {
             if (!is_dir($this->cacheDir)) {
@@ -72,7 +80,7 @@ public function __construct(AspectKernel $kernel)
                         "Can not create a directory {$this->cacheDir} for the cache.
                         Parent directory {$cacheRootDir} is not writable or not exist.");
                 }
-                mkdir($this->cacheDir, 0770);
+                mkdir($this->cacheDir, $this->fileMode, true);
             }
             if (!$this->kernel->hasFeature(Features::PREBUILT_CACHE) && !is_writable($this->cacheDir)) {
                 throw new \InvalidArgumentException("Cache directory {$this->cacheDir} is not writable");
@@ -178,9 +186,13 @@ public function flushCacheState()
                 '\'' . $cachePath => 'AOP_CACHE_DIR . \'',
                 '\'' . $rootPath  => 'AOP_ROOT_DIR . \''
             ));
-            file_put_contents($this->cacheDir . self::CACHE_FILE_NAME, $cacheData);
+            $fullCacheFileName = $this->cacheDir . self::CACHE_FILE_NAME;
+            file_put_contents($fullCacheFileName, $cacheData, LOCK_EX);
+            // For cache files we don't want executable bits by default
+            chmod($fullCacheFileName, $this->fileMode & (~0111));
+
             if (function_exists('opcache_invalidate')) {
-                opcache_invalidate($this->cacheDir . self::CACHE_FILE_NAME, true);
+                opcache_invalidate($fullCacheFileName, true);
             }
             $this->cacheState    = $this->newCacheState + $this->cacheState;
             $this->newCacheState = [];

diff --git a/src/Instrument/Transformer/CachingTransformer.php b/src/Instrument/Transformer/CachingTransformer.php
@@ -47,7 +47,7 @@ public function __construct(AspectKernel $kernel, $transformers, CachePathManage
     {
         parent::__construct($kernel);
         $this->cacheManager  = $cacheManager;
-        $this->cacheFileMode = (int) $this->options['cacheFileMode'];
+        $this->cacheFileMode = $this->options['cacheFileMode'];
         $this->transformers  = $transformers;
     }
 
@@ -84,12 +84,11 @@ public function transform(StreamMetaData $metadata)
             if ($wasProcessed) {
                 $parentCacheDir = dirname($cacheUri);
                 if (!is_dir($parentCacheDir)) {
-                    mkdir($parentCacheDir, 0770, true);
-                }
-                file_put_contents($cacheUri, $metadata->source);
-                if (!$cacheState && $this->cacheFileMode) {
-                    chmod($cacheUri, $this->cacheFileMode);
+                    mkdir($parentCacheDir, $this->cacheFileMode, true);
                 }
+                file_put_contents($cacheUri, $metadata->source, LOCK_EX);
+                // For cache files we don't want executable bits by default
+                chmod($cacheUri, $this->cacheFileMode & (~0111));
             }
             $this->cacheManager->setCacheState($originalUri, array(
                 'filemtime' => isset($_SERVER['REQUEST_TIME']) ? $_SERVER['REQUEST_TIME'] : time(),

diff --git a/src/Instrument/Transformer/WeavingTransformer.php b/src/Instrument/Transformer/WeavingTransformer.php
@@ -224,10 +224,12 @@ private function processFunctions(array $advisors, StreamMetaData $metadata, $na
             if (!file_exists($functionFileName) || !$this->container->isFresh(filemtime($functionFileName))) {
                 $dirname = dirname($functionFileName);
                 if (!file_exists($dirname)) {
-                    mkdir($dirname, 0770, true);
+                    mkdir($dirname, $this->options['cacheFileMode'], true);
                 }
                 $source = new FunctionProxy($namespace, $functionAdvices);
-                file_put_contents($functionFileName, $source);
+                file_put_contents($functionFileName, $source, LOCK_EX);
+                // For cache files we don't want executable bits by default
+                chmod($functionFileName, $this->options['cacheFileMode'] & (~0111));
             }
             $content = 'include_once AOP_CACHE_DIR . ' . var_export($cacheDirSuffix . $fileName, true) . ';' . PHP_EOL;
             $metadata->source .= $content;
@@ -261,7 +263,7 @@ private function saveProxyToCache($class, $child)
         $proxyFileName = $cacheDir . $fileName;
         $dirname       = dirname($proxyFileName);
         if (!file_exists($dirname)) {
-            mkdir($dirname, 0770, true);
+            mkdir($dirname, $this->options['cacheFileMode'], true);
         }
 
         $body      = '<?php' . PHP_EOL;
@@ -276,7 +278,9 @@ private function saveProxyToCache($class, $child)
         }
 
         $body .= $child;
-        file_put_contents($proxyFileName, $body);
+        file_put_contents($proxyFileName, $body, LOCK_EX);
+        // For cache files we don't want executable bits by default
+        chmod($proxyFileName, $this->options['cacheFileMode'] & (~0111));
 
         return 'include_once AOP_CACHE_DIR . ' . var_export($cacheDirSuffix . $fileName, true) . ';' . PHP_EOL;
     }

diff --git a/tests/Go/Instrument/Transformer/FilterInjectorTransformerTest.php b/tests/Go/Instrument/Transformer/FilterInjectorTransformerTest.php
@@ -28,10 +28,11 @@ public function setUp()
         if (!self::$transformer) {
             $kernelMock = $this->getKernelMock(
                 array(
-                    'cacheDir' => null,
-                    'appDir'   => '',
-                    'debug'    => false,
-                    'features' => 0
+                    'cacheDir'      => null,
+                    'cacheFileMode' => 0770,
+                    'appDir'        => '',
+                    'debug'         => false,
+                    'features'      => 0
                 ),
                 $this->getMock(GoAspectContainer::class)
             );

diff --git a/tests/Go/Instrument/Transformer/WeavingTransformerTest.php b/tests/Go/Instrument/Transformer/WeavingTransformerTest.php
@@ -38,10 +38,11 @@ public function setUp()
         $this->adviceMatcher = $this->getAdviceMatcherMock();
         $this->kernel        = $this->getKernelMock(
             array(
-                'appDir'       => dirname(__DIR__),
-                'cacheDir'     => null,
-                'includePaths' => [],
-                'excludePaths' => []
+                'appDir'        => dirname(__DIR__),
+                'cacheDir'      => null,
+                'cacheFileMode' => 0770,
+                'includePaths'  => [],
+                'excludePaths'  => []
             ),
             $container
         );