Skip to content

Commit 18dee43

Browse files
NattyNarwhalNattyNarwhal
authored
Add SAPI_HEADER_DELETE_PREFIX, make ext/session use it (#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
1 parent a262419 commit 18dee43

File tree

3 files changed

+28
-42
lines changed

3 files changed

+28
-42
lines changed

‎ext/session/session.c

Lines changed: 6 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1335,45 +1335,19 @@ static int php_session_cache_limiter(void)
13351335
* Cookie Management *
13361336
********************* */
13371337

1338-
/*
1339-
* Remove already sent session ID cookie.
1340-
* It must be directly removed from SG(sapi_header) because sapi_add_header_ex()
1341-
* removes all of matching cookie. i.e. It deletes all of Set-Cookie headers.
1342-
*/
13431338
static void php_session_remove_cookie(void) {
1344-
sapi_header_struct *header;
1345-
zend_llist *l = &SG(sapi_headers).headers;
1346-
zend_llist_element *next;
1347-
zend_llist_element *current;
13481339
char *session_cookie;
13491340
size_t session_cookie_len;
1350-
size_t len = sizeof("Set-Cookie")-1;
1341+
sapi_header_line header_line = {0};
13511342

13521343
ZEND_ASSERT(strpbrk(ZSTR_VAL(PS(session_name)), SESSION_FORBIDDEN_CHARS) == NULL);
13531344
session_cookie_len = spprintf(&session_cookie, 0, "Set-Cookie: %s=", ZSTR_VAL(PS(session_name)));
13541345

1355-
current = l->head;
1356-
while (current) {
1357-
header = (sapi_header_struct *)(current->data);
1358-
next = current->next;
1359-
if (header->header_len > len && header->header[len] == ':'
1360-
&& !strncmp(header->header, session_cookie, session_cookie_len)) {
1361-
if (current->prev) {
1362-
current->prev->next = next;
1363-
} else {
1364-
l->head = next;
1365-
}
1366-
if (next) {
1367-
next->prev = current->prev;
1368-
} else {
1369-
l->tail = current->prev;
1370-
}
1371-
sapi_free_header(header);
1372-
efree(current);
1373-
--l->count;
1374-
}
1375-
current = next;
1376-
}
1346+
header_line.line = session_cookie;
1347+
header_line.line_len = session_cookie_len;
1348+
header_line.header_len = sizeof("Set-Cookie") - 1;
1349+
sapi_header_op(SAPI_HEADER_DELETE_PREFIX, &header_line);
1350+
13771351
efree(session_cookie);
13781352
}
13791353

‎main/SAPI.c

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -597,15 +597,17 @@ static void sapi_update_response_code(int ncode)
597597
* since zend_llist_del_element only removes one matched item once,
598598
* we should remove them manually
599599
*/
600-
static void sapi_remove_header(zend_llist *l, char *name, size_t len) {
600+
static void sapi_remove_header(zend_llist *l, char *name, size_t len, size_t header_len)
601+
{
601602
sapi_header_struct *header;
602603
zend_llist_element *next;
603604
zend_llist_element *current=l->head;
604605

605606
while (current) {
606607
header = (sapi_header_struct *)(current->data);
607608
next = current->next;
608-
if (header->header_len > len && header->header[len] == ':'
609+
if (header->header_len > header_len
610+
&& (header->header[header_len] == ':' || len > header_len)
609611
&& !strncasecmp(header->header, name, len)) {
610612
if (current->prev) {
611613
current->prev->next = next;
@@ -653,7 +655,7 @@ static void sapi_header_add_op(sapi_header_op_enum op, sapi_header_struct *sapi_
653655
char sav = *colon_offset;
654656

655657
*colon_offset = 0;
656-
sapi_remove_header(&SG(sapi_headers).headers, sapi_header->header, strlen(sapi_header->header));
658+
sapi_remove_header(&SG(sapi_headers).headers, sapi_header->header, strlen(sapi_header->header), 0);
657659
*colon_offset = sav;
658660
}
659661
}
@@ -668,7 +670,7 @@ SAPI_API int sapi_header_op(sapi_header_op_enum op, void *arg)
668670
sapi_header_struct sapi_header;
669671
char *colon_offset;
670672
char *header_line;
671-
size_t header_line_len;
673+
size_t header_line_len, header_len;
672674
int http_response_code;
673675

674676
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)
691693

692694
case SAPI_HEADER_ADD:
693695
case SAPI_HEADER_REPLACE:
696+
case SAPI_HEADER_DELETE_PREFIX:
694697
case SAPI_HEADER_DELETE: {
695698
sapi_header_line *p = arg;
696699

@@ -699,7 +702,13 @@ SAPI_API int sapi_header_op(sapi_header_op_enum op, void *arg)
699702
}
700703
header_line = estrndup(p->line, p->line_len);
701704
header_line_len = p->line_len;
702-
http_response_code = p->response_code;
705+
if (op == SAPI_HEADER_DELETE_PREFIX) {
706+
header_len = p->header_len;
707+
http_response_code = 0;
708+
} else {
709+
header_len = 0;
710+
http_response_code = p->response_code;
711+
}
703712
break;
704713
}
705714

@@ -722,8 +731,8 @@ SAPI_API int sapi_header_op(sapi_header_op_enum op, void *arg)
722731
header_line[header_line_len]='\0';
723732
}
724733

725-
if (op == SAPI_HEADER_DELETE) {
726-
if (strchr(header_line, ':')) {
734+
if (op == SAPI_HEADER_DELETE || op == SAPI_HEADER_DELETE_PREFIX) {
735+
if (op == SAPI_HEADER_DELETE && strchr(header_line, ':')) {
727736
efree(header_line);
728737
sapi_module.sapi_error(E_WARNING, "Header to delete may not contain colon.");
729738
return FAILURE;
@@ -733,7 +742,7 @@ SAPI_API int sapi_header_op(sapi_header_op_enum op, void *arg)
733742
sapi_header.header_len = header_line_len;
734743
sapi_module.header_handler(&sapi_header, op, &SG(sapi_headers));
735744
}
736-
sapi_remove_header(&SG(sapi_headers).headers, header_line, header_line_len);
745+
sapi_remove_header(&SG(sapi_headers).headers, header_line, header_line_len, header_len);
737746
efree(header_line);
738747
return SUCCESS;
739748
} else {

‎main/SAPI.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,21 +185,24 @@ END_EXTERN_C()
185185
typedef struct {
186186
const char *line; /* If you allocated this, you need to free it yourself */
187187
size_t line_len;
188-
zend_long response_code; /* long due to zend_parse_parameters compatibility */
188+
union {
189+
zend_long response_code; /* long due to zend_parse_parameters compatibility */
190+
size_t header_len; /* the "Key" in "Key: Value", for optimization */
191+
};
189192
} sapi_header_line;
190193

191194
typedef enum { /* Parameter: */
192195
SAPI_HEADER_REPLACE, /* sapi_header_line* */
193196
SAPI_HEADER_ADD, /* sapi_header_line* */
194197
SAPI_HEADER_DELETE, /* sapi_header_line* */
198+
SAPI_HEADER_DELETE_PREFIX, /* sapi_header_line* */
195199
SAPI_HEADER_DELETE_ALL, /* void */
196200
SAPI_HEADER_SET_STATUS /* int */
197201
} sapi_header_op_enum;
198202

199203
BEGIN_EXTERN_C()
200204
SAPI_API int sapi_header_op(sapi_header_op_enum op, void *arg);
201205

202-
/* Deprecated functions. Use sapi_header_op instead. */
203206
SAPI_API int sapi_add_header_ex(const char *header_line, size_t header_line_len, bool duplicate, bool replace);
204207
#define sapi_add_header(a, b, c) sapi_add_header_ex((a),(b),(c),1)
205208

0 commit comments

Comments
 (0)