185960 2018-05-24 14:34:42 -0700 Update plugin search path to look for user installed plugins 2018-05-24 22:24:29 -0700 1 1 1 Unclassified WebKit WebKit2 WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 youennf youennf andersca bfulgham commit-queue ddkilzer mitz webkit-bug-importer oldest_to_newest 1427065 0 youennf 2018-05-24 14:34:42 -0700 Update plugin search path to look for user installed plugins 1427068 1 341224 youennf 2018-05-24 14:37:07 -0700 Created attachment 341224 Patch 1427085 2 341224 bfulgham 2018-05-24 15:55:33 -0700 Comment on attachment 341224 Patch Looks good! r=me. 1427086 3 andersca 2018-05-24 15:56:02 -0700 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. 1427089 4 youennf 2018-05-24 16:14:04 -0700 (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. 1427090 5 341224 youennf 2018-05-24 16:15:17 -0700 Comment on attachment 341224 Patch Cq+ing now. Anders, feel free to stop the cq if you think we should add both folders in case sandbox is enabled. 1427094 6 341224 commit-queue 2018-05-24 16:42:20 -0700 Comment on attachment 341224 Patch Clearing flags on attachment: 341224 Committed r232169: <https://trac.webkit.org/changeset/232169> 1427095 7 commit-queue 2018-05-24 16:42:22 -0700 All reviewed patches have been landed. Closing bug. 1427099 8 webkit-bug-importer 2018-05-24 16:43:17 -0700 <rdar://problem/40539448> 1427105 9 andersca 2018-05-24 16:54:45 -0700 (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. 1427110 10 bfulgham 2018-05-24 17:03:41 -0700 (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. 1427191 11 341224 mitz 2018-05-24 22:20:24 -0700 Comment on attachment 341224 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. 1427193 12 mitz 2018-05-24 22:24:29 -0700 (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? 341224 2018-05-24 14:37:07 -0700 2018-05-24 16:42:20 -0700 Patch bug-185960-20180524143707.patch text/plain 2104 youennf U3VidmVyc2lvbiBSZXZpc2lvbjogMjMyMTIzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDI3NzkyZTM5MTQ5NWI0OGUx NDBkMzA4YWU1NDdhM2JhMWZhOGI0MmUuLjRhY2E0OTU5NDM4ZWU0YzAzNjc4OTQzMTU4YmJlNTkx ZjE4Y2Y5NDggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTgtMDUtMjQgIFlvdWVubiBG YWJsZXQgIDx5b3Vlbm5AYXBwbGUuY29tPgorCisgICAgICAgIFVwZGF0ZSBwbHVnaW4gc2VhcmNo IHBhdGggdG8gbG9vayBmb3IgdXNlciBpbnN0YWxsZWQgcGx1Z2lucworICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTg1OTYwCisKKyAgICAgICAgUmV2aWV3 ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTm93IHRoYXQgVUlQcm9jZXNzIG1heSBi ZSBzYW5kYm94ZWQsIHRoZSBob21lIGRpcmVjdG9yeSBpcyBubyBsb25nZXIgdGhlIHVzZXIgaG9t ZSBkaXJlY3RvcnkuCisgICAgICAgIFVwZGF0ZSB0aGUgcGF0aCB0byBzdGlsbCBsb29rIGZvciBw bHVnaW5zIGluIHRoZSB1c2VyIGhvbWUgZGlyZWN0b3J5LgorCisgICAgICAgICogVUlQcm9jZXNz L1BsdWdpbnMvbWFjL1BsdWdpbkluZm9TdG9yZU1hYy5tbToKKyAgICAgICAgKFdlYktpdDo6UGx1 Z2luSW5mb1N0b3JlOjpwbHVnaW5zRGlyZWN0b3JpZXMpOgorCiAyMDE4LTA1LTIzICBZb3Vlbm4g RmFibGV0ICA8eW91ZW5uQGFwcGxlLmNvbT4KIAogICAgICAgICBNaWdyYXRlIEZyb20tT3JpZ2lu IHRvIENyb3NzLU9yaWdpbi1SZXNvdXJjZS1Qb2xpY3kKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJL aXQvVUlQcm9jZXNzL1BsdWdpbnMvbWFjL1BsdWdpbkluZm9TdG9yZU1hYy5tbSBiL1NvdXJjZS9X ZWJLaXQvVUlQcm9jZXNzL1BsdWdpbnMvbWFjL1BsdWdpbkluZm9TdG9yZU1hYy5tbQppbmRleCA1 NGI2MTI1NzJjYmY5Y2VhNDM3MmFkYjI3OWI0Zjg5Mjc1ZTcwZWUwLi4yNTdjOTRjNTc4NGQxY2Ux OTViNDQ2YzM3Y2M1ZmRiNzAyZTMwNTc2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L1VJUHJv Y2Vzcy9QbHVnaW5zL21hYy9QbHVnaW5JbmZvU3RvcmVNYWMubW0KKysrIGIvU291cmNlL1dlYktp dC9VSVByb2Nlc3MvUGx1Z2lucy9tYWMvUGx1Z2luSW5mb1N0b3JlTWFjLm1tCkBAIC0zMiw2ICsz Miw3IEBACiAjaW1wb3J0ICJOZXRzY2FwZVBsdWdpbk1vZHVsZS5oIgogI2ltcG9ydCAiU2FuZGJv eFV0aWxpdGllcy5oIgogI2ltcG9ydCA8V2ViQ29yZS9QbHVnaW5CbGFja2xpc3QuaD4KKyNpbXBv cnQgPHB3ZC5oPgogI2ltcG9ydCA8d3RmL0hhc2hTZXQuaD4KICNpbXBvcnQgPHd0Zi9SZXRhaW5Q dHIuaD4KICNpbXBvcnQgPHd0Zi90ZXh0L0NTdHJpbmcuaD4KQEAgLTQzLDkgKzQ0LDEzIEBAIG5h bWVzcGFjZSBXZWJLaXQgewogVmVjdG9yPFN0cmluZz4gUGx1Z2luSW5mb1N0b3JlOjpwbHVnaW5z RGlyZWN0b3JpZXMoKQogewogICAgIFZlY3RvcjxTdHJpbmc+IHBsdWdpbnNEaXJlY3RvcmllczsK KyAgICBwbHVnaW5zRGlyZWN0b3JpZXMucmVzZXJ2ZUluaXRpYWxDYXBhY2l0eSgyKTsKIAotICAg IHBsdWdpbnNEaXJlY3Rvcmllcy5hcHBlbmQoW05TSG9tZURpcmVjdG9yeSgpIHN0cmluZ0J5QXBw ZW5kaW5nUGF0aENvbXBvbmVudDpAIkxpYnJhcnkvSW50ZXJuZXQgUGx1Zy1JbnMiXSk7Ci0gICAg cGx1Z2luc0RpcmVjdG9yaWVzLmFwcGVuZCgiL0xpYnJhcnkvSW50ZXJuZXQgUGx1Zy1JbnMiKTsK KyAgICBBU0NJSUxpdGVyYWwgcGx1Z2luUGF0aCB7ICIvTGlicmFyeS9JbnRlcm5ldCBQbHVnLUlu cyIgfTsKKworICAgIGlmIChhdXRvKiBwdyA9IGdldHB3dWlkKGdldHVpZCgpKSkKKyAgICAgICAg cGx1Z2luc0RpcmVjdG9yaWVzLnVuY2hlY2tlZEFwcGVuZChtYWtlU3RyaW5nKHB3LT5wd19kaXIs IHBsdWdpblBhdGgpKTsKKyAgICBwbHVnaW5zRGlyZWN0b3JpZXMudW5jaGVja2VkQXBwZW5kKHBs dWdpblBhdGgpOwogICAgIAogICAgIHJldHVybiBwbHVnaW5zRGlyZWN0b3JpZXM7CiB9Cg==