Date: Mon, 4 Aug 1997 17:50:54 -0700 (PDT)
From: Dean Gaudet <dgaudet@arctic.org>
To: new-httpd@apache.org
Subject: Re: segfaults on CGI's with current CVS snapshot
In-Reply-To: <Pine.BSF.3.95q.970804183956.5502O-100000@valis.worldgate.com>
Message-ID: <Pine.LNX.3.95dg3.970804174140.15340X-100000@twinlark.arctic.org>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: new-httpd-owner@apache.org
Precedence: bulk
Reply-To: new-httpd@apache.org

Then that is probably the correct fix.  Up to my optimizations in rev 1.54
of http_request.c, it was effectively a private copy of the directory
string that was passed into parse_htaccess.  I didn't notice the lack of
"const" on that parm ... and should have suspected it ... 

As to why it's tickled by Lou's patches and not otherwise ... I'm not
sure.  Hmm, maybe this isn't the bug really, maybe something else is
futzing with cmd->path which shouldn't be, but it's not made obvious until
the memory is arranged the way it is by Lou's changes.

ACK!  Mea culpa.  Damn.  I still don't know why you're not seeing it with
anything 1.54 or later ... but, well... test_dirname *CHANGES* as
directory_walk moves along, this is the optimization I made.  This means
it's corrupting the cmd->path that everything was passed during
parse_htaccess. 

I think the fix is to change parse_htaccess so that "char *d" is "const
char *d", and it does a pstrdup when setting parms.path... note there's
already a pstrdup a little later on down, and the two probably can/should
share the same result. 

Dean

On Mon, 4 Aug 1997, Marc Slemko wrote:

> The pstrdup seems to fix it.  Hmm.
> 
> On Mon, 4 Aug 1997, Dean Gaudet wrote:
> 
> > 
> > 
> > On Mon, 4 Aug 1997, Marc Slemko wrote:
> > 
> > > On Mon, 4 Aug 1997, Dean Gaudet wrote:
> > > 
> > > > Can you post the exact set of changes you're backing out?  'cause as I say
> > > > that change to http_request there doesn't stand alone, you need the
> > > > parse_htaccess change as well.
> > > > 
> > > > I would believe that the changes around rev 1.57 of http_request.c could
> > > > corrupt memory... I may have committed an off-by-1 booboo.
> > > 
> > > Everything in that one commit message.
> > > 
> > > If I start with a tree from right after the below changes were made, it
> > > fails.
> > > 
> > > If I backout the below changes as follows:
> > > 
> > > http_config.c 1.56 -> 1.55
> > > http_config.h 1.33 -> 1.32
> > > http_core.c 1.91 -> 1.90
> > > http_request.c 1.58 -> 1.57
> > > 
> > > the problem stops.  I can't see anything obvious there.
> > > 
> > > What is:
> > > 
> > >   !     else {
> > >   !     filename = make_full_path(r->pool, d, 0);
> > >   !     f=pfopen(r->pool, filename, "r");
> > >   !     }
> > > 
> > > supposed to do?  No, that isn't the problem.
> > 
> > I was just checking that myself -- it shouldn't ever be executed.  It's
> > only used if sconf->access_conf == NULL, but the final server merge during
> > config inherits sconf->access_conf from the main server to the vhosts.  So
> > it should be removed.
> > 
> > Ok, in directory_walk try adding a pstrdup around the test_dirname
> > parameter to parse_htaccess. 
> > 
> > Dean
> > 
> > 
> 
>