Bug 194494

Summary: [WPE] Send client host fd and library name as web process creation parameters
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WPE WebKitAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, zan
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=194216
Attachments:
Description Flags
Patch
none
Patch
zan: review+
Patch for landing none

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>