Re: There must be a better way (to clean a filename)

Verdon Vaillancourt wrote: > Pretty much a newbie question... > > The following lines have been copied from examples I've looked at and are > part of a bigger function I am attempting to write, that uploads a file to > the server and then does some db work. The purpose of the following lines > are to get a 'clean' name for the uploaded file that is then used for other > tasks later on. > > What I mean by 'clean' is that if a user were to upload a file named > something silly like "User's Uploaded fiLe.jpg", I'll end up with > "user_s_uploaded_file.jpg" The following lines actually return > "user_s_uploaded_file_jpg.jpg" which is OK but could be better. > > I understand what is happening, and I've tried a few variations to the > ereg_replace to get it to leave the period alone (so I don't have to put the > extension back on) but haven't had any success. Would anyone care to suggest > the right expression, or an overall better way I could/should handle > something like this? > > $realname = strtolower($HTTP_POST_FILES['userfile']['name']); > $extension = substr(strrchr($realname,"."),1); > // remove %20 and the like from name > $realname = urldecode($realname); > // replace multiple non-word characters with a single "_" > $cleanedFilename = ereg_replace("[^[:alnum:]]", "_", $realname); > // Get rid of extra underscores > $cleanedFilename = ereg_replace("_+", "_", $cleanedFilename); > $cleanedFilename = ereg_replace("(^_|_$)", "", $cleanedFilename); > // The final name > $save_name = "$cleanedFilename"."."."$extension" Rule #1. Instead of trying to think of ALL the things a user could do to be stupid with a filename, define what is a VALID name, and throw out everything else. $goodname = ereg_replace('[^A-Za-z0-9_\\.-]', '', $realname); Because there will ALWAYS be somebody who manages to come up with some character you didn't think of that you do *NOT* want. Like, completely non-ASCII characters, for example, which are missing in your current code. This follows the generall security rule of deciding what is kosher, allowing that, and disallowing anything else, rather than trying to disallow everything that might not be kosher -- You'll never find all the non-kosher things. Now, you want to go a bit farther and: Make sure there is only one period: $parts = explode(".", $goodname); if (count($parts) > 1){ $extension = '.' . $parts[count($parts) - 1]; //note we put the . back in here... unset($parts[count($parts) - 1]); $name = implode('', $parts); } else{ //if you want to allow no extension: $name = implode('', $parts); $extension = ''; //note the lack of a . here //If you want do just dis-allow no extension: //die("Invalid filename<BR>\n"); //Under *NO* circumstances should you trust the $_FILES //mime type. //A) Different browsers will send different mime types for // the same file (browsers are stupid) //B) Hackers can send a trojan binary with a image/jpeg type } //Finally, it's possible that *NO* characters were valid. //It's also possible that there is ALREADY a file with the name //you currently have... $unique = strlen($name) == 0 ? '1' : ''; //The above will force the filename to not be completely empty //without ALWAYS tacking on a 0, which you don't want while (file_exists("/pathtodestination/$name$unique$extension")){ //It's kinda hacky to ++ the empty string //but who wants to have most files with a '0' tacked on? $unique++; } $finalname = "$name$unique$extension"; Note that I did not force lower-case filenames because, well, users who use case properly should not be penalized by idiots. :-) You may choose to do a strtolower on the input before the other processing if you really like lowercase, of course. Of course, there *IS* a built-in PHP function to do something like this, but I always forget what it is called, and there are characters it allows I dis-like or vice versa... Dig into the manual and find it, rather than hacking your own, if you can live with the PHP rules. PS Any relation to the musician Dan Vaillancourt playing at Uncomon Ground http://uncommonground.com shortly? I'm the guy who booked him :-)