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 WebKit

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bugs-noreply, zan

Priority:

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

Carlos Garcia Campos

Reported 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.

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
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Carlos Garcia Campos

Comment 1 2019-02-11 00:37:29 PST

Created attachment 361666 [details] Patch

Zan Dobersek

Comment 2 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().

Carlos Garcia Campos

Comment 3 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.

Zan Dobersek

Comment 4 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.

Carlos Garcia Campos

Comment 5 2019-02-11 06:18:36 PST

Created attachment 361677 [details] Patch

Carlos Garcia Campos

Comment 6 2019-02-20 02:34:06 PST

Created attachment 362489 [details] Patch for landing

Carlos Garcia Campos

Comment 7 2019-02-20 06:34:58 PST

Committed r241816: <https://trac.webkit.org/changeset/241816>

Note You need to log in before you can comment on or make changes to this bug.