Bug 151783 – Allow JavaScript to iterate over plugins for local files

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Brent Fulgham]

In Bug 149014 we hid all plugins (except Flash, Java, and QuickTime) so that sites could not inspect the user configuration looking for fingerprinting entropy.

However, there are some cases where we would like to allow this information to be accessed by JavaScript, such as in Safari's "Help | Installed Plug-ins" page.

Therefore, allow documents loaded from the local file system to iterate over all plugins via JavaScript.

[Brent Fulgham]

Created attachment 266489 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 266489 [details]
Patch

Wouldn't it be safer to have Safari call some SPI on a WKWebView that allows it to enumerate all plug-ins?

[Brent Fulgham]

<rdar://problem/23692113>

[Brent Fulgham]

(In reply to comment #2)
> Comment on attachment 266489 [details]
> Patch
> 
> Wouldn't it be safer to have Safari call some SPI on a WKWebView that allows
> it to enumerate all plug-ins?

That was my original proposal, but after discussion with Anders and Alexey it was decided that the "local file" test is sufficient.

[Said Abou-Hallawa]

Comment on attachment 266489 [details]
Patch

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

> LayoutTests/http/tests/plugins/plugin-javascript-access.html:16
> +        else

I think there is no need for else here.

> LayoutTests/http/tests/plugins/plugin-javascript-access.html:34
> +            }

I think the style guidelines (for C++ at least) state that there is no need for braces for single line blocks.

> LayoutTests/http/tests/plugins/plugin-javascript-access.html:79
> +    document.writeln("<p>FAILURE!  (Failed to find netscape test plugin)<\/p>");

Why do we have two different messages for not finding the netscape test pulgin:
1. (Could not find the netscape test plugin on a non-local file!)
2. (Failed to find netscape test plugin)

Also can't we move writing the pass/failure messages to checkForTestPlugin()? You may need to pass a parameter like shouldFindTestPlugin and it's equal to foundTestPlugin, then you write PASS otherwise you write FAILURE.

[Brent Fulgham]

Comment on attachment 266489 [details]
Patch

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

>> LayoutTests/http/tests/plugins/plugin-javascript-access.html:16
>> +        else
> 
> I think there is no need for else here.

it's how the 'found" state is reached. However, I restructured things so that we don't need the 'foundTestPlugin' variable or the else.

>> LayoutTests/http/tests/plugins/plugin-javascript-access.html:34
>> +            }
> 
> I think the style guidelines (for C++ at least) state that there is no need for braces for single line blocks.

Sure. I'll change that.

>> LayoutTests/http/tests/plugins/plugin-javascript-access.html:79
>> +    document.writeln("<p>FAILURE!  (Failed to find netscape test plugin)<\/p>");
> 
> Why do we have two different messages for not finding the netscape test pulgin:
> 1. (Could not find the netscape test plugin on a non-local file!)
> 2. (Failed to find netscape test plugin)
> 
> Also can't we move writing the pass/failure messages to checkForTestPlugin()? You may need to pass a parameter like shouldFindTestPlugin and it's equal to foundTestPlugin, then you write PASS otherwise you write FAILURE.

I revised the messages so they have the same phrasing, but left the two cases. However, I don't really want the function to be responsible for the pass/fail state (or the message), since in the first part of the test finding the plugin has a different meaning than after we turn on the debugging feature.

[Brent Fulgham]

Committed r193363: <http://trac.webkit.org/changeset/193363>

[Darin Adler]

I’d really prefer that this security check be done with SecurityOrigin function rather than a direct call to isLocalFile.

[Brent Fulgham]

(In reply to comment #8)
> I’d really prefer that this security check be done with SecurityOrigin
> function rather than a direct call to isLocalFile.

Please see Bug 152489, where I make that correction.