You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Nick Kew <ni...@webthing.com> on 2008/05/27 15:53:56 UTC
Re: svn commit: r660461 -
/httpd/httpd/trunk/modules/mappers/mod_rewrite.c
On Tue, 27 May 2008 10:50:24 -0000
rpluem@apache.org wrote:
> Author: rpluem
> Date: Tue May 27 03:50:23 2008
> New Revision: 660461
>
> URL: http://svn.apache.org/viewvc?rev=660461&view=rev
> Log:
> * Handle the case that secure is NULL
>
> Modified:
> httpd/httpd/trunk/modules/mappers/mod_rewrite.c
>
> Modified: httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_rewrite.c?rev=660461&r1=660460&r2=660461&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original) +++
> httpd/httpd/trunk/modules/mappers/mod_rewrite.c Tue May 27 03:50:23
> 2008 @@ -2492,7 +2492,9 @@ "; domain=", domain,
> expires ? "; expires=" : NULL,
> expires ? exp_time : NULL,
> - (strcasecmp(secure, "true") == 0 ||
> strcasecmp(secure, "1") == 0) ? "; secure" : NULL,
> + secure ? ((strcasecmp(secure,
> "true") == 0
> + || strcasecmp(secure,
> "1") == 0) ?
> + "; secure" : NULL) : NULL,
> httponly ? "; HttpOnly" : NULL,
> NULL);
Fixes the expected segfault, but should it be more robust?
As in, check the value of httponly rather than rely on it
only having one non-NULL value in the spec?
--
Nick Kew
Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/
Re: svn commit: r660461 - /httpd/httpd/trunk/modules/mappers/mod_rewrite.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 06/05/2008 11:33 PM, Nick Kew wrote:
> On Mon, 02 Jun 2008 21:35:24 +0200
> Ruediger Pluem <rp...@apache.org> wrote:
>
>>> 1. Accept only "true" or "1"
>>> 2. Accept only "secure" and "HttpOnly" (but case insensitive)
>>> 3. Accept "true" or "1" or "secure" and "HttpOnly" (but case
>>> insensitive)
>>>
>>> What do you like most (if none please add your favourite
>>> proposal :-)?
>
> Sorry, I still owe you a reply. Got distracted.
>
> Should I parse your (3) as ("true" or "1" or "secure") and "HttpOnly"?
> If so, that's probably the best option. Descriptive values are
> nice and less easy to confuse, but rejecting spec values true/1
> would seem perverse. Ugh.
>
> But (2) would be fine. As would (true|1) and HttpOnly. IMHO.
>
Thanks for the reply. Does r664330 (http://svn.apache.org/viewvc?rev=664330&view=rev)
solve this for you?
If yes I would update the backport proposal accordingly.
Regards
Rüdiger
Re: svn commit: r660461 -
/httpd/httpd/trunk/modules/mappers/mod_rewrite.c
Posted by Nick Kew <ni...@webthing.com>.
On Mon, 02 Jun 2008 21:35:24 +0200
Ruediger Pluem <rp...@apache.org> wrote:
> >
> > 1. Accept only "true" or "1"
> > 2. Accept only "secure" and "HttpOnly" (but case insensitive)
> > 3. Accept "true" or "1" or "secure" and "HttpOnly" (but case
> > insensitive)
> >
> > What do you like most (if none please add your favourite
> > proposal :-)?
Sorry, I still owe you a reply. Got distracted.
Should I parse your (3) as ("true" or "1" or "secure") and "HttpOnly"?
If so, that's probably the best option. Descriptive values are
nice and less easy to confuse, but rejecting spec values true/1
would seem perverse. Ugh.
But (2) would be fine. As would (true|1) and HttpOnly. IMHO.
--
Nick Kew
Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/
Re: svn commit: r660461 - /httpd/httpd/trunk/modules/mappers/mod_rewrite.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 05/28/2008 12:36 AM, Ruediger Pluem wrote:
>
>
> On 05/28/2008 12:14 AM, Nick Kew wrote:
>> On Wed, 28 May 2008 00:09:34 +0200
>> Ruediger Pluem <rp...@apache.org> wrote:
>>
>>
>>> Sorry but I do not get your point at the Moment. MS and the PR talk
>>> about "HttpOnly" (with exactly this mixture of capital and small
>>> letters). The only location where this is different is in the patch
>>> for mod_rewrite.xml (which can be easily fixed of course).
>>> So what is your exact point of concern now?
>>
>> Comparing not to "HttpOnly" but to "true" and "1". AKA cut&paste.
>>
>
> Ok, now I get your point. To be honest this wasn't copy and paste.
> I used the same behaviour as for the secure flag and adjusted the
> documentation accordingly. I guess we should keep it the same for
> both options:
>
> 1. Accept only "true" or "1"
> 2. Accept only "secure" and "HttpOnly" (but case insensitive)
> 3. Accept "true" or "1" or "secure" and "HttpOnly" (but case insensitive)
>
> What do you like most (if none please add your favourite proposal :-)?
Any opinions on the alternatives?
Regards
Rüdiger
Re: svn commit: r660461 - /httpd/httpd/trunk/modules/mappers/mod_rewrite.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 05/28/2008 12:14 AM, Nick Kew wrote:
> On Wed, 28 May 2008 00:09:34 +0200
> Ruediger Pluem <rp...@apache.org> wrote:
>
>
>> Sorry but I do not get your point at the Moment. MS and the PR talk
>> about "HttpOnly" (with exactly this mixture of capital and small
>> letters). The only location where this is different is in the patch
>> for mod_rewrite.xml (which can be easily fixed of course).
>> So what is your exact point of concern now?
>
> Comparing not to "HttpOnly" but to "true" and "1". AKA cut&paste.
>
Ok, now I get your point. To be honest this wasn't copy and paste.
I used the same behaviour as for the secure flag and adjusted the
documentation accordingly. I guess we should keep it the same for
both options:
1. Accept only "true" or "1"
2. Accept only "secure" and "HttpOnly" (but case insensitive)
3. Accept "true" or "1" or "secure" and "HttpOnly" (but case insensitive)
What do you like most (if none please add your favourite proposal :-)?
> ObPedant - strcasecomp for "1" seems OTT.
:-). This is really cut and paste. If there is need for further changes
I will fix this.
Regards
Rüdiger
Re: svn commit: r660461 -
/httpd/httpd/trunk/modules/mappers/mod_rewrite.c
Posted by Nick Kew <ni...@webthing.com>.
On Wed, 28 May 2008 00:09:34 +0200
Ruediger Pluem <rp...@apache.org> wrote:
> Sorry but I do not get your point at the Moment. MS and the PR talk
> about "HttpOnly" (with exactly this mixture of capital and small
> letters). The only location where this is different is in the patch
> for mod_rewrite.xml (which can be easily fixed of course).
> So what is your exact point of concern now?
Comparing not to "HttpOnly" but to "true" and "1". AKA cut&paste.
ObPedant - strcasecomp for "1" seems OTT.
--
Nick Kew
Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/
Re: svn commit: r660461 - /httpd/httpd/trunk/modules/mappers/mod_rewrite.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 05/27/2008 11:05 PM, Nick Kew wrote:
> On Tue, 27 May 2008 17:50:49 +0200
> Ruediger Pluem <rp...@apache.org> wrote:
>
>
>> Does r660566 (http://svn.apache.org/viewvc?rev=660566&view=rev) fix
>> your concerns?
>
> No, it's a mismatch with the literal "HTTPOnly" value of the attribute
> in the PR and the Microsoft document it references.
>
> I guess that's the trouble with supporting elasticated specs.
>
Sorry but I do not get your point at the Moment. MS and the PR talk about
"HttpOnly" (with exactly this mixture of capital and small letters).
The only location where this is different is in the patch for mod_rewrite.xml
(which can be easily fixed of course).
So what is your exact point of concern now?
Regards
Rüdiger
Re: svn commit: r660461 -
/httpd/httpd/trunk/modules/mappers/mod_rewrite.c
Posted by Nick Kew <ni...@webthing.com>.
On Tue, 27 May 2008 17:50:49 +0200
Ruediger Pluem <rp...@apache.org> wrote:
> Does r660566 (http://svn.apache.org/viewvc?rev=660566&view=rev) fix
> your concerns?
No, it's a mismatch with the literal "HTTPOnly" value of the attribute
in the PR and the Microsoft document it references.
I guess that's the trouble with supporting elasticated specs.
--
Nick Kew
Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/
Re: svn commit: r660461 - /httpd/httpd/trunk/modules/mappers/mod_rewrite.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 05/27/2008 03:53 PM, Nick Kew wrote:
> On Tue, 27 May 2008 10:50:24 -0000
> rpluem@apache.org wrote:
>
>> Author: rpluem
>> Date: Tue May 27 03:50:23 2008
>> New Revision: 660461
>>
>> URL: http://svn.apache.org/viewvc?rev=660461&view=rev
>> Log:
>> * Handle the case that secure is NULL
>>
>> Modified:
>> httpd/httpd/trunk/modules/mappers/mod_rewrite.c
>>
>> Modified: httpd/httpd/trunk/modules/mappers/mod_rewrite.c
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_rewrite.c?rev=660461&r1=660460&r2=660461&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original) +++
>> httpd/httpd/trunk/modules/mappers/mod_rewrite.c Tue May 27 03:50:23
>> 2008 @@ -2492,7 +2492,9 @@ "; domain=", domain,
>> expires ? "; expires=" : NULL,
>> expires ? exp_time : NULL,
>> - (strcasecmp(secure, "true") == 0 ||
>> strcasecmp(secure, "1") == 0) ? "; secure" : NULL,
>> + secure ? ((strcasecmp(secure,
>> "true") == 0
>> + || strcasecmp(secure,
>> "1") == 0) ?
>> + "; secure" : NULL) : NULL,
>> httponly ? "; HttpOnly" : NULL,
>> NULL);
>
> Fixes the expected segfault, but should it be more robust?
> As in, check the value of httponly rather than rely on it
> only having one non-NULL value in the spec?
Does r660566 (http://svn.apache.org/viewvc?rev=660566&view=rev) fix your concerns?
Regards
Rüdiger