Bug 194494 - [WPE] Send client host fd and library name as web process creation parameters
Summary: [WPE] Send client host fd and library name as web process creation parameters
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WPE WebKit (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-02-11 00:35 PST by Carlos Garcia Campos
Modified: 2019-02-20 06:34 PST (History)
2 users (show)

See Also:


Attachments
Patch (8.38 KB, patch)
2019-02-11 00:37 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch (8.49 KB, patch)
2019-02-11 06:18 PST, Carlos Garcia Campos
zan: review+
Details | Formatted Diff | Diff
Patch for landing (8.58 KB, patch)
2019-02-20 02:34 PST, Carlos Garcia Campos
no flags 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 2019-02-11 00:35:03 PST
Instead of using command line arguments. The code is simpler and we don't need wpe specific code in process launcher glib implementation.
Comment 1 Carlos Garcia Campos 2019-02-11 00:37:29 PST
Created attachment 361666 [details]
Patch
Comment 2 Zan Dobersek 2019-02-11 03:43:53 PST
Comment on attachment 361666 [details]
Patch

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

> Source/WebKit/WebProcess/glib/WebProcessGLib.cpp:53
> +    if (!parameters.implementationLibraryName.isNull())

isEmpty() here, should avoid potential problems of passing empty strings to wpe_loader_init().
Comment 3 Carlos Garcia Campos 2019-02-11 03:58:28 PST
Comment on attachment 361666 [details]
Patch

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

>> Source/WebKit/WebProcess/glib/WebProcessGLib.cpp:53
>> +    if (!parameters.implementationLibraryName.isNull())
> 
> isEmpty() here, should avoid potential problems of passing empty strings to wpe_loader_init().

It's a CString not a String, so we don't have isEmpty there. We could explicitly check data()[0] != '\0' but I don't think it's needed for something sent the from the UI process. The only reason we are checking this is because wpe_loader_get_loaded_implementation_library_name() is new api, so when libwpe < 0.2 we receive a null string here.
Comment 4 Zan Dobersek 2019-02-11 05:45:12 PST
Comment on attachment 361666 [details]
Patch

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

>>> Source/WebKit/WebProcess/glib/WebProcessGLib.cpp:53
>>> +    if (!parameters.implementationLibraryName.isNull())
>> 
>> isEmpty() here, should avoid potential problems of passing empty strings to wpe_loader_init().
> 
> It's a CString not a String, so we don't have isEmpty there. We could explicitly check data()[0] != '\0' but I don't think it's needed for something sent the from the UI process. The only reason we are checking this is because wpe_loader_get_loaded_implementation_library_name() is new api, so when libwpe < 0.2 we receive a null string here.

The condition here should prevent wpe_loader_init() misbehaving. Currently it only covers a null CString, which is fine. But a non-null CString with a strlen() of 0 will pass the condition, but the dlopen() call in wpe_loader_init() will end up returning a handle to the process executable, which is not desired.

More than depending on what the UIProcess sends over IPC here, it should be preferable to validate arguments properly on the spot.
Comment 5 Carlos Garcia Campos 2019-02-11 06:18:36 PST
Created attachment 361677 [details]
Patch
Comment 6 Carlos Garcia Campos 2019-02-20 02:34:06 PST
Created attachment 362489 [details]
Patch for landing
Comment 7 Carlos Garcia Campos 2019-02-20 06:34:58 PST
Committed r241816: <https://trac.webkit.org/changeset/241816>