refix #69111 and one related test

It is the least evil as the test just reduces the fail path. 5.6 seems
still broken in this regard, a backport should follow if travis is happy.

diff --git a/ext/session/mod_files.c b/ext/session/mod_files.c
index de6cccce4ea38f6922f33a8f5208a4e0c82b0b09..b37b212e20262f4a752d898149696d1a318899f6 100644 (file)
--- a/ext/session/mod_files.c
+++ b/ext/session/mod_files.c
@@ -113,6 +113,10 @@ static char *ps_files_path_create(char *buf, size_t buflen, ps_files *data, cons
        int i;
        size_t n;
 
+       if (!data) {
+               return NULL;
+       }
+
        key_len = strlen(key);
        if (key_len <= data->dirdepth ||
                buflen < (data->basedir_len + 2 * data->dirdepth + key_len + 5 + sizeof(FILE_PREFIX))) {
@@ -153,7 +157,7 @@ static void ps_files_close(ps_files *data)
        }
 }
 
-static void ps_files_open(ps_files *data, const char *key)
+static int ps_files_open(ps_files *data, const char *key)
 {
        char buf[MAXPATHLEN];
 #if !defined(O_NOFOLLOW) || !defined(PHP_WIN32)
@@ -170,18 +174,12 @@ static void ps_files_open(ps_files *data, const char *key)
                ps_files_close(data);
 
                if (php_session_valid_key(key) == FAILURE) {
-                       if (data->basedir) {
-                               efree(data->basedir);
-                               data->basedir = NULL;
-                               data->basedir_len = 0;
-                       }
-                       efree(data);
                        php_error_docref(NULL, E_WARNING, "The session id is too long or contains illegal characters, valid characters are a-z, A-Z, 0-9 and '-,'");
-                       return;
+                       return FAILURE;
                }
 
                if (!ps_files_path_create(buf, sizeof(buf), data, key)) {
-                       return;
+                       return FAILURE;
                }
 
                data->lastkey = estrdup(key);
@@ -193,7 +191,7 @@ static void ps_files_open(ps_files *data, const char *key)
                /* Check to make sure that the opened file is not outside of allowable dirs.
                   This is not 100% safe but it's hard to do something better without O_NOFOLLOW */
                if(PG(open_basedir) && lstat(buf, &sbuf) == 0 && S_ISLNK(sbuf.st_mode) && php_check_open_basedir(buf)) {
-                       return;
+                       return FAILURE;
                }
                data->fd = VCWD_OPEN_MODE(buf, O_CREAT | O_RDWR | O_BINARY, data->filemode);
 #endif
@@ -205,7 +203,7 @@ static void ps_files_open(ps_files *data, const char *key)
                        if (fstat(data->fd, &sbuf) || (sbuf.st_uid != 0 && sbuf.st_uid != getuid() && sbuf.st_uid != geteuid())) {
                                close(data->fd);
                                data->fd = -1;
-                               return;
+                               return FAILURE;
                        }
 #endif
                        do {
@@ -222,8 +220,11 @@ static void ps_files_open(ps_files *data, const char *key)
 #endif
                } else {
                        php_error_docref(NULL, E_WARNING, "open(%s, O_RDWR) failed: %s (%d)", buf, strerror(errno), errno);
+                       return FAILURE;
                }
        }
+
+       return SUCCESS;
 }
 
 static int ps_files_write(ps_files *data, zend_string *key, zend_string *val)
@@ -233,7 +234,9 @@ static int ps_files_write(ps_files *data, zend_string *key, zend_string *val)
        /* PS(id) may be changed by calling session_regenerate_id().
           Re-initialization should be tried here. ps_files_open() checks
        data->lastkey and reopen when it is needed. */
-       ps_files_open(data, ZSTR_VAL(key));
+       if (FAILURE == ps_files_open(data, ZSTR_VAL(key))) {
+               return FAILURE;
+       }
        if (data->fd < 0) {
                return FAILURE;
        }
@@ -471,7 +474,18 @@ PS_READ_FUNC(files)
        zend_stat_t sbuf;
        PS_FILES_DATA;
 
-       ps_files_open(data, ZSTR_VAL(key));
+       if (FAILURE == ps_files_open(data, ZSTR_VAL(key))) {
+               if (data->basedir) {
+                       efree(data->basedir);
+                       data->basedir = NULL;
+                       data->basedir_len = 0;
+               }
+               efree(data);
+               PS_SET_MOD_DATA(NULL);
+
+               return FAILURE;
+       }
+
        if (data->fd < 0) {
                return FAILURE;
        }
@@ -542,7 +556,18 @@ PS_WRITE_FUNC(files)
 {
        PS_FILES_DATA;
 
-       return ps_files_write(data, key, val);
+       if (FAILURE == ps_files_write(data, key, val)) {
+               if (data->basedir) {
+                       efree(data->basedir);
+                       data->basedir = NULL;
+                       data->basedir_len = 0;
+               }
+               efree(data);
+               PS_SET_MOD_DATA(NULL);
+               return FAILURE;
+       }
+
+       return SUCCESS;
 }
 
 
@@ -581,7 +606,16 @@ PS_UPDATE_TIMESTAMP_FUNC(files)
        ret = VCWD_UTIME(buf, newtime);
        if (ret == -1) {
                /* New session ID, create data file */
-               return ps_files_write(data, key, val);
+               if (FAILURE == ps_files_write(data, key, val)) {
+                       if (data->basedir) {
+                               efree(data->basedir);
+                               data->basedir = NULL;
+                               data->basedir_len = 0;
+                       }
+                       efree(data);
+                       PS_SET_MOD_DATA(NULL);
+                       return FAILURE;
+               }
        }
 
        return SUCCESS;
@@ -603,11 +637,11 @@ PS_DESTROY_FUNC(files)
        char buf[MAXPATHLEN];
        PS_FILES_DATA;
 
-       if (!ps_files_path_create(buf, sizeof(buf), data, ZSTR_VAL(key))) {
+       if (data && !ps_files_path_create(buf, sizeof(buf), data, ZSTR_VAL(key))) {
                return FAILURE;
        }
 
-       if (data->fd != -1) {
+       if (data && data->fd != -1) {
                ps_files_close(data);
 
                if (VCWD_UNLINK(buf) == -1) {

diff --git a/ext/session/tests/rfc1867_sid_invalid.phpt b/ext/session/tests/rfc1867_sid_invalid.phpt
index 4dd8f1f9799236a7a9932e77f7f982fde2753a34..c4c6f26449af6431e6b3f7f48b645bba364bb1e1 100644 (file)
--- a/ext/session/tests/rfc1867_sid_invalid.phpt
+++ b/ext/session/tests/rfc1867_sid_invalid.phpt
@@ -47,14 +47,10 @@ session_destroy();
 --EXPECTF--
 Warning: Unknown: The session id is too long or contains illegal characters, valid characters are a-z, A-Z, 0-9 and '-,' in Unknown on line 0
 
-Warning: Unknown: The session id is too long or contains illegal characters, valid characters are a-z, A-Z, 0-9 and '-,' in Unknown on line 0
-
 Warning: Unknown: Failed to write session data (files). Please verify that the current setting of session.save_path is correct () in Unknown on line 0
 
 Warning: Unknown: The session id is too long or contains illegal characters, valid characters are a-z, A-Z, 0-9 and '-,' in Unknown on line 0
 
-Warning: Unknown: The session id is too long or contains illegal characters, valid characters are a-z, A-Z, 0-9 and '-,' in Unknown on line 0
-
 Warning: Unknown: Failed to write session data (files). Please verify that the current setting of session.save_path is correct () in Unknown on line 0
 string(%d) "%s"
 bool(true)