API for host objects for iframes #1122

vbryh-msft · 2021-03-26T16:19:44Z

API spec for CoreWebView2Frame.AddHostObjectToScriptWithOrigins and frame created/deleted/name changed events:

WebView2 FrameCreated event;
New interface for iframes ICoreWebView2Frame with name property, FrameDeleted and NameChanged events;
AddHostObjectToScriptWithOrigins method for adding host objects to iframe and corresponding RemoveHostObjectFromScript method.

API spec for CoreWebView2Frame.AddHostObjectToScriptWithOrigins and frame created/deleted/name changed events: - WebView2 FrameCreated event; - New interface for iframes ICoreWebView2Frame with name property, FrameDeleted and NameChanged events; - AddHostObjectToScriptWithOrigins method for adding host objects to iframe and corresponding RemoveHostObjectFromScript method.

ptorr-msft · 2021-03-29T23:51:48Z

specs/AddHostObjectToScriptWithOrigins.md

+using `AddHostObjectToScriptWithOrigins` method.
+When script in the iframe tries to access a host object the origin of the iframe will be
+checked against the provided list. The Frame object has a Name property with the value of the
+name attribute from the iframe html tag declaring it. The NameChanged event is raised when


What is the principle of least surprise here? Meaning if I don't see that event (or somehow forget to hook it up) and now the frame has a new name... is that most likely to be a bug (possibly a security bug) or is it just some obscure edge-cases that care? Reason for asking: if it's more likely to be a (security) bug, then make the de-registration automatic and provide a way to opt-out of it.

Wouldn't removing the object when the name changes be closing the door after the horse has bolted? The page already got the object and could have uploaded your sensitive information to a secret server. Revoking access after the name changed won't delete your sensitive information from the secret server.

Consider doing something else with the name event in the sample. This is unrelated to normal web security practices.

ptorr-msft · 2021-03-29T23:54:04Z

specs/AddHostObjectToScriptWithOrigins.md

+```c#
+void SubscribeToFrameCreated()
+{
+    webView.CoreWebView2.FrameCreated += delegate (object sender, CoreWebView2FrameCreatedEventArgs args)


Sample: adding lambdas as event handlers means you can't unregister them. It is better to have a specific method that you can unregister later (if needed).

Replace with ( ) => { } syntax rather than delegate () { } syntax.
Because we never plan to remove the event handler we're ok with lambda.

You might not in this sample, but people might copy-paste and then have problems

That's true but that's a general principle of event handlers. This sounds like a suggestion that all sample code avoid lambda event handlers, but I think that's taking the issue too far. In samples and in my code I don't like the noise of a method for the typical case of a short handler that doesn't need to be unregistered.

ptorr-msft · 2021-03-29T23:58:19Z

specs/AddHostObjectToScriptWithOrigins.md

+        if (args.Frame.Name.Equals("iframe_name"))
+        {
+            String[] origins = new String[] { "https://appassets.example" };
+            args.Frame.AddHostObjectToScriptWithOrigins("bridge", new BridgeAddRemoteObject(), origins);


Why do you need WithOrigins? That can be the ABI name but it looks like a trivial overload of the existing API?

Yes, see above

ptorr-msft · 2021-03-29T23:59:08Z

specs/AddHostObjectToScriptWithOrigins.md

+            String[] origins = new String[] { "https://appassets.example" };
+            args.Frame.AddHostObjectToScriptWithOrigins("bridge", new BridgeAddRemoteObject(), origins);
+        }
+        args.Frame.NameChanged += delegate (object nameChangedSender, object nameChangedArgs)


Any reason why the sender and args are not strongly typed?

Use actual types instead of object. (Or just remove type names and use () => { } syntax)

ptorr-msft · 2021-03-30T00:04:49Z

specs/AddHostObjectToScriptWithOrigins.md

+        event Windows.Foundation.TypedEventHandler<CoreWebView2Frame, Object> NameChanged;
+        /// The FrameDeleted event is raised when the iframe corresponding to this CoreWebView2Frame
+        /// object is removed or the document containing that iframe is destroyed
+        event Windows.Foundation.TypedEventHandler<CoreWebView2Frame, Object> FrameDeleted;


Is the frame still valid during this event? E.g. can I access the Name property or not?

ptorr-msft · 2021-03-30T00:05:38Z

specs/AddHostObjectToScriptWithOrigins.md

+        /// Calling this method fails if it is called after the iframe is deleted.
+        /// \snippet ScenarioAddHostObject.cpp AddHostObjectToScriptWithOrigins
+        /// For more information about host objects navigate to [AddHostObjectToScript]
+        void AddHostObjectToScriptWithOrigins(String name, Object rawObject, String[] origins);


String[] or IVector<String>?

IEnumerable for C#

IIterable for IDL3

remains pointer+size for COM

ptorr-msft · 2021-03-30T00:06:20Z

specs/AddHostObjectToScriptWithOrigins.md

+        /// attempts are denied, if the object is already obtained by JavaScript code
+        /// in the iframe, the JavaScript code continues to have access to that
+        /// object. Calling this method for a name that is already removed or was never
+        /// added fails. If the iframe is deleted this method will return fail also.


What does "fail" mean? It should be a no-op to delete something that was never there, for example (not an exception).

oldnewthing · 2021-04-01T17:44:51Z

specs/AddHostObjectToScriptWithOrigins.md

+
+# Description
+The FrameCreated event is raised when an iframe is created and before starting any
+navigation and content loading. The FrameDeleted event is raised when an iframe is deleted


I'm guessing that a "deleted" frame is one that has been removed from the document, but may not have been destroyed. Is there a FrameAdded event that lets me know when it has been added back to the document? Right now, there seems to be a mismatch in event pairing.

✓ Created ✘ Destroyed

✘ Added ✓ Deleted

Another way of looking at it is with a frame lifetime diagram:

but we are told only about Created and Deleted. So if we create some tracking object in Created, we might leak it if it never got Added. And if a frame is Deleted, we can prematurely destroy the tracking object if the frame is subsequently Added back.

Let's use Created and Destroyed for naming based on conversation.

oldnewthing · 2021-04-01T17:52:12Z

specs/AddHostObjectToScriptWithOrigins.md

+or the document containing that iframe is destroyed.
+By providing event handler for frame created event,
+user can obtain the iframe and add host objects to it specifying the list of origins
+using `AddHostObjectToScriptWithOrigins` method.


I assume name collisions are resolved the same way as AddHostObjectToScript, whatever that is. (AddHostObjectToScript does not document how name collisions are resolved. Second caller wins? second caller fails? Second caller silently ignored?)

Update docs here and for AddHostObjectToScript how this works (or make it clearer if its already in the existing method docs).

oldnewthing · 2021-04-01T17:56:15Z

specs/AddHostObjectToScriptWithOrigins.md

+or the document containing that iframe is destroyed.
+By providing event handler for frame created event,
+user can obtain the iframe and add host objects to it specifying the list of origins
+using `AddHostObjectToScriptWithOrigins` method.


What origins does AddHostObjectToScript allow? Is AddHostObjectToScript(name, o) equivalent to

AddHostObjectToScriptWithOrigins(name, o, new[] {}) - disallow all

AddHostObjectToScriptWithOrigins(name, o, new[] { "*" }) - allow all

AddHostObjectToScriptWithOrigins(name, o, new[] { coreWebView2.Source }) - allow same origin as document

Something else?

Update docs for old method to say what equivalent is in new method

oldnewthing · 2021-04-01T17:58:51Z

specs/AddHostObjectToScriptWithOrigins.md

+or the document containing that iframe is destroyed.
+By providing event handler for frame created event,
+user can obtain the iframe and add host objects to it specifying the list of origins
+using `AddHostObjectToScriptWithOrigins` method.


I found the name AddHostObjectToScriptWithOrigins confusing because it sounds like the host object is the one with origins. I thought this was "Treat the host object as having come from any of these origins, and determine whether the frame can access the object by resolving the frame's source against these origins, following standard CORS rules."

For .NET & WinRT lets name this AddHostObjectToScript (like overload) and then in the ABI and in COM AddHostObjectToScriptWithAllowedOrigins.

oldnewthing · 2021-04-01T18:00:35Z

specs/AddHostObjectToScriptWithOrigins.md

+using `AddHostObjectToScriptWithOrigins` method.
+When script in the iframe tries to access a host object the origin of the iframe will be
+checked against the provided list. The Frame object has a Name property with the value of the
+name attribute from the iframe html tag declaring it. The NameChanged event is raised when


Wouldn't removing the object when the name changes be closing the door after the horse has bolted? The page already got the object and could have uploaded your sensitive information to a secret server. Revoking access after the name changed won't delete your sensitive information from the secret server.

oldnewthing · 2021-04-01T18:17:22Z

specs/AddHostObjectToScriptWithOrigins.md

+        {
+            // Wrap host object
+            VARIANT remoteObjectAsVariant = {};
+            m_hostObject.query_to<IDispatch>(&remoteObjectAsVariant.pdispVal);


Note that this will throw if m_hostObject fails the QI.

Add a comment noting this and that its our object so should always succeed.

oldnewthing · 2021-04-01T18:19:19Z

specs/AddHostObjectToScriptWithOrigins.md

+  /// If the iframe is declared with no src attribute its origin is considered
+  /// the same as the origin of the parent document.
+  /// List of origins will be treated as following:
+  /// 1. empty list - call will fail and no host object will be added for the


Or should this successfully add an object that is visible to no one? Could be an edge case where somebody builds a vector of allowed origins, and wind up with no allowed origins, and now they have to catch the edge case here and bypass the call to avoid a crash.

(Depending on the rules for how name collisions are resolved, it might be useful to create an object that is visible to no one, just so you can squat the name.)

Sure. Let's allow empty list for that reason (but still not null). And empty list means the object is not exposed to any origin

oldnewthing · 2021-04-01T18:20:35Z

specs/AddHostObjectToScriptWithOrigins.md

+  /// added fails. If the iframe is deleted this method will return fail also.
+  HRESULT RemoveHostObjectFromScript([in] LPCWSTR name);
+
+  /// The FrameDeleted event is raised when the iframe corresponding to this CoreWebView2Frame


This event is already on the Frame object, so would this just be "Deleted"?

Remove 'Frame' prefix from the one on this runtimeclass (the created event still needs the prefix since its for a different runtimeclass)

oldnewthing · 2021-04-01T18:28:56Z

specs/AddHostObjectToScriptWithOrigins.md

+     runtimeclass CoreWebView2Frame
+    {
+        /// The name of the iframe from the iframe html tag declaring it.
+        /// Calling this method fails if it is called after the iframe is deleted.


This can be a problem for code that posts work to the UI thread, since by the time the posted work runs, the frame may already be dead.

webView.CoreWebView2.FrameCreated += async (s, e) => { var bridge = await Bridge.CreateAsync(); // creating a bridge takes time bridge.Server = GetServerFromFrameName(e.Frame.Name); // crash e.Frame.AddHostObjectToScriptWithOrigins("bridge", bridge, trustedOrigins); // crash };

We'll add a property IsDestroyed that works even after the frame is destroyed. Must be true during the destroyed event. Allow removing event handlers without throwing.

oldnewthing · 2021-04-01T18:30:08Z

specs/AddHostObjectToScriptWithOrigins.md

+        event Windows.Foundation.TypedEventHandler<CoreWebView2Frame, Object> NameChanged;
+        /// The FrameDeleted event is raised when the iframe corresponding to this CoreWebView2Frame
+        /// object is removed or the document containing that iframe is destroyed
+        event Windows.Foundation.TypedEventHandler<CoreWebView2Frame, Object> FrameDeleted;


Is there a way to ask a frame, "Have you been deleted"? Or does the app have to keep its own HashSet<Frame> to keep track of which frames have been created and not yet deleted?

webView2.FrameCreated += (s, e) => { activeFrames.Add(e.Frame); e.Frame.FrameDeleted += (s, e) => { activeFrames.Remove(s); }; }; string TryGetFrameName(Frame frame) { if (activeFrames.Contains(frame)) return frame.Name; return ""; } void TryAddHostObjectToFrame(Frame frame, string name, object o) { if (activeFrames.Contains(frame)) return frame.AddHostObject(name, o); return; } ...etc for every member of Frame...

oldnewthing · 2021-04-01T19:11:20Z

specs/AddHostObjectToScriptWithOrigins.md

+        Callback<ICoreWebView2FrameNameChangedEventHandler>(
+            [this](ICoreWebView2Frame* sender,
+                IUnknown* args) -> HRESULT {
+            LPWSTR newName;


Does this string need to be freed?

Suggested change

LPWSTR newName;

wil::unique_cotaskmem_string newName;

Similarly below.

Yes. Pls update

oldnewthing · 2021-04-01T19:12:36Z

specs/AddHostObjectToScriptWithOrigins.md

+        if (std::wcscmp(name, L"iframe_name") == 0)
+        {
+            // Wrap host object
+            VARIANT remoteObjectAsVariant = {};


RAII type will auto-VariantClear the object at destruction.

Suggested change

VARIANT remoteObjectAsVariant = {};

wil::unique_variant remoteObjectAsVariant;

Yes. Pls use this

oldnewthing · 2021-04-01T19:27:32Z

specs/AddHostObjectToScriptWithOrigins.md

+navigation and content loading. The FrameDeleted event is raised when an iframe is deleted
+or the document containing that iframe is destroyed.
+By providing event handler for frame created event,
+user can obtain the iframe and add host objects to it specifying the list of origins


What happens if I use a malformed origin, like https://microsoft.com/jobs? Is it silently ignored? Is it stripped down to https://microsoft.com? Is the call rejected?

Is https://microsoft.com/ a legal origin, with the trailing slash?

Document and implement that less than an origin (scheme, host, port) or more than an origin, or just not any of those things will fail.

(with allowance for optional trailing slash)

david-risney · 2021-04-01T20:11:41Z

specs/AddHostObjectToScriptWithOrigins.md

+
+# Description
+The FrameCreated event is raised when an iframe is created and before starting any
+navigation and content loading. The FrameDeleted event is raised when an iframe is deleted


Let's use Created and Destroyed for naming based on conversation.

david-risney · 2021-04-01T20:12:59Z

specs/AddHostObjectToScriptWithOrigins.md

+navigation and content loading. The FrameDeleted event is raised when an iframe is deleted
+or the document containing that iframe is destroyed.
+By providing event handler for frame created event,
+user can obtain the iframe and add host objects to it specifying the list of origins


Document and implement that less than an origin (scheme, host, port) or more than an origin, or just not any of those things will fail.

david-risney · 2021-04-01T20:13:28Z

specs/AddHostObjectToScriptWithOrigins.md

+navigation and content loading. The FrameDeleted event is raised when an iframe is deleted
+or the document containing that iframe is destroyed.
+By providing event handler for frame created event,
+user can obtain the iframe and add host objects to it specifying the list of origins


(with allowance for optional trailing slash)

david-risney · 2021-04-01T20:14:29Z

specs/AddHostObjectToScriptWithOrigins.md

+or the document containing that iframe is destroyed.
+By providing event handler for frame created event,
+user can obtain the iframe and add host objects to it specifying the list of origins
+using `AddHostObjectToScriptWithOrigins` method.


Update docs here and for AddHostObjectToScript how this works (or make it clearer if its already in the existing method docs).

david-risney · 2021-04-01T20:17:23Z

specs/AddHostObjectToScriptWithOrigins.md

+or the document containing that iframe is destroyed.
+By providing event handler for frame created event,
+user can obtain the iframe and add host objects to it specifying the list of origins
+using `AddHostObjectToScriptWithOrigins` method.


Update docs for old method to say what equivalent is in new method

david-risney · 2021-04-01T20:48:11Z

specs/AddHostObjectToScriptWithOrigins.md

+     runtimeclass CoreWebView2Frame
+    {
+        /// The name of the iframe from the iframe html tag declaring it.
+        /// Calling this method fails if it is called after the iframe is deleted.


We'll add a property IsDestroyed that works even after the frame is destroyed. Must be true during the destroyed event. Allow removing event handlers without throwing.

david-risney · 2021-04-01T20:51:54Z

specs/AddHostObjectToScriptWithOrigins.md

+        event Windows.Foundation.TypedEventHandler<CoreWebView2Frame, Object> NameChanged;
+        /// The FrameDeleted event is raised when the iframe corresponding to this CoreWebView2Frame
+        /// object is removed or the document containing that iframe is destroyed
+        event Windows.Foundation.TypedEventHandler<CoreWebView2Frame, Object> FrameDeleted;


david-risney · 2021-04-01T20:52:43Z

specs/AddHostObjectToScriptWithOrigins.md

+        event Windows.Foundation.TypedEventHandler<CoreWebView2Frame, Object> NameChanged;
+        /// The FrameDeleted event is raised when the iframe corresponding to this CoreWebView2Frame
+        /// object is removed or the document containing that iframe is destroyed
+        event Windows.Foundation.TypedEventHandler<CoreWebView2Frame, Object> FrameDeleted;


david-risney · 2021-04-01T20:53:46Z

specs/AddHostObjectToScriptWithOrigins.md

+        /// Calling this method fails if it is called after the iframe is deleted.
+        /// \snippet ScenarioAddHostObject.cpp AddHostObjectToScriptWithOrigins
+        /// For more information about host objects navigate to [AddHostObjectToScript]
+        void AddHostObjectToScriptWithOrigins(String name, Object rawObject, String[] origins);


IEnumerable for C#

IIterable for IDL3

remains pointer+size for COM

david-risney · 2021-04-01T20:54:01Z

specs/AddHostObjectToScriptWithOrigins.md

+        /// attempts are denied, if the object is already obtained by JavaScript code
+        /// in the iframe, the JavaScript code continues to have access to that
+        /// object. Calling this method for a name that is already removed or was never
+        /// added fails. If the iframe is deleted this method will return fail also.


MikeHillberg · 2021-04-01T20:59:24Z

specs/AddHostObjectToScriptWithOrigins.md

+{
+    webView.CoreWebView2.FrameCreated += delegate (object sender, CoreWebView2FrameCreatedEventArgs args)
+    {
+        if (args.Frame.Name.Equals("iframe_name"))


Name can never be null? Note that in WinRT, an empty HSTRING projects as null.

I believe we decided that unnamed frames return String.Empty instead of null. (Similarly for other string properties.)

That works, I think I got it backwards; in HSTRING there's no support for empty, but if you pass a null HSTRING to C# it projects as String.Empty.

Null projects as empty string. https://docs.microsoft.com/en-us/windows/win32/winrt/hstring.

vbryh-msft requested a review from david-risney March 26, 2021 16:19

ptorr-msft reviewed Mar 29, 2021

View reviewed changes

ptorr-msft reviewed Mar 30, 2021

View reviewed changes

oldnewthing reviewed Apr 1, 2021

View reviewed changes

david-risney requested changes Apr 1, 2021

View reviewed changes

MikeHillberg reviewed Apr 1, 2021

View reviewed changes

david-risney added API Proposal Review WebView2 API Proposal for review. review completed WebView2 API Proposal that's been reviewed and now needs final update and push labels Apr 22, 2021

	VARIANT remoteObjectAsVariant = {};
	wil::unique_variant remoteObjectAsVariant;

API for host objects for iframes #1122

Are you sure you want to change the base?

API for host objects for iframes #1122

Conversation

vbryh-msft commented Mar 26, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment