[PATCH 0/2] MR580: mshtml: postMessage improvements.
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> IE10 and up actually pass cloned data without converting them to strings, using the Structured Clone Algorithm. Implement support for the basic variant types for now. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlevent.c | 46 ++++++++++++++--- dlls/mshtml/htmlevent.h | 2 +- dlls/mshtml/htmlwindow.c | 83 ++++++++++++++++++++++++++++++- dlls/mshtml/tests/documentmode.js | 8 ++- 4 files changed, 126 insertions(+), 13 deletions(-) diff --git a/dlls/mshtml/htmlevent.c b/dlls/mshtml/htmlevent.c index 24c107487af..149611174f0 100644 --- a/dlls/mshtml/htmlevent.c +++ b/dlls/mshtml/htmlevent.c @@ -2314,7 +2314,7 @@ static void DOMCustomEvent_destroy(DOMEvent *event) typedef struct { DOMEvent event; IDOMMessageEvent IDOMMessageEvent_iface; - WCHAR *data; + VARIANT data; } DOMMessageEvent; static inline DOMMessageEvent *impl_from_IDOMMessageEvent(IDOMMessageEvent *iface) @@ -2376,7 +2376,26 @@ static HRESULT WINAPI DOMMessageEvent_get_data(IDOMMessageEvent *iface, BSTR *p) TRACE("(%p)->(%p)\n", This, p); - return (*p = SysAllocString(This->data)) ? S_OK : E_OUTOFMEMORY; + if(V_VT(&This->data) != VT_BSTR) { + FIXME("non-string data\n"); + return E_NOTIMPL; + } + + return (*p = SysAllocString(V_BSTR(&This->data))) ? S_OK : E_OUTOFMEMORY; +} + +static HRESULT DOMMessageEvent_get_data_hook(DispatchEx *dispex, WORD flags, DISPPARAMS *dp, VARIANT *res, + EXCEPINFO *ei, IServiceProvider *caller) +{ + DOMMessageEvent *This = CONTAINING_RECORD(dispex, DOMMessageEvent, event.dispex); + + if(!(flags & DISPATCH_PROPERTYGET) || !res) + return S_FALSE; + + TRACE("(%p)->(%p)\n", This, res); + + V_VT(res) = VT_EMPTY; + return VariantCopy(res, &This->data); } static HRESULT WINAPI DOMMessageEvent_get_origin(IDOMMessageEvent *iface, BSTR *p) @@ -2433,7 +2452,16 @@ static void *DOMMessageEvent_query_interface(DOMEvent *event, REFIID riid) static void DOMMessageEvent_destroy(DOMEvent *event) { DOMMessageEvent *message_event = DOMMessageEvent_from_DOMEvent(event); - heap_free(message_event->data); + VariantClear(&message_event->data); +} + +static void DOMMessageEvent_init_dispex_info(dispex_data_t *info, compat_mode_t compat_mode) +{ + static const dispex_hook_t hooks[] = { + {DISPID_IDOMMESSAGEEVENT_DATA, DOMMessageEvent_get_data_hook}, + {DISPID_UNKNOWN} + }; + dispex_info_add_interface(info, IDOMMessageEvent_tid, compat_mode >= COMPAT_MODE_IE10 ? hooks : NULL); } typedef struct { @@ -2646,7 +2674,6 @@ static dispex_static_data_t DOMCustomEvent_dispex = { static const tid_t DOMMessageEvent_iface_tids[] = { IDOMEvent_tid, - IDOMMessageEvent_tid, 0 }; @@ -2654,7 +2681,8 @@ dispex_static_data_t DOMMessageEvent_dispex = { L"MessageEvent", NULL, DispDOMMessageEvent_tid, - DOMMessageEvent_iface_tids + DOMMessageEvent_iface_tids, + DOMMessageEvent_init_dispex_info }; static const tid_t DOMProgressEvent_iface_tids[] = { @@ -2878,7 +2906,7 @@ HRESULT create_document_event(HTMLDocumentNode *doc, eventid_t event_id, DOMEven return S_OK; } -HRESULT create_message_event(HTMLDocumentNode *doc, BSTR data, DOMEvent **ret) +HRESULT create_message_event(HTMLDocumentNode *doc, VARIANT *data, DOMEvent **ret) { DOMMessageEvent *message_event; DOMEvent *event; @@ -2889,9 +2917,11 @@ HRESULT create_message_event(HTMLDocumentNode *doc, BSTR data, DOMEvent **ret) return hres; message_event = DOMMessageEvent_from_DOMEvent(event); - if(!(message_event->data = heap_strdupW(data))) { + V_VT(&message_event->data) = VT_EMPTY; + hres = VariantCopy(&message_event->data, data); + if(FAILED(hres)) { IDOMEvent_Release(&event->IDOMEvent_iface); - return E_OUTOFMEMORY; + return hres; } *ret = event; diff --git a/dlls/mshtml/htmlevent.h b/dlls/mshtml/htmlevent.h index d0c72d87a5d..ed11e660789 100644 --- a/dlls/mshtml/htmlevent.h +++ b/dlls/mshtml/htmlevent.h @@ -111,7 +111,7 @@ void dispatch_event(EventTarget*,DOMEvent*) DECLSPEC_HIDDEN; HRESULT create_document_event(HTMLDocumentNode*,eventid_t,DOMEvent**) DECLSPEC_HIDDEN; HRESULT create_document_event_str(HTMLDocumentNode*,const WCHAR*,IDOMEvent**) DECLSPEC_HIDDEN; HRESULT create_event_from_nsevent(nsIDOMEvent*,compat_mode_t,DOMEvent**) DECLSPEC_HIDDEN; -HRESULT create_message_event(HTMLDocumentNode*,BSTR,DOMEvent**) DECLSPEC_HIDDEN; +HRESULT create_message_event(HTMLDocumentNode*,VARIANT*,DOMEvent**) DECLSPEC_HIDDEN; void init_nsevents(HTMLDocumentNode*) DECLSPEC_HIDDEN; void release_nsevents(HTMLDocumentNode*) DECLSPEC_HIDDEN; diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index 64e6ae25849..2406f9c615f 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -2218,6 +2218,7 @@ static HRESULT WINAPI HTMLWindow6_postMessage(IHTMLWindow6 *iface, BSTR msg, VAR HTMLWindow *This = impl_from_IHTMLWindow6(iface); DOMEvent *event; HRESULT hres; + VARIANT var; FIXME("(%p)->(%s %s) semi-stub\n", This, debugstr_w(msg), debugstr_variant(&targetOrigin)); @@ -2226,7 +2227,9 @@ static HRESULT WINAPI HTMLWindow6_postMessage(IHTMLWindow6 *iface, BSTR msg, VAR return E_FAIL; } - hres = create_message_event(This->inner_window->doc, msg, &event); + V_VT(&var) = VT_BSTR; + V_BSTR(&var) = msg; + hres = create_message_event(This->inner_window->doc, &var, &event); if(FAILED(hres)) return hres; @@ -3792,8 +3795,84 @@ static void HTMLWindow_bind_event(DispatchEx *dispex, eventid_t eid) ensure_doc_nsevent_handler(This->doc, NULL, eid); } +static HRESULT IHTMLWindow6_postMessage_hook(DispatchEx *dispex, WORD flags, DISPPARAMS *dp, VARIANT *res, + EXCEPINFO *ei, IServiceProvider *caller) +{ + HTMLInnerWindow *This = impl_from_DispatchEx(dispex); + VARIANT msg, *targetOrigin, *transfer; + struct post_message_task *task; + DOMEvent *event; + HRESULT hres; + + if(!(flags & DISPATCH_METHOD) || dp->cArgs < 2 || dp->cNamedArgs) + return S_FALSE; + msg = dp->rgvarg[dp->cArgs - 1]; + targetOrigin = &dp->rgvarg[dp->cArgs - 2]; + transfer = (dp->cArgs > 2) ? &dp->rgvarg[dp->cArgs - 3] : NULL; + + TRACE("(%p)->(%s %s %s)\n", This, debugstr_variant(&msg), debugstr_variant(targetOrigin), debugstr_variant(transfer)); + + if(transfer) + FIXME("transfer not implemented, ignoring\n"); + + switch(V_VT(&msg)) { + case VT_EMPTY: + case VT_NULL: + case VT_VOID: + case VT_I1: + case VT_I2: + case VT_I4: + case VT_I8: + case VT_UI1: + case VT_UI2: + case VT_UI4: + case VT_UI8: + case VT_INT: + case VT_UINT: + case VT_R4: + case VT_R8: + case VT_BOOL: + case VT_BSTR: + case VT_CY: + case VT_DATE: + case VT_DECIMAL: + case VT_HRESULT: + break; + case VT_ERROR: + V_VT(&msg) = VT_EMPTY; + break; + default: + FIXME("Unsupported vt %d\n", V_VT(&msg)); + return E_NOTIMPL; + } + + if(!This->doc) { + FIXME("No document\n"); + return E_FAIL; + } + + hres = create_message_event(This->doc, &msg, &event); + if(FAILED(hres)) + return hres; + + if(!(task = heap_alloc(sizeof(*task)))) { + IDOMEvent_Release(&event->IDOMEvent_iface); + return E_OUTOFMEMORY; + } + + task->event = event; + task->window = This; + IHTMLWindow2_AddRef(&task->window->base.IHTMLWindow2_iface); + return push_task(&task->header, post_message_proc, post_message_destr, This->task_magic); +} + static void HTMLWindow_init_dispex_info(dispex_data_t *info, compat_mode_t compat_mode) { + static const dispex_hook_t window6_ie10_hooks[] = { + {DISPID_IHTMLWINDOW6_POSTMESSAGE, IHTMLWindow6_postMessage_hook}, + {DISPID_UNKNOWN} + }; + if(compat_mode >= COMPAT_MODE_IE9) dispex_info_add_interface(info, IHTMLWindow7_tid, NULL); else @@ -3802,6 +3881,7 @@ static void HTMLWindow_init_dispex_info(dispex_data_t *info, compat_mode_t compa dispex_info_add_interface(info, IWineHTMLWindowPrivate_tid, NULL); dispex_info_add_interface(info, IHTMLWindow5_tid, NULL); + dispex_info_add_interface(info, IHTMLWindow6_tid, compat_mode >= COMPAT_MODE_IE10 ? window6_ie10_hooks : NULL); EventTarget_init_dispex_info(info, compat_mode); } @@ -3831,7 +3911,6 @@ static const tid_t HTMLWindow_iface_tids[] = { IHTMLWindow2_tid, IHTMLWindow3_tid, IHTMLWindow4_tid, - IHTMLWindow6_tid, 0 }; diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 76fa6bacac8..0ec4785450e 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -2078,10 +2078,14 @@ sync_test("__defineSetter__", function() { async_test("postMessage", function() { var v = document.documentMode; var onmessage_called = false; - window.onmessage = function() { + window.onmessage = function(e) { onmessage_called = true; + if(v < 9) + ok(e === undefined, "e = " + e); + else + ok(e.data === (v < 10 ? "10" : 10), "e.data = " + e.data); next_test(); } - window.postMessage("test", "*"); + window.postMessage(10, "*"); ok(onmessage_called == (v < 9 ? true : false), "onmessage not called"); }); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/580
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlwindow.c | 74 ++++++++++++++++++++++++++++++- dlls/mshtml/tests/documentmode.js | 53 ++++++++++++++++++++-- dlls/mshtml/tests/events.js | 2 +- 3 files changed, 123 insertions(+), 6 deletions(-) diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index 2406f9c615f..cd61b8942f9 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -25,6 +25,7 @@ #include "winuser.h" #include "ole2.h" #include "mshtmdid.h" +#include "wininet.h" #include "shlguid.h" #include "shobjidl.h" #include "exdispid.h" @@ -2213,6 +2214,58 @@ static void post_message_destr(task_t *_task) IHTMLWindow2_Release(&task->window->base.IHTMLWindow2_iface); } +static HRESULT check_target_origin(HTMLInnerWindow *window, const VARIANT *target_origin) +{ + URL_COMPONENTSW url = { sizeof(URL_COMPONENTSW) }; + HRESULT hres; + DWORD port; + IUri *uri; + BSTR bstr; + + if(V_VT(target_origin) != VT_BSTR) + return E_INVALIDARG; + + if(!wcscmp(V_BSTR(target_origin), L"*")) + return S_OK; + + url.dwSchemeLength = 1; + url.dwHostNameLength = 1; + if(!InternetCrackUrlW(V_BSTR(target_origin), 0, 0, &url)) { + SetLastError(ERROR_SUCCESS); + return E_INVALIDARG; + } + + if(!window->base.outer_window || !(uri = window->base.outer_window->uri)) + return S_OK; + + hres = IUri_GetSchemeName(uri, &bstr); + if(FAILED(hres)) + return hres; + if(SysStringLen(bstr) != url.dwSchemeLength || wcsnicmp(bstr, url.lpszScheme, url.dwSchemeLength)) + hres = S_FALSE; + SysFreeString(bstr); + if(hres != S_OK) + return hres; + + hres = IUri_GetHost(uri, &bstr); + if(FAILED(hres)) + return hres; + if(SysStringLen(bstr) != url.dwHostNameLength || wcsnicmp(bstr, url.lpszHostName, url.dwHostNameLength)) + hres = S_FALSE; + SysFreeString(bstr); + if(hres != S_OK) + return hres; + + /* Legacy modes ignore port */ + if(dispex_compat_mode(&window->event_target.dispex) < COMPAT_MODE_IE9) + return S_OK; + + hres = IUri_GetPort(uri, &port); + if(FAILED(hres)) + return hres; + return (port != url.nPort) ? S_FALSE : S_OK; +} + static HRESULT WINAPI HTMLWindow6_postMessage(IHTMLWindow6 *iface, BSTR msg, VARIANT targetOrigin) { HTMLWindow *This = impl_from_IHTMLWindow6(iface); @@ -2220,7 +2273,11 @@ static HRESULT WINAPI HTMLWindow6_postMessage(IHTMLWindow6 *iface, BSTR msg, VAR HRESULT hres; VARIANT var; - FIXME("(%p)->(%s %s) semi-stub\n", This, debugstr_w(msg), debugstr_variant(&targetOrigin)); + TRACE("(%p)->(%s %s)\n", This, debugstr_w(msg), debugstr_variant(&targetOrigin)); + + hres = check_target_origin(This->inner_window, &targetOrigin); + if(hres != S_OK) + return SUCCEEDED(hres) ? S_OK : hres; if(!This->inner_window->doc) { FIXME("No document\n"); @@ -3799,7 +3856,7 @@ static HRESULT IHTMLWindow6_postMessage_hook(DispatchEx *dispex, WORD flags, DIS EXCEPINFO *ei, IServiceProvider *caller) { HTMLInnerWindow *This = impl_from_DispatchEx(dispex); - VARIANT msg, *targetOrigin, *transfer; + VARIANT msg, targetOrigin_var, *targetOrigin, *transfer; struct post_message_task *task; DOMEvent *event; HRESULT hres; @@ -3815,6 +3872,19 @@ static HRESULT IHTMLWindow6_postMessage_hook(DispatchEx *dispex, WORD flags, DIS if(transfer) FIXME("transfer not implemented, ignoring\n"); + V_VT(&targetOrigin_var) = VT_EMPTY; + if(V_VT(targetOrigin) != VT_BSTR) { + hres = change_type(&targetOrigin_var, targetOrigin, VT_BSTR, caller); + if(FAILED(hres)) + return hres; + targetOrigin = &targetOrigin_var; + } + + hres = check_target_origin(This, targetOrigin); + VariantClear(&targetOrigin_var); + if(hres != S_OK) + return SUCCEEDED(hres) ? S_OK : hres; + switch(V_VT(&msg)) { case VT_EMPTY: case VT_NULL: diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 0ec4785450e..5b746598dba 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -2082,10 +2082,57 @@ async_test("postMessage", function() { onmessage_called = true; if(v < 9) ok(e === undefined, "e = " + e); - else + else { ok(e.data === (v < 10 ? "10" : 10), "e.data = " + e.data); - next_test(); + next_test(); + } + } + + var invalid = [ + v < 10 ? { toString: function() { return "http://winetest.example.org"; } } : null, + (function() { return "http://winetest.example.org"; }), + "winetest.example.org", + "example.org", + undefined + ]; + for(var i = 0; i < invalid.length; i++) { + try { + window.postMessage("invalid " + i, invalid[i]); + ok(false, "expected exception with targetOrigin " + invalid[i]); + }catch(ex) { + var n = ex.number >>> 0; + todo_wine_if(v >= 10). + ok(n === (v < 10 ? 0x80070057 : 0), "postMessage with targetOrigin " + invalid[i] + " threw " + n); + if(v >= 10) + todo_wine. + ok(ex.name === "SyntaxError", "postMessage with targetOrigin " + invalid[i] + " threw " + ex.name); + } } - window.postMessage(10, "*"); + try { + window.postMessage("invalid empty", ""); + ok(false, "expected exception with empty targetOrigin"); + }catch(ex) { + var n = ex.number >>> 0; + ok(n === 0x80070057, "postMessage with empty targetOrigin threw " + n); + } + + window.postMessage("wrong port", "http://winetest.example.org:1234"); + ok(onmessage_called == (v < 9 ? true : false), "onmessage not called with wrong port"); + onmessage_called = false; + + var not_sent = [ + "http://winetest.example.com", + "ftp://winetest.example.org", + "http://wine.example.org", + "http://example.org" + ]; + for(var i = 0; i < not_sent.length; i++) { + window.postMessage("not_sent " + i, not_sent[i]); + ok(onmessage_called == false, "onmessage called with targetOrigin " + not_sent[i]); + onmessage_called = false; + } + + window.postMessage(10, (v < 10 ? "*" : { toString: function() { return "*"; } })); ok(onmessage_called == (v < 9 ? true : false), "onmessage not called"); + if(v < 9) next_test(); }); diff --git a/dlls/mshtml/tests/events.js b/dlls/mshtml/tests/events.js index e67bf07d210..997e583ad20 100644 --- a/dlls/mshtml/tests/events.js +++ b/dlls/mshtml/tests/events.js @@ -818,6 +818,6 @@ async_test("message event", function() { next_test(); }); - window.postMessage("test", "http://winetest.example.org"); + window.postMessage("test", "httP://wineTest.example.org"); ok(listener_called == false, "listener already called"); }); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/580
Jacek Caban (@jacek) commented about dlls/mshtml/htmlwindow.c:
ensure_doc_nsevent_handler(This->doc, NULL, eid); }
+static HRESULT IHTMLWindow6_postMessage_hook(DispatchEx *dispex, WORD flags, DISPPARAMS *dp, VARIANT *res, + EXCEPINFO *ei, IServiceProvider *caller)
Do we really need another hook? We already have IWineHTMLWindowPrivate, you could just have a postMessage that takes message argument as VARIANT there. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/580#note_5685
On Thu Aug 4 18:43:23 2022 +0000, Jacek Caban wrote:
Do we really need another hook? We already have IWineHTMLWindowPrivate, you could just have a postMessage that takes message argument as VARIANT there. I actually forgot that they can override existing methods from other interfaces, but unfortunately it seems like we need a hook because we need the IServiceProvider for targetOrigin (second patch).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/580#note_5713
On Wed Aug 3 19:32:13 2022 +0000, **** wrote:
Marvin replied on the mailing list: ``` Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=120464 Your paranoid android. === w10pro64_en_AE_u8 (64 bit report) === mshtml: htmldoc.c:2541: Test failed: unexpected call UpdateUI htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS ``` The last time these failures happened was on 2022-05-17 in the 32-bit tests of the debian11 VM. Do you know what's up with those?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/580#note_5804
Jacek Caban (@jacek) commented about dlls/mshtml/htmlwindow.c:
+{ + URL_COMPONENTSW url = { sizeof(URL_COMPONENTSW) }; + HRESULT hres; + DWORD port; + IUri *uri; + BSTR bstr; + + if(V_VT(target_origin) != VT_BSTR) + return E_INVALIDARG; + + if(!wcscmp(V_BSTR(target_origin), L"*")) + return S_OK; + + url.dwSchemeLength = 1; + url.dwHostNameLength = 1; + if(!InternetCrackUrlW(V_BSTR(target_origin), 0, 0, &url)) { I think this should use IUri interface instead.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/580#note_5827
On Mon Aug 8 09:13:55 2022 +0000, Francois Gouget wrote:
The last time these failures happened was on 2022-05-17 in the 32-bit tests of the debian11 VM. Do you know what's up with those? To be honest, I see them happen all the time (even before gitlab transition). Maybe we should mark them as flaky or broken(), but I don't really understand that part of the code (the OleObj part) to make good judgment there yet.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/580#note_5925
participants (3)
-
Francois Gouget (＠fgouget) -
Gabriel Ivăncescu
-
Jacek Caban (＠jacek)