LocalFS cross platform bug
If you upload from a browser on a Windows machine to a server running on Linux, the id (and thus the file name) of the new file will be the complete Windows path instead of just the file name. This happens because os.basename on Linux doesn't know how to handle Windows pathname syntax. Here's a rather crude diff -u patch that works for me: --- LocalFS.py-orig Fri Apr 14 00:36:06 2000 +++ LocalFS.py Thu Jun 15 15:36:19 2000 @@ -625,10 +625,16 @@ "to this directory.<p>") else: raise + def _canonicalize_pathname(self, path): + if (string.count(path, '\\')): + if (len(path) > 1 and path[0] in string.letters and path[1] == ':'): + path = path[2:] + path = string.translate(path, string.maketrans('\\', '/')) + def manage_upload(self, file, REQUEST=None): """Upload a file to the local file system. The 'file' parameter is a FileUpload instance representing the uploaded file.""" - id = os.path.basename(file.filename) + id = os.path.basename(self._canonicalize_pathname(file.filename)) path = self._getpath(id) data = file.read(_test_read) if (find_binary(data) >= 0):
This is a known bug. Thanks for the patch. The problem with it is that filenames containing backslash characters are valid on Unix. I haven't been able to come up with a solution to this. Any ideas? --jfarr "Perl is worse than Python because people wanted it worse." Larry Wall, 14 Oct 1998 ----- Original Message ----- From: Dan L. Pierson <dan@sol.control.com> To: <zope@zope.org> Sent: Thursday, June 15, 2000 12:58 PM Subject: [Zope] LocalFS cross platform bug
If you upload from a browser on a Windows machine to a server running on Linux, the id (and thus the file name) of the new file will be the complete Windows path instead of just the file name. This happens because os.basename on Linux doesn't know how to handle Windows pathname syntax.
Here's a rather crude diff -u patch that works for me:
--- LocalFS.py-orig Fri Apr 14 00:36:06 2000 +++ LocalFS.py Thu Jun 15 15:36:19 2000 @@ -625,10 +625,16 @@ "to this directory.<p>") else: raise
+ def _canonicalize_pathname(self, path): + if (string.count(path, '\\')): + if (len(path) > 1 and path[0] in string.letters and path[1] == ':'): + path = path[2:] + path = string.translate(path, string.maketrans('\\', '/')) + def manage_upload(self, file, REQUEST=None): """Upload a file to the local file system. The 'file' parameter is a FileUpload instance representing the uploaded file.""" - id = os.path.basename(file.filename) + id = os.path.basename(self._canonicalize_pathname(file.filename)) path = self._getpath(id) data = file.read(_test_read) if (find_binary(data) >= 0):
_______________________________________________ Zope maillist - Zope@zope.org http://lists.zope.org/mailman/listinfo/zope ** No cross posts or HTML encoding! ** (Related lists - http://lists.zope.org/mailman/listinfo/zope-announce http://lists.zope.org/mailman/listinfo/zope-dev )
Jonothan Farr writes:
This is a known bug. Thanks for the patch. The problem with it is that filenames containing backslash characters are valid on Unix. I haven't been able to come up with a solution to this. Any ideas?
That's why I check for a file name that starts with a letter followed by a colon (hmm, maybe should check that it contains at least one backslash as well). While a file name that looks like a full DOS pathname is legal on Unix, I don't remember ever seeing one. IMHO, preventing someone from intentionally uploading a file with a bizarre name like that is less of a problem that making all uploads from Windows machines product bizarre results. Note that this patch only restricts the names of uploaded files. Files that get in the LocalFS directories some other way are untouched. BTW: I'm working on associating other annotation data with LocalFS files. My current thinking is to release the result as a separate product, since I can't think of a way to do it that doesn't hack LocalFS sources. Basically, I'm adding a PersistentMapping (from id to arbitrary class) to LocalFS that can contain one entry for every item in the top level directory with items for lower level directories being PersistentMappings themselves. This seems less of a pain that one mapping with the key being the full relative pathname when the time comes to handle rename, add and delete... The motivation for all of this is an archive of PDF files. I don't want to fill up the ZODB with the PDF files, but am perfectly happy to keep contributor name, short description, etc. there. We'll use a ZClass to store this info, but I don't think that anything but our DTML will need to be aware of the details of it. Comments?
This is a known bug. Thanks for the patch. The problem with it is that filenames containing backslash characters are valid on Unix. I haven't been able to come up with a solution to this. Any ideas?
That's why I check for a file name that starts with a letter followed by a colon (hmm, maybe should check that it contains at least one backslash as well).
That's a good point. Although, your patch actually does the backslash replacement regardless of whether the drive specification is present. How about this: + drive,rest=os.path.splitdrive(path) + if drive: + path = string.replace(rest, '\\', '/') This will only work if win32 paths are always specified as full paths by the browser. Do you know if this is the case? --jfarr
Jonothan Farr writes:
That's a good point. Although, your patch actually does the backslash replacement regardless of whether the drive specification is present.
How about this:
+ drive,rest=os.path.splitdrive(path) + if drive: + path = string.replace(rest, '\\', '/')
I like this better than mine, thanks!
This will only work if win32 paths are always specified as full paths by the browser. Do you know if this is the case?
It seems to be, but I've only done limited testing.
Jonothan Farr writes:
How about this:
+ drive,rest=os.path.splitdrive(path) + if drive: + path = string.replace(rest, '\\', '/')
Oops, looks like this doesn't work after all. The Linux version of splitdrive doesn't actually do the split :-(.
How about this:
+ drive,rest=os.path.splitdrive(path) + if drive: + path = string.replace(rest, '\\', '/')
Oops, looks like this doesn't work after all. The Linux version of splitdrive doesn't actually do the split :-(.
Doh! Looks like we'll need to roll our own splitdrive then. --jfarr "Perl is worse than Python because people wanted it worse." Larry Wall, 14 Oct 1998
BTW: I'm working on associating other annotation data with LocalFS files. My current thinking is to release the result as a separate product, since I can't think of a way to do it that doesn't hack LocalFS sources. Basically, I'm adding a PersistentMapping (from id to arbitrary class) to LocalFS that can contain one entry for every item in the top level directory with items for lower level directories being PersistentMappings themselves. This seems less of a pain that one mapping with the key being the full relative pathname when the time comes to handle rename, add and delete...
The motivation for all of this is an archive of PDF files. I don't want to fill up the ZODB with the PDF files, but am perfectly happy to keep contributor name, short description, etc. there. We'll use a ZClass to store this info, but I don't think that anything but our DTML will need to be aware of the details of it.
Comments?
That sounds like a mess. LocalDirectory objects are not persistent, they are created each time they are requested. So you'll have nowhere to put your PersistentMappings except in the top-level LocalFS object. Trying to make the LocalDirectories persistent would be painful. For one, it would be hard to keep in synch with the file system. You could store everything in a PM at the top and use paths relative to the base but that would make copy,rename,etc. also messy, like you said. What about just storing your meta-data in files in the local file system? Then you wouldn't need to modify the LocalFS product at all. Just write a ZClass that can read/write you meta-data file format. --jfarr
Jonothan Farr writes:
That sounds like a mess. LocalDirectory objects are not persistent, they are created each time they are requested. So you'll have nowhere to put your PersistentMappings except in the top-level LocalFS object. Trying to make the LocalDirectories persistent would be painful. For one, it would be hard to keep in synch with the file system. You could store everything in a PM at the top and use paths relative to the base but that would make copy,rename,etc. also messy, like you said.
I was planning to put the only top level PersistentMapping in the LocalFS object, all the others would be values in it. Still, you're right, it's not pretty.
What about just storing your meta-data in files in the local file system? Then you wouldn't need to modify the LocalFS product at all. Just write a ZClass that can read/write you meta-data file format.
Might well be a better approach. I'll think about it. Unfortunately, it doesn't remove the need for automatically handling move, copy and rename, since these files wouldn't be visible to the normal archive users.
I would suggest putting an extra text box on the upload form for the filename in ZOPE. You could include javascript to automatically fill in the text box whenever the ZOPE name in the file box changed. The javaScript code could use the C:\ or D:\ or whatever pattern at the beginning to determine that it might be a windows file and strip off the path. On the unlikely occassions it "guesses" wrong the user could change the name before uploading. Jim Sanford ----- Original Message ----- From: Dan L. Pierson <dan@sol.control.com> To: Jonothan Farr <jfarr@real.com> Cc: Dan L. Pierson <dan@sol.control.com>; <zope@zope.org> Sent: Tuesday, June 20, 2000 7:51 AM Subject: [Zope] Re: LocalFS w/ annotation data Jonothan Farr writes:
That sounds like a mess. LocalDirectory objects are not persistent, they are created each time they are requested. So you'll have nowhere to put your PersistentMappings except in the top-level LocalFS object. Trying to make the LocalDirectories persistent would be painful. For one, it would be hard to keep in synch with the file system. You could store everything in a PM at the top and use paths relative to the base but that would make copy,rename,etc. also messy, like you said.
I was planning to put the only top level PersistentMapping in the LocalFS object, all the others would be values in it. Still, you're right, it's not pretty.
What about just storing your meta-data in files in the local file system? Then you wouldn't need to modify the LocalFS product at all. Just write a ZClass that can read/write you meta-data file format.
Might well be a better approach. I'll think about it. Unfortunately, it doesn't remove the need for automatically handling move, copy and rename, since these files wouldn't be visible to the normal archive users. _______________________________________________ Zope maillist - Zope@zope.org http://lists.zope.org/mailman/listinfo/zope ** No cross posts or HTML encoding! ** (Related lists - http://lists.zope.org/mailman/listinfo/zope-announce http://lists.zope.org/mailman/listinfo/zope-dev )
What about just storing your meta-data in files in the local file system? Then you wouldn't need to modify the LocalFS product at all. Just write a ZClass that can read/write you meta-data file format.
Might well be a better approach. I'll think about it. Unfortunately, it doesn't remove the need for automatically handling move, copy and rename, since these files wouldn't be visible to the normal archive users.
Good point. Well, for my next trick I plan to add support for generic object meta-data in the file system. If you need it right away I'd say go for it, because I don't know when I'll get to it. I'd like it if you could let me know what you come up with, though. That'll probably make things easier when I get around to implementing a general solution, which will eliminate the maintenance chore for you of keeping your branch synched up with mine. --jfarr
participants (3)
-
Dan L. Pierson -
Jim Sanford -
Jonothan Farr