You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2007/01/23 22:06:46 UTC

Re: svn commit: r490156 - /httpd/httpd/trunk/modules/metadata/mod_headers.c


On 12/25/2006 06:40 PM, niq@apache.org wrote:
> Author: niq
> Date: Mon Dec 25 09:40:10 2006
> New Revision: 490156
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=490156
> Log:
> PR#36609 - permit % as the last character of a Header value
> 
> Modified:
>     httpd/httpd/trunk/modules/metadata/mod_headers.c
> 
> Modified: httpd/httpd/trunk/modules/metadata/mod_headers.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/metadata/mod_headers.c?view=diff&rev=490156&r1=490155&r2=490156
> ==============================================================================
> --- httpd/httpd/trunk/modules/metadata/mod_headers.c (original)
> +++ httpd/httpd/trunk/modules/metadata/mod_headers.c Mon Dec 25 09:40:10 2006
> @@ -305,8 +305,8 @@
>      }
>      s++; /* skip the % */
>  
> -    /* Pass through %% as % */
> -    if (*s == '%') {
> +    /* Pass through %% or % at end of string as % */
> +    if ((*s == '%') || (*s == '\0')) {
>          tag->func = constant_item;
>          tag->arg = "%";
>          *sa = ++s;

Doesn't this create an off-by-one error?

Lets s look like the following: s = "%\0t"

This would result in pointing *sa to t.

But in line 360 we have the following loop:

   while (*s) {
        if ((res = parse_format_tag(p, (format_tag *) apr_array_push(hdr->ta), &s))) {
            return res;
        }
    }

It would then start to process the memory region starting with t with parse_format_tag.

I think the following should fix this:

Index: modules/metadata/mod_headers.c
===================================================================
--- modules/metadata/mod_headers.c      (Revision 495852)
+++ modules/metadata/mod_headers.c      (Arbeitskopie)
@@ -309,7 +309,9 @@
     if ((*s == '%') || (*s == '\0')) {
         tag->func = constant_item;
         tag->arg = "%";
-        *sa = ++s;
+        if (*s)
+            s++;
+        *sa = s;
         return NULL;
     }



Regards

RĂ¼diger


Re: svn commit: r490156 - /httpd/httpd/trunk/modules/metadata/mod_headers.c

Posted by Nick Kew <ni...@webthing.com>.
On Tue, 23 Jan 2007 22:06:46 +0100
Ruediger Pluem <rp...@apache.org> wrote:


> > -    /* Pass through %% as % */
> > -    if (*s == '%') {
> > +    /* Pass through %% or % at end of string as % */
> > +    if ((*s == '%') || (*s == '\0')) {
> >          tag->func = constant_item;
> >          tag->arg = "%";
> >          *sa = ++s;
> 
> Doesn't this create an off-by-one error?
> 
> Lets s look like the following: s = "%\0t"

%\0  ??  Oook!

> This would result in pointing *sa to t.
> 
> But in line 360 we have the following loop:
> 
>    while (*s) {
>         if ((res = parse_format_tag(p, (format_tag *)
> apr_array_push(hdr->ta), &s))) { return res;
>         }
>     }
> 
> It would then start to process the memory region starting with t with
> parse_format_tag.

Heh!

> I think the following should fix this:

Yep, looks right, thanks.

-- 
Nick Kew

Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/