From e4d90506e96ad05299d19fcccc5171ef55aea1fb Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Tue, 27 May 2025 16:57:29 -0300 Subject: [PATCH 1/6] 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 866b44c3eac7d..bfd8c45f2ceef 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 284f4cb96f1fa..221b7b51e2fd9 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 0fe9c0d11d6f8b01f1194d2e59baa204f203a84b Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Tue, 27 May 2025 17:02:18 -0300 Subject: [PATCH 2/6] 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 7b677249fb41b..7e7ad8541c3d7 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 97c2addadcf6b98ec62e30f48b5d7826f3719a26 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Wed, 28 May 2025 12:12:39 -0300 Subject: [PATCH 3/6] [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 7e7ad8541c3d7..b6dea04513aab 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}; From 251542b8fb5ae91046022310df54e63efa96094a Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Fri, 13 Jun 2025 15:02:49 -0300 Subject: [PATCH 4/6] Un-deprecate simple add/replace header API, use it Suggestion from Jakub. --- ext/session/session.c | 7 +++---- main/SAPI.h | 1 - 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/ext/session/session.c b/ext/session/session.c index b6dea04513aab..98ab802014aa2 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -1351,7 +1351,6 @@ static void php_session_remove_cookie(void) { 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; @@ -1417,9 +1416,9 @@ static zend_result php_session_send_cookie(void) smart_str_0(&ncookie); php_session_remove_cookie(); /* remove already sent session ID cookie */ - header_line.line = ZSTR_VAL(ncookie.s); - header_line.line_len = ZSTR_LEN(ncookie.s); - sapi_header_op(SAPI_HEADER_ADD, &header_line); + /* '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); smart_str_free(&ncookie); return SUCCESS; diff --git a/main/SAPI.h b/main/SAPI.h index 221b7b51e2fd9..f85b4da60e526 100644 --- a/main/SAPI.h +++ b/main/SAPI.h @@ -200,7 +200,6 @@ typedef enum { /* Parameter: */ BEGIN_EXTERN_C() SAPI_API int sapi_header_op(sapi_header_op_enum op, void *arg); -/* Deprecated functions. Use sapi_header_op instead. */ SAPI_API int sapi_add_header_ex(const char *header_line, size_t header_line_len, bool duplicate, bool replace); #define sapi_add_header(a, b, c) sapi_add_header_ex((a),(b),(c),1) From 2c0fb8934eb01ce3d502602325cb95e128974694 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Fri, 13 Jun 2025 15:46:42 -0300 Subject: [PATCH 5/6] Restore the optimization removing session cookies had I don't think this needs to be special cased with the parameter. --- main/SAPI.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/main/SAPI.c b/main/SAPI.c index bfd8c45f2ceef..80349c4c4ce8e 100644 --- a/main/SAPI.c +++ b/main/SAPI.c @@ -597,17 +597,23 @@ 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, bool check_separator) +static void sapi_remove_header(zend_llist *l, char *name, size_t len) { sapi_header_struct *header; zend_llist_element *next; zend_llist_element *current=l->head; + size_t header_len = len; + const char *colon = strchr(name, ':'); + if (colon) { + header_len = (size_t)(colon - name); + } + while (current) { header = (sapi_header_struct *)(current->data); next = current->next; - if (header->header_len > len - && (header->header[len] == ':' || !check_separator) + if (header->header_len > header_len + && (header->header[header_len] == ':' || len > header_len) && !strncasecmp(header->header, name, len)) { if (current->prev) { current->prev->next = next; @@ -655,7 +661,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), true); + sapi_remove_header(&SG(sapi_headers).headers, sapi_header->header, strlen(sapi_header->header)); *colon_offset = sav; } } @@ -736,7 +742,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, op == SAPI_HEADER_DELETE); + sapi_remove_header(&SG(sapi_headers).headers, header_line, header_line_len); efree(header_line); return SUCCESS; } else { From 8cd5992c99316de88c244041e226df7c004f1043 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Wed, 2 Jul 2025 15:47:15 -0300 Subject: [PATCH 6/6] Move setting header length to caller Suggestion from Jakub. --- ext/session/session.c | 1 + main/SAPI.c | 22 +++++++++++----------- main/SAPI.h | 5 ++++- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/ext/session/session.c b/ext/session/session.c index 98ab802014aa2..5cc5d963059d0 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -1344,6 +1344,7 @@ static void php_session_remove_cookie(void) { header_line.line = session_cookie; header_line.line_len = strlen(session_cookie); + header_line.header_len = sizeof("Set-Cookie") - 1; sapi_header_op(SAPI_HEADER_DELETE_PREFIX, &header_line); efree(session_cookie); diff --git a/main/SAPI.c b/main/SAPI.c index 80349c4c4ce8e..169ae572fa967 100644 --- a/main/SAPI.c +++ b/main/SAPI.c @@ -597,18 +597,12 @@ 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, size_t header_len) { sapi_header_struct *header; zend_llist_element *next; zend_llist_element *current=l->head; - size_t header_len = len; - const char *colon = strchr(name, ':'); - if (colon) { - header_len = (size_t)(colon - name); - } - while (current) { header = (sapi_header_struct *)(current->data); next = current->next; @@ -661,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), 0); *colon_offset = sav; } } @@ -676,7 +670,7 @@ SAPI_API int sapi_header_op(sapi_header_op_enum op, void *arg) sapi_header_struct sapi_header; char *colon_offset; char *header_line; - size_t header_line_len; + size_t header_line_len, header_len; int http_response_code; if (SG(headers_sent) && !SG(request_info).no_headers) { @@ -708,7 +702,13 @@ SAPI_API int sapi_header_op(sapi_header_op_enum op, void *arg) } header_line = estrndup(p->line, p->line_len); header_line_len = p->line_len; - http_response_code = p->response_code; + if (op == SAPI_HEADER_DELETE_PREFIX) { + header_len = p->header_len; + http_response_code = 0; + } else { + header_len = 0; + http_response_code = p->response_code; + } break; } @@ -742,7 +742,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, header_len); efree(header_line); return SUCCESS; } else { diff --git a/main/SAPI.h b/main/SAPI.h index f85b4da60e526..a5e2d70df7209 100644 --- a/main/SAPI.h +++ b/main/SAPI.h @@ -185,7 +185,10 @@ END_EXTERN_C() typedef struct { const char *line; /* If you allocated this, you need to free it yourself */ size_t line_len; - zend_long response_code; /* long due to zend_parse_parameters compatibility */ + union { + zend_long response_code; /* long due to zend_parse_parameters compatibility */ + size_t header_len; /* the "Key" in "Key: Value", for optimization */ + }; } sapi_header_line; typedef enum { /* Parameter: */