Re: $r->location bug

[Torsten Foertsch]

On Mon 18 Feb 2008, Torsten Foertsch wrote:
> Here is a patch and a test case.

Forget the patch. It fixes the wrong place. The only other directive (that I 
know of) that uses the location path ProxyPassReverse also assumes that the 
pointer is valid for the whole request.

So the correct place to approach is our $r->add_config.

Torsten


Index: src/modules/perl/modperl_config.c
===================================================================
--- src/modules/perl/modperl_config.c	(revision 36)
+++ src/modules/perl/modperl_config.c	(working copy)
@@ -624,6 +624,8 @@
     if (!path) {
         /* pass a non-NULL path if nothing else given and for compatibility */
         path = "/";
+    } else {
+        path = apr_pstrdup(r->pool, path);
     }
 
     errmsg = modperl_config_insert(aTHX_
Index: t/response/TestAPI/add_config.pm
===================================================================
--- t/response/TestAPI/add_config.pm	(revision 36)
+++ t/response/TestAPI/add_config.pm	(working copy)
@@ -59,6 +59,14 @@
     };
     $r->pnotes(followsymlinks => "$@");
 
+    eval {
+        my $path="/a/path/to/somewhere";
+        $r->add_config(['PerlResponseHandler '.__PACKAGE__], -1, $path);
+        # now overwrite the path in place to see if the location pointer
+        # is really copied: see apr_pstrdup in modperl_config_insert_request
+        $path=~tr[a-z][n-za-m];
+    };
+
     return Apache2::Const::DECLINED;
 }
 
@@ -83,7 +91,7 @@
     my ($self, $r) = @_;
     my $cf = $self->get_config($r->server);
 
-    plan $r, tests => 8;
+    plan $r, tests => 9;
 
     ok t_cmp $r->pnotes('add_config1'), qr/.+\n/;
     ok t_cmp $r->pnotes('add_config2'), (APACHE22 ? qr/.+\n/ : '');
@@ -103,6 +111,8 @@
     my $opts = APACHE22 ? Apache2::Const::OPT_SYM_LINKS : $expect;
     ok t_cmp $r->allow_override_opts, $opts;
 
+    ok t_cmp $r->location, '/a/path/to/somewhere';
+
     return Apache2::Const::OK;
 }
 
@@ -118,7 +128,6 @@
         <Directory @DocumentRoot@>
             AllowOverride All
         </Directory>
-        PerlResponseHandler TestAPI::add_config
         PerlMapToStorageHandler TestAPI::add_config::map2storage
         PerlFixupHandler TestAPI::add_config::fixup
     </VirtualHost>


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