jonathangeiger / kohana-jelly

See the link below for the most up-to-date code
https://github.com/creatoro/jelly
MIT License
146 stars 34 forks source link

Jelly_Field_File - Wrong directory separator on windows #134

Open tsh854 opened 14 years ago

tsh854 commented 14 years ago

field/file.php [34] $this->path = rtrim($this->path, '/').'/';

should be:

[34] $this->path = rtrim($this->path, '/\\').DIRECTORY_SEPARATOR;
jonathangeiger commented 14 years ago

Is this really an issue on windows? If you look at line 29 you'll see that all windows-style back-slashes are converted to unix-style forward-slashes.

I was under the impression that Windows accepted either.

tsh854 commented 14 years ago

You are correct that line 29 converts backslashes to forward-slashes, however after that replacement the path is put through realpath() which converts back to windows backslashes on Windows hosts - see realpath() example 2.

This then results in $this->path having mixed slashes if line 34 remains as it is, then when the save() method is called, it is unsuccessful in removing the path to return only the filename (line 60)

jonathangeiger commented 14 years ago

Thanks for the clarification.

For portability reasons paths should always be saved using UNIX style forward-slashes.

I don't have access to a windows machine, but would realpath()ing the filename and then converting back-slashes to forward-slashes be a viable solution?

tsh854 commented 14 years ago

Unfortunately it's not quite that simple - the save() method still fails to strip the path from the filename because Upload::save() also relies on realpath(), but Jelly_Field_File::save() would be using our standardised path with forward-slashes.

To get it to work the changes would have to be something like:

$this->path = realpath(str_replace('\\', '/', $this->path));

would become $this->path = str_replace('\', '/', realpath($this->path));

and // Chop off the original path $value = str_replace($this->path, '', $filename);
becomes // Standardise slashes $filename = str_replace('\', '/', $filename); // Chop off the original path $value = str_replace($this->path, '', $filename);

jonathangeiger commented 14 years ago

Thanks for your help. I'm going to look into this and get back to you.