Fix for issue 14265

[ruudboon]

Fixed issue #14265, added test and added method to get Path.

[codecov]

Codecov Report

Merging #14279 into 4.0.x will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##            4.0.x   #14279      +/-   ##
==========================================
- Coverage    66.4%   66.39%   -0.01%     
==========================================
  Files         489      489              
  Lines      116432   116428       -4     
==========================================
- Hits        77312    77308       -4     
  Misses      39120    39120

Impacted Files	Coverage Δ
ext/phalcon/session/adapter/stream.zep.c	83.16% <0%> (-0.34%)	⬇️
ext/phalcon/session/manager.zep.c	95.3% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13a1fa2...0df7f93. Read the comment docs.

[sergeyklay]

@ruudboon I would like suggest something like:

diff --git a/phalcon/Session/Adapter/Stream.zep b/phalcon/Session/Adapter/Stream.zep
index cb6d649f7..9f13990ea 100644
--- a/phalcon/Session/Adapter/Stream.zep
+++ b/phalcon/Session/Adapter/Stream.zep
@@ -54,10 +54,13 @@ class Stream extends Noop
          */
         if !fetch path, options["savePath"] {
             let path = ini_get("session.save_path");
+            if empty path {
+                let path = sys_get_temp_dir();
+            }
         }
 
         if unlikely !is_writable(path) {
-            throw new Exception("The save_path [" . path . "]is not writeable");
+            throw new Exception("The session save path [" . path . "] is not writeable");
         }
 
         let this->path = Str::dirSeparator(path);
@@ -69,7 +72,9 @@ class Stream extends Noop
 
         let file = this->path . this->getPrefixedName(id);
 
-        if file_exists(file) && is_file(file) {
+        // is_file() will return FALSE if the given path points to
+        // a directory or there is no file.
+        if is_file(file) {
             unlink(file);
         }
 
@@ -84,9 +89,9 @@ class Stream extends Noop
             time    = time() - maxlifetime;
 
         for file in glob(pattern) {
-            if file_exists(file) &&
-               is_file(file)     &&
-               (filemtime(file) < time) {
+            // is_file() will return FALSE if the given path points to
+            // a directory or there is no file.
+            if is_file(file) && (filemtime(file) < time) {
                 unlink(file);
             }
         }
@@ -96,33 +101,28 @@ class Stream extends Noop
 
     public function open(var savePath, var sessionName) -> bool
     {
-        var path;
-
-        if true !== ends_with(savePath, "/") {
-            let path = savePath . "/";
+        if unlikely !empty savePath {
+            if ends_with(savePath, "/") {
+                let this->path = savePath;
+            } else {
+                let this->path = savePath . "/";
+            }
         }
 
-        let this->path = path;
-
         return true;
     }
 
     public function read(var id) -> string
     {
-        var data, name;
+        var name;
 
-        let name = this->path . this->getPrefixedName(id),
-            data = "";
+        let name = this->path . this->getPrefixedName(id);
 
         if file_exists(name) {
-            let data = file_get_contents(name);
-
-            if false === data {
-                return "";
-            }
+            return (string) file_get_contents(name);
         }
 
-        return data;
+        return "";
     }
 
     public function write(var id, var data) -> bool

Also I'm not sure about getPath. Do you need it only for testing purposes? Is it possible to achieve the same result using reflection?

cphalcon/tests/_support/Helper/Unit.php

Lines 103 to 112 in a0b25ed

	public function getProtectedProperty($obj, $prop)
	{
	$reflection = new ReflectionClass($obj);
	$property = $reflection->getProperty($prop);
	$property->setAccessible(true);
	return $property->getValue($obj);
	}

[CameronHall]

@ruudboon can you please try rebasing instead of merging?