Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c

["William A. Rowe, Jr." <wr...@rowe-clan.net>]

From: <st...@apache.org>
Sent: Monday, July 30, 2001 10:11 AM


> stoddard    01/07/30 08:11:40
> 
>   Modified:    .        CHANGES
>                modules/generators mod_cgi.c mod_cgid.c
>   Log:
>   Win32: Get exec cgi tag working.  finfo.protection == 0 on Windows from the
>   apr_stat() call in directory walk (because discovering the permissions on
>   Windows is ridiculously expensive).  All we really need to know is whether the
>   file exists.

Wouldn't it make more sense, in this context, to give up if
rr->finfo.filetype != APR_FILE?  It's the responsibility of
dir walk/file walk to resolve symlinks to their target, so
we shouldn't have to test APR_LNK.

>   -    if (rr->finfo.protection == 0) {
>   +    if (rr->finfo.filetype == 0) {

Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c

["William A. Rowe, Jr." <wr...@rowe-clan.net>]

From: "Bill Stoddard" <bi...@wstoddard.com>
Sent: Monday, July 30, 2001 11:17 AM


> > It's the responsibility of
> > dir walk/file walk to resolve symlinks to their target, so
> > we shouldn't have to test APR_LNK.
> 
> Cool! Didn't recognise this.

That is my personal opinion.  My directory_walk rework basically _always_
performs a (required) lstat - first.  If that lstat returns a link, either 
the stat is retrieved and owners are compared, or the request is denied.  
If it returns a file/directory, then it can go on it's merry way, and doesn't 
care that we didn't do an actual stat.

There is a secondary issue.  Any apr_dir_open/read should retrieve lstat's
first, so we can decide how to resolve them.  It takes the onus off of the
ap_sub_request_lookup_dirent() to go back and check for a symlink.  If we
pass ap_sub_request_lookup_dirent() a specific file, it should be from lstat(),
never stat().  Of course ap_sub_request_lookup_file() has to go through the
entire set of gyrations, but that's why I created the optimized case :)

I'll dig out the directory_walk patch and post it again this early this week
(this time, as a patch).  Note we _really_ need the caching optimizations before 
we can consider it production quality, but I'd like to simply get it committed
and drop the back-asswards get_file_info pre-step.

Bill

Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c

[Bill Stoddard <bi...@wstoddard.com>]

> > stoddard    01/07/30 08:11:40
> >
> >   Modified:    .        CHANGES
> >                modules/generators mod_cgi.c mod_cgid.c
> >   Log:
> >   Win32: Get exec cgi tag working.  finfo.protection == 0 on Windows from the
> >   apr_stat() call in directory walk (because discovering the permissions on
> >   Windows is ridiculously expensive).  All we really need to know is whether the
> >   file exists.
>
> Wouldn't it make more sense, in this context, to give up if
> rr->finfo.filetype != APR_FILE?

My first cut was indeed checking filetype != APR_FILE, then I thought about the APR_LNK
issue...

> It's the responsibility of
> dir walk/file walk to resolve symlinks to their target, so
> we shouldn't have to test APR_LNK.

Cool! Didn't recognise this.


Bill