apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject Re: cvs commit: apr/include apr_file_info.h
Date Wed, 31 Jan 2001 21:22:10 GMT
> stoddard    01/01/31 12:01:16
>   Modified:    modules/http http_request.c
>                file_io/win32 filestat.c
>                include  apr_file_info.h
>   Log:
>   apr_stat() in http_request.c only needs size, type, mtime, ctime & atime values
>   the file. Modify apr_stat() under windows to accomodate apr_stat( APR_FINFO_MIN)
>   -        rv = apr_stat(&r->finfo, path, APR_FINFO_NORM, r->pool);
>   +        rv = apr_stat(&r->finfo, path, APR_FINFO_MIN, r->pool);

Good call, but we need to go back through the entire server to find 
out where we assume to know every APR_FINFO_ field.
>   Index: filestat.c
>   ===================================================================
>   RCS file: /home/cvs/apr/file_io/win32/filestat.c,v
>   @@ -445,10 +445,20 @@
>            finfo->filetype = APR_REG;
>        }
>   -    if (FileInformation.dwFileAttributes & FILE_ATTRIBUTE_READONLY)
>   -        finfo->protection = APR_FREADONLY;

This -was- an accessor for win9x to walk around the absence of 'security'.
this patch breaks that delegation, IMHO.  This bit needs to remain, and
the general schema of remain abstract, if not in the existing
more info part, than a new private protection_from_attributes() function?

>   +    /* 
>   +     * Hummm, should we assume the file is always executable? Is there a way
>   +     * to know other than guess based on the file extension or make an 
>   +     * expensive system call?
>   +     */

I see no reason to, myself.  Unless we coopt another attribute (such as the
archive bit, which is set every time the user twiddles with the file and
could be reset by the admin when it's 'approved'.)

We don't share this concept in the same way as unix, and win32 folks don't
play the same stratagies as a unix admin.  I have a post following on this.

>   +    if (FileInformation.dwFileAttributes & FILE_ATTRIBUTE_READONLY) {
>   +        finfo->protection |= S_IREAD | S_IEXEC;
>   +    }
>   +    else {
>   +        finfo->protection |= S_IREAD | S_IWRITE | S_IEXEC;
>   +    }

Did you mean to set the APR_ protection bits?  I'd suggest again we abstract
this out, and remove the redundant code from the more info function.

The bigger issue is that this isn't really the case.  You aren't returning
the real APR_FINFO_UPROT (and didn't claim to).  But it isn't valid to test
any field that isn't tagged in the bit mask.

We really should consider making this whole problem less ambigous.  What
do we test in the server?  User perms?  I guess my question is, who cares 
that the user can execute the file, when the server's permissions don't 
allow it.  Leads to a 'new' thought...

How trivial do you suppose it is to create the concept of 'effective'
security, that is, beyond the scope of World.  On unix, if daemonuser==user
or daemongroup==group then copy those permissions.  Finally, if it's
neither, then copy the world perms.

On win32 we could implement this two ways.  One is to get the effective
permissions for current user [less expensive than quering for a random
user, but probably still expensive].  Or we could base it on the test you
see here.

I'll summarize the .protection issue in a second message.

View raw message