Item13478: after sleeping on it, concluded that it is *not* appropria…

…te to decode either path_info or query_string. Both of these strings should always be byte strings, url-encoded if necessary, and it's not the engine's business to decode them. Callers may decode when - for example - extracting path components such as web and topic names from the path_info, or parameter values from the query_string. Note also that when composing a URL, it is essential to urlEncode - however this is not necessary with the query_string, as it is never urlDecoded

EmptyPlugin/lib/Foswiki/Plugins/EmptyPlugin.pm

@@ -263,7 +263,7 @@ exceptions handled by =Foswiki::UI= (for example, =Foswiki::OopsException=).
 ---++ initializeUserHandler( $loginName, $url, $pathInfo )
    * =$loginName= - login name recovered from $ENV{REMOTE_USER}
    * =$url= - request url
-   * =$pathInfo= - pathinfo from the CGI query
+   * =$path_info= - path_info from the Foswiki::Request
 Allows a plugin to set the username. Normally Foswiki gets the username
 from the login manager. This handler gives you a chance to override the
 login manager.
@@ -277,7 +277,7 @@ This handler is called very early, immediately after =earlyInitPlugin=.
 =cut
 
 #sub initializeUserHandler {
-#    my ( $loginName, $url, $pathInfo ) = @_;
+#    my ( $loginName, $url, $path_info ) = @_;
 #}
 
 =begin TML

HomePagePlugin/lib/Foswiki/Plugins/HomePagePlugin.pm

@@ -56,7 +56,8 @@ sub initializeUserHandler {
 
     # we don't know the user at this point so can only set up the
     # site wide default
-    my $path_info = $Foswiki::Plugins::SESSION->{request}->path_info();
+    my $path_info =
+      Foswiki::urlDecode( $Foswiki::Plugins::SESSION->{request}->path_info() );
 
     return
       unless ( ( $path_info eq '' or $path_info eq '/' )

TablePlugin/lib/Foswiki/Plugins/TablePlugin/Core.pm

@@ -673,8 +673,8 @@ sub _processTableRow {
                     my $nRows = scalar(@curTable);
                     my $rspan = $rowspan[$col] + 1;
                     if ( $rspan > 1 ) {
-                        $curTable[ $nRows - $rspan ][$col]->{attrs}->{rowspan}
-                          = $rspan;
+                        $curTable[ $nRows - $rspan ][$col]->{attrs}->{rowspan} =
+                          $rspan;
                     }
                     undef( $rowspan[$col] );
                 }
@@ -1895,8 +1895,6 @@ sub handler {
         $url = $cgi->url( -absolute => 1, -path => 1 ) . '?';
         my $queryString = $cgi->query_string();
         if ($queryString) {
-            $queryString = Foswiki::decode_utf8($queryString)
-              if $Foswiki::UNICODE;
             $url .= $queryString . ';';
         }
 

core/lib/Foswiki.pm

@@ -1485,7 +1485,11 @@ sub getSkin {
 Returns the URL to a Foswiki script, providing the web and topic as
 "path info" parameters.  The result looks something like this:
 "http://host/foswiki/bin/$script/$web/$topic".
-   * =...= - an arbitrary number of name,value parameter pairs that will be url-encoded and added to the url. The special parameter name '#' is reserved for specifying an anchor. e.g. <tt>getScriptUrl('x','y','view','#'=>'XXX',a=>1,b=>2)</tt> will give <tt>.../view/x/y?a=1&b=2#XXX</tt>
+   * =...= - an arbitrary number of name,value parameter pairs that will
+be url-encoded and added to the url. The special parameter name '#' is
+reserved for specifying an anchor. e.g.
+=getScriptUrl('x','y','view','#'=>'XXX',a=>1,b=>2)= will give
+=.../view/x/y?a=1&b=2#XXX=
 
 If $absolute is set, generates an absolute URL. $absolute is advisory only;
 Foswiki can decide to generate absolute URLs (for example when run from the
@@ -1501,6 +1505,9 @@ $script is not given, absolute URLs will always be generated.
 
 If either the web or the topic is defined, will generate a full url (including web and topic). Otherwise will generate only up to the script name. An undefined web will default to the main web name.
 
+As required by RFC3986, the returned URL will only contain the
+allowed characters -A-Za-z0-9_.~!*\'();:@&=+$,/?%#[]
+
 =cut
 
 sub getScriptUrl {
@@ -1554,8 +1561,8 @@ sub getScriptUrl {
 =begin TML
 
 ---++ StaticMethod make_params(...)
-Generate a URL parameters string from parameters given. A parameter named '#' will
-generate an anchor.
+Generate a URL parameters string from parameters given. A parameter
+named '#' will generate a fragment identifier.
 
 =cut
 
@@ -1611,6 +1618,9 @@ Note: for compatibility with older plugins, which use %PUBURL*% with
 a constructed URL path, do not use =*= unless =web=, =topic=, and
 =attachment= are all specified.
 
+As required by RFC3986, the returned URL will only contain the
+allowed characters -A-Za-z0-9_.~!*\'();:@&=+$,/?%#[]
+
 =cut
 
 sub getPubURL {
@@ -2164,17 +2174,20 @@ sub new {
         $this->{scriptUrlPath} = $1;
     }
 
-# The web/topic can be provided by either the query path_info,  or by URL Parameters
-# urlparams  topic:       Specifies web.topic or topic.   Overrides the path given in the URL
-#            defaultweb:  Overrides the default web, for use when topic= does not provide a web.
-# path_info  web/topic    Always provdes a web,   if topic not provided, defaults to the Home topic
+    # The web/topic can be provided by either the query path_info,
+    # or by URL Parameters:
+    # topic:       Specifies web.topic or topic.
+    #              Overrides the path given in the URL
+    # defaultweb:  Overrides the default web, for use when topic=
+    #              does not provide a web.
+    # path_info    Defaults to the Users web Home topic
 
     # Set the default for web
     # Development.AddWebParamToAllCgiScripts: enables
     # bin/script?topic=WebPreferences;defaultweb=Sandbox
     my $defaultweb = $query->param('defaultweb') || $Foswiki::cfg{UsersWebName};
 
-    my $webtopic      = $query->path_info() || '';
+    my $webtopic      = urlDecode( $query->path_info() || '' );
     my $topicOverride = '';
     my $topic         = $query->param('topic');
     if ( defined $topic ) {
@@ -2191,8 +2204,8 @@ sub new {
         }
     }
 
-# SMELL Scripts like rest, jsonrpc,  don't use web/topic path.  so this ends up all bogus
-# but doesn't do any harm.
+    # SMELL Scripts like rest, jsonrpc,  don't use web/topic path.
+    # So this ends up all bogus, but doesn't do any harm.
 
     ( my $web, $topic ) =
       $this->_parsePath( $webtopic, $defaultweb, $topicOverride );

core/lib/Foswiki/LoginManager.pm

@@ -632,7 +632,7 @@ sub redirectToLoggedOutUrl {
     );
 
     #TODO: consider if we should risk passing on the urlparams on logout
-    my $path_info = $session->{request}->path_info();
+    my $path_info = Foswiki::urlDecode( $session->{request}->path_info() );
     if ( my $topic = $session->{request}->param('topic') )
     {    #we should at least respect the ?topic= request
         my $topicRequest = Foswiki::Sandbox::untaintUnchecked($topic);
@@ -645,6 +645,8 @@ sub redirectToLoggedOutUrl {
         $path_info = substr( $path_info, 0, ( ( pos $path_info ) - 1 ) );
     }
 
+    $path_info = Foswiki::urlEncode($path_info);
+
     my $redirectUrl;
     if ( $Foswiki::cfg{ForceDefaultUrlHost} ) {
 

core/lib/Foswiki/Request.pm

@@ -171,6 +171,11 @@ Called without parameters returns current pathInfo.
 
 There is a =path_info()= alias for compatibility with CGI.
 
+Note that the string returned is a *URL encoded byte string*
+i.e. it will only contain characters -A-Za-z0-9_.~!*\'();:@&=+$,/?%#[]
+If you intend to analyse it, you will probably have to
+Foswiki::urlDecode it first.
+
 =cut
 
 *path_info = \&pathInfo;
@@ -212,6 +217,11 @@ Returns query_string part of request uri, if any.
 
 =query_string()= alias provided for compatibility with CGI.
 
+Note that the string returned is a *URL encoded byte string*
+i.e. it will only contain characters -A-Za-z0-9_.~!*\'();:@&=+$,/?%#[]
+If you intend to analyse it, you will probably have to
+Foswiki::urlDecode it first.
+
 =cut
 
 *query_string = \&queryString;

core/lib/Foswiki/Store.pm

@@ -194,6 +194,9 @@ fallback for distributed topics (such as those in System) when content is not
 held in the store itself (e.g. if the store doesn't recognise the web it
 can call SUPER::getAttachmentURL)
 
+As required by RFC3986, the returned URL may only contain the
+allowed characters -A-Za-z0-9_.~!*\'();:@&=+$,/?%#[]
+
 =cut
 
 sub getAttachmentURL {

core/lib/Foswiki/UI/Rest.pm

@@ -126,7 +126,7 @@ sub rest {
         "computing REST for $session->{webName}.$session->{topicName}")
       if Foswiki::PageCache::TRACE();
 
-    my $pathInfo = $req->path_info();
+    my $pathInfo = Foswiki::urlDecode( $req->path_info() );
 
     # Foswiki rest invocations are defined as having a subject (pluginName)
     # and verb (restHandler in that plugin). Make sure the path_info is

core/lib/Foswiki/UI/Viewfile.pm

@@ -76,7 +76,7 @@ sub viewfile {
 
         # This is a standard path extended by the attachment name e.g.
         # /Web/Topic/Attachment.gif
-        $pathInfo = $query->path_info();
+        $pathInfo = Foswiki::urlDecode( $query->path_info() );
     }
 
     # If we have path_info but no ?filename=