Browse Source

Removed creation of unneeded file handle object in FileEngine::write(). Added configuration option 'mask' to FileEngine::. Now able to provide a permission mask to create cache files with specific permissions.
Set '0664' as default value for the 'mask' setting in FileEngine::. Adjusted corresponding test cases.
Added new warning if the file could not be opened for writing.

Thomas Ploch 14 years ago
parent
commit
0a70963a74

+ 20 - 14
lib/Cake/Cache/Engine/FileEngine.php

@@ -67,7 +67,7 @@ class FileEngine extends CacheEngine {
 		parent::init(array_merge(
 			array(
 				'engine' => 'File', 'path' => CACHE, 'prefix'=> 'cake_', 'lock'=> true,
-				'serialize'=> true, 'isWindows' => false
+				'serialize'=> true, 'isWindows' => false, 'mask' => 0664
 			),
 			$settings
 		));
@@ -124,21 +124,16 @@ class FileEngine extends CacheEngine {
 		$expires = time() + $duration;
 		$contents = $expires . $lineBreak . $data . $lineBreak;
 
-		if (!$handle = fopen($this->_File->getPathName(), 'c')) {
-		    return false;
-		}
-
 		if ($this->settings['lock']) {
-		    flock($handle, LOCK_EX);
+		    $this->_File->flock(LOCK_EX);
 		}
 
-		$success = ftruncate($handle, 0) && fwrite($handle, $contents) && fflush($handle);
+		$success = $this->_File->ftruncate(0) && $this->_File->fwrite($contents) && $this->_File->fflush();
 
 		if ($this->settings['lock']) {
-		    flock($handle, LOCK_UN);
+		    $this->_File->flock(LOCK_UN);
 		}
 
-		fclose($handle);
 		return $success;
 	}
 
@@ -276,7 +271,8 @@ class FileEngine extends CacheEngine {
 	}
 
 /**
- * Sets the current cache key this class is managing
+ * Sets the current cache key this class is managing, and creates a writable SplFileObject
+ * for the cache file the key is refering to.
  *
  * @param string $key The key
  * @param boolean $createKey Whether the key should be created if it doesn't exists, or not
@@ -288,12 +284,22 @@ class FileEngine extends CacheEngine {
 		if (!$createKey && !$path->isFile()) {
 			return false;
 		}
-		$old = umask(0);
 		if (empty($this->_File) || $this->_File->getBaseName() !== $key) {
-			$this->_File = $path->openFile('a+');
-		}
-		umask($old);
+			$exists = file_exists($path->getPathname());
+			try {
+				$this->_File = $path->openFile('c+');
+			} catch (Exception $e) {
+				trigger_error(__d('cake_dev', $e->getMessage()), E_USER_WARNING);
+				return false;
+			}
+			unset($path);
 
+			if (!$exists && !chmod($this->_File->getPathname(), (int) $this->settings['mask'])) {
+				trigger_error(__d(
+					'cake_dev', 'Could not apply permission mask "%s" on cache file "%s"',
+					array($this->_File->getPathname(), $this->settings['mask'])), E_USER_WARNING);
+			}
+		}
 		return true;
 	}
 

+ 2 - 1
lib/Cake/Test/Case/Cache/CacheTest.php

@@ -202,7 +202,8 @@ class CacheTest extends CakeTestCase {
 			'duration' => 3600,
 			'probability' => 100,
 			'engine' => 'File',
-			'isWindows' => DIRECTORY_SEPARATOR == '\\'
+			'isWindows' => DIRECTORY_SEPARATOR == '\\',
+			'mask' => 0664
 		);
 		$this->assertEqual($expected, Cache::settings('sessions'));
 

+ 42 - 0
lib/Cake/Test/Case/Cache/Engine/FileEngineTest.php

@@ -312,6 +312,8 @@ class FileEngineTest extends CakeTestCase {
 		$this->assertIdentical(Cache::read('App.doubleQuoteTest', 'file_test'), '"this is a quoted string"');
 		Cache::write('App.singleQuoteTest', "'this is a quoted string'", 'file_test');
 		$this->assertIdentical(Cache::read('App.singleQuoteTest', 'file_test'), "'this is a quoted string'");
+		Cache::delete('App.singleQuoteTest', 'file_test');
+		Cache::delete('App.doubleQuoteTest', 'file_test');
 	}
 
 /**
@@ -330,4 +332,44 @@ class FileEngineTest extends CakeTestCase {
 
 		Cache::drop('failure');
 	}
+
+/**
+ * Testing the mask setting in FileEngine
+ * 
+ * @return void
+ */
+	public function testMaskSetting() {
+		Cache::config('mask_test', array('engine' => 'File', 'path' => TMP . 'tests'));
+		$data = 'This is some test content';
+		$write = Cache::write('masking_test', $data, 'mask_test');
+		$result = substr(sprintf('%o',fileperms(TMP . 'tests' . DS .'cake_masking_test')), -4);
+		$expected = '0664';
+		$this->assertEqual($result, $expected);
+		Cache::delete('masking_test', 'mask_test');
+		Cache::drop('mask_test');
+
+		Cache::config('mask_test', array('engine' => 'File', 'mask' => 0666, 'path' => TMP . 'tests'));
+		$write = Cache::write('masking_test', $data, 'mask_test');
+		$result = substr(sprintf('%o',fileperms(TMP . 'tests' . DS .'cake_masking_test')), -4);
+		$expected = '0666';
+		$this->assertEqual($result, $expected);
+		Cache::delete('masking_test', 'mask_test');
+		Cache::drop('mask_test');
+
+		Cache::config('mask_test', array('engine' => 'File', 'mask' => 0644, 'path' => TMP . 'tests'));
+		$write = Cache::write('masking_test', $data, 'mask_test');
+		$result = substr(sprintf('%o',fileperms(TMP . 'tests' . DS .'cake_masking_test')), -4);
+		$expected = '0644';
+		$this->assertEqual($result, $expected);
+		Cache::delete('masking_test', 'mask_test');
+		Cache::drop('mask_test');
+
+		Cache::config('mask_test', array('engine' => 'File', 'mask' => 0640, 'path' => TMP . 'tests'));
+		$write = Cache::write('masking_test', $data, 'mask_test');
+		$result = substr(sprintf('%o',fileperms(TMP . 'tests' . DS .'cake_masking_test')), -4);
+		$expected = '0640';
+		$this->assertEqual($result, $expected);
+		Cache::delete('masking_test', 'mask_test');
+		Cache::drop('mask_test');
+	}
 }