254777 2023-03-30 15:09:03 -0700 REGRESSION (Safari 16.4): PostMessage with transfer object is broken between contexts 2023-07-20 10:42:39 -0700 1 1 1 Unclassified WebKit WebCore Misc. Safari 16 All All RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=171216 https://bugs.webkit.org/show_bug.cgi?id=259362 BrowserCompat, InRadar P2 Major --- 1 tooker cdumez ajuma ap cdumez cockscomb karlcow michaeldo webkit-bug-importer wilander oldest_to_newest 1945507 0 tooker 2023-03-30 15:09:03 -0700 Sending a transferable object like a MessagePort to communicate between different contexts no longer works properly. When I use the transfer argument to postMessage the event handler gets an event with a null data property. Steps to reproduce: 1. Call postMessage({foo: 'bar', port}, self.origin, [port]) from page script. 2. Add window.message event handler to a different context such as an extension's content script context and evaluate the event callback arg. 3. Observe the ev.data is null instead of {foo: 'bar', port: port} Expected behavior: For the message handler to get an event object with a non-null data property filed with the contents sent from the postMessage caller. I first noticed this on the Orion Safari fork and filed a bug with them, https://orionfeedback.org/d/4730-postmessage-with-transfer-object-is-broken, but after updating to Safari 16.4 on the macOS 13.3 I see the same bug there, which leads me to believe this is a core WebKit regression. This is directly affecting my Safari extension which no longer works on anyone's 16.4 based Safari browsers. The same code works fine on all Chromium and Firefox. 1945514 1 tooker 2023-03-30 15:53:49 -0700 More observations. 1. I can only reproduce with MessagePort transferables. Using ArrayBuffer for example works as expected. 2. The data property of the MessageEvent is null but the ports[] array is populated with the MessagePort object. 3. Excluding the MessagePort from the sending side's data object works as expected. The data property of the MessageEvent is correctly set to the cloned object and the ports[] property contains the transferred MessagePort. I'll be refactoring my use case to use the technique from #3; It's slightly less safe because I need to make assumptions about the ordering of the ports[] property on the receiver side instead of being able to directly access the transferables from the data object property but at least I'll be able to get my Safari 16.4 users working again and support Orion. 1946072 2 karlcow 2023-04-02 17:49:53 -0700 It would be nice to have a simplified testcase 1946077 3 tooker 2023-04-02 18:18:47 -0700 (In reply to Karl Dubost from comment #2) > It would be nice to have a simplified testcase I'm not familiar enough with WebKit to simulate an extension's content-script context. I tried a simple test with a Worker and an iframe but neither are affected. 1946082 4 ap 2023-04-02 18:34:14 -0700 Would it be possible to provide easy to reproduce test steps, even if not minimized? This way, someone who isn't deeply familiar with the code could e.g. bisect to find the culprit change. 1946083 5 webkit-bug-importer 2023-04-02 18:34:24 -0700 <rdar://problem/107538083> 1946094 6 tooker 2023-04-02 18:55:20 -0700 Safari requires a significant amount of tooling and xcode interaction so I'll just skip that, but with Orion you can create a local extension folder and populate it with a manifest.json such as this... { "manifest_version": 2, "content_scripts": [{ "run_at": "document_start", "matches": ["*://*/*"], "js": ["ext-context.js"] }] ... } ext-context.js should do this... addEventListener('message', ev => { if (ev.data && ev.data.port instanceof MessagePort) { console.info("pass"); } else { console.error("fail"); } }); And then visit a mock page that runs this script... const ch = new MessageChannel(); const port = ch.port2; postMessage({port}, self.origin, [port]); I'll see if I can find where the regression came in too; I realize this is probably not helpful. 1947562 7 karlcow 2023-04-09 19:12:52 -0700 Justin Mayfield, does it work if you exclude the port in the data argument of postMessage? 1947563 8 tooker 2023-04-09 19:29:35 -0700 Yes, and that is the exact workaround I'm using in my extension. It only works because there is the .ports[] array to save the day when you transfer a MessagePort specifically. But then again, this only seems to affect MessagePort transfer objects. This is pattern I'm using now that works on all browsers.. const transfer = [port]; postMessage({portIndex: transfer.indexOf(port)}, transfer); And from my handler I do.. const port = event.ports[event.data.portIndex]; 1948049 9 wilander 2023-04-11 16:34:58 -0700 Hi, Justin! Thanks for filing. Could you share which extension this is for? And am I right in that you have already deployed a workaround so the bug won't reproduce with your extension? Thanks! 1948095 10 tooker 2023-04-11 17:32:36 -0700 (In reply to John Wilander from comment #9) > Hi, Justin! Thanks for filing. Could you share which extension this is for? > And am I right in that you have already deployed a workaround so the bug > won't reproduce with your extension? Thanks! Hello John, This is my extension: https://github.com/SauceLLC/sauce4strava https://apps.apple.com/us/app/sauce-for-strava/id1570922521 https://addons.mozilla.org/en-US/firefox/addon/sauce4strava/ https://chrome.google.com/webstore/detail/eigiefcapdcdmncdghkeahgfmnobigha And you are correct, I already released patched builds (v8.2.2) on the Apple and Firefox stores. This is the commit with my workaround: https://github.com/SauceLLC/sauce4strava/commit/9100e2db6245b5db7bf104b3044029d0b936ec7a I don't know how to get you a Safari based build with the old behavior from the app-store, but if you use Onion you can install this unpatched firefox build: https://addons.mozilla.org/firefox/downloads/file/4078653/sauce4strava-8.2.1.xpi I should warn you that you'll probably need to create an account with strava.com first. It's free, but there would be some tedium in trying to repro from my particular extension. You'll also need to be on a page that invokes these postMessage calls, such as this example: https://www.strava.com/activities/8355858982 1949620 11 cockscomb 2023-04-18 17:58:35 -0700 Hello. This issue probably affects Chrome iOS as well. I encountered a similar situation, and at first I thought it was a problem on Chrome's side, but it seems that a change on the WebKit side was having an effect instead. https://bugs.chromium.org/p/chromium/issues/detail?id=1431021 1949624 12 tooker 2023-04-18 18:16:42 -0700 (In reply to Hiroki Kato from comment #11) > Hello. > > This issue probably affects Chrome iOS as well. I encountered a similar > situation, and at first I thought it was a problem on Chrome's side, but it > seems that a change on the WebKit side was having an effect instead. > > https://bugs.chromium.org/p/chromium/issues/detail?id=1431021 Very interesting. This comment in particular is very interesting: https://bugs.chromium.org/p/chromium/issues/detail?id=1431021#c6 Thanks for cross referencing here Hiroki! 1949717 13 ajuma 2023-04-19 05:57:56 -0700 (In reply to Justin Mayfield from comment #12) > (In reply to Hiroki Kato from comment #11) > > Hello. > > > > This issue probably affects Chrome iOS as well. I encountered a similar > > situation, and at first I thought it was a problem on Chrome's side, but it > > seems that a change on the WebKit side was having an effect instead. > > > > https://bugs.chromium.org/p/chromium/issues/detail?id=1431021 > > Very interesting. This comment in particular is very interesting: > https://bugs.chromium.org/p/chromium/issues/detail?id=1431021#c6 https://bugs.chromium.org/p/chromium/issues/detail?id=1431021#c4b also has a small test app and test page. 1949718 14 ajuma 2023-04-19 05:58:32 -0700 (In reply to Ali Juma from comment #13) > (In reply to Justin Mayfield from comment #12) > > (In reply to Hiroki Kato from comment #11) > > > Hello. > > > > > > This issue probably affects Chrome iOS as well. I encountered a similar > > > situation, and at first I thought it was a problem on Chrome's side, but it > > > seems that a change on the WebKit side was having an effect instead. > > > > > > https://bugs.chromium.org/p/chromium/issues/detail?id=1431021 > > > > Very interesting. This comment in particular is very interesting: > > https://bugs.chromium.org/p/chromium/issues/detail?id=1431021#c6 > > https://bugs.chromium.org/p/chromium/issues/detail?id=1431021#c4b also has a > small test app and test page. Sorry, typo, I meant https://bugs.chromium.org/p/chromium/issues/detail?id=1431021#c4 1949821 15 cdumez 2023-04-19 13:17:11 -0700 Pull request: https://github.com/WebKit/WebKit/pull/12929 1949901 16 ews-feeder 2023-04-19 16:39:58 -0700 Committed 263155@main (ca6ca7d1895d): <https://commits.webkit.org/263155@main> Reviewed commits have been landed. Closing PR #12929 and removing active labels.