hi all

just following up on a thread in new-httpd from the weekend :)

in mp1, whenever a user set the filename via $r->filename($newfile) mod_perl would update r->finfo as well. the reason is because things like default_handler do this

        ap_update_mtime(r, r->finfo.mtime);
...
        ap_set_content_length(r, r->finfo.size);
        if ((errstatus = ap_meets_conditions(r)) != OK) {
...

so, if a handler sets r->filename after translation, the wrong value will be used in the meets_conditions test, and the client will get bad data.

the attached patch carries this over to mp2.

the only outstanding issue for me is that in mp1, there was an exception for Win32, which I didn't carry over, figuring apr now takes care of the compat issues. I'm also hoping that utime (used in the test suite) is portable :) so, I'd like to get some feedback/test results from Win32 before porting this functionality to mp2.

--Geoff
Index: xs/Apache/RequestRec/Apache__RequestRec.h
===================================================================
RCS file: /home/cvspublic/modperl-2.0/xs/Apache/RequestRec/Apache__RequestRec.h,v
retrieving revision 1.6
diff -u -r1.6 Apache__RequestRec.h
--- xs/Apache/RequestRec/Apache__RequestRec.h   14 Mar 2003 05:34:24 -0000      1.6
+++ xs/Apache/RequestRec/Apache__RequestRec.h   27 Oct 2003 19:13:13 -0000
@@ -53,3 +53,33 @@
     return modperl_table_get_set(aTHX_ r->subprocess_env,
                                  key, val, TRUE);
 }
+
+static MP_INLINE
+char *mpxs_Apache__RequestRec_filename(pTHX_ request_rec *r, 
+                                             SV *name)
+{
+    char *retval = r->filename;
+
+    if (name) {
+        STRLEN len;
+        const char *val = SvPV(name, len);
+
+        MP_TRACE_o(MP_FUNC, "setting r->filename to %s\n", 
+                   val);
+
+        /* set r->filename to the incoming value */
+        r->filename = apr_pstrndup(r->pool, val, len);
+
+        /* and update r->finfo so later calcuations work properly */
+        apr_status_t rv = apr_stat(&r->finfo, r->filename, 
+                                   APR_FINFO_MIN, r->pool);
+
+        if (rv != APR_SUCCESS) {
+             MP_TRACE_o(MP_FUNC, "unable to update finfo for %s\n", 
+                        name);
+             r->finfo.filetype = 0;
+        }
+    }
+
+    return retval;
+}
Index: xs/maps/apache_structures.map
===================================================================
RCS file: /home/cvspublic/modperl-2.0/xs/maps/apache_structures.map,v
retrieving revision 1.19
diff -u -r1.19 apache_structures.map
--- xs/maps/apache_structures.map       19 Feb 2003 14:12:02 -0000      1.19
+++ xs/maps/apache_structures.map       27 Oct 2003 19:13:13 -0000
@@ -51,7 +51,7 @@
    no_local_copy
    unparsed_uri
    uri
-   filename
+~  filename
    canonical_filename
    path_info
    args
Index: xs/maps/modperl_functions.map
===================================================================
RCS file: /home/cvspublic/modperl-2.0/xs/maps/modperl_functions.map,v
retrieving revision 1.58
diff -u -r1.58 modperl_functions.map
--- xs/maps/modperl_functions.map       30 Aug 2003 02:33:26 -0000      1.58
+++ xs/maps/modperl_functions.map       27 Oct 2003 19:13:13 -0000
@@ -15,6 +15,7 @@
  mpxs_Apache__RequestRec_content_type   | | r, type=Nullsv
  mpxs_Apache__RequestRec_proxyreq       | | r, val=Nullsv
  mpxs_Apache__RequestRec_subprocess_env | | r, key=NULL, val=Nullsv
+ mpxs_Apache__RequestRec_filename       | | r, name=Nullsv
 
 MODULE=Apache::RequestUtil   PACKAGE=guess
  mpxs_Apache__RequestRec_push_handlers
Index: xs/tables/current/ModPerl/FunctionTable.pm
===================================================================
RCS file: /home/cvspublic/modperl-2.0/xs/tables/current/ModPerl/FunctionTable.pm,v
retrieving revision 1.126
diff -u -r1.126 FunctionTable.pm
--- xs/tables/current/ModPerl/FunctionTable.pm  14 Oct 2003 17:32:40 -0000      1.126
+++ xs/tables/current/ModPerl/FunctionTable.pm  27 Oct 2003 19:13:17 -0000
@@ -5544,6 +5544,24 @@
     ]
   },
   {
+    'return_type' => 'char *',
+    'name' => 'mpxs_Apache__RequestRec_filename',
+    'args' => [
+      {
+        'type' => 'PerlInterpreter *',
+        'name' => 'my_perl'
+      },
+      {
+        'type' => 'request_rec *',
+        'name' => 'r'
+      },
+      {
+        'type' => 'SV *',
+        'name' => 'name'
+      }
+    ]
+  },
+  {
     'return_type' => 'SV *',
     'name' => 'mpxs_Apache__RequestRec_get_handlers',
     'args' => [

--- /dev/null   2003-01-30 05:24:37.000000000 -0500
+++ t/htdocs/modperl/change.html        2003-10-27 14:04:21.000000000 -0500
@@ -0,0 +1,2 @@
+file used in t/modperl/filename.t
+$r->filename('htdocs/modperl/change.html');

--- /dev/null   2003-01-30 05:24:37.000000000 -0500
+++ t/modperl/filename.t        2003-10-27 11:46:00.000000000 -0500
@@ -0,0 +1,92 @@
+use strict;
+use warnings FATAL => 'all';
+
+use Apache::Test;
+use Apache::TestRequest;
+use Apache::TestUtil qw(t_cmp t_debug);
+
+use File::Spec::Functions qw(catfile);
+
+use Apache2;
+use Apache::Const -compile => qw(HTTP_OK HTTP_NOT_MODIFIED);
+
+plan tests => 8;
+
+my $base = "/TestModperl__filename";
+my $last_modified;
+
+# first, touch the two files we will be using,
+# just to be sure we're in sync
+
+ok utime(undef, undef, catfile(qw(htdocs index.html)));
+ok utime(undef, undef, catfile(qw(htdocs modperl change.html)));
+
+# now, get index.html by itself
+{
+    my $uri = "$base/index.html";
+
+    my $response = GET $uri;
+    ok t_cmp(Apache::HTTP_OK,
+             $response->code,
+             "GET $uri");
+
+    # capture the Last-Modified header from index.html
+    $last_modified = $response->header('Last-Modified');
+}
+
+# send the same request with conditional headers
+# just to make sure things are going as planned
+{
+    my $uri = "$base/index.html";
+
+    my $response = GET $uri, 'If-Modified-Since' => $last_modified;
+    ok t_cmp(Apache::HTTP_NOT_MODIFIED,
+             $response->code,
+             "GET $uri, If-Modified-Since: $last_modified");
+}
+
+# touch index.html so that requests to it get 200
+# when using the old Last-Modified date
+
+sleep 2;  # make sure we get a time difference
+ok utime(undef, undef, catfile(qw(htdocs index.html)));
+
+# the setup is over, now for the real tests...
+
+# at this point, we have index.html and modperl/change.html
+# with different mtimes on the filesystem.  we expect
+# that using the same If-Modified-Since header would yield
+# different results: index.html ought to get 200, since it
+# has been changed, while modperl/change.html ought to get
+# 304 since it has not changed.
+{
+    my $uri = "$base/index.html";
+
+    my $response = GET $uri, 'If-Modified-Since' => $last_modified;
+    ok t_cmp(Apache::HTTP_OK,
+             $response->code,
+             "GET $uri, If-Modified-Since: $last_modified");
+}
+
+{
+    my $uri = "$base/modperl/change.html";
+
+    my $response = GET $uri, 'If-Modified-Since' => $last_modified;
+    ok t_cmp(Apache::HTTP_NOT_MODIFIED,
+             $response->code,
+             "GET $uri, If-Modified-Since: $last_modified");
+}
+
+# if $r->filename($newfile) didn't also update r->finfo.mtime
+# apache would use the mtime from index.html and not the 
+# file that was just set.  so, if things are going as expected
+# we should get the 304 from modperl/change.html and not the
+# 200 from index.html
+{
+    my $uri = "$base/index.html?modperl/change.html";
+
+    my $response = GET $uri, 'If-Modified-Since' => $last_modified;
+    ok t_cmp(Apache::HTTP_NOT_MODIFIED,
+             $response->code,
+             "GET $uri, If-Modified-Since: $last_modified");
+}

--- /dev/null   2003-01-30 05:24:37.000000000 -0500
+++ t/response/TestModperl/filename.pm  2003-10-27 12:34:44.000000000 -0500
@@ -0,0 +1,31 @@
+package TestModperl::filename;
+
+use strict;
+use warnings FATAL => 'all';
+
+use File::Spec::Functions qw(catfile);
+
+use Apache::ServerUtil ();
+
+use Apache::Const -compile => 'OK';
+
+sub handler {
+
+    my $r = shift;
+
+    if (my $new_filename = $r->args) {
+        $r->filename(Apache->server_root_relative(
+                       catfile('htdocs', $new_filename)));
+    }
+
+    return Apache::OK;
+}
+
+1;
+__DATA__
+<NoAutoConfig>
+    Alias /TestModperl__filename @DocumentRoot@
+    <Location /TestModperl__filename>
+        PerlFixupHandler TestModperl::filename
+    </Location>
+</NoAutoConfig>

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to