You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by gr...@apache.org on 2007/07/31 16:54:48 UTC
svn commit: r561352 - /httpd/httpd/trunk/modules/http/http_protocol.c
Author: gregames
Date: Tue Jul 31 07:54:46 2007
New Revision: 561352
URL: http://svn.apache.org/viewvc?view=rev&rev=561352
Log:
this appears to be a "mv" rather than a "swap", so we should be able to
simplify & shave off a couple of cycles.
Modified:
httpd/httpd/trunk/modules/http/http_protocol.c
Modified: httpd/httpd/trunk/modules/http/http_protocol.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_protocol.c?view=diff&rev=561352&r1=561351&r2=561352
==============================================================================
--- httpd/httpd/trunk/modules/http/http_protocol.c (original)
+++ httpd/httpd/trunk/modules/http/http_protocol.c Tue Jul 31 07:54:46 2007
@@ -1138,7 +1138,6 @@
}
if (!r->assbackwards) {
- apr_table_t *tmp = r->headers_out;
/* For all HTTP/1.x responses for which we generate the message,
* we need to avoid inheriting the "normal status" header fields
@@ -1146,7 +1145,6 @@
* error or redirect, except for Location on external redirects.
*/
r->headers_out = r->err_headers_out;
- r->err_headers_out = tmp;
apr_table_clear(r->err_headers_out);
if (ap_is_HTTP_REDIRECT(status) || (status == HTTP_CREATED)) {
Re: svn commit: r561352 -
/httpd/httpd/trunk/modules/http/http_protocol.c
Posted by Nick Kew <ni...@webthing.com>.
On Sat, 04 Aug 2007 13:58:19 +0200
Ruediger Pluem <rp...@apache.org> wrote:
> > So r->headers_out and r->err_headers_out point to the *same* table
> > and this table is *empty*. So we loose *all* output headers for
> > further processing.
Looks like that to me. The old code may be a tiny tad confusing,
in that it runs apr_table_clear after the swap and thus at-first-glance
on the wrong table. But that doesn't affect your point.
--
Nick Kew
Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/
Re: svn commit: r561352 - /httpd/httpd/trunk/modules/http/http_protocol.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 08/04/2007 04:46 PM, Jim Jagielski wrote:
>
> On Aug 4, 2007, at 7:58 AM, Ruediger Pluem wrote:
>
>>
>>
>> On 07/31/2007 08:52 PM, Ruediger Pluem wrote:
>>>
>>> On 07/31/2007 04:54 PM, wrote:
>>>> Author: gregames
>>>> Date: Tue Jul 31 07:54:46 2007
>>>> New Revision: 561352
>>>>
>>>> URL: http://svn.apache.org/viewvc?view=rev&rev=561352
>>>> Log:
>>>> this appears to be a "mv" rather than a "swap", so we should be able to
>>>> simplify & shave off a couple of cycles.
>>>>
>>>> Modified:
>>>> httpd/httpd/trunk/modules/http/http_protocol.c
>>>>
>>>> Modified: httpd/httpd/trunk/modules/http/http_protocol.c
>>>> URL:
>>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_protocol.c?view=diff&rev=561352&r1=561351&r2=561352
>>>>
>>>> ==============================================================================
>>>>
>>>> --- httpd/httpd/trunk/modules/http/http_protocol.c (original)
>>>> +++ httpd/httpd/trunk/modules/http/http_protocol.c Tue Jul 31
>>>> 07:54:46 2007
>>>> @@ -1138,7 +1138,6 @@
>>>> }
>>>>
>>>> if (!r->assbackwards) {
>>>> - apr_table_t *tmp = r->headers_out;
>>>>
>>>> /* For all HTTP/1.x responses for which we generate the
>>>> message,
>>>> * we need to avoid inheriting the "normal status" header
>>>> fields
>>>> @@ -1146,7 +1145,6 @@
>>>> * error or redirect, except for Location on external
>>>> redirects.
>>>> */
>>>> r->headers_out = r->err_headers_out;
>>>> - r->err_headers_out = tmp;
>>>> apr_table_clear(r->err_headers_out);
>>>>
>>>> if (ap_is_HTTP_REDIRECT(status) || (status == HTTP_CREATED)) {
>>>>
>>>
>>> IMHO this is wrong. Lets take the following example:
>>>
>>> r->headers_out points to a table T1 containing the key value pairs
>>> ((A1,a1), (B1,b1)).
>>> r->err_headers_out points to a table T2 containing the key value
>>> pairs ((A2,a2), (B2,b2)).
>>>
>>> After running thru the old code (until after the apr_table_clear) the
>>> the result is:
>>>
>>> r->headers_out points to table T2 containing the key value pairs
>>> ((A2,a2), (B2,b2)).
>>> r->err_headers_out points to table T1 which is empty.
>>>
>>> Using the new code the result is
>>>
>>> r->headers_out points to table T2.
>>> r->err_headers_out points to table T2.
>>>
>>> T2 is empty.
>>>
>>> So r->headers_out and r->err_headers_out point to the *same* table
>>> and this table is *empty*.
>>> So we loose *all* output headers for further processing.
>>
>> Ping? Any comments?
>>
>
> Looks like a viable and reasonable (and required) reversal of
> the patch is required...
Today I noticed that this completely breaks authentication on trunk.
As there was no further response / action from Greg I reverted this patch.
Regards
Rüdiger
Re: svn commit: r561352 - /httpd/httpd/trunk/modules/http/http_protocol.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Aug 4, 2007, at 7:58 AM, Ruediger Pluem wrote:
>
>
> On 07/31/2007 08:52 PM, Ruediger Pluem wrote:
>>
>> On 07/31/2007 04:54 PM, wrote:
>>> Author: gregames
>>> Date: Tue Jul 31 07:54:46 2007
>>> New Revision: 561352
>>>
>>> URL: http://svn.apache.org/viewvc?view=rev&rev=561352
>>> Log:
>>> this appears to be a "mv" rather than a "swap", so we should be
>>> able to
>>> simplify & shave off a couple of cycles.
>>>
>>> Modified:
>>> httpd/httpd/trunk/modules/http/http_protocol.c
>>>
>>> Modified: httpd/httpd/trunk/modules/http/http_protocol.c
>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/
>>> http_protocol.c?view=diff&rev=561352&r1=561351&r2=561352
>>> ====================================================================
>>> ==========
>>> --- httpd/httpd/trunk/modules/http/http_protocol.c (original)
>>> +++ httpd/httpd/trunk/modules/http/http_protocol.c Tue Jul 31
>>> 07:54:46 2007
>>> @@ -1138,7 +1138,6 @@
>>> }
>>>
>>> if (!r->assbackwards) {
>>> - apr_table_t *tmp = r->headers_out;
>>>
>>> /* For all HTTP/1.x responses for which we generate the
>>> message,
>>> * we need to avoid inheriting the "normal status"
>>> header fields
>>> @@ -1146,7 +1145,6 @@
>>> * error or redirect, except for Location on external
>>> redirects.
>>> */
>>> r->headers_out = r->err_headers_out;
>>> - r->err_headers_out = tmp;
>>> apr_table_clear(r->err_headers_out);
>>>
>>> if (ap_is_HTTP_REDIRECT(status) || (status ==
>>> HTTP_CREATED)) {
>>>
>>
>> IMHO this is wrong. Lets take the following example:
>>
>> r->headers_out points to a table T1 containing the key value pairs
>> ((A1,a1), (B1,b1)).
>> r->err_headers_out points to a table T2 containing the key value
>> pairs ((A2,a2), (B2,b2)).
>>
>> After running thru the old code (until after the apr_table_clear)
>> the the result is:
>>
>> r->headers_out points to table T2 containing the key value pairs
>> ((A2,a2), (B2,b2)).
>> r->err_headers_out points to table T1 which is empty.
>>
>> Using the new code the result is
>>
>> r->headers_out points to table T2.
>> r->err_headers_out points to table T2.
>>
>> T2 is empty.
>>
>> So r->headers_out and r->err_headers_out point to the *same* table
>> and this table is *empty*.
>> So we loose *all* output headers for further processing.
>
> Ping? Any comments?
>
Looks like a viable and reasonable (and required) reversal of
the patch is required...
Re: svn commit: r561352 - /httpd/httpd/trunk/modules/http/http_protocol.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 07/31/2007 08:52 PM, Ruediger Pluem wrote:
>
> On 07/31/2007 04:54 PM, wrote:
>> Author: gregames
>> Date: Tue Jul 31 07:54:46 2007
>> New Revision: 561352
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=561352
>> Log:
>> this appears to be a "mv" rather than a "swap", so we should be able to
>> simplify & shave off a couple of cycles.
>>
>> Modified:
>> httpd/httpd/trunk/modules/http/http_protocol.c
>>
>> Modified: httpd/httpd/trunk/modules/http/http_protocol.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_protocol.c?view=diff&rev=561352&r1=561351&r2=561352
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/http/http_protocol.c (original)
>> +++ httpd/httpd/trunk/modules/http/http_protocol.c Tue Jul 31 07:54:46 2007
>> @@ -1138,7 +1138,6 @@
>> }
>>
>> if (!r->assbackwards) {
>> - apr_table_t *tmp = r->headers_out;
>>
>> /* For all HTTP/1.x responses for which we generate the message,
>> * we need to avoid inheriting the "normal status" header fields
>> @@ -1146,7 +1145,6 @@
>> * error or redirect, except for Location on external redirects.
>> */
>> r->headers_out = r->err_headers_out;
>> - r->err_headers_out = tmp;
>> apr_table_clear(r->err_headers_out);
>>
>> if (ap_is_HTTP_REDIRECT(status) || (status == HTTP_CREATED)) {
>>
>
> IMHO this is wrong. Lets take the following example:
>
> r->headers_out points to a table T1 containing the key value pairs ((A1,a1), (B1,b1)).
> r->err_headers_out points to a table T2 containing the key value pairs ((A2,a2), (B2,b2)).
>
> After running thru the old code (until after the apr_table_clear) the the result is:
>
> r->headers_out points to table T2 containing the key value pairs ((A2,a2), (B2,b2)).
> r->err_headers_out points to table T1 which is empty.
>
> Using the new code the result is
>
> r->headers_out points to table T2.
> r->err_headers_out points to table T2.
>
> T2 is empty.
>
> So r->headers_out and r->err_headers_out point to the *same* table and this table is *empty*.
> So we loose *all* output headers for further processing.
Ping? Any comments?
Regards
Rüdiger
Re: svn commit: r561352 - /httpd/httpd/trunk/modules/http/http_protocol.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 07/31/2007 04:54 PM, wrote:
> Author: gregames
> Date: Tue Jul 31 07:54:46 2007
> New Revision: 561352
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=561352
> Log:
> this appears to be a "mv" rather than a "swap", so we should be able to
> simplify & shave off a couple of cycles.
>
> Modified:
> httpd/httpd/trunk/modules/http/http_protocol.c
>
> Modified: httpd/httpd/trunk/modules/http/http_protocol.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_protocol.c?view=diff&rev=561352&r1=561351&r2=561352
> ==============================================================================
> --- httpd/httpd/trunk/modules/http/http_protocol.c (original)
> +++ httpd/httpd/trunk/modules/http/http_protocol.c Tue Jul 31 07:54:46 2007
> @@ -1138,7 +1138,6 @@
> }
>
> if (!r->assbackwards) {
> - apr_table_t *tmp = r->headers_out;
>
> /* For all HTTP/1.x responses for which we generate the message,
> * we need to avoid inheriting the "normal status" header fields
> @@ -1146,7 +1145,6 @@
> * error or redirect, except for Location on external redirects.
> */
> r->headers_out = r->err_headers_out;
> - r->err_headers_out = tmp;
> apr_table_clear(r->err_headers_out);
>
> if (ap_is_HTTP_REDIRECT(status) || (status == HTTP_CREATED)) {
>
IMHO this is wrong. Lets take the following example:
r->headers_out points to a table T1 containing the key value pairs ((A1,a1), (B1,b1)).
r->err_headers_out points to a table T2 containing the key value pairs ((A2,a2), (B2,b2)).
After running thru the old code (until after the apr_table_clear) the the result is:
r->headers_out points to table T2 containing the key value pairs ((A2,a2), (B2,b2)).
r->err_headers_out points to table T1 which is empty.
Using the new code the result is
r->headers_out points to table T2.
r->err_headers_out points to table T2.
T2 is empty.
So r->headers_out and r->err_headers_out point to the *same* table and this table is *empty*.
So we loose *all* output headers for further processing.
Regards
Rüdiger