From ea43031b39e9951c8cb9a32abc9610f77618d83a Mon Sep 17 00:00:00 2001
From: =?utf8?q?Bartosz=20Dziewo=C5=84ski?= <matma.rex@gmail.com>
Date: Sun, 24 Jul 2016 21:44:35 +0200
Subject: [PATCH] Improve how slashes are stripped from filenames

* Add slash and backslash ('/' and '\') to $wgIllegalFileChars.
* Replace illegal chars before removing paths in wfStripIllegalFilenameChars().

This way users trying to upload a file with slashes in the name will
get a better filename suggestion (e.g. for 'Foo part 1/3.jpg', you
previously got '3.jpg', now you'll get 'Foo part 1-3.jpg'). Uploading
tools that don't special-case slashes will also behave better.

Change-Id: Ib78f48a5f8c92e8ab2dc773ea6789b96b3662177
---
 includes/DefaultSettings.php                     | 8 ++++++--
 includes/GlobalFunctions.php                     | 9 +++++----
 tests/phpunit/includes/upload/UploadBaseTest.php | 2 +-
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 1e6030226c..592632864c 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -398,9 +398,13 @@ $wgAllowImageMoving = true;
 $wgEnableAsyncUploads = false;
 
 /**
- * These are additional characters that should be replaced with '-' in filenames
+ * Additional characters that are not allowed in filenames. They are replaced with '-' when
+ * uploading. Like $wgLegalTitleChars, this is a regexp character class.
+ *
+ * Slashes and backslashes are disallowed regardless of this setting, but included here for
+ * completeness.
  */
-$wgIllegalFileChars = ":";
+$wgIllegalFileChars = ":\\/\\\\";
 
 /**
  * What directory to place deleted uploads in.
diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index 66e244082e..e262c8138d 100644
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -3350,9 +3350,9 @@ function wfCountDown( $seconds ) {
 }
 
 /**
- * Replace all invalid characters with -
- * Additional characters can be defined in $wgIllegalFileChars (see bug 20489)
- * By default, $wgIllegalFileChars = ':'
+ * Replace all invalid characters with '-'.
+ * Additional characters can be defined in $wgIllegalFileChars (see T22489).
+ * By default, $wgIllegalFileChars includes ':', '/', '\'.
  *
  * @param string $name Filename to process
  * @return string
@@ -3360,12 +3360,13 @@ function wfCountDown( $seconds ) {
 function wfStripIllegalFilenameChars( $name ) {
 	global $wgIllegalFileChars;
 	$illegalFileChars = $wgIllegalFileChars ? "|[" . $wgIllegalFileChars . "]" : '';
-	$name = wfBaseName( $name );
 	$name = preg_replace(
 		"/[^" . Title::legalChars() . "]" . $illegalFileChars . "/",
 		'-',
 		$name
 	);
+	// $wgIllegalFileChars may not include '/' and '\', so we still need to do this
+	$name = wfBaseName( $name );
 	return $name;
 }
 
diff --git a/tests/phpunit/includes/upload/UploadBaseTest.php b/tests/phpunit/includes/upload/UploadBaseTest.php
index 287af29d79..de6c4120a6 100644
--- a/tests/phpunit/includes/upload/UploadBaseTest.php
+++ b/tests/phpunit/includes/upload/UploadBaseTest.php
@@ -52,7 +52,7 @@ class UploadBaseTest extends MediaWikiTestCase {
 			[ 'ValidTitle.jpg', 'ValidTitle.jpg', UploadBase::OK,
 				'upload valid title' ],
 			/* A title with a slash */
-			[ 'A/B.jpg', 'B.jpg', UploadBase::OK,
+			[ 'A/B.jpg', 'A-B.jpg', UploadBase::OK,
 				'upload title with slash' ],
 			/* A title with illegal char */
 			[ 'A:B.jpg', 'A-B.jpg', UploadBase::OK,
-- 
2.20.1