Feature Proposal: We should reject outright, and not rename files that fail the name and upload filters

Motivation

It's confusing to have filenames changed during the upload process. Just reject the upload and let the user choose a different name.

The rename has been listed as a "SMELL", saying we should reject, but rename just because it's always been that way.

The rationale for this change is twofold; first, it is counter-intuitive to change filenames on users. The user uploads a file and then looks for it, but mystically, the name has changed! This is as bad as Windows ignoring trailing '.' on filenames when saving files, and is generally considered to be very bad UI practice. Second, it is a security risk. The rules may determine that the new filename is "safe" but in reality, strangenesses in the OS may result in the renaming rules being leveraged to allow a dangerous filename to leak through.

Description and Documentation

Upload and TinyMCE attach processes will be changed to throw 403 - Forbidden.

Also, the rejection of spaces in file names should be removed (i.e. spaces in file names should be supported).

Examples

Impact

WhatDoesItAffect:

%WHATDOESITAFFECT%

edit

Implementation

By CrawfordCurrie

diff --git a/TWikiCompatibilityPlugin/lib/TWiki/Sandbox.pm b/TWikiCompatibilityPlugin/lib/TWiki/Sandbox.pm
index 0ed15bb..b3aa5cf 100644
--- a/TWikiCompatibilityPlugin/lib/TWiki/Sandbox.pm
+++ b/TWikiCompatibilityPlugin/lib/TWiki/Sandbox.pm
@@ -20,7 +20,7 @@ sub normalizeFileName {
     Foswiki::Sandbox::untaint( shift,
         \&Foswiki::Sandbox::validateAttachmentName );
 }
-sub sanitizeAttachmentName { Foswiki::Sandbox::sanitizeAttachmentName(@_) }
+sub sanitizeAttachmentName { Foswiki::Func::sanitizeAttachmentName(@_) }
 sub sysCommand             { return Foswiki::Sandbox::sysCommand(@_) }
 
 1;
diff --git a/UnitTestContrib/test/unit/FuncTests.pm b/UnitTestContrib/test/unit/FuncTests.pm
index c5167a6..e4374bd 100644
--- a/UnitTestContrib/test/unit/FuncTests.pm
+++ b/UnitTestContrib/test/unit/FuncTests.pm
@@ -2620,7 +2620,7 @@ sub do_attachment {
     $this->assert( !$e, $e );
 
     # See also: RobustnessTests::test_sanitizeAttachmentNama_unicode
-    my ($sanitized) = Foswiki::Sandbox::sanitizeAttachmentName($name);
+    my ($sanitized) = Foswiki::Func::sanitizeAttachmentName($name);
     $this->assert_str_equals( $name, $sanitized );
 
     require File::Spec;
diff --git a/UnitTestContrib/test/unit/RobustnessTests.pm b/UnitTestContrib/test/unit/RobustnessTests.pm
index 92a0501..ac80667 100644
--- a/UnitTestContrib/test/unit/RobustnessTests.pm
+++ b/UnitTestContrib/test/unit/RobustnessTests.pm
@@ -85,7 +85,7 @@ sub test_validateAttachmentName {
 }
 
 sub _shittify {
-    my ( $a, $b ) = Foswiki::Sandbox::sanitizeAttachmentName(shift);
+    my ( $a, $b ) = Foswiki::Func::sanitizeAttachmentName(shift);
     return $a;
 }
 
diff --git a/UnitTestContrib/test/unit/UploadScriptTests.pm b/UnitTestContrib/test/unit/UploadScriptTests.pm
index ec04ebe..09d7938 100644
--- a/UnitTestContrib/test/unit/UploadScriptTests.pm
+++ b/UnitTestContrib/test/unit/UploadScriptTests.pm
@@ -241,8 +241,31 @@ sub test_illegal_upload {
     my $this = shift;
     local $/ = undef;
     my $data = 'asdfasdf';
-    my ( $goodfilename, $badfilename ) =
-      Foswiki::Sandbox::sanitizeAttachmentName('F$%^&&**()_ .php');
+
+    $Foswiki::cfg{NameFilter} = 'F';
+    $Foswiki::cfg{UploadFilter} = '\\.name$';
+    my $badfilename = 'A 8ad.name';
+    try {
+        $this->do_upload(
+            $badfilename,
+            $data,
+            undef,
+            hidefile         => 0,
+            filecomment      => 'Elucidate the goose',
+            createlink       => 0,
+            changeproperties => 0
+        );
+        $this->assert(0);
+    }
+    catch Foswiki::OopsException with {
+        my $e = shift;
+        $this->assert_str_equals( "upload_name_disallowed", $e->{def} );
+        $this->assert_str_equals( $badfilename, $e->{params}->[0] );
+        $this->assert_str_equals( "UploadFilter", $e->{params}->[1] );
+    };
+
+    $Foswiki::cfg{NameFilter} = '\\d';
+    $Foswiki::cfg{UploadFilter} = '';
     try {
         $this->do_upload(
             $badfilename,
@@ -257,8 +280,9 @@ sub test_illegal_upload {
     }
     catch Foswiki::OopsException with {
         my $e = shift;
-        $this->assert_str_equals( $goodfilename,         $e->{params}[1] );
-        $this->assert_str_equals( "upload_name_changed", $e->{def} );
+        $this->assert_str_equals( "upload_name_disallowed", $e->{def} );
+        $this->assert_str_equals( $badfilename, $e->{params}->[0] );
+        $this->assert_str_equals( "NameFilter", $e->{params}->[1] );
     };
 
     return;
@@ -268,8 +292,7 @@ sub test_illegal_propschange {
     my $this = shift;
     local $/ = undef;
     my $data = 'asdfasdf';
-    my ( $goodfilename, $badfilename ) =
-      Foswiki::Sandbox::sanitizeAttachmentName('F$%^&&**()_ .php');
+    my $badfilename = 'F$%^&&**()_ .php';
     try {
         $this->do_upload(
             $badfilename,
@@ -284,8 +307,7 @@ sub test_illegal_propschange {
     }
     catch Foswiki::OopsException with {
         my $e = shift;
-        $this->assert_str_equals( $goodfilename,         $e->{params}[1] );
-        $this->assert_str_equals( "upload_name_changed", $e->{def} );
+        $this->assert_str_equals( "upload_name_disallowed", $e->{def} );
     };
     try {
         $this->do_upload(
@@ -301,8 +323,7 @@ sub test_illegal_propschange {
     }
     catch Foswiki::OopsException with {
         my $e = shift;
-        $this->assert_str_equals( $goodfilename,         $e->{params}[1] );
-        $this->assert_str_equals( "upload_name_changed", $e->{def} );
+        $this->assert_str_equals( "upload_name_disallowed", $e->{def} );
     };
 
     return;
diff --git a/WysiwygPlugin/lib/Foswiki/Plugins/WysiwygPlugin/Handlers.pm b/WysiwygPlugin/lib/Foswiki/Plugins/WysiwygPlugin/Handlers.pm
index b2b8f84..e383618 100644
--- a/WysiwygPlugin/lib/Foswiki/Plugins/WysiwygPlugin/Handlers.pm
+++ b/WysiwygPlugin/lib/Foswiki/Plugins/WysiwygPlugin/Handlers.pm
@@ -801,8 +801,6 @@ sub _restUpload {
     $fileComment =~ s/\s+/ /go;
     $fileComment =~ s/^\s*//o;
     $fileComment =~ s/\s*$//o;
-    $fileName =~ s/\s*$//o;
-    $filePath =~ s/\s*$//o;
 
     unless (
         Foswiki::Func::checkAccessPermission(
@@ -823,9 +821,17 @@ sub _restUpload {
 
     unless ($doPropsOnly) {
 
-        # SMELL: call to unpublished function
-        ( $fileName, $origName ) =
-          Foswiki::Sandbox::sanitizeAttachmentName($fileName);
+        $origName = $fileName;
+
+        $fileName = Foswiki::Sandbox::untaint(
+            $fileName,
+            \&Foswiki::Sandbox::validateAttachmentName);
+
+        if ( !$fileName
+             || defined $Foswiki::cfg{UploadFilter}
+             && $fileName =~ /$Foswiki::cfg{UploadFilter}/) {
+            returnRESTResult( $response, 403, "Illegal upload name" );
+        }
 
         # check if upload has non zero size
         if ($stream) {
diff --git a/core/lib/Foswiki.pm b/core/lib/Foswiki.pm
index c4b076a..36a7cd0 100644
--- a/core/lib/Foswiki.pm
+++ b/core/lib/Foswiki.pm
@@ -550,9 +550,10 @@ qr(AERO|ARPA|ASIA|BIZ|CAT|COM|COOP|EDU|GOV|INFO|INT|JOBS|MIL|MOBI|MUSEUM|NAME|NE
     #
     # $regex{filenameInvalidCharRegex} = qr/[^$regex{mixedAlphaNum}\. _-]/o;
     #
-    # It was only used in Foswiki::Sandbox::sanitizeAttachmentName(), which now
-    # uses $Foswiki::cfg{NameFilter} instead.
-    # See RobustnessTests::test_sanitizeAttachmentName
+    # It was only used in Foswiki::Sandbox::sanitizeAttachmentName(),
+    # which no longer exists. Foswiki::Func::sanitizeAttachmentName is
+    # maintained for compatibility but uses uses $Foswiki::cfg{NameFilter}
+    # instead.
     #
     # Actually, this is used in GenPDFPrincePlugin; let's copy NameFilter
     $regex{filenameInvalidCharRegex} = $Foswiki::cfg{NameFilter};
diff --git a/core/lib/Foswiki.spec b/core/lib/Foswiki.spec
index 0dc7f92..8377201 100644
--- a/core/lib/Foswiki.spec
+++ b/core/lib/Foswiki.spec
@@ -451,7 +451,7 @@ $Foswiki::cfg{LegacyRESTSecurity} = $FALSE;
 # script. The =login= and =logon= script will always accept the username and
 # password, but only from POST requests. In order to add support for the
 # =rest= and =restauth>> scripts, specify =/^(view|rest)(auth)?$/=
-$Foswiki::cfg{Session}{AcceptUserPwParam} = qr/^view(auth)?$/;
+$Foswiki::cfg{Session}{AcceptUserPwParam} = '^view(auth)?$';
 
 # **BOOLEAN EXPERT**
 # For backwards compatibility, enable this setting if you want
@@ -970,14 +970,27 @@ $Foswiki::cfg{RenderLoggedInButUnknownUsers} = $FALSE;
 $Foswiki::cfg{DenyDotDotInclude} = $TRUE;
 
 # **REGEX EXPERT**
-# Filter-in regex for uploaded (attached) file names. This is a filter
-# *in*, so any files that match this filter will be renamed on upload
-# to prevent upload of files with the same file extensions as executables.
+# Regex used to detect illegal names for uploaded (attached) files.
+#
+# Normally your web server should be configured to control what can be
+# done with files in the =pub= directory (see
+# [[http://foswiki.org/Support/FaqSecureFoswikiAgainstAttacks#Configure_the_web_server_to_protect_attachments][Support.FaqSecureFoswikiAgainstAttacks]]
+# for help doing this. In this case, this configuration item can be set to
+# the null string.
+#
+# On some hosted installations, you don't have access to the web server
+# configuration in order to secure it. In this case, you can use this option
+# to detect filenames that present a security threat (e.g. that the webserver
+# might interpret as executables).
 # 
 # *Note:* Make sure you update this list with any configuration or script
 # filetypes that are automatically run by your web server.
-$Foswiki::cfg{UploadFilter} =
-  qr/^(\.htaccess|.*\.(?i)(?:php[0-9s]?(\..*)?|[sp]htm[l]?(\..*)?|pl|py|cgi))$/;
+#
+# *Note:* this will only filter files during upload. It won't affect
+# files that were already uploaded, or files that were created directly
+# on the server.
+#
+$Foswiki::cfg{UploadFilter} = '^(\.htaccess|.*\.(?i)(?:php[0-9s]?(\..*)?|[sp]htm[l]?(\..*)?|pl|py|cgi))$';
 
 # **REGEX EXPERT**
 # Filter-out regex for webnames, topic names, file attachment names, usernames,
diff --git a/core/lib/Foswiki/Func.pm b/core/lib/Foswiki/Func.pm
index 23fb2de..b8e0c65 100644
--- a/core/lib/Foswiki/Func.pm
+++ b/core/lib/Foswiki/Func.pm
@@ -2982,8 +2982,34 @@ attachment names are uploaded.
 =cut
 
 sub sanitizeAttachmentName {
-    require Foswiki::Sandbox;
-    return Foswiki::Sandbox::sanitizeAttachmentName(@_);
+    my $fileName = shift;    # Full pathname if browser is IE
+
+    # Homegrown split equivalent because File::Spec functions will assume that
+    # directory path is using / in UNIX and \ in Windows as defined in the HOST
+    # environment.  And we don't know the client OS. Problem is specific to IE
+    # which sends the full original client path when you upload files. See
+    # Item2859 and Item2225 before trying again to use File::Spec functions and
+    # remember to test with IE.
+    # This should take care of any silly ../ shenanigans
+    $fileName =~ s{[\\/]+$}{};  # Get rid of trailing slash/backslash (unlikely)
+    $fileName =~ s!^.*[\\/]!!;  # Get rid of leading directory components
+
+    my $origName = $fileName;
+
+    # Change spaces to underscore
+    $fileName =~ s/ /_/go;
+
+    # See Foswiki.pm filenameInvalidCharRegex definition and/or Item11185
+    #$fileName =~ s/$Foswiki::regex{filenameInvalidCharRegex}//go;
+    $fileName =~ s/$Foswiki::cfg{NameFilter}//go;
+
+    # Append .txt to some files
+    $fileName =~ s/$Foswiki::cfg{UploadFilter}/$1\.txt/goi;
+
+    # Untaint
+    $fileName = Foswiki::Sandbox::untaintUnchecked($fileName);
+
+    return ( $fileName, $origName );
 }
 
 =begin TML
diff --git a/core/lib/Foswiki/Sandbox.pm b/core/lib/Foswiki/Sandbox.pm
index b2561e8..94df88d 100644
--- a/core/lib/Foswiki/Sandbox.pm
+++ b/core/lib/Foswiki/Sandbox.pm
@@ -181,7 +181,8 @@ standard Web/Topic/attachment level e.g
 Web/Topic/attachmentdir/subdir/attachment.gif. While such attachments cannot
 be created via the UI, they *can* be created manually on the server.
 
-The individual path components are filtered by $Foswiki::cfg{NameFilter}
+The individual path components are checked against $Foswiki::cfg{NameFilter}
+The validation fails if it matches.
 
 =cut
 
@@ -215,7 +216,7 @@ sub validateAttachmentName {
         else {
 
             # Filter nasty characters
-            $component =~ s/$Foswiki::cfg{NameFilter}//g;
+            return undef if $component =~ /$Foswiki::cfg{NameFilter}/;
             push( @result, $component );
         }
     }
@@ -280,54 +281,6 @@ sub normalizeFileName {
     return _cleanUpFilePath(@_);
 }
 
-=begin TML
-
----++ StaticMethod sanitizeAttachmentName($fname) -> ($fileName, $origName)
-
-Given a file name received in a query parameter, sanitise it. Returns
-the sanitised name together with the basename before sanitisation.
-
-Sanitation includes removal of all leading path components,
-filtering illegal characters and mapping client
-file names to a subset of legal server file names.
-
-Avoid using this if you can; encoding attachment names this way is badly
-broken, much better to use point-of-source validation to ensure only valid
-attachment names are ever uploaded.
-
-=cut
-
-sub sanitizeAttachmentName {
-    my $fileName = shift;    # Full pathname if browser is IE
-
-    # Homegrown split equivalent because File::Spec functions will assume that
-    # directory path is using / in UNIX and \ in Windows as defined in the HOST
-    # environment.  And we don't know the client OS. Problem is specific to IE
-    # which sends the full original client path when you upload files. See
-    # Item2859 and Item2225 before trying again to use File::Spec functions and
-    # remember to test with IE.
-    # This should take care of any silly ../ shenanigans
-    $fileName =~ s{[\\/]+$}{};  # Get rid of trailing slash/backslash (unlikely)
-    $fileName =~ s!^.*[\\/]!!;  # Get rid of leading directory components
-
-    my $origName = $fileName;
-
-    # Change spaces to underscore
-    $fileName =~ s/ /_/go;
-
-    # See Foswiki.pm filenameInvalidCharRegex definition and/or Item11185
-    #$fileName =~ s/$Foswiki::regex{filenameInvalidCharRegex}//go;
-    $fileName =~ s/$Foswiki::cfg{NameFilter}//go;
-
-    # Append .txt to some files
-    $fileName =~ s/$Foswiki::cfg{UploadFilter}/$1\.txt/goi;
-
-    # Untaint
-    $fileName = untaintUnchecked($fileName);
-
-    return ( $fileName, $origName );
-}
-
 sub _buildCommandLine {
     my ( $template, %params ) = @_;
     my @arguments;
diff --git a/core/lib/Foswiki/UI/Upload.pm b/core/lib/Foswiki/UI/Upload.pm
index c75029f..a9a99dc 100644
--- a/core/lib/Foswiki/UI/Upload.pm
+++ b/core/lib/Foswiki/UI/Upload.pm
@@ -127,22 +127,39 @@ sub _upload {
     $fileComment =~ s/\s+/ /go;
     $fileComment =~ s/^\s*//o;
     $fileComment =~ s/\s*$//o;
-    $fileName    =~ s/\s*$//o;
-    $filePath    =~ s/\s*$//o;
+
+    my $origName = $fileName;
+
+    $fileName = Foswiki::Sandbox::untaint(
+        $fileName,
+        \&Foswiki::Sandbox::validateAttachmentName);
+
+    unless ($fileName) {
+         throw Foswiki::OopsException(
+            'attention',
+            def    => 'upload_name_disallowed',
+            web    => $web,
+            topic  => $topic,
+            params => [ ( $origName || '""' ), 'NameFilter' ]
+            );
+    }
+
+    if ( defined $Foswiki::cfg{UploadFilter}
+         && $fileName =~ /$Foswiki::cfg{UploadFilter}/) {
+        throw Foswiki::OopsException(
+            'attention',
+            def    => 'upload_name_disallowed',
+            web    => $web,
+            topic  => $topic,
+            params => [ ( $origName || '""' ), 'UploadFilter' ]
+            );
+     }
 
     Foswiki::UI::checkWebExists( $session, $web, $topic, 'attach files to' );
     Foswiki::UI::checkTopicExists( $session, $web, $topic, 'attach files to' );
     my ($topicObject) = Foswiki::Func::readTopic( $web, $topic );
     Foswiki::UI::checkAccess( $session, 'CHANGE', $topicObject );
 
-    my $origName = $fileName;
-
-    # SMELL: would be much better to throw an exception if an attempt
-    # is made to upload an invalid filename. However, it has always
-    # been this way :-(
-    ( $fileName, $origName ) =
-      Foswiki::Sandbox::sanitizeAttachmentName($fileName);
-
     my $stream;
     my ( $fileSize, $fileDate, $tmpFilePath ) = '';
 
@@ -222,17 +239,6 @@ sub _upload {
     };
     close($stream) if $stream;
 
-    if ( $fileName ne $origName ) {
-        throw Foswiki::OopsException(
-            'attention',
-            status => 200,
-            def    => 'upload_name_changed',
-            web    => $web,
-            topic  => $topic,
-            params => [ $origName, $fileName ]
-        );
-    }
-
     # generate a message useful for those calling this script
     # from the command line
     return ($doPropsOnly)
diff --git a/core/templates/messages.tmpl b/core/templates/messages.tmpl
index e1e0f95..abf6f3f 100644
--- a/core/templates/messages.tmpl
+++ b/core/templates/messages.tmpl
@@ -222,17 +222,12 @@ registermessages.tmpl
 %MAKETEXT{"You will probably have to ask your administrator, [_1], to do this." args="%WIKIWEBMASTER%"}%
 %TMPL:END%
 %{==============================================================================}%
-%TMPL:DEF{"upload_name_changed"}%
----+++ %MAKETEXT{"File has been uploaded with different name"}%
-%MAKETEXT{"The file has been uploaded and attached properly to the [_1] topic." args="<nop>%TOPIC%"}%
-%MAKETEXT{"However, the filename has been changed from [_1] to [_2]. Please make a note of it." args="<b><code>%PARAM1%</code></b>,<b><code>%PARAM2%</code></b>"}%
-
-*%MAKETEXT{"Note:"}%*
-%MAKETEXT{"In some cases, Foswiki changes the name of the uploaded file to make it safe and accessible across all platforms:"}%
-   * %MAKETEXT{"Spaces are replaced by underscores"}%
-   * %MAKETEXT{"A =.txt= extension is appended to some filenames for security reasons"}%
-   * %MAKETEXT{"Some characters such as =&#126;=, =$=, =@=, =%= are removed"}%
-   * %MAKETEXT{"International (8-bit) characters are removed (replaced by US-ASCII equivalents for Latin-1)"}%
+%TMPL:DEF{"upload_name_disallowed"}%
+---+++ %MAKETEXT{"Filename is not allowed"}%
+%MAKETEXT{"Filename <code>[_1]</code> failed check against ={[_2]}=" args="%PARAM1%,%PARAM2%"}%
+
+%MAKETEXT{"Foswiki will not allow you to upload filenames that may compromise the security of the web server, or are not safe and accessible across all platforms"}%
+
 %MAKETEXT{"You may be able to get your Wiki administrator to change the settings if they are inappropriate for your environment."}%
 
 %TMPL:P{"oktopicaction"}%

-- Contributors: GeorgeClark - 08 Oct 2014

Discussion

Why would we reject filenames with blanks in them?

-- MichaelDaum - 09 Oct 2014

You mean why do we reject them? It's to do with the way the RCS command line is composed, I think. Do we need to reject them? No, I don't believe so, and the patch above doesn't.

-- CrawfordCurrie - 09 Oct 2014

The patch above - as well as current core code - replaces blanks in filenames with underscores. Can we please allow filenames with blanks in them. As far as I know there is no real reason to do that, is there?

-- MichaelDaum - 09 Oct 2014

As far I can see, this patch does two things.

1. sanitizeAttachmentName is no longer called when you upload a file. The filename is just "validated".
2. Foswiki::Func::sanitizeAttachmentName is refactored. Its behaviour is unchanged. It still replaces blanks in filenames.

Thus, with this patch, you should be able to attach files with spaces in the filenames.

-- MichaelTempest - 09 Oct 2014

Everything MichaelTempest says above is correct.

-- CrawfordCurrie - 10 Oct 2014

Remember that many years ago when we looked at this area in the old project we were a bit surprised about the way Apache looked at file names and related then to e.g. PHP

We tried to prevent uploading files ending with .php and some other suffixes that often also are used with PHP or similar.

What we found out is that Apache does not require the .php to be a suffix. You could upload a file called evilscript.php.harmless and it would still be seen as a php script.

The conclusion after a long list of tries was that it is ESSENTIAL that the Apache configuration disables any serving of PHP, perl, CGI, Python etc in the entire pub tree.

In practical life it is PHP that is the issue only because by default most distros enable PHP everywhere.

There are also some people that find it convenient to let the php interpreter process .htm and .html files so they can enbed little 1-liner php scripts in them like counters etc. So it is really important that PHP is disabled. The admin needs to be aware of this. Protecting against illegal filenames is important but it does not in any way replace the disabling of php and we need to take care that we do not give the impression that the admin is safe just because the name filter contains .php

Disabling serving html files is also important for many admins because they want to prevent people from uploading html files in a pub dir under a topic and use this as a web server. All sorts of little websites serving as distribution of "nature" films or blood vessel expansion drugs starting with V, or plain spam containing links to enhance Google ranking is the concern and whatever other things html can contain.

-- KennethLavrsen - 12 Oct 2014

Thanks for that perspective, Kenneth. I agree with everything you say. However it's not directly pertinent; the proposal here is to change the response to a suspect file from "change the file name to something 'harmless'" to "reject upload of a dodgy filename outright". We are not changing the rules by which dodgy files get identified; those will remain.

I added a more descriptive rationale to the proposal to document why we think this change is needed.

-- CrawfordCurrie - 13 Oct 2014

It was just a reminder all of the old surprising information. I have no issues with the proposal

-- KennethLavrsen - 13 Oct 2014

The deadline to raise a concern is long since passed, but I don't think we can just remove the Sandbox function. It is part of our published API and needs to be deprecated.

I'd also like to see us separate filtering of attachment names from topic names. Topics and webs probably need to be more restrictive than attachment names.

-- GeorgeClark - 23 Jan 2015

The best UX would be to accept each and every file no matter how crazy it is named and be able to retrieve that file again clicking on exactly the same filename being displayed on the user interface. The rest of the process to effectively store the file on the server should not be of any concern to the user. It is the store's business to make that happen.

Two solutions:

1. rename the file while storing
2. reject storing the file

... as the original filename is "illegal" in some respect ... for the filesystem and/or http server underneath ... not for the user.

Problem with solution (1) is that the original filename is lost in the sense that the user isn't able to access the file again using the same filename. That's what this proposal is trying to fix.

Problems with solution (2) are that

• (a) the user has to guess which filename might be okay for Foswiki to accept
• (b) the user will have to rename the original file at least temporarily which might not be possible or at least create redundancy on the user's side having to perform this operation on his end now.
• (c) the user requires to do more work on his behalf to perform a rather simple task
• (d) the user is interrupted in his workflow thus lowering productivity
• (e) the user might be in the middle of a batch upload which either is completely rejected or just in parts; files not being uploaded have to be picked out and renamed each and re-uploaded again until Foswiki finally accepts them all
• (d) the user might repeatedly fail to upload one of more files in a very undesirable trial and error fashion.

That's what this proposal is trying to do.

I really can't see how solution (2) is any improvement.

I'd like to propose a third solution.

• While storing files being uploaded create a sane canonical file ID (similar to the cUIDs)
• Keeping the original filename.
• Use the original filename to be displayed on the userinterface.
• Store the file under its canonical file ID ... not the user-provided filename.
• A download of the file can be performed either directly using the http server in which case the file ID must be used.
• A download via viewfile or xsendfile will provide a "Content-Disposition" HTTP header so that the "File Save" dialog on the user's side will let him re-create the very same file using the original filename on his side again.

Similar solutions:

• topic vs topic title
• cUID vs WikiName

But that's not been proposed here and therefore I have to raise concerns on this feature proposal for the list of reasons explained above.

-- MichaelDaum - 23 Jan 2015

I'm not commenting too much the development process, but based on my own experience from my users, one of the really serious Foswiki issues are the attachments naming.

The optimal solution would be (from my point of view):

• The Fosiki should accept any filename for upload (stupid ones too, like: Report from the "Do it better" congress.doc (yes, with quotes).
• Foswiki should sanitise the filename for store, e.g. it should be stored for example as Report_from_the__Do_it_better__congress.doc
• The download URL should be composed with this sanitised filename, e.g: /Myweb/MyTopic/Report_from_the__Do_it_better__congress.doc
• Anywhere in the UI the user should see the original filename, and when the user downloads the file, should get it with its original filename too.

-- JozefMojzis - 23 Jan 2015

How about a html5 solution with the download attribute. If we store all files as a md5 named blob, and then generate the download link with the original filename. This completely eliminates any current or future file system / os sensitivities to filenames.

Unfortunately the support is fairly weak. Chrome, FF20+, IE 15+ but not IE, or Safari. I tested a few more today. Konqueror and android, no. Dolphin - partial (not filename). But that will probably improve over time.

-- GeorgeClark - 26 Jul 2015

I am discarding this proposal, as I agree with Michael. NameFilter and friends are stupid.

Commenting is disabled while not logged in