ext/session: add recursive session cleanup for dirname > 0#21491
ext/session: add recursive session cleanup for dirname > 0#21491jorgsowa wants to merge 3 commits intophp:masterfrom
Conversation
Girgias
left a comment
Girgias
left a comment
There was a problem hiding this comment.
I don't really understand why we need to loops that are basically identical? Also I'd prefer if those new GC tests are in a subfolder (maybe even a mod_files subfolder)
|
| 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; |
There was a problem hiding this comment.
Can you please add comments to explain what you're doing.
The previous comments existed for a reason.
3 participants
When
dirnameis bigger than 0, there is no automatic cleanup of old sessions. I think it's oversight and shouldn't work like this. If the performance is the issue then lower GC probability should be set instead ignoring cleanup.This is not security issue, because documentation clearly state the current behavior, but still should work differently from the first place.