Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:36631
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain peytz.dk from 80.160.174.174 cause and error)
Message-ID: <47EC2065.2000002@peytz.dk>
Date: Thu, 27 Mar 2008 23:32:05 +0100
User-Agent: Thunderbird 2.0.0.12 (X11/20080226)
MIME-Version: 1.0
To: internals@lists.php.net
Content-Type: multipart/mixed;
 boundary="------------030602000408080408000704"
Subject: [PATCH] Allow unsetting headers previously set by header()
From: chsc@peytz.dk (Christian Schmidt)

--------------030602000408080408000704
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

When a header has been set using header('Foo: bar') it can be replaced with 
other headers, but it cannot be removed.

For instance, one could imagine that the auto_prepend_file calls 
header('Vary: Accept-Language') because most pages do language-negotiation. 
But if a single file generates, say, an image, it would improve caching if 
the Vary header was removed. Setting it to the empty string using 
header('Vary:') is not equivalent to unsetting the header.

I suggest extending the behaviour of header() so that when the first 
argument does not contain a colon (and does not begin with "HTTP/"), e.g. 
header('Vary'), it unsets the header with the specified name.

AFAICT this change is backwards compatible. According to the HTTP spec, RFC 
2616, section 4.2, header lines must include a colon, so calling e.g. 
header('Vary') does currently not generate a valid header:
http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2

The attached patch contains an implementation for the cgi and apache2handler 
SAPIs.

AFAICT these SAPIs don't need to be changed:
cli, embed, isapi, milter, pi3web, thttpd, tux, webjames.

These SAPIs need additional patching:
apache, apache_hooks, apache2filter, continuity/capi, caudium, nsapi, 
phttpd, roxen
I don't have an easy way to test these SAPIs, so any help with adding 
support for them is appreciated. AFAICT some of these don't have full 
header() support (in particular for the second $replace parameter), so it 
may not be a big issue if they aren't patched right away.

For some reason I couldn't build apache2filter on my machine.

The atteched phpt file is intended to be put in 
ext/standard/tests/general_functions.


What do you think of the general idea of being able to unset headers?
And what do you think of my patch?

This is my first patch for PHP. I am not a strong C programmer.


Christian

--------------030602000408080408000704
Content-Type: text/plain;
 name="header.patch.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="header.patch.txt"

Index: sapi/apache2handler/sapi_apache2.c
===================================================================
RCS file: /repository/php-src/sapi/apache2handler/sapi_apache2.c,v
retrieving revision 1.57.2.10.2.15.2.3
diff -u -9 -p -r1.57.2.10.2.15.2.3 sapi_apache2.c
--- sapi/apache2handler/sapi_apache2.c	18 Mar 2008 22:23:20 -0000	1.57.2.10.2.15.2.3
+++ sapi/apache2handler/sapi_apache2.c	27 Mar 2008 21:39:21 -0000
@@ -87,20 +87,20 @@ php_apache_sapi_header_handler(sapi_head
 {
 	php_struct *ctx;
 	char *val, *ptr;
 
 	ctx = SG(server_context);
 
 	val = strchr(sapi_header->header, ':');
 
 	if (!val) {
-		sapi_free_header(sapi_header);
-		return 0;
+		apr_table_unset(ctx->r->headers_out, sapi_header->header);
+		return SAPI_HEADER_ADD;
 	}
 	ptr = val;
 
 	*val = '\0';
 	
 	do {
 		val++;
 	} while (*val == ' ');
 
Index: main/SAPI.c
===================================================================
RCS file: /repository/php-src/main/SAPI.c,v
retrieving revision 1.202.2.7.2.15.2.4
diff -u -9 -p -r1.202.2.7.2.15.2.4 SAPI.c
--- main/SAPI.c	18 Mar 2008 21:42:50 -0000	1.202.2.7.2.15.2.4
+++ main/SAPI.c	27 Mar 2008 21:39:21 -0000
@@ -491,19 +491,20 @@ static void sapi_update_response_code(in
 	if (SG(sapi_headers).http_status_line) {
 		efree(SG(sapi_headers).http_status_line);
 		SG(sapi_headers).http_status_line = NULL;
 	}
 	SG(sapi_headers).http_response_code = ncode;
 }
 
 static int sapi_find_matching_header(void *element1, void *element2)
 {
-	return strncasecmp(((sapi_header_struct*)element1)->header, (char*)element2, strlen((char*)element2)) == 0;
+	int len = strlen((char*)element2);
+	return strncasecmp(((sapi_header_struct*)element1)->header, (char*)element2, len) == 0 && ((sapi_header_struct*)element1)->header[len] == ':';
 }
 
 SAPI_API int sapi_add_header_ex(char *header_line, uint header_line_len, zend_bool duplicate, zend_bool replace TSRMLS_DC)
 {
 	sapi_header_line ctr = {0};
 	int r;
 	
 	ctr.line = header_line;
 	ctr.line_len = header_line_len;
@@ -730,32 +731,36 @@ SAPI_API int sapi_header_op(sapi_header_
 	if (sapi_module.header_handler) {
 		retval = sapi_module.header_handler(&sapi_header, &SG(sapi_headers) TSRMLS_CC);
 	} else {
 		retval = SAPI_HEADER_ADD;
 	}
 	if (retval & SAPI_HEADER_DELETE_ALL) {
 		zend_llist_clean(&SG(sapi_headers).headers);
 	}
 	if (retval & SAPI_HEADER_ADD) {
-		/* in replace mode first remove the header if it already exists in the headers llist */
-		if (replace) {
-			colon_offset = strchr(sapi_header.header, ':');
-			if (colon_offset) {
+		colon_offset = strchr(sapi_header.header, ':');
+		if (!colon_offset) {
+			/* no colon means delete header */
+			zend_llist_del_element(&SG(sapi_headers).headers, sapi_header.header, (int(*)(void*, void*))sapi_find_matching_header);
+			sapi_free_header(&sapi_header);
+
+		} else {
+			/* in replace mode first remove the header if it already exists in the headers llist */
+			if (replace) {
 				char sav;
-				colon_offset++;
 				sav = *colon_offset;
 				*colon_offset = 0;
 				zend_llist_del_element(&SG(sapi_headers).headers, sapi_header.header, (int(*)(void*, void*))sapi_find_matching_header);
 				*colon_offset = sav;
 			}
-		}
 
-		zend_llist_add_element(&SG(sapi_headers).headers, (void *) &sapi_header);
+			zend_llist_add_element(&SG(sapi_headers).headers, (void *) &sapi_header);
+		}
 	}
 	return SUCCESS;
 }
 
 
 SAPI_API int sapi_send_headers(TSRMLS_D)
 {
 	int retval;
 	int ret = FAILURE;

--------------030602000408080408000704
Content-Type: text/plain;
 name="head_cgi.phpt.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="head_cgi.phpt.txt"

--TEST--
header() and friends
--SKIPIF--
<?php
if (php_sapi_name() != "cgi-fcgi") {
	die("skip this test is for CGI");
}
?>
--FILE--
<?php

ini_set('html_errors', 0);

header("X-Powered-By");
header("CONTENT-type: text/plain");

header("Foo1: bar1 1");
header("Foo1: bar1 2", false);
header("Foo1: bar1 3", true);

header("FOO2: bar2");
header("foo2");

ob_start();
var_dump("foo3");
var_dump(headers_sent());
header("Foo3: bar3");

while (ob_get_level()) ob_end_flush();
var_dump("foo4");
var_dump(headers_sent());

var_dump(header("Foo4: bar4"));

var_dump(headers_list());

echo "Done\n";
?>
--EXPECTF--	
CONTENT-type: text/plain
Foo1: bar1 2
Foo1: bar1 3
Foo3: bar3

string(4) "foo3"
bool(false)
string(4) "foo4"
bool(true)

Warning: Cannot modify header information - headers already sent by (output started at %s:%d) in %s on line %d
NULL
array(4) {
  [0]=>
  string(24) "CONTENT-type: text/plain"
  [1]=>
  string(12) "Foo1: bar1 2"
  [2]=>
  string(12) "Foo1: bar1 3"
  [3]=>
  string(10) "Foo3: bar3"
}
Done

--------------030602000408080408000704--