Item13478: Viewfile multiple issues with utf8 filenames and topics
Priority: Urgent
Current State: Closed
Released In: 2.0.0
Target Release: major
Viewfile has a couple of issues
-
bin/viewfile/Litterbox/TestTöpic/AśčÁŠŤśěž-utf8-errors.txt
fails with: wide print at lib/Foswiki/UI/Viewfile.pm line 152
-
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