Bug 227343 - [GTK] MiniBrowser: add an option to enable the web process sandbox
Summary: [GTK] MiniBrowser: add an option to enable the web process sandbox
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2021-06-24 02:36 PDT by Carlos Garcia Campos
Modified: 2021-06-28 05:41 PDT (History)
2 users (show)

See Also:


Attachments
Patch (2.99 KB, patch)
2021-06-24 02:38 PDT, Carlos Garcia Campos
mcatanzaro: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2021-06-24 02:36:48 PDT
.
Comment 1 Carlos Garcia Campos 2021-06-24 02:38:05 PDT
Created attachment 432143 [details]
Patch
Comment 2 Michael Catanzaro 2021-06-24 08:46:28 PDT
Comment on attachment 432143 [details]
Patch

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

> Tools/MiniBrowser/gtk/main.c:149
> +    { "enable-sandbox", 0, 0, G_OPTION_ARG_NONE, &enableSandbox, "Enable web process sandbox support", NULL },

Ideally it would be on by default, to have the best chance of avoiding mistakes like developing new features that are incompatible with the sandbox (it has happened recently). I guess the sandbox will probably break layout tests if we're not careful, though. And I know you want to avoid waiting for D-Bus registration when running layout tests.

So I would flip it to disable-sandbox, if possible.

> Tools/MiniBrowser/gtk/main.c:682
> +        if (!webkit_website_data_manager_is_ephemeral(manager)) {
> +            webkit_web_context_add_path_to_sandbox(webContext, webkit_website_data_manager_get_base_data_directory(manager), TRUE);
> +            webkit_web_context_add_path_to_sandbox(webContext, webkit_website_data_manager_get_base_cache_directory(manager), TRUE);
> +        }

Um, I thought this was supposed to be done automatically, but I can't find any code for it, so I guess it has to be manual. :/
Comment 3 Michael Catanzaro 2021-06-24 10:03:44 PDT
(In reply to Michael Catanzaro from comment #2)
> So I would flip it to disable-sandbox, if possible.

If you do this, then we can also close bug #213174.

> > Tools/MiniBrowser/gtk/main.c:682
> > +        if (!webkit_website_data_manager_is_ephemeral(manager)) {
> > +            webkit_web_context_add_path_to_sandbox(webContext, webkit_website_data_manager_get_base_data_directory(manager), TRUE);
> > +            webkit_web_context_add_path_to_sandbox(webContext, webkit_website_data_manager_get_base_cache_directory(manager), TRUE);
> > +        }
> 
> Um, I thought this was supposed to be done automatically, but I can't find
> any code for it, so I guess it has to be manual. :/

Discussing this with Patrick, we think these should not be needed. Does something break if you don't add them? It might be a bug.
Comment 4 Carlos Garcia Campos 2021-06-28 05:41:14 PDT
Committed r279337 (239205@main): <https://commits.webkit.org/239205@main>