Fixed bug #61605 header_remove() does not remove all headers #36

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@reeze
reeze commented Apr 3, 2012

Hi,
after looking at the code,I found that SAPI it self did't handle multiple headers properly, apache2handler handles it for itself. so apache2handler can remove it correctly.

SAPI headers are saved in zend_llist. when try to remove or replace it simply try to find the first one it found and stop searching. I've looked at sapi/cli&cgi they both didn't handle it itself.
Sine header() doc says: "The optional replace parameter indicates whether the header should replace a previous similar header". but it did't behavior like this.
So I made a patch for this. can someone review this pull request for me?

Thanks

@reeze reeze commented on the diff Apr 4, 2012
main/SAPI.c
{
- int len = strlen((char*)element2);
- return strncasecmp(((sapi_header_struct*)element1)->header, (char*)element2, len) == 0 && ((sapi_header_struct*)element1)->header[len] == ':';
+ zend_llist_element *element, *next;
+ int len = strlen(name);
+ char *header;
+
+ element = headers->head;
+ while (element) {
+ next = element->next;
+ header = (char *)((sapi_header_struct *)element->data)->header;
+
+ if (strncasecmp(header, name, len) == 0 && header[len] == ':') {
+ if (element->prev) {
@reeze
reeze Apr 4, 2012

zend_llist doesn't have any api to remove more than one element like HashTable's apply. so I simply do it my self (original macro DEL_LLIST_ELEMENT(current, l)). if there is a easy way please tell me.thanks.

@php-pulls
Collaborator

Comment on behalf of laruence at php.net:

thanks for your work, but I got a fix and phpt test. and it also seems better.

@php-pulls
Collaborator

Comment on behalf of laruence at php.net:

close request

@php-pulls php-pulls closed this Apr 4, 2012
@reeze
reeze commented Apr 4, 2012

thanks. but I think we choose the same way.
The reason I didn't add extra str len parameter is that we always have to call strlen. so I hide it to make interface clear.

@laruence
Member
laruence commented Apr 4, 2012

Nope, for header_remove, it's no need to call strlen, it already there, and also the php test script, and the pdtor ,,,etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment