Bug 186788 – Provide a way for Injected Bundles to indicate classes approved for secure encoding/decoding

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Brent Fulgham]

We want to use NSSecureCoding for data passed over IPC. However, WebKit clients using the Injected Bundle mechanism sometimes want to serialize parameter values that are unique to the WebKit client. WebKit doesn't know anything about these classes, and so cannot enable them for NSSecureCoding.

This patch adds a new WKBundle method, WKBundleExtendClassesForParameterCoder, WKBundleExtendClassesForParameterCoder, which is used to define additional classes that the coder/decoder should permit.

[Brent Fulgham]

<rdar://problem/41094167>

[Brent Fulgham]

Created attachment 343117 [details]
Patch

[Brent Fulgham]

Created attachment 343157 [details]
Patch v2 (Fix 32-bit build)

[Brent Fulgham]

Created attachment 343207 [details]
Patch

[Brent Fulgham]

Created attachment 343209 [details]
Patch

[Brent Fulgham]

Created attachment 343214 [details]
Patch

[Brent Fulgham]

Created attachment 343241 [details]
Patch

[EWS Watchlist]

Comment on attachment 343241 [details]
Patch

Attachment 343241 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8278062

New failing tests:
accessibility/mac/selection-notification-focus-change.html

[EWS Watchlist]

Created attachment 343245 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Brent Fulgham]

Created attachment 343246 [details]
Patch

[EWS Watchlist]

Comment on attachment 343246 [details]
Patch

Attachment 343246 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8278944

New failing tests:
accessibility/smart-invert-reference.html
imported/w3c/web-platform-tests/streams/piping/error-propagation-forward.html

[EWS Watchlist]

Created attachment 343255 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 343246 [details]
Patch

Attachment 343246 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8279639

New failing tests:
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-audio.html

[EWS Watchlist]

Created attachment 343256 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Brent Fulgham]

Created attachment 343271 [details]
Patch

[EWS Watchlist]

Comment on attachment 343271 [details]
Patch

Attachment 343271 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8281808

New failing tests:
http/tests/misc/object-embedding-svg-delayed-size-negotiation-2.htm
accessibility/mac/selection-notification-focus-change.html

[EWS Watchlist]

Created attachment 343282 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Brent Fulgham]

performance-api/performance-observer-no-document-leak.html was a flaky timeout, and doesn't seem related to this patch.

[Chris Dumez]

Comment on attachment 343271 [details]
Patch

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

> Source/WebKit/WebProcess/InjectedBundle/API/c/mac/WKBundleMac.h:39
> +WK_EXPORT void WKBundleExtendClassesForParameterCoder(WKBundleRef bundle, WKArrayRef classes);

It would be good to confirm with Brady / Alex / Geoff if we need a corresponding Cocoa SPI for this, and where this SPI should be. I have been told no new C API without Cocoa equivalent.

> Source/WebKit/WebProcess/InjectedBundle/API/c/mac/WKBundleMac.mm:42
> +void WKBundleExtendClassesForParameterCoder(WKBundleRef bundle, WKArrayRef classes)

I think we should return early if classes is null instead of crashing.

[Brent Fulgham]

Comment on attachment 343271 [details]
Patch

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

>> Source/WebKit/WebProcess/InjectedBundle/API/c/mac/WKBundleMac.mm:42
>> +void WKBundleExtendClassesForParameterCoder(WKBundleRef bundle, WKArrayRef classes)
> 
> I think we should return early if classes is null instead of crashing.

OK -- I'll correct that.

[Brent Fulgham]

Created attachment 343616 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 343616 [details]
Patch for landing

Clearing flags on attachment: 343616

Committed r233207: <https://trac.webkit.org/changeset/233207>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Michael Catanzaro]

Hmm:

[608/829] Building CXX object Source/WebKit/CMakeFiles/WebKit.dir/WebProcess/InjectedBundle/API/c/WKBundle.cpp.o
/home/mcatanzaro/Projects/WebKit/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundle.cpp:300:5: warning: "WK_API_ENABLED" is not defined, evaluates to 0 [-Wundef]
 #if WK_API_ENABLED
     ^~~~~~~~~~~~~~


I'm guessing this should be:

#if PLATFORM(COCOA) && WK_API_ENABLED

[Michael Catanzaro]

Committed r233400: <https://trac.webkit.org/changeset/233400>