578579 - Firefox failing to find plugin when path too long >= 130 chars

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Plug-ins, defect)

Description

[Andy Idsinga (Intel)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US) AppleWebKit/533.4 (KHTML, like Gecko) Chrome/5.0.375.99 Safari/533.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.6) Gecko/20100625 Firefox/3.6.6

Firefox is not finding all of our plugins.
The same plugins are found in recent, older versions of Firefox.

Other versions tested:
3.5.5       - works
3.5.10      - works
3.6.4       - fails
4.0 beta 1  - fails

Symptoms:
- Some plugins are missing from about:plugins.
- When adding an embed tag to a page using JavaScript the browser displays the
  missing plugin UI

Hypothesis:
After comparing the windows registry settings of a plugin that was found vs
one that wasn't it appears the browser may be failing to find the plugin 
becuase of a path length limitation.
The steps to reproduce in this report seem to validate this hypothesis.



Reproducible: Always

Steps to Reproduce:
1.Edit windows registry for a given plugin to use a path >= 130 characters (us ascii) in length.
Registry key\value used in these steps is:
[HKEY_CURRENT_USER\Software\MozillaPlugins\@your_org/your_plugin_desc;version=a.b.c.d]\Path="absolute_path\\to\\your\\npYourPlugin.dll"

2. Place the plugin at the path you specified in the registry in #1 above (>= 130 chars).

3. Start firefox and observe that the plugin does NOT appear in about:plugins

4. Close firefox, re-edit the windows registry using a path with < 130 characters.

5. Restart firefox and observe that plugin will now appear in about:plugins.

6. Yay - you reproduced the problem - get coffee now :)
Actual Results:  
The browser will find plugins, as observed in about:plugins when the path to the plugin DLL is < 130 characters and will not find them when it is >= 130.

Expected Results:  
Firefox should be able to support the max path length for the filesystem from which it searches for and loads plugins.
In past versions (see summary in this report) it supported longer paths.

Comment 1

[Andy Idsinga (Intel)]

re - regression window: 
Problem started sometime between 3.5.10 and 3.6.4.

Comment 2

[Thomas Harning]

Similar, but unrelated to this is a limit on the maximum number of characters in the registry key to less than the 'real' maximum for a single API call is 32 * (255 + 1) characters [32 levels deep w/ 255 key length limit + \ separator].

Dynamic length calculation should be used alongside malloc, not _MAX_PATH which is not correct in Windows, especially when Unicode is involved.

Problem #1:
_MAX_PATH is 260 in Windows, whereas we are hitting a 130 character limit...

This is due to the fact that NS_ARRAY_LENGTH is returning the number of elements (characters) in the array.  RegQueryValueExW expects that the length passed into it be the number of bytes in the buffer... even though the output is in wide characters.

Changing:

DWORD pathlen = NS_ARRAY_LENGTH(path);
to
DWORD pathlen = NS_ARRAY_LENGTH(path) * sizeof(path[0]);

Should at least bring the length to 260.

Problem #2: Assuming _MAX_PATH in Windows is "really" the maximum path length.  This may (hopefully) hold for many function in the C API that Windows exports, especially ones that don't accept an input length on the buffer to be written to (ex: tmpnam).
The retrieval of the path from the registry should query the length of the value, then use that to get a properly sized buffer to store the full path in.
Querying the length of the path would be done through passing a NULL data buffer in, but passing the address of the data buffer length variable.

Comment 3

[(mostly gone) XtC4UaLL [:xtc4uall]]

This regressed on MC within:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=44c392db6672&tochange=96e8d529b2d3
This includes an Electrolysis Branch Merge to MC.
Hence i tried to find a Range on that Branch, but even going back to the first working Windows Build (Built from http://hg.mozilla.org/projects/electrolysis/rev/ef5eac530f66) the Issue is given.

Comment 4

[Masatoshi Kimura [:emk]]

(In reply to comment #2)
> DWORD pathlen = NS_ARRAY_LENGTH(path) * sizeof(path[0]);
It's redundant because the preprocessor will expand this to
"sizeof(path) / sizeof(path[0]) * sizeof(path[0])".

We don't have to consider about "real" maximum path length here because we already depends on MAX_PATH in many places.
http://mxr.mozilla.org/mozilla-central/search?string=MAX_PATH
Fixing this will be too big to land on the branch for the regression.

Comment 5

[GeorgesZ]

I understand about not considering about "real" maximum path length. Also, in depoyment to end user machine, it should usually not be much of a problem. Except when the plugin is deployed for an individual non-admin user instead of the whole machine. In this case, the path can get closer to the limit. For example Google update can have a path like "C:\Documents and Settings\gmzwinge\Local Settings\Application Data\Google\Update\1.2.183.39\npGoogleOneClick8.dll" which is 114 characters. Also, while developing plugin, that path length can be more easily reached due to the multiple hierarchy of folders usually involved in development. Would it be possible to make a change similar to the one proposed by Thomas above? The change goes in mozilla-1.9.\modules\plugin\base\src\nsPluginDirServiceProvider.cpp, in the function nsPluginDirServiceProvider::GetPLIDDirectoriesWithHKEY(). The change could be from:
      WCHAR path[_MAX_PATH];
      DWORD pathlen = NS_ARRAY_LENGTH(path);
to:
      WCHAR path[_MAX_PATH];
      DWORD pathlen = sizeof(path);
This would double the length of the path. And hopefully should not be too big of a change. Thanks.

Comment 6

[LeeRoy]

Hi Guys, just to update you, finally I found a way to solve it
Do you have error messages?, like :

* Path too long
* Error cannot delete file: cannot read from source file or disk
* Cannot delete file: Access is denied
* There has been a sharing violation.
* Cannot delete file or folder The file name you specified is not valid or too long. Specify a different file name.

For that I tried it with: http://longpathtool.com/

Comment 7

[Benjamin Smedberg]

Resolving old bugs which are likely not relevant any more, since NPAPI plugins are deprecated.