Bug 164103 – PluginInfoStore::loadPluginsIfNecessary can still load plugins multiple times

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[Michael Catanzaro]

PluginInfoStore::loadPluginsIfNecessary is trying hard to make sure plugin paths are unique, but it's possible for two unique paths to refer to the same directory. For instance, on Arch, /usr/lib64 is a symlink to /usr/lib. So all plugins that we find when scanning /usr/lib64/mozilla/plugins/ get found again when scanning /usr/lib/mozilla/plugins/ because it's the same directory.

[Carlos Garcia Campos]

I guess we could follow symlinks when adding paths to uniquePluginPaths

[Carlos Garcia Campos]

Looking at about:plugins in firefox, it seems they follow symlinks, because the plugin path they show is always the symlink destination.

[Carlos Garcia Campos]

Created attachment 293429 [details]
Patch

[WebKit Commit Bot]

This patch modifies one of the wasm.json files. Please ensure that any changes in one have been mirrored to the other. You can find the wasm.json files at "Source/JavaScriptCore/wasm/wasm.json" and "JSTests/wasm/wasm.json".

[Michael Catanzaro]

Comment on attachment 293429 [details]
Patch

Wow, the readlink API is terrible.

[Michael Catanzaro]

Don't break EFL, looks like a missing #include:

../../Source/WebKit2/UIProcess/Plugins/unix/PluginInfoStoreUnix.cpp: In function 'WTF::String WebKit::followSymlink(const WTF::String&)':
../../Source/WebKit2/UIProcess/Plugins/unix/PluginInfoStoreUnix.cpp:69:50: error: 'filenameToString' was not declared in this scope
             return filenameToString(buffer.data());
                                                  ^

Also a question: this works not only when the last component of the pathname is not a symlink, but also when an intermediate component is a symlink, right? Since that's the case we're trying to fix.

[Carlos Garcia Campos]

Comment on attachment 293429 [details]
Patch

Good point, we readlink only works if the given path is a link, we need realpath instead.

[Carlos Garcia Campos]

Created attachment 293548 [details]
Updated patch using realpath

The patch is indeed simpler now :-)

[Carlos Garcia Campos]

Created attachment 293549 [details]
Fix EFL build

filenameToString is GTK+ specific.

[Michael Catanzaro]

Comment on attachment 293549 [details]
Fix EFL build

View in context: https://bugs.webkit.org/attachment.cgi?id=293549&action=review

Bonus points if you tested this manually.

> Source/WebKit2/UIProcess/Plugins/unix/PluginInfoStoreUnix.cpp:64
> +            String pluginPath = String::fromUTF8(normalizedPath);

Looking at the comment above the definition of filenameToString, I think it'd be better to copy that function into FileSystemEfl and use it here unconditionally.

[Carlos Garcia Campos]

(In reply to comment #10)
> Comment on attachment 293549 [details]
> Fix EFL build
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=293549&action=review
> 
> Bonus points if you tested this manually.
> 
> > Source/WebKit2/UIProcess/Plugins/unix/PluginInfoStoreUnix.cpp:64
> > +            String pluginPath = String::fromUTF8(normalizedPath);
> 
> Looking at the comment above the definition of filenameToString, I think
> it'd be better to copy that function into FileSystemEfl and use it here
> unconditionally.

We already have that :-P stringFromFileSystemRepresentation()

[Carlos Garcia Campos]

Committed r208273: <http://trac.webkit.org/changeset/208273>