From e7c3bf7e29ab2a9fb84a00b403ab3fd28cb84d58 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Tue, 27 May 2025 16:57:29 -0300 Subject: [PATCH 1/8] 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 afc1c24c8fd3ecac3fc539af9236e1dab5973b16 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Tue, 27 May 2025 17:02:18 -0300 Subject: [PATCH 2/8] 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 | 40 +++++++++------------------------------- 1 file changed, 9 insertions(+), 31 deletions(-) diff --git a/ext/session/session.c b/ext/session/session.c index edcfe9c433607..e78491791aaed 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -1341,44 +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(ZSTR_VAL(PS(session_name)), SESSION_FORBIDDEN_CHARS) == NULL); session_cookie_len = spprintf(&session_cookie, 0, "Set-Cookie: %s=", ZSTR_VAL(PS(session_name))); - 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; @@ -1444,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 c636d667b4e4a048599e39e213726a86480c0582 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Wed, 28 May 2025 12:12:39 -0300 Subject: [PATCH 3/8] [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 e78491791aaed..0a6af9baccb7d 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 8a2713fcbc800a770403f445b11292ac31e0de41 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Fri, 13 Jun 2025 15:02:49 -0300 Subject: [PATCH 4/8] 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 0a6af9baccb7d..bbd3d862c5b0c 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 f263bf162621954098da780b1f1f1bf7f84dc075 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Fri, 13 Jun 2025 15:46:42 -0300 Subject: [PATCH 5/8] 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 284132ab09015e0fbef3e228100a97b1883b6364 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Wed, 2 Jul 2025 15:47:15 -0300 Subject: [PATCH 6/8] 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 bbd3d862c5b0c..dc8e35a8ed5c4 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: */ From 926fa17a654541f69d65c2eb1075cfba6078e8ff Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Mon, 21 Jul 2025 14:39:52 -0300 Subject: [PATCH 7/8] [ci skip] adjust tab count It may be better to use spaces in here instead. --- main/SAPI.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main/SAPI.h b/main/SAPI.h index a5e2d70df7209..f7a64f243104e 100644 --- a/main/SAPI.h +++ b/main/SAPI.h @@ -195,7 +195,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_PREFIX, /* sapi_header_line* */ SAPI_HEADER_DELETE_ALL, /* void */ SAPI_HEADER_SET_STATUS /* int */ } sapi_header_op_enum; From e2b3ce79ff523914f6f774bf65a2588ee93b2a7f Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Mon, 21 Jul 2025 14:54:12 -0300 Subject: [PATCH 8/8] Use session_cookie_len rather than calling strlen --- ext/session/session.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/session/session.c b/ext/session/session.c index dc8e35a8ed5c4..d466db2932ec3 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -1337,13 +1337,14 @@ static int php_session_cache_limiter(void) static void php_session_remove_cookie(void) { char *session_cookie; + size_t session_cookie_len; sapi_header_line header_line = {0}; ZEND_ASSERT(strpbrk(ZSTR_VAL(PS(session_name)), SESSION_FORBIDDEN_CHARS) == NULL); session_cookie_len = spprintf(&session_cookie, 0, "Set-Cookie: %s=", ZSTR_VAL(PS(session_name))); header_line.line = session_cookie; - header_line.line_len = strlen(session_cookie); + header_line.line_len = session_cookie_len; header_line.header_len = sizeof("Set-Cookie") - 1; sapi_header_op(SAPI_HEADER_DELETE_PREFIX, &header_line);