httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ken Parzygnat" <kp...@raleigh.ibm.com>
Subject RE: [PATCH] RE: Canonical filename overhaul
Date Mon, 02 Nov 1998 16:15:51 GMT
> > 
> > > Ok, here's the patch for you to play with.  This patch is what
> > > was previously discussed with one addition.  I heard through
> > > the grapevine (credit Marc) that one problem was related to
> > > use of alias names (the loveable "blah~1" type names).  Therefore,
> > > I had another piece of code up my sleeve to address another problem,
> > > but I'm putting it on the table now because it will solve this problem.
> > 
> > This doesn't want to apply for me; the util_win32.c chunks fail
> > completely.
> > 
> > Could you either see if that may be something on your end or just send the
> > whole util_win32.c file?
> > 
> > A few notes:
> > 
> > Get rid of the ap_assert()s.  There is no need for them and it is bad
> > coding practice to use them for "normal" error conditions.  Yea, I
> > know, they are a carryover from the existing code but it is broken.
> 
> There's only one mentioned in the patch that I can see (on the + side),
> and that's:
> 
> +    ap_assert(szFile != NULL);
> +    if (szFile == NULL || strlen(szFile) == 0)
> +        return ap_pstrdup(pPool, "");
> 
> which is either correct, if we don't think you should pass a NULL to
> ap_os_canonical_filename, in which case the following if is wrong, or
> incorrect if the following if is correct.

Yep.  I was thinking that ap_assert was only in affect on debug
builds.  I've changed this in the last repost.

> 
> BTW, wouldn't it make more sense to return NULL if szFile is NULL?

Probably.  I've updated this too.  I was originally thinking that we
needed to return some string.  This sort of falls under your next
comment.

> 
> Double-BTW, there isn't a defined way for ap_os_canonical_filename to
> return an error. Do we want one?
> 
I'm back and forth on this.  In one respect it would be nice to have error
codes to indicate things like NULL parameters and filenames that are way
too long.  But on the other hand, I think that ap_os_canonical_filename
should make the best attempt it can at returning something since we don't
want to make too many assumptions about the input.  I'm really not sold
either way.

Thanks for your comments Ben!
- - - - - - - - - - - - - - - - - -
Ken Parzygnat
email: kparz@raleigh.ibm.com 


Mime
View raw message