Item13897: Converted Foswiki.pm static _isRedirectSafe() to Foswiki::…

…App object method.
foswiki · Oct 10, 2016 · 2c1cef5 · 2c1cef5
1 parent 95e8a95
commit 2c1cef5
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 62 deletions.
diff --git a/UnitTestContrib/test/unit/ResponseTests.pm b/UnitTestContrib/test/unit/ResponseTests.pm
@@ -16,14 +16,15 @@ around set_up => sub {
     my $this = shift;
     $orig->( $this, @_ );
 
-    $this->_saved->{AllowRedirectUrl} = $Foswiki::cfg{AllowRedirectUrl};
-    $this->_saved->{DefaultUrlHost}   = $Foswiki::cfg{DefaultUrlHost};
+    $this->_saved->{AllowRedirectUrl} =
+      $this->app->cfg->data->{AllowRedirectUrl};
+    $this->_saved->{DefaultUrlHost} = $this->app->cfg->data->{DefaultUrlHost};
     $this->_saved->{PermittedRedirectHostUrls} =
-      $Foswiki::cfg{PermittedRedirectHostUrls};
+      $this->app->cfg->data->{PermittedRedirectHostUrls};
 
-    $Foswiki::cfg{AllowRedirectUrl}          = 0;
-    $Foswiki::cfg{DefaultUrlHost}            = 'http://wiki.server';
-    $Foswiki::cfg{PermittedRedirectHostUrls} = 'http://other.wiki';
+    $this->app->cfg->data->{AllowRedirectUrl}          = 0;
+    $this->app->cfg->data->{DefaultUrlHost}            = 'http://wiki.server';
+    $this->app->cfg->data->{PermittedRedirectHostUrls} = 'http://other.wiki';
 
     return;
 };
@@ -32,9 +33,10 @@ around tear_down => sub {
     my $orig = shift;
     my $this = shift;
 
-    $Foswiki::cfg{AllowRedirectUrl} = $this->_saved->{AllowRedirectUrl};
-    $Foswiki::cfg{DefaultUrlHost}   = $this->_saved->{DefaultUrlHost};
-    $Foswiki::cfg{PermittedRedirectHostUrls} =
+    $this->app->cfg->data->{AllowRedirectUrl} =
+      $this->_saved->{AllowRedirectUrl};
+    $this->app->cfg->data->{DefaultUrlHost} = $this->_saved->{DefaultUrlHost};
+    $this->app->cfg->data->{PermittedRedirectHostUrls} =
       $this->_saved->{PermittedRedirectHostUrls};
 
     $orig->($this);
@@ -306,34 +308,36 @@ sub test_header {
 sub test_isRedirectSafe {
     my ($this) = @_;
 
-    $this->assert( not Foswiki::_isRedirectSafe('http://slashdot.org') );
-    $this->assert( Foswiki::_isRedirectSafe('/relative') );
+    my $app = $this->app;
 
-    #$Foswiki::cfg{DefaultUrlHost} based
-    my $baseUrlMissingSlash = $Foswiki::cfg{DefaultUrlHost};
+    $this->assert( not $app->_isRedirectSafe('http://slashdot.org') );
+    $this->assert( $app->_isRedirectSafe('/relative') );
+
+    #$this->app->cfg->data->{DefaultUrlHost} based
+    my $baseUrlMissingSlash = $this->app->cfg->data->{DefaultUrlHost};
 
     #http://wiki.server.com (missing trailing slash)
     $baseUrlMissingSlash =~ s/(.*)\/$/$1/;
     my $url = $baseUrlMissingSlash;
-    $this->assert( Foswiki::_isRedirectSafe($url) );
+    $this->assert( $app->_isRedirectSafe($url) );
     $url = $baseUrlMissingSlash . '?somestuff=12';
-    $this->assert( Foswiki::_isRedirectSafe($url) );
+    $this->assert( $app->_isRedirectSafe($url) );
     $url = $baseUrlMissingSlash . '#header';
-    $this->assert( Foswiki::_isRedirectSafe($url) );
+    $this->assert( $app->_isRedirectSafe($url) );
 
-    $Foswiki::cfg{DefaultUrlHost} = 'http://wiki.server';
-    $Foswiki::cfg{PermittedRedirectHostUrls} =
+    $this->app->cfg->data->{DefaultUrlHost} = 'http://wiki.server';
+    $this->app->cfg->data->{PermittedRedirectHostUrls} =
       'http://wiki.other,http://other.wiki';
 
-    $this->assert( Foswiki::_isRedirectSafe('/wiki.server') );
-    $this->assert( Foswiki::_isRedirectSafe('http://wiki.server') );
-    $this->assert( Foswiki::_isRedirectSafe('http://wiki.server/') );
-    $this->assert( Foswiki::_isRedirectSafe('http://other.wiki') );
-    $this->assert( Foswiki::_isRedirectSafe('http://other.wiki/') );
-    $this->assert( Foswiki::_isRedirectSafe('http://wiki.other') );
-    $this->assert( Foswiki::_isRedirectSafe('http://wiki.other/') );
-    $this->assert( not Foswiki::_isRedirectSafe('http://slashdot.org') );
-    $this->assert( not Foswiki::_isRedirectSafe('http://slashdot.org/') );
+    $this->assert( $app->_isRedirectSafe('/wiki.server') );
+    $this->assert( $app->_isRedirectSafe('http://wiki.server') );
+    $this->assert( $app->_isRedirectSafe('http://wiki.server/') );
+    $this->assert( $app->_isRedirectSafe('http://other.wiki') );
+    $this->assert( $app->_isRedirectSafe('http://other.wiki/') );
+    $this->assert( $app->_isRedirectSafe('http://wiki.other') );
+    $this->assert( $app->_isRedirectSafe('http://wiki.other/') );
+    $this->assert( not $app->_isRedirectSafe('http://slashdot.org') );
+    $this->assert( not $app->_isRedirectSafe('http://slashdot.org/') );
 
     return;
 }

diff --git a/core/lib/Foswiki.pm b/core/lib/Foswiki.pm
@@ -167,39 +167,6 @@ BEGIN {
 #use Foswiki::Plugins  ();
 #use Foswiki::Users    ();
 
-# Tests if the $redirect is an external URL, returning false if
-# AllowRedirectUrl is denied
-sub _isRedirectSafe {
-    my $redirect = shift;
-
-    return 1 if ( $Foswiki::cfg{AllowRedirectUrl} );
-
-    # relative URL - OK
-    return 1 if $redirect =~ m#^/#;
-
-    #TODO: this should really use URI
-    # Compare protocol, host name and port number
-    if ( $redirect =~ m!^(.*?://[^/?#]*)! ) {
-
-        # implicit untaints OK because result not used. uc retaints
-        # if use locale anyway.
-        my $target = uc($1);
-
-        $Foswiki::cfg{DefaultUrlHost} =~ m!^(.*?://[^/]*)!;
-        return 1 if ( $target eq uc($1) );
-
-        if ( $Foswiki::cfg{PermittedRedirectHostUrls} ) {
-            foreach my $red (
-                split( /\s*,\s*/, $Foswiki::cfg{PermittedRedirectHostUrls} ) )
-            {
-                $red =~ m!^(.*?://[^/]*)!;
-                return 1 if ( $target eq uc($1) );
-            }
-        }
-    }
-    return 0;
-}
-
 =begin TML
 
 ---++ StaticMethod splitAnchorFromUrl( $url ) -> ( $url, $anchor )

diff --git a/core/lib/Foswiki/App.pm b/core/lib/Foswiki/App.pm
@@ -835,7 +835,7 @@ sub redirect {
     # prevent phishing by only allowing redirect to configured host
     # do this check as late as possible to catch _any_ last minute hacks
     # TODO: this should really use URI
-    if ( !Foswiki::_isRedirectSafe($url) ) {
+    if ( !$this->_isRedirectSafe($url) ) {
 
         # goto oops if URL is trying to take us somewhere dangerous
         $url = $this->cfg->getScriptUrl(
@@ -895,7 +895,7 @@ sub redirectto {
     if ( $redirecturl =~ m#^$regex{linkProtocolPattern}://# ) {
 
         # assuming URL
-        return $redirecturl if Foswiki::_isRedirectSafe($redirecturl);
+        return $redirecturl if $this->_isRedirectSafe($redirecturl);
         return;
     }
 
@@ -1701,6 +1701,42 @@ sub _validateWTA {
     return ( $w, $t, $a );
 }
 
+# Tests if the $redirect is an external URL, returning false if
+# AllowRedirectUrl is denied
+sub _isRedirectSafe {
+    my $this     = shift;
+    my $redirect = shift;
+
+    my $cfgData = $this->cfg->data;
+
+    return 1 if ( $cfgData->{AllowRedirectUrl} );
+
+    # relative URL - OK
+    return 1 if $redirect =~ m#^/#;
+
+    #TODO: this should really use URI
+    # Compare protocol, host name and port number
+    if ( $redirect =~ m!^(.*?://[^/?#]*)! ) {
+
+        # implicit untaints OK because result not used. uc retaints
+        # if use locale anyway.
+        my $target = uc($1);
+
+        $cfgData->{DefaultUrlHost} =~ m!^(.*?://[^/]*)!;
+        return 1 if ( $target eq uc($1) );
+
+        if ( $cfgData->{PermittedRedirectHostUrls} ) {
+            foreach my $red (
+                split( /\s*,\s*/, $cfgData->{PermittedRedirectHostUrls} ) )
+            {
+                $red =~ m!^(.*?://[^/]*)!;
+                return 1 if ( $target eq uc($1) );
+            }
+        }
+    }
+    return 0;
+}
+
 =begin TML
 
 ---+++ ObjectMethod getUrlHost( ) -> $host