Bug 185960 – Update plugin search path to look for user installed plugins

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[youenn fablet]

Update plugin search path to look for user installed plugins

[youenn fablet]

Created attachment 341224 [details]
Patch

[Brent Fulgham]

Comment on attachment 341224 [details]
Patch

Looks good! r=me.

[Anders Carlsson]

Won't this break any users that have plug-ins installed in the container directory? 

I think you should either:

- Pick the home folder based on whether sandboxing is enabled or not.
- Add both sets of directories.

[youenn fablet]

(In reply to Anders Carlsson from comment #3)
> Won't this break any users that have plug-ins installed in the container
> directory? 

The plug-ins usually install themselves globally or on user home folder, not in the container of a specific application embedding WebKit.

> I think you should either:
> 
> - Pick the home folder based on whether sandboxing is enabled or not.
> - Add both sets of directories.

I am fine adding both if sandboxing is enabled but I am not sure this is worth it.
AFAIK, WebKit UIProcess is currently not sandboxed in platforms supporting plug-ins so I do not think that any user is currently using a plug-in that is installed in the container directory.

[youenn fablet]

Comment on attachment 341224 [details]
Patch

Cq+ing now.
Anders, feel free to stop the cq if you think we should add both folders in case sandbox is enabled.

[WebKit Commit Bot]

Comment on attachment 341224 [details]
Patch

Clearing flags on attachment: 341224

Committed r232169: <https://trac.webkit.org/changeset/232169>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/40539448>

[Anders Carlsson]

(In reply to youenn fablet from comment #4)
> (In reply to Anders Carlsson from comment #3)
> > Won't this break any users that have plug-ins installed in the container
> > directory? 
> 
> The plug-ins usually install themselves globally or on user home folder, not
> in the container of a specific application embedding WebKit.

An app can put a plug-in inside the user home folder - I don't think there's any other way for an application to expose a plug-in to WebKit.

> 
> > I think you should either:
> > 
> > - Pick the home folder based on whether sandboxing is enabled or not.
> > - Add both sets of directories.
> 
> I am fine adding both if sandboxing is enabled but I am not sure this is
> worth it.
> AFAIK, WebKit UIProcess is currently not sandboxed in platforms supporting
> plug-ins so I do not think that any user is currently using a plug-in that
> is installed in the container directory.

There are many macOS apps that are sandboxed. This patch could potentially break them.

[Brent Fulgham]

(In reply to Anders Carlsson from comment #9)
> > The plug-ins usually install themselves globally or on user home folder, not
> > in the container of a specific application embedding WebKit.
> 
> An app can put a plug-in inside the user home folder - I don't think there's
> any other way for an application to expose a plug-in to WebKit.

I'm confused -- do you mean there are apps that have plugins hosted inside their container, and need WebKit to access that location? I wasn't aware of that.

In that case, it seems like adding the container directory as well would be a reasonable thing to do.

Comment on attachment 341224 [details]
Patch

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

> Source/WebKit/UIProcess/Plugins/mac/PluginInfoStoreMac.mm:52
> +    if (auto* pw = getpwuid(getuid()))
> +        pluginsDirectories.uncheckedAppend(makeString(pw->pw_dir, pluginPath));

The change log doesn’t explain what’s motivating this change or how it can be tested, but this change doesn’t really help sandboxed WebKit clients use plug-ins from the user’s home directory, because by default, sandboxed apps don’t have access to Library/Internet Plug-Ins under the user’s home directory. Perhaps you tested with MiniBrowser and concluded that this fix is good, but MiniBrowser has an entitlement that gives it read-only access to the entire file system. Most other sandboxed apps that use WebKit don’t have this sort of access. The way sandboxed apps enjoy access to some locations under the user’s Library directory is that (a) the application sandbox (/System/Library/Sandbox/Profiles/application.sb) allows this access and (b) the sandbox machinery creates a symlink from the Library directory inside the app’s container to the appropriate location in the user’s home directory. The code in the app (or any framework used by the app) only ever needs to use NSHomeDirectory(), and the symlink and the sandbox take care of the rest. You can see all those symlinks if you look in ~/Library/Containers/org.webkit.MiniBrowser/Data/Library.

(In reply to Anders Carlsson from comment #9)
> (In reply to youenn fablet from comment #4)
> > (In reply to Anders Carlsson from comment #3)
> > > Won't this break any users that have plug-ins installed in the container
> > > directory? 
> > 
> > The plug-ins usually install themselves globally or on user home folder, not
> > in the container of a specific application embedding WebKit.
> 
> An app can put a plug-in inside the user home folder - I don't think there's
> any other way for an application to expose a plug-in to WebKit.

In Legacy WebKit, WebPluginDatabase searches in

        [NSHomeDirectory() stringByAppendingPathComponent:@"Library/Internet Plug-Ins"],
        @"/Library/Internet Plug-Ins",
        [[NSBundle mainBundle] builtInPlugInsPath],

applications that bundle plug-ins are expected to use the last location. Perhaps modern WebKit is missing that last search path?