On Thu, 03 Mar 2005 08:28:26 -0500, Stas Bekman <[EMAIL PROTECTED]> wrote:
> Damon Buckwalter wrote:
> > Software error:
> >
> > Argument "Error opening '/home/www/foo.cgi...." isn't numeric in
> > numeric eq (==) at /usr/lib/perl5/Apache2/ModPerl/RegistryCooker.pm
> > line 543.
> >
> > Looking at the APR::Error docs, I changed this to something like:
> >
> > $rc = ref $@ eq 'APR::Error' && $@ == APR::EACCES ? Apache::FORBIDDEN
> > : Apache::NOT_FOUND
> >
> > But I only ever get NOT_FOUND, even for files that exist but do not
> > have read permisson.
>
> Right, because what I said was bogus, slurp_filename didn't throw an
> APR::Error object. Please try with this patch:
>
[patch snipped]
The patch works nicely, but every error code returns FORBIDDEN still.
I have a tweaked version attached that I think works better.
> sure, let's give them best info we can. with the patch above you will get
> the exact rc.
>
I had to dig into [EMAIL PROTECTED]>{rc} to get the exact return code. $@ ==
APR::EACCES seemed to always evaluate true, even though the error log
showed different error codes for file not found versus permission
denied:
[Thu Mar 03 10:22:14 2005] [error] slurp_filename('/home/damon/test/apache2/htdo
cs/mp/notexist.cgi') / opening: (2) No such file or directory at /home/damon/tes
t/perl/lib/site_perl/5.9.1/i686-linux-thread-multi-64int-ld/ModPerl/RegistryCook
er.pm line 540
[Thu Mar 03 10:22:14 2005] [error] rc = 2 EACCESS = 13 ENOENT = 2
[Thu Mar 03 10:41:08 2005] [error] slurp_filename('/home/damon/test/apache2/htdo
cs/mp/unreadable.cgi') / opening: (13) Permission denied at /home/damon/test/per
l/lib/site_perl/5.9.1/i686-linux-thread-multi-64int-ld/ModPerl/RegistryCooker.pm
line 540
[Thu Mar 03 10:41:08 2005] [error] rc = 13 EACCESS = 13 ENOENT = 2
> > I think it is a bit confusing to return 404 if the script is there,
> > just not readable.
>
> user-wise it doesn't make any difference. developers get a detailed error
> message in the logs. May be I'm biased, since when I develop my code I
> always first watch the error_log and only then the browser :)
>
> but as I said above if we can match mod_cgi behavior, that would be perfect.
See my patch below, which includes your changes. I believe that this
matches mod_cgi behavior pretty closely, returning FORBIDDEN on
EACCES, NOT_FOUND on ENOENT, and SERVER_ERROR by default if there is
an error in [EMAIL PROTECTED]
Index: src/modules/perl/modperl_util.c
===================================================================
--- src/modules/perl/modperl_util.c (revision 156065)
+++ src/modules/perl/modperl_util.c (working copy)
@@ -597,11 +597,13 @@
return (svp && *svp != &PL_sv_undef) ? 1 : 0;
}
-#define SLURP_SUCCESS(action) \
- if (rc != APR_SUCCESS) { \
- SvREFCNT_dec(sv); \
- Perl_croak(aTHX_ "Error " action " '%s': %s ", r->filename, \
- modperl_error_strerror(aTHX_ rc)); \
+#define SLURP_SUCCESS(action) \
+ if (rc != APR_SUCCESS) { \
+ SvREFCNT_dec(sv); \
+ modperl_croak(aTHX_ rc, \
+ apr_psprintf(r->pool, \
+ "slurp_filename('%s') / " action, \
+ r->filename)); \
}
MP_INLINE SV *modperl_slurp_filename(pTHX_ request_rec *r, int tainted)
Index: ModPerl-Registry/lib/ModPerl/RegistryCooker.pm
===================================================================
--- ModPerl-Registry/lib/ModPerl/RegistryCooker.pm (revision 156065)
+++ ModPerl-Registry/lib/ModPerl/RegistryCooker.pm (working copy)
@@ -41,6 +41,7 @@
use File::Spec::Functions ();
use File::Basename;
+use APR::Const -compile => qw(EACCES ENOENT);
use Apache::Const -compile => qw(:common &OPT_EXECCGI);
use ModPerl::Const -compile => 'EXIT';
@@ -254,21 +255,10 @@
my $self = shift;
my $r = $self->{REQ};
- unless (-r $r->my_finfo && -s _) {
- $self->log_error("$self->{FILENAME} not found or unable to stat");
- return Apache::NOT_FOUND;
- }
+ return Apache::DECLINED if -d $r->my_finfo;
- return Apache::DECLINED if -d _;
-
$self->{MTIME} = -M _;
- unless (-x _ or IS_WIN32) {
- $r->log_error("file permissions deny server execution",
- $self->{FILENAME});
- return Apache::FORBIDDEN;
- }
-
if (!($r->allow_options & Apache::OPT_EXECCGI)) {
$r->log_error("Options ExecCGI is off in this directory",
$self->{FILENAME});
@@ -372,10 +362,13 @@
sub convert_script_to_compiled_handler {
my $self = shift;
+ my $rc = Apache::OK;
+
$self->debug("Adding package $self->{PACKAGE}") if DEBUG & D_NOISE;
# get the script's source
- $self->read_script;
+ $rc = $self->read_script;
+ return $rc unless $rc == Apache::OK;
# convert the shebang line opts into perl code
$self->rewrite_shebang;
@@ -408,7 +401,7 @@
${ $self->{CODE} },
"\n}"; # last line comment without newline?
- my $rc = $self->compile(\$eval);
+ $rc = $self->compile(\$eval);
return $rc unless $rc == Apache::OK;
$self->debug(qq{compiled package \"$self->{PACKAGE}\"}) if DEBUG & D_NOISE;
@@ -534,16 +527,33 @@
# dflt: read_script
# desc: reads the script in
# args: $self - registry blessed object
-# rtrn: nothing
+# rtrn: Apache::OK on success, some other code on failure
# efct: initializes the CODE field with the source script
#########################################################################
# reads the contents of the file
sub read_script {
my $self = shift;
+ my $rc = Apache::OK;
$self->debug("reading $self->{FILENAME}") if DEBUG & D_NOISE;
- $self->{CODE} = $self->{REQ}->slurp_filename(0); # untainted
+ $self->{CODE} = eval { $self->{REQ}->slurp_filename(0) }; # untainted
+
+ if ($@) {
+ $self->log_error("$@");
+
+ if (ref $@ eq 'APR::Error') {
+ if ([EMAIL PROTECTED]>{rc} == APR::EACCES) {
+ $rc = Apache::FORBIDDEN;
+ } elsif ([EMAIL PROTECTED]>{rc} == APR::ENOENT) {
+ $rc = Apache::NOT_FOUND;
+ } else {
+ $rc = Apache::SERVER_ERROR;
+ }
+ }
+ }
+
+ return $rc;
}
#########################################################################
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
