360070 - Extension manager should use nsIFile.createUnique to extract into the temp dir

Status: RESOLVED WORKSFORME

(Toolkit :: Add-ons Manager, defect)

Description

[John Woods]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.7) Gecko/20060909 Firefox/1.5.0.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.7) Gecko/20060909 Firefox/1.5.0.7

When the EM creates a temporary copy of install.rdf, it does this at the location pointed to by the TEMP environment variable. Though Firefox may have a minimum requirements that exclude the FAT16 filesystem, it cannot be sure that temp files aren't stored on such a filesystem. Ecomstation's democd is one such case, though there may be others out there. Digging through the cobwebs of my fat16 days... the rules used to be, if you format a drive to a certain size, say 4 Meg, it will be given a filesystem determined by the size of the drive. Sometimes there were options to override the default filesystem, other times there were not. TEMP= drives do not necessarily need big drives, so this may happen elsewhere.

Reproducible: Always

Steps to Reproduce:
1. Boot ecomstation democd
2. Start firefox provided on cd, or better upgrade
3. Install any extension
Actual Results:  
Firefox could not install this item because "install-qx4..rdf" (provided  by
the item) is not well-formed or does not exist. Please contact the author about
this problem.

Expected Results:  
Extension installs without problems.

The EM is taking the basename and adding 4 chars to the name after generating a random 3 char set like so... install.rdf becomes install-xyz.rdf. FAT 16 filesystems can only have 8 chars in the name, and 3 chars for an extension. Ecomstation does not use the fat16 drive to store firefox on, just some of it's  own files and any temp files, log files etc. Firefox is stored on a fat32 ramdrive. Otoh, if you create a small enough ramdrive, it will be fat16.

I started to address this in this in a different bug, but #firefox consensus was make new bug. Here is the attachment I posted there.
https://bugzilla.mozilla.org/attachment.cgi?id=244927&action=view

What should nsExtensionManager do instead? 
1. Possibly use nsIFile.createUnique()

Comment 1

[Dave Townsend [:mossop]]

This is probably a pretty simple fix, but anything you do is going to exacerbate bug 292854. There's also a more fundamental question about how far Firefox should go to deal with platforms that have what in my view is a pretty basic feature these days.

Comment 2

[John Woods]

I need to amend step 2. The current ecomstation democd has firefox 1.0. I never ran it after the first time I saw it. It can install extensions, although the ones I like to use wouldn't install as minVersion was too high for that. So I upgraded the Firefox it used and found this error. So after getting a warpzilla version of Firefox, this bug becomes relevant to newer changes in nsExtensionManager.js. I am also creating another attachment. 

This bug... it is a small problem I admit. Until you get it. Then nothing installs. Here's my first stab at a fix.

827 function extractRDFFileToTempDir(zipFile, fileName, suppressErrors) {
828   var file = getFile(KEY_TEMPDIR, [getRandomFileName(fileName)]);
829   try {
830     var zipReader = getZipReaderForFile(zipFile);
831     zipReader.extract(fileName, file);
832     zipReader.close();
833   }
834   catch (e) {
835     if (!suppressErrors) {


		if( e=0x8052000f) alert("The system would not allow creating a "
			+ "temporary file due to the length of the file name.");
		else

or
		if( e=NS_FILE_TOO_BIG) alert("The system would not allow creating a "
			+ "temporary file due to the length of the file name.");
		else





836       showMessage("missingFileTitle", [], "missingFileMessage", 
837                   [BundleManager.appName, fileName]);
838       throw e;
839     }
840   }
841   return file;
842 }
843 

Comment 3

[John Woods]

Attachment: nsExtensionManager.js relevant sections and some venkman stuff

Here also is a way to get a unique, tempfile that works on any drive. The naming error thrown in venkman.

Comment 4

[John Woods]

First of all, I made an assignment when the equality test was what I was trying to do. Late night... and the e.result part was left out. e.result==0x8052000f

Since getRandomFileName is called in file's creation, there is no 
way to check if it is legal in that filesystem, and maybe make a correction.
That's why I check the result.

828   var file = getFile(KEY_TEMPDIR, [getRandomFileName(fileName)]);
829   try {

Ok, so here is a test that shows what I'm saying. This is what you get when a
small ramdrive is used for temp files. They will usually be fat16, because that what ramdrive.sys vdisk.sys all made. They can be used for up to 32 meg. Just paste into your favorite javascript environment and run. This same error happens when install-abc.rdf is used as a temp filename, and Firefox tries to create it on a ramdrive. So at present it may be just eComstation...


try{file1 = Components.classes["@mozilla.org/file/directory_service;1"]
.getService(Components.interfaces.nsIProperties)
.get("TmpD",Components.interfaces.nsIFile);
file1.append("22222222222222222222222222222222222222222222222222222222222222222"
+"22222222222222222222222222222222222222222222222222222222222222222222222222222"
+"22222222222222222222222222222222222222222222222222222222222222222222222222222"
+"22222222222222222222222222222222222222222222222222222222222222222222222222222"
222222222222222222222222222suggestedName.tmp");
file1.createUnique(Components.interfaces.nsIFile.NORMAL_FILE_TYPE, 0664);
alert(file1.path);
}
catch(e)
  {
  if( e.result==0x8052000f)
  alert("Your message goes here... but the filename"
         +"was not allowed to be created");
}

Comment 5

[Peter Weilbacher]

Not sure if this is a valid bug of Firefox. It is stated in the README:

     + Installation on a file system supporting long file names
       (i.e. HPFS or JFS but not FAT)

Well, it doesn't _explicitely_ say that the filesystem of TMP/TEMP also needs to be able to handle long filenames. Do we need to add that?

But even on OS/2 there are RAM drives that can definitely handle long filenames with multiple dots. I have been using RamFS v1.21 from
   http://hobbes.nmsu.edu/cgi-bin/h-search?key=ramfs121
for quite some time now, and especially as holder of the temporary directory it works great, installing extensions and all.

Perhaps the Demo CD feedback form would be a better place to leave a comment:
   http://www.ecomstation.com/demo/?l=man
I will also contact the Mensys guys about this.

Comment 6

[John Woods]

(In reply to comment #5)
> Not sure if this is a valid bug of Firefox. It is stated in the README:

yup. I'm sure.

> 
>      + Installation on a file system supporting long file names
>        (i.e. HPFS or JFS but not FAT)
> 
> Well, it doesn't _explicitely_ say that the filesystem of TMP/TEMP also needs
> to be able to handle long filenames. Do we need to add that?
> 

That's correct, it does not... and they are 2 separate things aren't they.
Besides, that would be more work that fixing the source. Especially since the error emitted throws blame elsewhere like to extension devlopers like me, it had better be right when it does that! It was not correct in this eComstation instance. Hence the bug.

http://lxr.mozilla.org/mozilla/source/toolkit/mozapps/extensions/src/nsExtensionManager.js.in#535
 /*
  * Randomize the specified file name. Used to force RDF to bypass the cache
  * when loading certain types of files.
  * @param   fileName 
  *          A file name to randomize, e.g. install.rdf
  * @returns A randomized file name, e.g. install-xyz.rdf
  */
 function getRandomFileName(fileName) {
  var extensionDelimiter = fileName.lastIndexOf(".");
  var prefix = fileName.substr(0, 4);
  var suffix = fileName.substr(extensionDelimiter);
  var characters = "abcdefghijklmnopqrstuvwxyz0123456789";
  var nameString = prefix + "-";

   for (var i = 0; i < 3; ++i) {
     var index = Math.round((Math.random()) * characters.length);
     nameString += characters.charAt(index);
   }
   return nameString + suffix;
 }
 
which would return files like inst-xyz.rdf without any double dots, since
suffix already has a dot, and gets rid of the problem. The function here 
differs only on lines 545 and 554.


> But even on OS/2 there are RAM drives that can definitely handle long filenames
> with multiple dots. I have been using RamFS v1.21 from
>    http://hobbes.nmsu.edu/cgi-bin/h-search?key=ramfs121
> for quite some time now, and especially as holder of the temporary directory it
> works great, installing extensions and all.

I din't say that eComstation's OS2 had no support for long filenames on any ramdrives. The D: drive is a ramdrive, and it supports long filenames. Just not C:. Curious. It must have it's reasons though.

> 
> Perhaps the Demo CD feedback form would be a better place to leave a comment:
>    http://www.ecomstation.com/demo/?l=man
> I will also contact the Mensys guys about this.
> 

Why? Firefox needs to be correct when it throws an error, and blame. At least when it is pointed out to someone who can do something about it. I'm still waiting for the developer's os2 they promised. Till then, I'll just sympathize with ya.

Comment 7

[Peter Weilbacher]

I don't have the Demo CD to try that, but the guys who created it said that they certainly use a RAMFS that is capable of long filenames. (And regarding the "Developer OS/2", I certainly paid for my copy of eComStation...)

But OK I see your point and understand why you want to fix the Firefox behavior. Nothing I can help you with, though, so I remove the CC.

Comment 8

[John Woods]

(In reply to comment #7)
> I don't have the Demo CD to try that, but the guys who created it said that
> they certainly use a RAMFS that is capable of long filenames. (And regarding
> the "Developer OS/2", I certainly paid for my copy of eComStation...)

It's not on C: drive, where %TEMP% points to.

After what I've been through with this headache, I wouldn't give $20 for it.
When I saw how much they wanted, it made me remember why Microsoft won the OS struggle with IBM. They give more for less. My $250 operating system budget is already allocated to Vista. (For my wife's machine, as I use debian-knoppix.)

> 
> But OK I see your point and understand why you want to fix the Firefox
> behavior. Nothing I can help you with, though, so I remove the CC.
> 

Since you were in touch with the developers of the democd, did you at least point out the temporary file problem and the ramdrive they do use for drive C: ?

Comment 9

[Dave Townsend [:mossop]]

Confirming as a valid issue.

Comment 10

[Blair McBride [:Unfocused] (UNAVAILABLE)]

The Add-ons Manager has since been rewritten, and uses nsIFile.createUnique() in the appropriate places. And there's been no recent reports of long filename issues (extensions remain packed by default now anyway, so don't need to be extracted). Please file a new bug for any such new issue relating to the new Add-ons Manager.