You are viewing a plain text version of this content. The canonical link for it is here.
Posted to apreq-dev@httpd.apache.org by Bojan Smojver <bo...@rexursive.com> on 2004/11/16 23:21:25 UTC

Bug in mod_apreq.c

This piece of code in mod_apreq.c:

--------------------------------
static apr_status_t apache2_header_out(void *env, const char *name,
                                       char *value)
{
    dR;
    apr_table_addn(r->headers_out, name, value);
    return APR_SUCCESS;
}
--------------------------------

assumes that name and value will be something from "permanent" memory and
therefore don't need to be copied, but rather that their pointers should simply
be stored in the table (therefore the addn, not add). However, this code in
apreq_cookie.c:

--------------------------------
APREQ_DECLARE(apr_status_t) apreq_cookie_bake(const apreq_cookie_t *c,
                                              void *env)
{
    char s[APREQ_COOKIE_MAX_LENGTH];
    int len = apreq_cookie_serialize(c, s, APREQ_COOKIE_MAX_LENGTH);
    if (len < APREQ_COOKIE_MAX_LENGTH)
        return apreq_env_set_cookie(env, s);

    apreq_log(APREQ_ERROR APR_INCOMPLETE, env,
              "serialized cookie length exceeds limit %d",
              APREQ_COOKIE_MAX_LENGTH - 1);
    return APR_INCOMPLETE;
}
--------------------------------

uses values from the stack. The apreq_env_set_cookie() call eventually calls
apache2_header_out() function, which then assigns completely temporary strings
(the ones from the stack) to the request headers, which are normally associated
with the request pool.

Function apr_table_add() should be used instead, to make copies of both keys and
values. My test show that this fixes the problem of Weird Cookies (TM).

--
Bojan

Re: Bug in mod_apreq.c

Posted by Bojan Smojver <bo...@rexursive.com>.
Quoting Joe Schaefer <jo...@sunstarsys.com>:

> > Function apr_table_add() should be used instead, to make copies
> > of both keys and values. My test show that this fixes the problem
> > of Weird Cookies (TM).
>
> +1.  Recently we discussed (on the modperl@ user list)
> that using r->headers_out was also a bug- it should be using
> the r->err_headers_out table instead.  Can you post a patch for
> mod_apreq.c that includes this bugfix as well?

Sure. Once I get to the box where I can pull CVS out...

--
Bojan

Re: Bug in mod_apreq.c

Posted by Stas Bekman <st...@stason.org>.
Bojan Smojver wrote:

> I'm guessing the fact that I can't see the change through the CVS web
> interface is related to Subversion?

Yes. Please use:
http://svn.apache.org/viewcvs.cgi/httpd/apreq/trunk/

(Joe, Randy, should this link end up on the apreq website as well?)

-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

Re: Bug in mod_apreq.c

Posted by Bojan Smojver <bo...@rexursive.com>.
On Fri, 2004-11-19 at 22:19 -0500, Joe Schaefer wrote:

> Thanks, applied!

First, sorry for the late reply - I moved house and my Internet was down
for a long time.

I'm guessing the fact that I can't see the change through the CVS web
interface is related to Subversion?

-- 
Bojan


Re: Bug in mod_apreq.c

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Bojan Smojver <bo...@rexursive.com> writes:

> On Tue, 2004-11-16 at 17:43 -0500, Joe Schaefer wrote:
>
>> +1.  Recently we discussed (on the modperl@ user list) 
>> that using r->headers_out was also a bug- it should be using
>> the r->err_headers_out table instead.  Can you post a patch for
>> mod_apreq.c that includes this bugfix as well?
>
> Attached. Let me know if that's what you had in mind. The fix for
> _add()  v. _addn() is also in it.

Thanks, applied!

-- 
Joe Schaefer


Re: Bug in mod_apreq.c

Posted by Bojan Smojver <bo...@rexursive.com>.
On Tue, 2004-11-16 at 17:43 -0500, Joe Schaefer wrote:

> +1.  Recently we discussed (on the modperl@ user list) 
> that using r->headers_out was also a bug- it should be using
> the r->err_headers_out table instead.  Can you post a patch for
> mod_apreq.c that includes this bugfix as well?

Attached. Let me know if that's what you had in mind. The fix for _add()
v. _addn() is also in it.

-- 
Bojan

Re: Bug in mod_apreq.c

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Bojan Smojver <bo...@rexursive.com> writes:

> This piece of code in mod_apreq.c:

[...]

> Function apr_table_add() should be used instead, to make copies
> of both keys and values. My test show that this fixes the problem
> of Weird Cookies (TM). 

+1.  Recently we discussed (on the modperl@ user list) 
that using r->headers_out was also a bug- it should be using
the r->err_headers_out table instead.  Can you post a patch for
mod_apreq.c that includes this bugfix as well?

-- 
Joe Schaefer