Skip to content
This repository

Joomla CMS [#27526] Media manager doesn't support multi-byte characters #997

Closed
wants to merge 1 commit into from

4 participants

Ofer Cohen Rouven Weßling Louis Landry Sam Moffatt
Ofer Cohen
oc666 commented March 18, 2012

This will add support for multi-byte file name on onsafe method of file class. The default is full backward compatible, and not using multi-byte file name.

http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=27526

Rouven Weßling
Collaborator

Wouldn't it make sense to always support multibypte characters? (without the argument)

Also I wonder if phputf8 doesn't have some function that could help you.

Ofer Cohen
oc666 commented March 24, 2012

I left it without support by default for backward compatibility.
If you found phputf8 function that make this trick, please, let me know.

Rouven Weßling
Collaborator

I don't even know what your change does, I don't speak regex ;) That's why I haven't pulled it either.

Sam Moffatt pasamio commented on the diff August 14, 2012
libraries/joomla/filesystem/file.php
((14 lines not shown))
63 64
 	{
64  
-		$regex = array('#(\.){2,}#', '#[^A-Za-z0-9\.\_\- ]#', '#^\.#');
65  
-
66  
-		return preg_replace($regex, '', $file);
  65
+		if ($multibyte)
  66
+		{
  67
+			$search = "/\?%*:|\"<>#;()&;, ";
  68
+			return str_replace(str_split($search), '_', $filename);
1
Sam Moffatt
pasamio added a note August 14, 2012

Shouldn't we use something that is UTF-8 safe here? Perhaps a preg_replace with the u modifier to ensure we don't clobber parts of multibyte characters?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Louis Landry

@oc666 I'd love to see this get merged. Would you mind putting together a few test cases so that we can verify that everything works as expected? I do think that we may have an issue with multibyte characters, but trust that you can/will test that out for us. I'm going to close this until we get a couple of tests to ensure everything works and we don't have any regressions down the road. Please re-open this when you get that sorted out.

Louis Landry LouisLandry closed this October 08, 2012
Ofer Cohen

@LouisLandry, can you guide me to some simple guide or example for Joomla unit test?

Thanks, Ofer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Mar 18, 2012
Ofer Cohen fix bug #27526: Media manager doesn't support multi-byte characters 595334b
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 14 additions and 5 deletions. Show diff stats Hide diff stats

  1. 19  libraries/joomla/filesystem/file.php
19  libraries/joomla/filesystem/file.php
@@ -53,17 +53,26 @@ public static function stripExt($file)
53 53
 	/**
54 54
 	 * Makes file name safe to use
55 55
 	 *
56  
-	 * @param   string  $file  The name of the file [not full path]
  56
+	 * @param   string   $filename   The name of the file [not full path]
  57
+	 * @param   boolean  $multibyte  Support for multibyte file-name
57 58
 	 *
58 59
 	 * @return  string  The sanitised string
59 60
 	 *
60 61
 	 * @since   11.1
61 62
 	 */
62  
-	public static function makeSafe($file)
  63
+	public static function makeSafe($filename, $multibyte = false)
63 64
 	{
64  
-		$regex = array('#(\.){2,}#', '#[^A-Za-z0-9\.\_\- ]#', '#^\.#');
65  
-
66  
-		return preg_replace($regex, '', $file);
  65
+		if ($multibyte)
  66
+		{
  67
+			$search = "/\?%*:|\"<>#;()&;, ";
  68
+			return str_replace(str_split($search), '_', $filename);
  69
+		}
  70
+		else
  71
+		{
  72
+			$regex = array('#(\.){2,}#', '#[^A-Za-z0-9\.\_\- ]#', '#^\.#');
  73
+			return preg_replace($regex, '', $filename);
  74
+		}
  75
+		
67 76
 	}
68 77
 
69 78
 	/**
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.