BUG: Apache::Cookie saving expired data

[David Robins <db...@davidrobins.net>]

I just upgraded to the newest Apache2/mod_perl2, which needed 
libapreq2-2.05-dev for Apache2::Request.  Apache 2.0.54, mod_perl 2.0.0, perl 
5.8.6.

The following statement died with an error about err_headers_out:

 Apache2::Cookie->new($r, name => 'blah', value => $sid, expires => '+1D', 
path => '/path')->bake();

Fine, checked the code, found that bake now wants $r (strange to require it 
two places, but OK, it's 'dev'):

 Apache2::Cookie->new($r, name => 'blah', value => $sid, expires => '+1D', 
path => '/path')->bake($r);

But now it's outputting a cookie with path '+1D' (and everything else 
correct).

if I call path() and expires() separately, it's OK.

I put some trace prints in Apache2::Cookie::new (this is the code that sets 
expires and path, name and value are done explicitly):

    while(my ($k, $v) = each %attrs) {
        $k =~ s/^-//;
print STDERR "setting '$k' to '$v'\n";
print STDERR "pre: ".$cookie->as_string()."\n";
        $cookie->$k($v);
print STDERR "post: ".$cookie->as_string()."\n";
    }
    return $cookie;

Oddly, the 'path' value is corrupt before expires() is even called.  I smell 
rodent and look for the C code for path(), finding it in Cookie.xs:

--- begin function ---
char *
path(obj, val=NULL)
    APR::Request::Cookie obj
    char * val
  PREINIT:
    /*nada*/

  CODE:
    RETVAL = (char *)  obj->path;

    if (items > 1) {
        obj->path = (char *) val;
    }
  OUTPUT:
    RETVAL
--- end function ---

C programmers will probably see what's wrong here: copying a volatile pointer 
(aliasing bug).  The char* comes from SvPV_nolen(), which points into the SV, 
in this case $v from the 'each'.  When $v changes, so will the value of path.  
When $v goes out of scope and its PV gets freed, 'val' will be overwritten by 
whatever uses the space.

The same problem occurs in the other generated char* functions, e.g. domain, 
port, comment, etc.

Should the passed-in strings could be allocated space from the Apache pool 
like name and value?  I figure someone else is better positioned to make a 
clean fix, but if not I can look into writing up a patch.

-- 
Dave
Isa. 40:31

BUG: version_check reports false negatives

[David Robins <db...@davidrobins.net>]

On Monday June 27, 2005 10:57, Joe Schaefer wrote:
> David Robins <db...@davidrobins.net> writes:
> 
> > C programmers will probably see what's wrong here: copying a volatile
> > pointer  (aliasing bug).  The char* comes from SvPV_nolen(), which
> > points into the SV, in this case $v from the 'each'.  When $v changes,
> > so will the value of path. When $v goes out of scope and its PV gets
> > freed, 'val' will be overwritten by  whatever uses the space.
> >
> > The same problem occurs in the other generated char* functions,
> > e.g. domain,  port, comment, etc.
> 
> Exactly- that's another bug we inherited from ExtUtils::XSBuilder, but 
> our current svn trunk includes a fix for that.

Thanks, will grab latest from svn.

Do you know about this one?

httpd-apreq-2 # perl Makefile.PL
perl: 5.8.6 ok
mod_perl: 2.000000 ok
Apache::Test: 1.25 ok
ExtUtils::MakeMaker: 6.17 ok
ExtUtils::XSBuilder: 0.27 ok
build/version_check.pl failed: Test::More version 0.6 unsupported (0.47 or 
greater is required).
# Looks like your test died before it could output anything.
Please upgrade Test::More first.
./configure --enable-perl-glue --with-perl="/usr/bin/perl"
sh: ./configure: No such file or directory

Doesn't seem to realize that 0.6 > 0.47.  The splitting code in 
build/version_check.pl looks very suspicious; perhaps use regular numeric 
comparison if there's < 2 dots (see attached diff)?

-- 
Dave
Isa. 40:31

Re: BUG: Apache::Cookie saving expired data

[Joe Schaefer <jo...@sunstarsys.com>]

David Robins <db...@davidrobins.net> writes:

> C programmers will probably see what's wrong here: copying a volatile
> pointer  (aliasing bug).  The char* comes from SvPV_nolen(), which
> points into the SV, in this case $v from the 'each'.  When $v changes,
> so will the value of path. When $v goes out of scope and its PV gets
> freed, 'val' will be overwritten by  whatever uses the space.
>
> The same problem occurs in the other generated char* functions,
> e.g. domain,  port, comment, etc.

Exactly- that's another bug we inherited from ExtUtils::XSBuilder, but 
our current svn trunk includes a fix for that.

-- 
Joe Schaefer