Item13378: simplify things a lot by make Foswiki::Request behave like…

… CGI with -utf8 (except not broken, like CGI)

JsonRpcContrib/lib/Foswiki/Contrib/JsonRpcContrib/Request.pm

@@ -202,7 +202,7 @@ sub toSiteCharSet {
     return $string unless $string;
 
     # Convert to unicode if the core supports it
-    return Encode::decode_utf8($string) if $Foswiki::UNICODE;
+    return $string if $Foswiki::UNICODE;
 
     return $string
       if ( $Foswiki::cfg{Site}{CharSet} =~ /^utf-?8/i );

JsonRpcContrib/lib/Foswiki/Contrib/JsonRpcContrib/Server.pm

@@ -92,7 +92,7 @@ sub dispatch {
 
     # get topic parameter and set the location overriding any
     #  other value derived from the namespace param
-    my $topic = $request->unicode_param('topic')
+    my $topic = $request->param('topic')
       || $Foswiki::cfg{HomeTopicName};
     ( $session->{webName}, $session->{topicName} ) =
       Foswiki::Func::normalizeWebTopicName( $Foswiki::cfg{UsersWebName},
@@ -184,7 +184,7 @@ sub dispatch {
     };
 
     # finally
-    my $redirectto = $request->unicode_param('redirectto');
+    my $redirectto = $request->param('redirectto');
     if ( $code == 0 && defined $redirectto ) {
         my $url;
         if ( $redirectto =~ /^https?:/ ) {

NatEditPlugin/lib/Foswiki/Plugins/NatEditPlugin/RestSave.pm

@@ -118,8 +118,7 @@ sub toSiteCharSet {
 
     return $string unless $string;
 
-    # Convert to unicode if the core supports it
-    return Encode::decode_utf8($string) if $Foswiki::UNICODE;
+    return $string if $Foswiki::UNICODE;
 
     return $string
       if ( $Foswiki::cfg{Site}{CharSet} =~ /^utf-?8/i );

WysiwygPlugin/lib/Foswiki/Plugins/WysiwygPlugin/Handlers.pm

@@ -28,8 +28,7 @@ sub toSiteCharSet {
 
     return $string unless defined $string;
 
-    # Convert to unicode if the core supports it
-    return Encode::decode_utf8($string) if $Foswiki::UNICODE;
+    return $string if $Foswiki::UNICODE;
 
     return $string
       if ( $Foswiki::cfg{Site}{CharSet} =~ /^utf-?8/i );

core/lib/Foswiki.pm

@@ -1003,7 +1003,7 @@ Returns undef if the target is not valid, and the target URL otherwise.
 sub redirectto {
     my ( $this, $url ) = @_;
 
-    my $redirecturl = $this->{request}->unicode_param('redirectto');
+    my $redirecturl = $this->{request}->param('redirectto');
     $redirecturl = $url unless $redirecturl;
 
     return unless $redirecturl;
@@ -2039,12 +2039,11 @@ sub new {
     # Set the default for web
     # Development.AddWebParamToAllCgiScripts: enables
     # bin/script?topic=WebPreferences;defaultweb=Sandbox
-    my $defaultweb =
-      $query->unicode_param('defaultweb') || $Foswiki::cfg{UsersWebName};
+    my $defaultweb = $query->param('defaultweb') || $Foswiki::cfg{UsersWebName};
 
     my $webtopic      = Encode::decode_utf8( $query->path_info() || '' );
     my $topicOverride = '';
-    my $topic         = $query->unicode_param('topic');
+    my $topic         = $query->param('topic');
     if ( defined $topic ) {
         if ( $topic =~ m/[\/.]+/ ) {
             $webtopic = $topic;

core/lib/Foswiki/Engine.pm

@@ -173,10 +173,11 @@ sub prepareConnection { }
 
 ---++ ObjectMethod prepareQueryParameters( $req, $queryString )
 
-Should fill $req's query parameters field.
+Populates $req with parameters extracted by parsing a
+byte string (which may include url-encoded characters, which may
+in turn be parts of utf8-encoded characters).
 
-This method populates $req as it should if given $queryString parameter.
-Subclasses may redefine this method and call SUPER with query string obtained.
+Note that parameter names and values are decoded to unicode.
 
 =cut
 
@@ -191,12 +192,14 @@ sub prepareQueryParameters {
         if ( defined $value ) {
             $value =~ tr/+/ /;
             $value =~ s/%([0-9A-F]{2})/chr(hex($1))/gei;
+            $value = Encode::decode_utf8($value);
         }
         if ( defined $param ) {
             $param =~ tr/+/ /;
             $param =~ s/%([0-9A-F]{2})/chr(hex($1))/gei;
-            push @{ $params{$param} }, $value;
-            push @plist, $param;
+            $param = Encode::decode_utf8($param);
+            push( @{ $params{$param} }, $value );
+            push( @plist,               $param );
         }
     }
     foreach my $param (@plist) {
@@ -270,7 +273,9 @@ sub prepareBody { }
 Abstract method, must be defined by inherited classes.
    * =$req= - Foswiki::Request object to populate
 
-Should fill $req's body parameters.
+Should fill $req's body parameters (parameters that are set in the
+request body, as against the query string). Implementations must
+convert parameter values to unicode.
 
 =cut
 
@@ -283,8 +288,10 @@ sub prepareBodyParameters { }
 Abstract method, must be defined by inherited classes.
    * =$req= - Foswiki::Request object to populate
 
-Should fill $req's uploads field. Its a hashref whose keys are
-parameter names and values Foswiki::Request::Upload objects.
+Should fill $req's {uploads} field. This is a hashref whose keys are
+upload names and values Foswiki::Request::Upload objects.
+
+Implementations must convert upload names to unicode.
 
 =cut
 

core/lib/Foswiki/Engine/CGI.pm

@@ -20,6 +20,7 @@ use Foswiki::Engine ();
 our @ISA = ('Foswiki::Engine');
 
 use Assert;
+use Foswiki                  ();
 use Foswiki::Request         ();
 use Foswiki::Request::Upload ();
 use Foswiki::Response        ();
@@ -213,6 +214,11 @@ sub prepareBody {
     # sub-processes (see long comment ****)
     $CONSTRUCTOR_PID = $$;
 
+    # Note that we handle unicode conversion in the various prepare*
+    # methods, rather than using the CGI -utf8 option, which is documented
+    # as breaking uploads (though cdot believes this is because of the
+    # deprecated dual nature of param delivering lightweight file handles,
+    # and it would probably work in Foswiki. Just not tried it)
     my $cgi = new CGI();
     my $err = $cgi->cgi_error;
     throw Foswiki::EngineException( $1, $2 )
@@ -226,8 +232,19 @@ sub prepareBodyParameters {
     return unless $ENV{CONTENT_LENGTH};
     my @plist = $this->{cgi}->multi_param();
     foreach my $pname (@plist) {
-        my @values = map { "$_" } $this->{cgi}->multi_param($pname);
-        (undef) = $req->bodyParam( -name => $pname, -value => \@values );
+        my $upname = Encode::decode_utf8($pname);
+        my @values;
+        if ($Foswiki::UNICODE) {
+            @values =
+              map { Encode::decode_utf8($_) } $this->{cgi}->multi_param($pname);
+        }
+        else {
+            @values = $this->{cgi}->multi_param($pname);
+        }
+        $req->bodyParam( -name => $upname, -value => \@values );
+
+        # Note that we record the encoded name of the upload. It will be
+        # decoded in prepareUploads, which rewrites the {uploads} hash.
         $this->{uploads}{$pname} = 1 if scalar( $this->{cgi}->upload($pname) );
     }
 }
@@ -238,8 +255,9 @@ sub prepareUploads {
     return unless $ENV{CONTENT_LENGTH};
     my %uploads;
     foreach my $key ( keys %{ $this->{uploads} } ) {
-        my $fname = $this->{cgi}->param($key);
-        $uploads{"$fname"} = new Foswiki::Request::Upload(
+        my $fname  = $this->{cgi}->param($key);
+        my $ufname = Encode::decode_utf8($fname);
+        $uploads{$ufname} = new Foswiki::Request::Upload(
             headers => $this->{cgi}->uploadInfo($fname),
             tmpname => $this->{cgi}->tmpFileName($fname),
         );

core/lib/Foswiki/If/OP_dollar.pm

@@ -40,7 +40,7 @@ sub evaluate {
       unless $session;
     my $text = $a->_evaluate(@_) || '';
     if ( $text && defined( $session->{request}->param($text) ) ) {
-        return $session->{request}->unicode_param($text);
+        return $session->{request}->param($text);
     }
 
     $text = "%$text%";

core/lib/Foswiki/LoginManager.pm

@@ -633,7 +633,7 @@ sub redirectToLoggedOutUrl {
 
     #TODO: consider if we should risk passing on the urlparams on logout
     my $path_info = Encode::decode_utf8( $session->{request}->path_info() );
-    if ( my $topic = $session->{request}->unicode_param('topic') )
+    if ( my $topic = $session->{request}->param('topic') )
     {    #we should at least respect the ?topic= request
         my $topicRequest = Foswiki::Sandbox::untaintUnchecked($topic);
         ( my $web, $topic ) =

core/lib/Foswiki/Macros/TOC.pm

@@ -117,7 +117,7 @@ sub TOC {
         next if ( $name eq 'topic' );
         next if ( $name eq 'text' );
         next if ( $name eq 'refresh' );
-        push( @qparams, $name => scalar( $query->unicode_param($name) ) );
+        push( @qparams, $name => scalar( $query->param($name) ) );
     }
 
     # Extract anchor targets. This has to generate *identical* anchor

core/lib/Foswiki/Macros/URLPARAM.pm

@@ -51,7 +51,7 @@ sub URLPARAM {
             }
         }
         else {
-            $value = $this->{request}->unicode_param($param);
+            $value = $this->{request}->param($param);
             $value =
               _handleURLPARAMValue( $value, $newLine, $encode, $default );
         }

core/lib/Foswiki/UI/Edit.pm

@@ -61,8 +61,8 @@ sub init_edit {
       Foswiki::isTrue( scalar( $query->param('onlywikiname') ) );
     my $onlyNewTopic =
       Foswiki::isTrue( scalar( $query->param('onlynewtopic') ) );
-    my $formTemplate  = $query->unicode_param('formtemplate')  || '';
-    my $templateTopic = $query->unicode_param('templatetopic') || '';
+    my $formTemplate  = $query->param('formtemplate')  || '';
+    my $templateTopic = $query->param('templatetopic') || '';
     my $notemplateexpansion =
       Foswiki::isTrue( scalar( $query->param('notemplateexpansion') ) );
 
@@ -71,8 +71,8 @@ sub init_edit {
          $query->param('contenttype')
       || $query->param('apptype')
       || 'text/html';
-    my $parentTopic = $query->unicode_param('topicparent') || '';
-    my $ptext = $query->unicode_param('text');
+    my $parentTopic = $query->param('topicparent') || '';
+    my $ptext = $query->param('text');
 
     my $revision;
     if ( defined $query->param('rev') ) {
@@ -186,7 +186,7 @@ sub init_edit {
 
     # Get edit template
     my $template =
-         $query->unicode_param('template')
+         $query->param('template')
       || $session->{prefs}->getPreference('EDIT_TEMPLATE')
       || $templateName;
 

core/lib/Foswiki/UI/Passwords.pm

@@ -57,7 +57,7 @@ sub resetPassword {
     unless (@userNames) {
         throw Foswiki::OopsException( 'register', def => 'no_users_to_reset' );
     }
-    my $introduction = $query->unicode_param('Introduction') || '';
+    my $introduction = $query->param('Introduction') || '';
 
     # need admin priv if resetting bulk, or resetting another user
     my $isBulk = ( scalar(@userNames) > 1 );
@@ -251,7 +251,7 @@ sub changePasswordAndOrEmail {
     my $passwordA   = $query->param('password');
     my $passwordB   = $query->param('passwordA');
     my $email       = $query->param('email');
-    my $topicName   = $query->unicode_param('TopicName');
+    my $topicName   = $query->param('TopicName');
 
     # check if required fields are filled in
     unless ($login) {

core/lib/Foswiki/UI/Preview.pm

@@ -108,7 +108,7 @@ sub preview {
     my $saveCmd = $query->param('cmd') || '';
     $tmpl =~ s/%CMD%/$saveCmd/g;
 
-    my $redirectTo = $query->unicode_param('redirectto') || '';
+    my $redirectTo = $query->param('redirectto') || '';
     $tmpl =~ s/%REDIRECTTO%/$redirectTo/g;
 
     $formName ||= '';
@@ -160,14 +160,14 @@ sub preview {
          #ASSERT($originalrev ne '%ORIGINALREV%') if DEBUG;
     $tmpl =~ s/%ORIGINALREV%/$originalrev/g if ( defined($originalrev) );
 
-    my $templatetopic = $query->unicode_param('templatetopic');
+    my $templatetopic = $query->param('templatetopic');
 
     #ASSERT($templatetopic ne '%TEMPLATETOPIC%') if DEBUG;
     $tmpl =~ s/%TEMPLATETOPIC%/$templatetopic/g if ( defined($templatetopic) );
 
     #this one's worrying, its special, and not set much at all
     #$tmpl =~ s/%SETTINGSTOPIC%/$settingstopic/g;
-    my $newtopic = $query->unicode_param('newtopic');
+    my $newtopic = $query->param('newtopic');
 
     #ASSERT($newtopic ne '%NEWTOPIC%') if DEBUG;
     $tmpl =~ s/%NEWTOPIC%/$newtopic/g if ( defined($newtopic) );

core/lib/Foswiki/UI/Save.pm

@@ -177,16 +177,17 @@ sub buildNewTopic {
         $ancestorRev = 0;
     }
 
-    if ( defined( $query->param('text') ) ) {
+    $text = $query->param('text');
+    if ( defined $text ) {
 
         # text is defined in the query, save that text, overriding anything
         # from the template or the previous rev of the topic
-        $text = $query->param('text');
         $text =~ s/\r//g;
         $text .= "\n" unless $text =~ m/\n$/s;
     }
-
-    $text = '' unless defined $text;
+    else {
+        $text = '';
+    }
 
     # Change the parent, if appropriate
     my $newParent = $query->param('topicparent');
@@ -631,7 +632,7 @@ WARN
         # replace top revision with the text from the query, trying to
         # make it look as much like the original as possible. The query
         # text is expected to contain %META as well as text.
-        $topicObject->text( scalar( $query->param('text') ) );
+        $topicObject->text( $query->param('text') );
 
         try {
             $topicObject->replaceMostRecentRevision( forcedate => 1 );

core/lib/Foswiki/UI/Upload.pm

@@ -115,12 +115,12 @@ sub _upload {
 
     Foswiki::UI::checkValidationKey($session);
 
-    my $hideFile    = $query->param('hidefile')            || '';
-    my $fileComment = $query->unicode_param('filecomment') || '';
-    my $createLink  = $query->param('createlink')          || '';
+    my $hideFile    = $query->param('hidefile')    || '';
+    my $fileComment = $query->param('filecomment') || '';
+    my $createLink  = $query->param('createlink')  || '';
     my $doPropsOnly = $query->param('changeproperties');
-    my $filePath    = $query->unicode_param('filepath')    || '';
-    my $fileName    = $query->unicode_param('filename')    || '';
+    my $filePath    = $query->param('filepath')    || '';
+    my $fileName    = $query->param('filename')    || '';
     if ( $filePath && !$fileName ) {
         $filePath =~ m|([^/\\]*$)|;
         $fileName = $1;
@@ -149,7 +149,7 @@ sub _upload {
     my ( $fileSize, $fileDate, $tmpFilePath ) = '';
 
     unless ($doPropsOnly) {
-        my $fh = $query->unicode_param('filepath');
+        my $fh = $query->param('filepath');
 
         try {
             $tmpFilePath = $query->tmpFileName($fh);

core/lib/Foswiki/UI/Viewfile.pm

@@ -70,7 +70,7 @@ sub viewfile {
 
         # Attachment name is passed in URL params. This is a (possibly
         # / separated) path relative to the pub/Web/Topic
-        $fileName = $query->unicode_param('filename');
+        $fileName = $query->param('filename');
     }
     else {