Posted by markus_petrux on March 11, 2009 at 6:39pm
| Project: | File (Field) Paths |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
When transliteration_clean_filename() is applied to file paths, slashes are removed and the resulting file path is wrong.
For example:
<?php
print transliteration_clean_filename('sites/eaxmple.com/files/test.txt');
// This will print: 'siteseaxmple.comfilestest.txt'
?>Here's a mini-patch for function filefield_paths_process_string() that seems to work well on my site.
<?php
// Transliterate string
if (module_exists('transliteration') && $settings['transliterate']) {
$value = transliteration_get($value);
if ($type == 'field') {
- $value = transliteration_clean_filename($value);
+ if (strpos($value, '/') !== FALSE) {
+ $parts = explode('/', $value);
+ foreach ($parts as $i => $part) {
+ $parts[$i] = transliteration_clean_filename($part);
+ }
+ $value = implode('/', array_filter($parts));
+ }
+ else {
+ $value = transliteration_clean_filename($value);
+ }
}
}
?>There may be other ways to fix this, though.
Comments
#1
Ok, this is actually worst. Critical. Though, I was afraid to report it like this because I could not believe it was happening.
Slashes are removed from the file path in filefield_paths_process_file(), this line:
<?php$file['filepath']['new'] = filefield_paths_process_string($file['filepath']['new'], 'field', array(0 => $file['field']), $settings['filepath']);
?>
For example, if $file['filepath']['new'] = 'sites/example.com/files/test.txt', it ends up as 'sitesexample.comfilestest.txt'.
Then, the following code is executed:
<?php// Finalize files if necessary
if (dirname($file['filepath']['new']) != dirname($file['field']['filepath']) || $file['filename']['new'] != $file['field']['filename']) {
if (filefield_paths_file_move($file)) {
?>
Where dirname($file['filepath']['new']) is '.', and dirname($file['field']['filepath']) is 'sites/example.com/files', so the IF condition is TRUE, then filefield_paths_file_move($file) is evaluated, and here we have the following code:
<?php$dest = _filefield_paths_strip_path(dirname($file['filepath']['new'])); // equals to $dest = '.';
...
...
if (!file_move($file['field']['filepath'], $dest .'/'. $file['filename']['new'], $replace)) {
?>
That is equal to file_move('sites/example.com/files/test.txt', './test.txt', TRUE). Looking at file_move() in drupal's include/file.inc we have:
<?php
function file_move(&$source, $dest = 0, $replace = FILE_EXISTS_RENAME) {
$path_original = is_object($source) ? $source->filepath : $source;
if (file_copy($source, $dest, $replace)) {
$path_current = is_object($source) ? $source->filepath : $source;
if ($path_original == $path_current || file_delete($path_original)) {
return 1;
}
drupal_set_message(t('The removal of the original file %file has failed.', array('%file' => $path_original)), 'error');
}
return 0;
}
?>
First the file is copied to destination, then if original and destination paths are not equal, the file removed. Seems good, but it isn't, because the same file that's copied here, is then removed. We can see this by looking at file_copy(), where the first line of code does this:
<?php$dest = file_create_path($dest); // file_create_path('./test.txt') = 'sites/darwin.cvs.gamefilia.com/files/./test.txt'
?>
So destination really points to the same source file, that's first copied and then removed in file_move().
Solution to this might involve a patch to drupal core, but in the meantime, maybe filefield_paths could normalize the source and destination paths before running a file_move().
That being said, I solved my issue with the patch in the issue starter, but I believe something else should be done to ensure files are not removed when other configurations generate a destination file path without subdirectories, which is when dirname($file) will return '.'.
#2
For reference, I have created a bug report to Drupal core: #398874: file_move('sites/example.com/files/test.txt', './test.txt') removes destination file!
#3
Are you putting slashes in the filename?
If so, you shouldn't be, they will removed intentionally as there shouldn't be slashes in your filename. Put them in your filepath.
If not, I will be sure to look into this. And for future reference, what you supplied is not a patch, checkout http://drupal.org/patch/create for more information.
#4
Stated posting the last comment before seeing the last two comments, will look into this more shortly.
#5
Nope, the filename is just 'test.txt'.
In filefield settings I have the following:
- Path settings / File path: empty
- Path settings / File path cleanup settings / Transliterate: enabled
When File Path is empty, filefield places the file in the site files directory, and the filepath looks like that, which is what then filefield_paths tries to transform causing this problem I mentioned.
PS: I known how to create a patch, but I think it is a bit more complex, so you may have to try and test for yourself. My first aim was to describe the problem the best I could, not just give you a patch.
#6
Sorry, reverting the issue status.
#7
Tested with the settings from#5 and nothing went wrong. My file ended up at /sites/default/files/test.txt as expected.
Are you sure you have transliteration turned on for the filepath? Don't mean filename?
Reducing from critical until I can confirm the bug is reproducible.
PS. My comment about the patch was because some developers will get very angry at people who don't do things correctly, so I just wanted to make sure you knew how to do things correctly and got in the habit of doing so.
#8
Ok, Issue is confirmed.
I'm not marking it as critical, as it only effects those using the transliteration module, it'd be critical if it affected everyone.
Working on fix now.
#9
#10
Hi Markus,
Thanks for that, it just seemed odd that you would be transliterating an empty field, but I did confirm the issue and have fixed it. In the process of commit the fix right now.
#11
Fixed and committed to DRUPAL-6--1 and DRUPAL-5.
#12
Wow, thanks for the super quick response. I'll try to test the fixes later today.
Yes, it's confusing, but I noticed this happens against the filepath where the file is originally uploaded by filefield, so it is the files directory of the site. ie. something like files/test.txt, sites/all/files/test.txt, sites/example.com/files/test.txt, etc.
filefield_pahts starts to transform the filepath from that, from where filefield uploaded the file, not before filefield computes the detiation of the file, so filefield_paths computes the new location and performs a move operation.
#13
Automatically closed -- issue fixed for 2 weeks with no activity.