some points i think should be improved in file.inc:

PHP_OS == 'WINNT'
this doesn't kick in for Win95(98?), as this has PHP_OS == 'WIN32'. change this test to substr(PHP_OS, 0, 3) == 'WIN'. it might be useful to add a define('OS_WINDOWS', substr(PHP_OS, 0, 3) == 'WIN' ? TRUE : FALSE); (from pear/PEAR.php) for further tests, if needed.
define('FILE_SEPARATOR', PHP_OS == 'WINNT' ? '\\' : '/');
according to the PHP Manual, both slash (/) and backslash (\) are used as path separator character. let's use forward slashes everywhere and get rid of all the os switches, str_replace's etc. in the code
$dest = variable_get('file_directory_temp', (PHP_OS == 'WINNT' ? 'c:\\windows\\temp' : '/tmp'));
on my system, the TMP-dir is D:/tmp. would be nice if this code would use get_cfg_var('upload_tmp_dir') and/or the system default tmp-dir. see this code snippet from the PHP Manual.
CommentFileSizeAuthor
#5 file-inc-windows-95-98-issue_0.patch1.11 KBStefan Nagtegaal
#3 file-inc-windows-95-98-issue.patch1.56 KBclaybutterfly

Comments

ax’s picture

ax commented

and a related issue i forgot in my first post: the default "File System Path" ({drupal-dir}/files) should be created and included in drupal-4.4.0.tar.gz. see also Unnecessary step of creating folders for filestore (and maybe also Add directory-properties into README for proper filesystem usage).

ax’s picture

ax commented

the points from my original post still haven't been implemented and would still make much sense.

claybutterfly’s picture

claybutterfly commented
Assigned: Unassigned » claybutterfly
Priority: Normal » Critical
StatusFileSize
new file-inc-windows-95-98-issue.patch1.56 KB

This solves the file issues on windows 95/98..
Tested this and works fine, also not noticed any problems on Windows XP.

chx’s picture

chx commented

Stefan, I can not make a patch from here, but 

- * @param $dest Path to verify
+ * @param $dest Path to verifiy

these lines are not needed, that's sure.

Stefan Nagtegaal’s picture

Stefan Nagtegaal commented
StatusFileSize
new file-inc-windows-95-98-issue_0.patch1.11 KB

Updated patch without the typo...

Steven’s picture

Steven commented

I think this can be done cleaner with a IS_WINDOWS constant defined once.

dries’s picture

dries commented

I had a look at it, integrated Steven's suggestions, and committed it to HEAD.

Should this be backported to DRUPAL-4-5? That is, does it fix a bug? According to Stefan's comment (comment #3) it does, however, no bug reports are referenced.

Steven’s picture

Steven commented

I'm not exactly sure what the $regex is supposed to do... it would seem to check for a valid windows drive ('?:\\') but it allows any character for the first character. Also remember that windows PHP allows you to use forward slashes just fine, and that you can go to the root using / as well.

dries’s picture

dries commented
moshe weitzman’s picture

moshe weitzman commented
Anonymous’s picture

(not verified) commented