From 4ec7c3c77bea56a215425c01072a058d144d91fd Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Tue, 27 May 2025 16:57:29 -0300 Subject: [PATCH 1/3] Add SAPI_HEADER_DELETE_PREFIX operation The session ext currently munges into the linked list of headers itself, because the delete header API is given the key for headers to delete. The session ext wants to use a prefix past the colon separator, for i.e. "Set-Cookie: PHPSESSID=", to eliminate only the specific cookie rather than all cookies. This changes the SAPI code to add a new header op to take a prefix instead. Call sites are yet unchanged. Also fix some whitespace. --- main/SAPI.c | 15 +++++++++------ main/SAPI.h | 1 + 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/main/SAPI.c b/main/SAPI.c index 866b44c3eac7..bfd8c45f2cee 100644 --- a/main/SAPI.c +++ b/main/SAPI.c @@ -597,7 +597,8 @@ static void sapi_update_response_code(int ncode) * since zend_llist_del_element only removes one matched item once, * we should remove them manually */ -static void sapi_remove_header(zend_llist *l, char *name, size_t len) { +static void sapi_remove_header(zend_llist *l, char *name, size_t len, bool check_separator) +{ sapi_header_struct *header; zend_llist_element *next; zend_llist_element *current=l->head; @@ -605,7 +606,8 @@ static void sapi_remove_header(zend_llist *l, char *name, size_t len) { while (current) { header = (sapi_header_struct *)(current->data); next = current->next; - if (header->header_len > len && header->header[len] == ':' + if (header->header_len > len + && (header->header[len] == ':' || !check_separator) && !strncasecmp(header->header, name, len)) { if (current->prev) { current->prev->next = next; @@ -653,7 +655,7 @@ static void sapi_header_add_op(sapi_header_op_enum op, sapi_header_struct *sapi_ char sav = *colon_offset; *colon_offset = 0; - sapi_remove_header(&SG(sapi_headers).headers, sapi_header->header, strlen(sapi_header->header)); + sapi_remove_header(&SG(sapi_headers).headers, sapi_header->header, strlen(sapi_header->header), true); *colon_offset = sav; } } @@ -691,6 +693,7 @@ SAPI_API int sapi_header_op(sapi_header_op_enum op, void *arg) case SAPI_HEADER_ADD: case SAPI_HEADER_REPLACE: + case SAPI_HEADER_DELETE_PREFIX: case SAPI_HEADER_DELETE: { sapi_header_line *p = arg; @@ -722,8 +725,8 @@ SAPI_API int sapi_header_op(sapi_header_op_enum op, void *arg) header_line[header_line_len]='\0'; } - if (op == SAPI_HEADER_DELETE) { - if (strchr(header_line, ':')) { + if (op == SAPI_HEADER_DELETE || op == SAPI_HEADER_DELETE_PREFIX) { + if (op == SAPI_HEADER_DELETE && strchr(header_line, ':')) { efree(header_line); sapi_module.sapi_error(E_WARNING, "Header to delete may not contain colon."); return FAILURE; @@ -733,7 +736,7 @@ SAPI_API int sapi_header_op(sapi_header_op_enum op, void *arg) sapi_header.header_len = header_line_len; sapi_module.header_handler(&sapi_header, op, &SG(sapi_headers)); } - sapi_remove_header(&SG(sapi_headers).headers, header_line, header_line_len); + sapi_remove_header(&SG(sapi_headers).headers, header_line, header_line_len, op == SAPI_HEADER_DELETE); efree(header_line); return SUCCESS; } else { diff --git a/main/SAPI.h b/main/SAPI.h index 284f4cb96f1f..221b7b51e2fd 100644 --- a/main/SAPI.h +++ b/main/SAPI.h @@ -192,6 +192,7 @@ typedef enum { /* Parameter: */ SAPI_HEADER_REPLACE, /* sapi_header_line* */ SAPI_HEADER_ADD, /* sapi_header_line* */ SAPI_HEADER_DELETE, /* sapi_header_line* */ + SAPI_HEADER_DELETE_PREFIX, /* sapi_header_line* */ SAPI_HEADER_DELETE_ALL, /* void */ SAPI_HEADER_SET_STATUS /* int */ } sapi_header_op_enum; From 1999dfc116cc978e27e5356b67cca0eb9d358163 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Tue, 27 May 2025 17:02:18 -0300 Subject: [PATCH 2/3] Simplify cookie setting code in ext/session Use the modern SAPI header ops API, including the remove prefix op we just added. --- ext/session/session.c | 41 +++++++++-------------------------------- 1 file changed, 9 insertions(+), 32 deletions(-) diff --git a/ext/session/session.c b/ext/session/session.c index 7b677249fb41..7e7ad8541c3d 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -1341,45 +1341,22 @@ static int php_session_cache_limiter(void) * removes all of matching cookie. i.e. It deletes all of Set-Cookie headers. */ static void php_session_remove_cookie(void) { - sapi_header_struct *header; - zend_llist *l = &SG(sapi_headers).headers; - zend_llist_element *next; - zend_llist_element *current; char *session_cookie; - size_t session_cookie_len; - size_t len = sizeof("Set-Cookie")-1; + sapi_header_line header_line = {0}; ZEND_ASSERT(strpbrk(PS(session_name), SESSION_FORBIDDEN_CHARS) == NULL); spprintf(&session_cookie, 0, "Set-Cookie: %s=", PS(session_name)); - session_cookie_len = strlen(session_cookie); - current = l->head; - while (current) { - header = (sapi_header_struct *)(current->data); - next = current->next; - if (header->header_len > len && header->header[len] == ':' - && !strncmp(header->header, session_cookie, session_cookie_len)) { - if (current->prev) { - current->prev->next = next; - } else { - l->head = next; - } - if (next) { - next->prev = current->prev; - } else { - l->tail = current->prev; - } - sapi_free_header(header); - efree(current); - --l->count; - } - current = next; - } + header_line.line = session_cookie; + header_line.line_len = strlen(session_cookie); + sapi_header_op(SAPI_HEADER_DELETE_PREFIX, &header_line); + efree(session_cookie); } static zend_result php_session_send_cookie(void) { + sapi_header_line header_line = {0}; smart_str ncookie = {0}; zend_string *date_fmt = NULL; zend_string *e_id; @@ -1445,9 +1422,9 @@ static zend_result php_session_send_cookie(void) smart_str_0(&ncookie); php_session_remove_cookie(); /* remove already sent session ID cookie */ - /* 'replace' must be 0 here, else a previous Set-Cookie - header, probably sent with setcookie() will be replaced! */ - sapi_add_header_ex(estrndup(ZSTR_VAL(ncookie.s), ZSTR_LEN(ncookie.s)), ZSTR_LEN(ncookie.s), 0, 0); + header_line.line = ZSTR_VAL(ncookie.s); + header_line.line_len = ZSTR_LEN(ncookie.s); + sapi_header_op(SAPI_HEADER_ADD, &header_line); smart_str_free(&ncookie); return SUCCESS; From 0ef7bffa525d51ed8fd1dabc812fa14ad93c073b Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Wed, 28 May 2025 12:12:39 -0300 Subject: [PATCH 3/3] [ci skip] Remove redundant and unnecessary comment The purpose of this is clear, and after refactoring, the special case is no longer there, so it has no value. --- ext/session/session.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/ext/session/session.c b/ext/session/session.c index 7e7ad8541c3d..b6dea04513aa 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -1335,11 +1335,6 @@ static int php_session_cache_limiter(void) * Cookie Management * ********************* */ -/* - * Remove already sent session ID cookie. - * It must be directly removed from SG(sapi_header) because sapi_add_header_ex() - * removes all of matching cookie. i.e. It deletes all of Set-Cookie headers. - */ static void php_session_remove_cookie(void) { char *session_cookie; sapi_header_line header_line = {0};