Item13478: Viewfile multiple issues with utf8 filenames and topics

Viewfile has a couple of issues

1. bin/viewfile/Litterbox/TestTöpic/AśčÁŠŤśěž-utf8-errors.txt fails with: wide print at lib/Foswiki/UI/Viewfile.pm line 152
2. bin/viewfile/Litterbox/TestTöpic?filename=AśčÁŠŤśěž-utf8-errors.txt;rev=1 also fails with same error.

Two issues in viewfile.

• Query parameters like filename are already decoded
• The pathinfo is not decoded in all cases.

Crawford, for your review: I'm running into these while testing TopicInteractionPlugin, but as I can recreate them with plain old pattern skin, it seems to be a core issue. What I'm unsure of is the original code doesn't decode the pathInfo if it's passed in a redirect. I need to test viewfile redirection, but I'm assuming that the pathInfo always will require a decode.

diff --git a/core/lib/Foswiki/UI/Viewfile.pm b/core/lib/Foswiki/UI/Viewfile.pm
index 098736b..6874471 100644
--- a/core/lib/Foswiki/UI/Viewfile.pm
+++ b/core/lib/Foswiki/UI/Viewfile.pm
@@ -76,11 +76,12 @@ sub viewfile {
 
         # This is a standard path extended by the attachment name e.g.
         # /Web/Topic/Attachment.gif
-        $pathInfo = Foswiki::decode_utf8( $query->path_info() );
+        $pathInfo = $query->path_info();
     }
 
     # If we have path_info but no ?filename=
     if ($pathInfo) {
+        $pathInfo = Foswiki::decode_utf8( $pathInfo );
         my @path = split( /\/+/, $pathInfo );
         shift(@path) unless ( $path[0] );    # remove leading empty string
 
@@ -148,9 +149,6 @@ sub viewfile {
         );
     }
 
-    # decode filename in case it is urlencoded and/or utf8, see Item9462
-    $fileName = Foswiki::urlDecode($fileName);
-
     # Note that there may be directories below the pub/web/topic, so
     # simply sanitizing the attachment name won't work.
     $fileName = Foswiki::Sandbox::untaint( $fileName,

-- GeorgeClark - 28 Jun 2015

viewfile rewriting works fine, but I don't think that covers the redirect handling in viewfile.pm.

-- GeorgeClark - 28 Jun 2015

Checking in.

-- GeorgeClark - 28 Jun 2015

You had the right idea, but we really needed to pull the decode up into the Engine::CGI module. That way we don't need to decode piecemeal.

-- Main.CrawfordCurrie - 29 Jun 2015 - 16:37

Hijacked this to fix TablePlugin. Please review. Thanks

-- GeorgeClark - 29 Jun 2015

Crawford, my patch has broken two of the TablePlugin unit tests, and I have no idea why.

-- GeorgeClark - 30 Jun 2015

I applied a rationality filter, and t all looks good now.

-- CrawfordCurrie - 30 Jun 2015

Commenting is disabled while not logged in