Add SAPI_HEADER_DELETE_PREFIX, make ext/session use it (php#18678)

* 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.

* Simplify cookie setting code in ext/session

Use the modern SAPI header ops API, including the remove prefix op we
just added.

* [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.

* Un-deprecate simple add/replace header API, use it

Suggestion from Jakub.

* Restore the optimization removing session cookies had

I don't think this needs to be special cased with the parameter.

* Move setting header length to caller

Suggestion from Jakub.

* [ci skip] adjust tab count

It may be better to use spaces in here instead.

* Use session_cookie_len rather than calling strlen

Author: NattyNarwhal

ext/session/session.c

@@ -1335,45 +1335,19 @@ 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) {
-	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 = session_cookie_len;
+	header_line.header_len = sizeof("Set-Cookie") - 1;
+	sapi_header_op(SAPI_HEADER_DELETE_PREFIX, &header_line);
+
 	efree(session_cookie);
 }
 

main/SAPI.c

@@ -597,15 +597,17 @@ 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;
 
 	while (current) {
 		header = (sapi_header_struct *)(current->data);
 		next = current->next;
-		if (header->header_len > len && header->header[len] == ':'
+		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;
@@ -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), 0);
 				*colon_offset = sav;
 			}
 		}
@@ -668,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) {
@@ -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;
 
@@ -699,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;
 			}
 
@@ -722,8 +731,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 +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 {

main/SAPI.h

@@ -185,21 +185,24 @@ 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: 			*/
 	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;
 
 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)