Closed
Bug 56295
Opened 24 years ago
Closed 23 years ago
nsLocalFileMac doesn't work for file names > 31 characters
Categories
(Core :: XPCOM, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla0.9.8
People
(Reporter: mscott, Assigned: ccarlen)
References
Details
(Keywords: dataloss, Whiteboard: [PDT-] [ETA ?],AOLTW-)
Attachments
(1 file, 4 obsolete files)
4.23 KB,
patch
|
sdagley
:
review+
sfraser_bugs
:
superreview+
|
Details | Diff | Splinter Review |
This bug spun out of Bug #55628.
Go to www.mozilla.org. On the nightly builds part of the page, click on the
Linux link. This trys to download the following file:
"mozilla-i686-pc-linux-gnu.tar"
We fail to download this file.
On the mac, we have a nsLocalFileMac object which points to the destination
directory we want to start writing our temp file to. Let's pretend it's
"HD:DesktopFolder" We append the temp file name to this object.
In my case, I expect to get:
HD:DesktopFolder:mozilla-i686-pc-linux.gnu.tar
Unfortunately appending a file name that's greater than 31 characters on the Mac
causes a corruption to occur so the file object now points to a random location.
For me, I kept getting:
"HD:Seamonkey:view-debug" which is completely bogus. It lost all path information.
As a result the helper app content is never downloaded.
We need to add some code to nsLocalFileMac to properly "fixup" long file names
with the following constraints:
1) preserve the file extension
2) ensure uniqueness of the file so we don't stomp on an exisiting file when we
generate this new file name.
3) any other constraints?
Reporter | ||
Comment 1•24 years ago
|
||
changing to browser general I didn't mean to select xp-com as the component.
adding rtm and data loss keywords as this problem prevents us from downloading
files from the web > 31 characters.
I can probably come up with a hack for the file download case which might work
for RTM but we might want the Mac owner for nsLocalFileMac to investigate a long
term solution there.
Comment 2•24 years ago
|
||
Note that nsIFile has a CreateUnique() which should do the right thing.
Component: Browser-General → XPCOM
Hardware: PC → Macintosh
Reporter | ||
Comment 3•24 years ago
|
||
Changing my mind a little bit, I'd be happy just having a generic routine anyone
can call (off nsILocalFile???) that would cannocalize a file name making sure it
met the Mac requirement of file length.
I'm sure I'm not the only user of nsLocalFileMac that has file names that could
be >= 31 characters.
Hardware: Macintosh → PC
Assignee | ||
Comment 4•24 years ago
|
||
Isn't there some existing convention that we could use in nsLocalFileMac? Isn't
there something in NSPR made to deal with java class names on the Mac? If so, it
should be brought over to nsLocalFileMac. Depends on what we want to happen when
a > 31 char name is used and then somebody calls nsIFile::GetPath(). I suppose
we return the shortened path?
Reporter | ||
Comment 5•24 years ago
|
||
Apparently this used to work with the old nsFileSpec code:
1) Append the word "dummy" to your file spec.
2) Then call MakeUnique passing in the REAL file name you want as the suggested
file name. MakeUnique would properly truncate your suggested name and replace
"dummy" with a truncated suggested name.
I'm looking at nsIFile::CreateUnique right now and it currently completely
ignores the passed in suggested file name. But it did have code to truncate a
suggested name if it was too long.
I'm going to try to modify nsLocalFile::CreateUnique to behave like the old
nsFileSpec::MakeUnique used to.
This could be a work around we could use for rtm....
Apparently we've had this problem in mail and other places before in code that
still uses nsFileSpec. That code uses this dummy technique I described above.
Updated•24 years ago
|
Whiteboard: [rtm need info]
Comment 7•24 years ago
|
||
mscott, can i help you with this bug?
Comment 8•24 years ago
|
||
JF, we discussed making this fix be Mac-specific since other platforms are not
affected by it. (Do we have any 8.3 problems to worry about? Phil filed a bug
that may boil down to this problem.) If you can find a small, safe patch it
will help things along.
Reporter | ||
Comment 9•24 years ago
|
||
Reporter | ||
Comment 10•24 years ago
|
||
Okay here's a fix to CreateUnique that makes things better. I modified create
unique to do a couple of things:
1) If a suggested name is passed in, then we should honor it!! Before, this
parameter was completely ignored. So the only way to pass in your suggested name
was to Append the name and then call create Unique. This is what I was doing in
the exthandler. And Append barfs big time with long file names on the mac.
2) For the Mac only, truncate the suggested file name or the leaf name if the
non extension part is over 27 characters. This code was already there before.
Now in the uriloader I can call CreateUnique directly with a long suggested file
name and on the mac, we'll properly truncate the suggested name into a valid
file name.
Status: NEW → ASSIGNED
Reporter | ||
Comment 11•24 years ago
|
||
Simon and Conrad, can I get code reviews from you guys for this patch?
Comment 12•24 years ago
|
||
The patch looks good. r=sfraser
Assignee | ||
Comment 13•24 years ago
|
||
I'm not sure about this. If suggestedName != nsnull, it always enters the loop.
So, if you wanted a file named "foo", passed that as suggestedName, and even if
"foo" didn't exist in that dir, you'd get foo-1.
Reporter | ||
Comment 14•24 years ago
|
||
oh good catch your right, that's not the behavior I intended. I'll change the
loop so we test for existence with the suggested name before attempting to add
uniqueness code.
Comment 15•24 years ago
|
||
No comments for a week now. Is this dead for rtm?
Comment 16•24 years ago
|
||
Seems like a new patch is required. Is this happening soon? I'd like to see
this get into rtm+ state ASAP or moved to rtm- if we've given up.
Comment 17•24 years ago
|
||
rtm-, we don't appear to be pursuing this for its own sake anymore.
Whiteboard: [rtm need info] → [rtm-]
Reporter | ||
Updated•24 years ago
|
Target Milestone: --- → Future
Keywords: nsenterprise
Comment 18•23 years ago
|
||
adding nsenterprise+
Keywords: nsenterprise → nsenterprise+
Target Milestone: Future → mozilla0.9.4
Reporter | ||
Comment 19•23 years ago
|
||
Not sure why enterprise wants this but over to the nslocalFileMac owners. Conrad?
Assignee: mscott → ccarlen
Status: ASSIGNED → NEW
Assignee | ||
Comment 20•23 years ago
|
||
Accepting. I'd like to go with the original idea:
> 1) preserve the file extension
> 2) ensure uniqueness of the file so we don't stomp on an exisiting file when we
making sure it's applied to anything which can affect the leafname of the file.
Status: NEW → ASSIGNED
Comment 21•23 years ago
|
||
This hasn't been touched in a month. Is this still an enterprise show stopper?
Comment 22•23 years ago
|
||
Is there a current workaround that we can apply to the branch, like we did for
previous release.
Whiteboard: PDT
Target Milestone: mozilla0.9.4 → mozilla0.9.5
Reporter | ||
Comment 23•23 years ago
|
||
I really don't believe this is an enterprise stopper. I'd like to see some
justificaiton as to why they think this is nsEnterprise+. In the mean time this
should be PDT-.
Comment 25•23 years ago
|
||
Pls provide an ETA, if you believe we should fix this on the branch.
Whiteboard: PDT → [PDT] [ETA ?]
Assignee | ||
Comment 26•23 years ago
|
||
mscott, is this problem even possible when downloading files anymore? The temp
file has a salted name < 31 chars and then, when it comes to picking the final
location, the file picker UI (on the OS level) will ensure that the file name is
not too long. Not saying this isn't a problem but just one that's not likely
(possible) to be encountered and therefore not PDT.
Reporter | ||
Comment 27•23 years ago
|
||
conrad: your exactly right which is why I recommended a minus for this bug. I
think grega made it a minus and it's no longer nominated for the branch.
Reporter | ||
Comment 28•23 years ago
|
||
Just read Jaime's comment. Jaime why do you think you are interested in this
bug? It's not even nominated for the branch. I'm not sure where the [PDT] came
from. Maybe that's how it accidentally got on their radar.
Comment 29•23 years ago
|
||
Looks like the PDT was added when this was a enterprise+, to keep it on the
radar. Since it is now a enterprise+, and not a stopper = PDT-
Whiteboard: [PDT] [ETA ?] → [PDT-] [ETA ?]
Assignee | ||
Comment 30•23 years ago
|
||
-> 0.9.7
Pushing off since this it is unlikely, if not impossible, to encounter this problem.
Target Milestone: mozilla0.9.5 → mozilla0.9.7
Assignee | ||
Comment 31•23 years ago
|
||
While not a dup of bug 95481, will be fixed (on OS X anyway) by that.
-> 0.9.8
Target Milestone: mozilla0.9.7 → mozilla0.9.8
Assignee | ||
Comment 33•23 years ago
|
||
The patch limits the length to 31 chars, truncating it from the middle if need
be. It's done on Append() and SetLeafName(). It's not done on InitWithPath()
because, since that's a native path, it's required to be valid. BTW, I'll
replace the ellipsis as character constant with kEllipsisChar = 0X?? as soon as
I can find its value. Works when used with Steve's patch for bug 60203 and
downloading Linux nightly from mozilla.org.
Attachment #17462 -
Attachment is obsolete: true
Comment 34•23 years ago
|
||
Comment on attachment 64402 [details] [diff] [review]
patch to truncate name to 31 chars
r=sdagley (yes, please change the hardcoded ... to a const)
Attachment #64402 -
Flags: review+
Comment 35•23 years ago
|
||
Hrmm. I think truncating a char* representation of the filename like that may do
bad things on systems with a non-Roman system script (e.g. Japanese).
OS: Windows NT → Mac System 8.5
Hardware: PC → Macintosh
Assignee | ||
Comment 36•23 years ago
|
||
Looking at the Script Mgr and others, I can't yet find a way to determine
character boundaries in a sequence of multibyte chars. Anybody know? Also, is
the HFS name length limit 31 bytes or 31 characters? Anyway, if we don't
truncate a file name > 31 bytes on a MacRoman system, that file operation is
hosed anyway.
Comment 37•23 years ago
|
||
You'd need to use CharacterByteType and/or FillParseTable. See the Script
Manager documentation for details.
Assignee | ||
Comment 38•23 years ago
|
||
Thanks - FillParseTable seemed like the way to go.
Attachment #64402 -
Attachment is obsolete: true
Comment 39•23 years ago
|
||
Hrmm, I think that the ellipsis character will show up as something else in
Japanese. ftang?
Assignee | ||
Comment 40•23 years ago
|
||
sTable[charPos] should be sTable[aNode[charPos]]
Attachment #64569 -
Attachment is obsolete: true
Updated•23 years ago
|
Assignee | ||
Comment 41•23 years ago
|
||
> I think that the ellipsis character will show up as something else in
> Japanese.
It will. Although, it is possible to look up the string which is smTokenEllipsis
in the intl4 table. Working on that...
Assignee | ||
Comment 42•23 years ago
|
||
This should be it. Gets the tokenEllipsis from the itl4 tables, resorts to
ASCII "..." if that cannot be found. Tested, need r=/sr=
Attachment #64576 -
Attachment is obsolete: true
Comment 43•23 years ago
|
||
Comment on attachment 64938 [details] [diff] [review]
Gets ellipsis from itl4 tables
r=sdagley
Attachment #64938 -
Flags: review+
Comment 44•23 years ago
|
||
Comment on attachment 64938 [details] [diff] [review]
Gets ellipsis from itl4 tables
Looks good. sr=sfraser
Attachment #64938 -
Flags: superreview+
Assignee | ||
Comment 45•23 years ago
|
||
Checked in. Thanks.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•