diff --git a/ext/session/mod_files.c b/ext/session/mod_files.c index 60c6bfc1bd4c7..6f0a3c5bde761 100644 --- a/ext/session/mod_files.c +++ b/ext/session/mod_files.c @@ -276,7 +276,7 @@ static zend_result ps_files_write(ps_files *data, zend_string *key, zend_string return SUCCESS; } -static int ps_files_cleanup_dir(const zend_string *dirname, zend_long maxlifetime) +static int ps_files_cleanup_dir(const zend_string *dirname, zend_long maxlifetime, size_t remaining_depth) { DIR *dir; struct dirent *entry; @@ -291,44 +291,54 @@ static int ps_files_cleanup_dir(const zend_string *dirname, zend_long maxlifetim return -1; } - time(&now); - if (ZSTR_LEN(dirname) >= MAXPATHLEN) { php_error_docref(NULL, E_NOTICE, "ps_files_cleanup_dir: dirname(%s) is too long", ZSTR_VAL(dirname)); closedir(dir); return -1; } - /* Prepare buffer (dirname never changes) */ memcpy(buf, ZSTR_VAL(dirname), ZSTR_LEN(dirname)); buf[ZSTR_LEN(dirname)] = PHP_DIR_SEPARATOR; + if (remaining_depth == 0) { + time(&now); + } + while ((entry = readdir(dir))) { - /* does the file start with our prefix? */ - if (!strncmp(entry->d_name, FILE_PREFIX, sizeof(FILE_PREFIX) - 1)) { - size_t entry_len = strlen(entry->d_name); - - /* does it fit into our buffer? */ - if (entry_len + ZSTR_LEN(dirname) + 2 < MAXPATHLEN) { - /* create the full path.. */ - memcpy(buf + ZSTR_LEN(dirname) + 1, entry->d_name, entry_len); - - /* NUL terminate it and */ - buf[ZSTR_LEN(dirname) + entry_len + 1] = '\0'; - - /* check whether its last access was more than maxlifetime ago */ - if (VCWD_STAT(buf, &sbuf) == 0 && - (now - sbuf.st_mtime) > maxlifetime) { - VCWD_UNLINK(buf); - nrdels++; - } + if (entry->d_name[0] == '.' && + (entry->d_name[1] == '\0' || + (entry->d_name[1] == '.' && entry->d_name[2] == '\0'))) { + continue; + } + if (remaining_depth == 0 && strncmp(entry->d_name, FILE_PREFIX, sizeof(FILE_PREFIX) - 1) != 0) { + continue; + } + size_t entry_len = strlen(entry->d_name); + if (ZSTR_LEN(dirname) + 1 + entry_len >= MAXPATHLEN) { + continue; + } + memcpy(buf + ZSTR_LEN(dirname) + 1, entry->d_name, entry_len); + buf[ZSTR_LEN(dirname) + 1 + entry_len] = '\0'; + if (VCWD_STAT(buf, &sbuf) != 0) { + continue; + } + if (remaining_depth == 0) { + if ((now - sbuf.st_mtime) > maxlifetime) { + VCWD_UNLINK(buf); + nrdels++; + } + } else if (S_ISDIR(sbuf.st_mode)) { + zend_string *subdir = zend_string_init(buf, ZSTR_LEN(dirname) + 1 + entry_len, 0); + int n = ps_files_cleanup_dir(subdir, maxlifetime, remaining_depth - 1); + zend_string_release(subdir); + if (n >= 0) { + nrdels += n; } } } closedir(dir); - - return (nrdels); + return nrdels; } static zend_result ps_files_key_exists(ps_files *data, const zend_string *key) @@ -624,15 +634,7 @@ PS_GC_FUNC(files) { PS_FILES_DATA; - /* We don't perform any cleanup, if dirdepth is larger than 0. - we return SUCCESS, since all cleanup should be handled by - an external entity (i.e. find -ctime x | xargs rm) */ - - if (data->dirdepth == 0) { - *nrdels = ps_files_cleanup_dir(data->basedir, maxlifetime); - } else { - *nrdels = -1; // Cannot process multiple depth save dir - } + *nrdels = ps_files_cleanup_dir(data->basedir, maxlifetime, data->dirdepth); return *nrdels; } diff --git a/ext/session/tests/mod_files/gc_dirdepth2.phpt b/ext/session/tests/mod_files/gc_dirdepth2.phpt new file mode 100644 index 0000000000000..a8724285125a4 --- /dev/null +++ b/ext/session/tests/mod_files/gc_dirdepth2.phpt @@ -0,0 +1,45 @@ +--TEST-- +session GC cleans expired sessions with save_path dirdepth=2 (two subdir levels) +--EXTENSIONS-- +session +--SKIPIF-- + +--INI-- +session.gc_probability=0 +session.gc_maxlifetime=10 +--FILE-- + +--CLEAN-- + +--EXPECT-- +session_gc() return value: int(1) +expired file removed: bool(true) diff --git a/ext/session/tests/mod_files/gc_dirdepth_disabled.phpt b/ext/session/tests/mod_files/gc_dirdepth_disabled.phpt new file mode 100644 index 0000000000000..81c62430d157a --- /dev/null +++ b/ext/session/tests/mod_files/gc_dirdepth_disabled.phpt @@ -0,0 +1,68 @@ +--TEST-- +session GC correctly cleans expired sessions when save_path dirdepth > 0 +--EXTENSIONS-- +session +--SKIPIF-- + +--INI-- +session.gc_probability=0 +session.gc_maxlifetime=1 +--FILE-- + +--CLEAN-- + +--EXPECT-- +dirdepth=1 — session_gc() return value: int(1) +dirdepth=1 — expired session file removed: bool(true) +dirdepth=0 — session_gc() return value: int(1) +dirdepth=0 — expired session file removed: bool(true) diff --git a/ext/session/tests/mod_files/gc_dirdepth_multi_subdir_count.phpt b/ext/session/tests/mod_files/gc_dirdepth_multi_subdir_count.phpt new file mode 100644 index 0000000000000..1ba047502f5ea --- /dev/null +++ b/ext/session/tests/mod_files/gc_dirdepth_multi_subdir_count.phpt @@ -0,0 +1,54 @@ +--TEST-- +session GC accumulates correct total count across multiple subdirs, including empty ones (dirdepth=1) +--EXTENSIONS-- +session +--SKIPIF-- + +--INI-- +session.gc_probability=0 +session.gc_maxlifetime=10 +--FILE-- + +--CLEAN-- + +--EXPECT-- +session_gc() return value: int(3) +all expired files removed: bool(true) diff --git a/ext/session/tests/mod_files/gc_dirdepth_selective.phpt b/ext/session/tests/mod_files/gc_dirdepth_selective.phpt new file mode 100644 index 0000000000000..a173324171e27 --- /dev/null +++ b/ext/session/tests/mod_files/gc_dirdepth_selective.phpt @@ -0,0 +1,57 @@ +--TEST-- +session GC deletes only expired sess_* files and leaves all other files untouched (dirdepth=1) +--EXTENSIONS-- +session +--SKIPIF-- + +--INI-- +session.gc_probability=0 +session.gc_maxlifetime=10 +--FILE-- + gc_maxlifetime=10 → deleted + +file_put_contents($fresh, 'user|s:5:"alice";'); +touch($fresh, time() - 1); // 1 s old < gc_maxlifetime=10 → kept + +file_put_contents($other, 'untouched'); +touch($other, time() - 100); // old but no sess_ prefix → kept + +session_id('a0000000000000000000000000'); // first char 'a' → $base/a/ +session_start(); +$result = session_gc(); // int(1): exactly one deletion proves selectivity +session_destroy(); + +echo "session_gc() return value: "; +var_dump($result); + +echo "expired sess_ file removed: "; +var_dump(!file_exists($expired)); + +echo "other file kept: "; +var_dump(file_exists($other)); +?> +--CLEAN-- + +--EXPECT-- +session_gc() return value: int(1) +expired sess_ file removed: bool(true) +other file kept: bool(true) diff --git a/php.ini-development b/php.ini-development index 6f93f440112ea..ee75459ea56c3 100644 --- a/php.ini-development +++ b/php.ini-development @@ -1386,13 +1386,6 @@ session.gc_divisor = 1000 ; https://php.net/session.gc-maxlifetime session.gc_maxlifetime = 1440 -; NOTE: If you are using the subdirectory option for storing session files -; (see session.save_path above), then garbage collection does *not* -; happen automatically. You will need to do your own garbage -; collection through a shell script, cron entry, or some other method. -; For example, the following script is the equivalent of setting -; session.gc_maxlifetime to 1440 (1440 seconds = 24 minutes): -; find /path/to/sessions -cmin +24 -type f | xargs rm ; Check HTTP Referer to invalidate externally stored URLs containing ids. ; HTTP_REFERER has to contain this substring for the session to be diff --git a/php.ini-production b/php.ini-production index 9aafad21e9c74..b10e2ba9944ae 100644 --- a/php.ini-production +++ b/php.ini-production @@ -1388,13 +1388,6 @@ session.gc_divisor = 1000 ; https://php.net/session.gc-maxlifetime session.gc_maxlifetime = 1440 -; NOTE: If you are using the subdirectory option for storing session files -; (see session.save_path above), then garbage collection does *not* -; happen automatically. You will need to do your own garbage -; collection through a shell script, cron entry, or some other method. -; For example, the following script is the equivalent of setting -; session.gc_maxlifetime to 1440 (1440 seconds = 24 minutes): -; find /path/to/sessions -cmin +24 -type f | xargs rm ; Check HTTP Referer to invalidate externally stored URLs containing ids. ; HTTP_REFERER has to contain this substring for the session to be