On Wed, 02 Mar 2005 18:32:40 -0500, Stas Bekman <[EMAIL PROTECTED]> wrote:
failing to open a file will cover the -r check. What will cover the -x check? Or do you suggest that we shouldn't require cgi scripts to be executable? At least mod_cgi requires so when doing exec, but we don't exec. So we should probably keep it in, no?
I'd say no, because we're not really exec'ing anything. It will be a
user education issue, that PerlRun/Registry scripts will run if you
tell Apache to handle then and ExecCGI is enabled in that context. Plus it breaks my whole scheme to make mod_perl ACL compatible...
That's fine with me. Unless other developers disagree we can go with it. [...]
I managed to come up with something very similar to this, minus the
last bit. I like it, but I did manage to (partially) implement some
code on the C/XS side to return different statuses based on whether
the file is unreadable or non-existent. It is broken because I can't
figure out how to return the script from modperl_slurp_filehandle() to
$self->{CODE}. I just started learning XS today ;^)... I was able to
make missing or unreadable files return a proper status code at least.
You don't need to do any XS changes. Just the following will do:
$self->{CODE} = eval { $self->{REQ}->slurp_filename(0) }; # untainted
if ($@) {
$self->log_error("$@");
$rc = $@ == APR::EACCES ? Apache::FORBIDDEN : Apache::NOT_FOUND;
}$@ is a magic object under mod_perl 2. Take a look at: http://perl.apache.org/docs/2.0/api/APR/Error.html
Your case is certainly faulty:
+ if ($rc == APR::EACCES) {
+ return Apache::DECLINED;why declined and not forbidden?
+ } elsif ($rc == APR::ENOENT) {
+ return Apache::NOT_FOUND;
+ } else {
+ return Apache::OK;why default to Apache::OK, which may miss some other errors you didn't encounter for. We should either default to Apache::NOT_FOUND or 500, but see below.
Further APR::EACCES will be given no matter whether the file exists or its perms are not right. So you will never end up with Apache::NOT_FOUND.
You can see that tests like t/404.t are failing with it, when using this version in addition to my last patch:
sub read_script {
my $self = shift;
my $rc = Apache::OK; $self->debug("reading $self->{FILENAME}") if DEBUG & D_NOISE;
$self->{CODE} = eval { $self->{REQ}->slurp_filename(0) }; # untainted
if ($@) {
$rc = $@ == APR::EACCES ? Apache::FORBIDDEN : Apache::NOT_FOUND;
$self->log_error("$@");
}return $rc; }
I'd rather always return Apache::NOT_FOUND, and log_error() logs the right error message. I don't think the error code makes any difference to the user, when they can't reach the page.
regarding changing the slurp_filename() API: passing a buffer by reference is ugly and not perlish (though we do it sometimes where we have no choice). Here $@ gives us the exact error code and stringified error message if wanted. Ideally it should have been $!, but unfortunately it's impossible to extend perl-core functionality on this one. Hence the $@ object.
-- __________________________________________________________________ Stas Bekman JAm_pH ------> Just Another mod_perl Hacker http://stason.org/ mod_perl Guide ---> http://perl.apache.org mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com http://modperlbook.org http://apache.org http://ticketmaster.com
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
