Rather than using '/' to separate directories when we're building paths, we should use the PATH_SEPARATOR constant. The change fixes some bugs I've encountered on Windows using file uploads.
According to the docs on php.net it was introduced in PHP 4.3.0-RC2. The installation notes list 4.3.3 as our minimum requirement in so compatibility shouldn't be a problem.
Please review the attached patch. I've tried to make sure that the changes only affect files, not URLs.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | file.inc.USING_DIRECTORY_SEPARATOR_0.patch | 3.45 KB | drewish |
| file.inc.USING_DIRECTORY_SEPARATOR.patch | 3 KB | drewish |
Comments
Comment #1
drewish commentedthat last patch had a bug in it. it was setting \s on urls.
Comment #2
drewish commentedwhoops, wrong constant in the title
Comment #3
morbus iffI'm a little ... ... well, I can't say with any certainty if this is a good idea or not. Neither constant seems to have very decent documentation and my tests, on OS X and webchick's win32 box, shows that carry the same value (I originally thought that PATH may be related to a URL, but it's not - perhaps they're thinking UNC paths?). They return what I expect, certainly ("/" on *nix and "\" on Win32), but I'd like to hear more about the bugs you're facing with the current code.
Comment #4
morbus iffBased on some discussions in freenode's #php, "/" is supposed to be handled transparently and crossplatform-like internally to PHP. So, instead of "c:\\winnt\\tmp", for example, it'd be "c:/winnt/tmp". They suggest there isn't a need for DIRECTORY SEPARATOR at all. I've been unable, however, to find official documentation that suggests the same.
Comment #5
morbus iffAccording to dirname (and other directory/path related functions): On Windows, both slash (/) and backslash (\) are used as directory separator character. In other environments, it is the forward slash (/). Which would suggest that the _SEPARATOR stuff isn't needed in Drupal, and we should change my hardcoded "c:\\" stuff to "c:/" instead.
Comment #6
moshe weitzman commentedComment #7
ax commentedwe'll definitely want to use / instead of DIRECTORY_SEPARATOR. see also http://drupal.org/node/7235. marking WON'T FIX.