[HybridWebView] Filter Android message events by source - #35717
Conversation
HybridWebView generally expects all hosted code to be local app content, so this is general hardening — it''s just for correctness, not security enforcement. On Android, the JS side subscribes to `window`''s `message` event to receive messages from .NET, but did not check where each event came from. Anything else on the page that calls `window.postMessage` (for example, a nested iframe posting to its parent) would be dispatched through `HybridWebViewMessageReceived` the same as a real native message. Native messages arrive via `WebView.postWebMessage` targeted at the app origin and produce a `MessageEvent` with `origin === window.location.origin` and `source === null`. Only dispatch when both are true; otherwise drop the event and log a warning so unexpected senders are visible during development. Windows (WebView2) and iOS/MacCatalyst (WKWebView) use platform- specific channels and are unaffected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35717Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35717" |
|
/azp run maui-pr-devicetests |
|
/azp run maui-pr-uitests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 1 pipeline(s). |
The previous commit gated the Android `message` listener on both `source === null` and `origin === window.location.origin`. The existing `HybridWebViewTests_SendRawMessage` device test caught that the origin check is wrong: Android WebView delivers `postWebMessage` events with an empty-string `origin`, not the page's origin, so all real native messages were being dropped. `source === null` alone is the invariant we can rely on across WebView versions and is sufficient to reject `window.postMessage` senders (including a same-origin nested iframe). Remove the origin comparison and add a comment explaining why. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run maui-pr-devicetests |
|
/azp run maui-pr-uitests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 1 pipeline(s). |
jonathanpeppers
changed the title
message events by origin and source
|
/review -b feature/enhanced-reviewer |
|
Does someone have access to merge? I retried the Windows Helix tests once already. |
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 1 findings
See inline comments for details.
| // arg.origin here because Android WebView delivers postWebMessage events with an | ||
| // empty-string origin, not window.location.origin. | ||
| window.addEventListener('message', (arg: MessageEvent) => { | ||
| if (arg.source !== null) { |
There was a problem hiding this comment.
[major] Regression Prevention — This Android-only guard is the key behavior that prevents iframe/window postMessage traffic from being raised as HybridWebViewMessageReceived, but the PR has no regression test covering that rejection path. Existing native SendRawMessage coverage would still pass if this guard were removed; please add an Android HybridWebView test that posts from a nested iframe/window and asserts the event is not dispatched while native SendRawMessage still is.
MauiBot
added
s/agent-fix-pr-picked
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@jonathanpeppers — new AI review results are available based on this last commit:
b81b864.
[HybridWebView] Drop origin equality check on Android To request a fresh review after new comments or commits, comment/review rerun.
Review Sessions — click to expand
Gate — Test Before & After Fix
Gate Result: ⚠️ SKIPPED
No tests were detected in this PR.
Recommendation: Add tests to verify the fix using the write-tests-agent.
UI Tests — ViewBaseTests,WebView
Detected UI test categories: ViewBaseTests,WebView
Pre-Flight — Context & Validation
Issue: N/A - No linked issue
PR: #35717 - [HybridWebView] Filter Android message events by source
Platforms Affected: Android
Files Changed: 2 implementation, 0 test
Key Findings
- PR modifies
src/Core/src/Handlers/HybridWebView/HybridWebView.tsand generatedHybridWebView.jsto prevent Androidwindow.postMessagetraffic from being dispatched as native HybridWebView messages. - Current PR fix filters Android
messageevents withMessageEvent.source === null, relying on AndroidWebView.postWebMessagenative messages having no sending window. - Gate was already skipped before this phase because no tests were detected in the PR; no gate content was modified.
- Public PR discussion requested Android device/UI pipelines, but there are no inline review comments on the PR from the public API data.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: medium
Errors: 0 | Warnings: 1 | Suggestions: 0
Key code review findings:
⚠️ Missing Android regression coverage atsrc/Core/src/Handlers/HybridWebView/HybridWebView.ts:98: existingHybridWebViewTests_SendRawMessageverifies native delivery still works, but would not fail if the new iframe/window-message guard were removed.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #35717 | Android JS listener drops message events unless MessageEvent.source === null; updates TS and generated JS. |
src/Core/src/Handlers/HybridWebView/HybridWebView.ts, src/Core/src/Handlers/HybridWebView/HybridWebView.js |
Original PR |
Code Review — Deep Analysis
Code Review — PR #35717
Independent Assessment
What this changes: Android HybridWebView now ignores window message events unless MessageEvent.source === null, so iframe/window-originated postMessage events are not treated as native messages.
Inferred motivation: Prevent non-native page messages from being dispatched through HybridWebViewMessageReceived.
Reconciliation with PR Narrative
Author claims: This is Android-only hardening/correctness; native postWebMessage has source === null; origin checks broke native delivery.
Agreement/disagreement: Matches the code and existing Android messaging path.
Findings
⚠️ Warning — Missing Android regression coverage
src/Core/src/Handlers/HybridWebView/HybridWebView.ts:98
The new guard is the key behavior preventing iframe/window postMessage traffic from being mistaken for native HybridWebView messages. Existing HybridWebViewTests_SendRawMessage verifies native delivery still works, but would not fail if this guard were removed. Please add an Android HybridWebView test that posts from a nested iframe/window and verifies HybridWebViewMessageReceived is not dispatched.
Local inline findings written to:
CustomAgentLogsTmp/PRState/35717/PRAgent/inline-findings.json
Devil's Advocate
The source === null invariant appears well-scoped to Android WebView.postWebMessage, and Windows/iOS use different channels. I did not find a correctness bug in the implementation itself. CI currently has failing Windows Helix device-test work item crashes, which look unrelated to this Android JS change, but prevent an LGTM verdict.
Verdict: NEEDS_DISCUSSION
Confidence: medium
Summary: Code approach looks sound, but the main behavior lacks regression coverage. Also, required CI is not fully green due to failing maui-pr-devicetests Windows Helix work items.
Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix | Native message envelope: C# prefixes native raw messages and JS only dispatches prefixed data. | ✅ PASS | 3 files | Not selected: cross-file string protocol and generated-JS churn risk. |
| 2 | try-fix | Stop using Android window message channel; native calls window.HybridWebView.__ReceiveRawMessage(...) via EvaluateJavascript. |
✅ PASS | 3 files | Not selected: broader redesign and new callable internal JS entry point. |
| 3 | try-fix | Stop using Android window message channel; native directly dispatches HybridWebViewMessageReceived via EvaluateJavascript. |
✅ PASS | 3 files | Not selected: broader native-delivery redesign and JS event construction from C#. |
| 4 | try-fix | Android JavaScriptInterface pull channel with native queue drained from JS. | ❌ FAIL | 3 files | Rejected: page-callable queue drain can consume messages; Android instrumentation did not complete. |
| PR | PR #35717 | Android JS listener keeps using WebView.postWebMessage but drops events unless MessageEvent.source === null. |
2 files | Original PR; smallest focused fix. |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 1 | Yes | Suggested native message envelope/magic prefix. Passed, but adds protocol complexity. |
| claude-opus-4.7 | 1 | Yes | Suggested bypassing window.message with an internal JS receiver invoked from native. Passed, but larger redesign. |
| gpt-5.3-codex | 1 | Yes | Investigated WebMessagePort; found bootstrap still depends on spoofable window.message, then tried direct CustomEvent dispatch. Passed, but larger redesign. |
| gpt-5.5 | 1 | Yes | Tried JavaScriptInterface pull channel. Failed due page-callable queue-drain risk and hung instrumentation. |
| all | 2 | No | Remaining non-trivial ideas collapse to one of: source-null filter, magic/envelope discriminator, direct native script dispatch, or page-callable bridge/channel. |
Learning Loop
- Candidate 1 showed an explicit discriminator can preserve native delivery, but it adds a protocol contract and can create generated-JS churn if TypeScript is rebuilt with the wrong target.
- Candidate 2 showed removing the shared
messagechannel is viable, but exposes a new callable JS entry point and changes Android native delivery semantics. - Candidate 3 removed the extra JS entry point, but still moves public event construction to C# and is a larger redesign than necessary.
- Candidate 4 demonstrated that a JavaScriptInterface pull channel is unsafe because page script can drain the queue before framework code receives native messages.
- All passing candidates were tested with the closest available Android regression (
HybridWebViewTests_SendRawMessagevia Controls device tests). Because PR #35717 has no new regression test, these passes verify native delivery is preserved but do not directly prove iframe/windowpostMessagerejection.
Exhausted: Yes
Selected Fix: PR #35717 — The PR MessageEvent.source === null filter is the smallest focused fix, changes only the JS bridge files, preserves the existing Android native delivery mechanism, and avoids the extra protocol/API/native-dispatch risks found in the alternatives. The main recommendation remains adding an Android regression test that posts from a nested iframe/window and verifies HybridWebViewMessageReceived is not dispatched.
Report — Final Recommendation
Comparative Report — PR #35717
Ranking
| Rank | Candidate | Result | Assessment |
|---|---|---|---|
| 1 | pr-plus-reviewer |
Same implementation as the PR, with expert-reviewed guidance recorded. Best balance of correctness, narrow scope, generated JS consistency, and low Android bridge risk. | |
| 2 | pr |
Smallest focused fix. It preserves the existing Android native PostWebMessage path and filters out page/iframe messages by MessageEvent.source === null. |
|
| 3 | try-fix-1 |
✅ PASS | Explicit native-message envelope works for native delivery, but adds a cross-file string protocol between C# and JS and the attempt produced generated-JS churn risk. |
| 4 | try-fix-2 |
✅ PASS | Bypasses window.message with EvaluateJavascript and a new window.HybridWebView.__ReceiveRawMessage entry point. Correctly avoids the ambiguous channel, but is a broader native-delivery redesign and exposes a new callable JS surface. |
| 5 | try-fix-3 |
✅ PASS | Bypasses window.message and dispatches HybridWebViewMessageReceived directly from Android C#. Avoids a new JS entry point, but moves public event construction into native code and is still a larger redesign than necessary. |
| 6 | try-fix-4 |
❌ FAIL | Rejected. The JavaScriptInterface pull queue can be drained by page JavaScript before the framework receives messages, and the Android instrumentation run did not complete successfully. Per rule, this failed-regression candidate is ranked below passing candidates. |
Candidate analysis
pr
The PR changes only HybridWebView.ts and generated HybridWebView.js. Android continues using WebView.postWebMessage for native-to-JS raw messages, but the JS listener now dispatches only message events with source === null. This directly addresses the iframe/window postMessage confusion without changing C# native delivery or public APIs.
Weakness: no Android regression test was added, so the exact rejection behavior is not protected by PR tests. Existing HybridWebViewTests_SendRawMessage validates native delivery but would not fail if the source guard were removed.
pr-plus-reviewer
The expert reviewer found one actionable finding: add Android regression coverage. No safe production-code change was recommended, so this candidate is functionally identical to pr and carries the same implementation risk profile. It ranks first only because the expert-reviewed output is the most complete candidate state for this phase and preserves the focused PR approach.
try-fix-1
The magic-prefix envelope passed the available Android native delivery regression. However, it creates a new application-layer protocol that must stay synchronized between Android C# and JS, and its generated JS output showed broad toolchain churn. It is more complex than the PR for the same behavioral goal.
try-fix-2
The direct EvaluateJavascript receiver passed the available Android native delivery regression and eliminates the shared window.message receive channel. The cost is a broader redesign of Android native message delivery and a new callable internal JS API on window.HybridWebView, which is unnecessary if the source === null invariant is reliable.
try-fix-3
The direct CustomEvent dispatch passed the available Android native delivery regression and avoids adding the __ReceiveRawMessage JS entry point from try-fix-2. It still changes native delivery semantics and constructs the public JS event from C#, making it a larger and less conventional change than the PR.
try-fix-4
The JavaScriptInterface pull channel failed. The page-callable queue drain is a concrete correctness issue because page JavaScript can consume native messages before the framework polling loop dispatches them, and instrumentation did not complete.
Winner
Winner: pr-plus-reviewer
pr-plus-reviewer wins because the expert review did not identify a production-code defect in the PR fix, and the PR's focused Android MessageEvent.source === null filter avoids the extra protocol, bridge-surface, and native-dispatch risks introduced by the passing try-fix alternatives. The winner still needs Android regression coverage before merge because the PR currently has no test for rejecting nested iframe/window postMessage traffic.
Future Action — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
<!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Description of Change HybridWebView generally expects all hosted code to be local app content, so this is general hardening -- it's just for correctness, not security enforcement. On Android, the JS side subscribes to `window`'s `message` event to receive messages from .NET, but did not check where each event came from. Anything else on the page that calls `window.postMessage` (for example, a nested iframe posting to its parent) would be dispatched through `HybridWebViewMessageReceived` the same as a real native message. Native messages arrive via `WebView.postWebMessage` and produce a `MessageEvent` with `source === null` (no sending window). Only dispatch when that's true; otherwise drop the event and log a warning so unexpected senders are visible during development. This rejects every `window.postMessage` caller, including same-origin iframes, and is the one invariant we can rely on across Android WebView versions. (An earlier revision of this PR also gated on `origin === window.location.origin`. The existing `HybridWebViewTests_SendRawMessage` device test caught that this is wrong on Android: `postWebMessage` events arrive with an empty-string `origin`, not the page origin, so the equality check dropped all real native messages. The origin comparison has been removed.) Windows (WebView2) and iOS/MacCatalyst (WKWebView) use platform-specific channels and are unaffected. ### Issues Fixed N/A --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
5 participants
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description of Change
HybridWebView generally expects all hosted code to be local app content, so this is general hardening -- it's just for correctness, not security enforcement.
On Android, the JS side subscribes to
window'smessageevent to receive messages from .NET, but did not check where each event came from. Anything else on the page that callswindow.postMessage(for example, a nested iframe posting to its parent) would be dispatched throughHybridWebViewMessageReceivedthe same as a real native message.Native messages arrive via
WebView.postWebMessageand produce aMessageEventwithsource === null(no sending window). Only dispatch when that's true; otherwise drop the event and log a warning so unexpected senders are visible during development. This rejects everywindow.postMessagecaller, including same-origin iframes, and is the one invariant we can rely on across Android WebView versions.(An earlier revision of this PR also gated on
origin === window.location.origin. The existingHybridWebViewTests_SendRawMessagedevice test caught that this is wrong on Android:postWebMessageevents arrive with an empty-stringorigin, not the page origin, so the equality check dropped all real native messages. The origin comparison has been removed.)Windows (WebView2) and iOS/MacCatalyst (WKWebView) use platform-specific channels and are unaffected.
Issues Fixed
N/A