[inflight regression] Fix HybridWebView JS↔.NET bridge: Android request interception order and Windows NUL-character handling - #36544
Conversation
|
@jonathanpeppers The tests were failing due to changs in your PR #35850, and we have fixed it. Could you please review the changes in this PR and share if you have any concerns? |
|
/azp run maui-pr-uitests , maui-pr-devicetests |
|
Azure Pipelines successfully started running 2 pipeline(s). |
| catch (Exception ex) | ||
| { | ||
| // ShouldInterceptRequest runs on a native WebView thread; letting an exception | ||
| // unwind across the JNI boundary crashes the app (and breaks in the debugger). | ||
| // Log it and return an error response instead. | ||
| logger?.LogError(ex, "SendMessage handler threw while processing a JS -> .NET message."); | ||
| return new WebResourceResponse(null, "UTF-8", 500, "Internal Server Error", null, new MemoryStream()); | ||
| } |
There was a problem hiding this comment.
What exceptions are we expecting would be thrown here? Is it related to the Uri parsing code? Should we be using TryParse() related methods instead?
There was a problem hiding this comment.
This try/catch is not related to Uri parsing—it specifically wraps Handler.MessageReceived(messageBody).
The purpose is to prevent unhandled exceptions thrown by application code. When MessageReceived is invoked, it raises the .NET-side message handlers (for example, RawMessageReceived). If a developer's event handler throws an exception, it propagates back through this call.
Since this code runs on the native Android WebView thread (ShouldInterceptRequest), allowing the exception to escape would cross the JNI boundary and terminate the Android process. Catching the exception here prevents the application from crashing, logs the failure, and returns a 500 Internal Server Error response to the WebView instead.
There was a problem hiding this comment.
If this is to catch a user's exception, we should remove this, this is effectively "swallowing" their exception!
Since this code runs on the native Android WebView thread (ShouldInterceptRequest), allowing the exception to escape would cross the JNI boundary and terminate the Android process.
The Button.Click event also does this, and we allow it to crash the process. If a user wants this behavior, they can catch the exception themselves (recommended).
There was a problem hiding this comment.
I removed the try/catch around Handler.MessageReceived(messageBody) entirely, so exceptions thrown by the app's RawMessageReceived handler are no longer swallowed or converted into an HTTP 500. They now propagate naturally, consistent with how MAUI treats other user event handlers like Button.Click.
kubaflo
left a comment
There was a problem hiding this comment.
Could you please check the suggestions?
@jonathanpeppers The test (SendRawMessageRoundTripsSpecialCharacters) was added in #35850 and was failing on Windows at original PR. No new test need to be added. For Android, below existing test is failed
|
|
@Dhivya-SF4094 how do you know this? When I click on the run I see:
I'm just checking if copilot hallucinated something. |
This comment has been minimized.
This comment has been minimized.
Tests Failure Analysis
Test Failure Review: Needs human investigation - click to expandOverall verdict: Needs human investigation 193 distinct failures were extracted across Android, Windows, iOS, and macOS legs. 16 also appear on the base branch (1 confirmed pre-existing-on-base; 15 indeterminate due to partial baseline inspection), while 177 could not be attributed deterministically. The changed files touch only HybridWebView handlers (Android and Windows), so iOS/macOS/layout/CollectionView failures are almost certainly pre-existing infra noise, but a human must confirm given the large number of unattributed failures and unexplained build legs. Coverage: 175 checks total · 88 passing/neutral/skipped · 86 failing · 1 pending · 0 inaccessible · 2 unmapped · 147 unexplained build legs · 0 unaccounted failing checks · 2 aborted failing checks · 0 canceled-build checks · 3 device-test unverified · 192 unattributed · 0 regressed-vs-base. Deterministic ceiling: Needs human investigation — pending check (macOS UITests CollectionView), 2 unmapped checks (pre_activation, Build Analysis), 147 unexplained build legs, 192 unattributed failures, 2 aborted checks (macOS UITests Editor group, macOS UITests Shell), 3 device-test unverified green checks.
Recommended actionA human should verify: (1) the aborted macOS UITests checks (Editor group and Shell), (2) the still-pending macOS UITests CollectionView check, (3) the 3 unconfirmed green device-test checks (Android CoreCLR, Windows, iOS/Catalyst/Android Mono Helix), and (4) the 147 unexplained build legs. The HybridWebView-only file scope (Android + Windows) makes broad iOS/macOS/gesture/CollectionView failures unlikely to be PR-caused, but that cannot be confirmed without a complete baseline read. Evidence details
|
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 1 findings
See inline comments for details.
On Windows agents core.autocrlf renormalizes some text files (notably *.js like HybridWebView.js) LF->CRLF during the initial checkout, leaving the working tree dirty. The deep 'Merge PR for testing' step then runs 'git merge --squash pr-N' (and a 'git checkout pr-N' fallback), both of which abort with 'local changes would be overwritten by merge/checkout' — failing the ENTIRE Windows deep stage before the PR is even merged, on a base-repo file that isn't part of the PR (build 14666239, #36544). Pin core.autocrlf=false + core.eol=lf and discard the renormalization ('git checkout -- .') right after configuring the git identity, so the squash merge runs against a clean base tree. YAML safe_load OK. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 15d2af20-e4ab-4e88-9011-cfbd83513bc0
…the build) The last-resort Add-MissingUITestResultsNote fallback (fired when the deep loop is skipped entirely, so no per-category dirs exist and the smarter ci-copilot.yml classifier emits no deep block) always said: "most often because the PR build failed (see the Gate section) or the deep UI test stage was skipped. Fix the build/gate issues..." That is misleading when the gate did NOT fail. On PR #36544 the gate was SKIPPED (no tests detected) and the deep stage died at the Windows "Merge PR for testing" step (the .js autocrlf renormalization, fixed separately in 1b12e8c) — yet the note told the author to "fix the build/gate", which were both fine. The note already receives the full comment body, which contains the Gate section, so read the gate outcome and tailor the guidance: * Gate FAILED -> "The PR build failed — see the Gate section ... Fix the build error and comment /review rerun." * Gate PASSED / SKIPPED / INCONCLUSIVE / unknown -> "The PR build itself was fine — the deep UI stage was skipped or interrupted on INFRASTRUCTURE (the merge-for-testing step, emulator/simulator boot, or an Appium hang) ... usually transient; comment /review rerun." Both branches keep the "No UI test results were produced" + "/review rerun" phrases the existing tests assert. Added 3 gate-aware tests (FAILED -> blames build; SKIPPED/PASSED -> points at infrastructure, never "fix the build"). All 41 Pester tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 15d2af20-e4ab-4e88-9011-cfbd83513bc0
@jonathanpeppers Verified the failing test on inflight/candidate branch and confirmed that SendRawMessageRoundTripsSpecialCharacters fails on the Windows platform. Additionally, I verified the behavior on the main branch by applying the changes from PR #35850, and the same test failure was observed there as well. Windows_TestFailure.mp4 |
|
Reviewed the AI summary and incorporated the necessary changes to address the valid concern. |
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@Dhivya-SF4094 — new AI review results are available based on this last commit:
089681c. 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
❌ Deep UI tests — 163 passed, 2 failed across 2 categories on platform-pool agent (replaces in-process counts above).
🧪 UI Test Execution Results (deep, platform pool)
| Category | Tests | Snapshot diffs |
|---|---|---|
ViewBaseTests |
115/115 ✓ | — |
WebView |
48/50 (2 ❌) | 2 diff PNGs |
❌ WebView — 2 failed tests
WebViewCanLoadFileFromSubdirectory
Expected to load the file from the subdirectory, but got '' instead.
Assert.That(text, Is.EqualTo("Success"))
Expected string length 7 but was 0. Strings differ at index 0.
Expected: "Success"
But was: <string.Empty>
-----------^
at Microsoft.Maui.TestCases.Tests.Issues.Issue23315.WebViewCanLoadFileFromSubdirectory() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue23315.cs:line 21
1) at Microsoft.Maui.TestCases.Tests.Issues.Issue23315.WebViewCanLoadFileFromSubdirectory() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue23315.cs:line 21
VerifyHybridWebViewWithShadow
VisualTestUtils.VisualTestFailedException :
Snapshot different than baseline: VerifyHybridWebViewWithShadow.png (0.62% difference)
If the correct baseline has changed (this isn't a a bug), then update the baseline image.
See test attachment or download the build artifacts to get the new snapshot file.
More info: https://aka.ms/visual-test-workflow
at Microsoft.Maui.TestCases.Tests.UITest.VerifyScreenshot(String name, Nullable`1 retryDelay, Nullable`1 retryTimeout, Int32 cropLeft, Int32 cropRight, Int32 cropTop, Int32 cropBottom, Double tolerance, Boolean includeTitleBar) in /_/src/Controls/tests/TestCases.Shared.Tests/UITest.cs:line 296
at Microsoft.Maui.TestCases.Tests.HybridWebViewFeatureTests.VerifyHybridWebViewWithShadow() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/HybridWebViewFeatureTests.cs:line 126
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.RuntimeMethodInfo.Invoke(Ob
...
🔍 AI analysis of failures — PR-related vs unrelated
🔍 AI-generated triage (GitHub Copilot CLI) — a heuristic judgement of whether each deep UI test failure is connected to this PR's changes. Verify before relying on it.
Likely PR-related: one or more failures appear connected to this PR's changes.
- ✗ PR-related — Windows HybridWebView rendering/screenshot (~1 test):
VerifyHybridWebViewWithShadowis a Windows deep-run snapshot failure in the exact HybridWebView area touched by the PR's Windows handler/platform and shared JavaScript changes, so the rendering change is plausibly PR-caused rather than a generic baseline issue. - ● Unrelated — Plain WebView local-file loading (~1 test):
WebViewCanLoadFileFromSubdirectoryexercises regular WebView file loading, while the PR changes HybridWebView-specific transport/message handling; the empty loaded text does not reference the modified HybridWebView paths.
Strongest signal: the run is Windows, and the PR includes Windows HybridWebView changes, so the HybridWebView visual diff should be checked against the PR's intended rendering/transport behavior.
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs)
🔗 Regression Cross-Reference
🔍 Regression Cross-Reference
⚠ Overlaps with prior bug-fix PRs — same files modified, but no exact line revert detected.
📋 Pre-Flight — Context & Validation
Issue: Unknown - GitHub CLI authentication unavailable; linked issue could not be fetched in this environment.
PR: #36544 - HybridWebView bridge transport fixes for Android interception and Windows NUL message preservation
Platforms Affected: Android, Windows
Files Changed: 5 implementation, 0 test
Key Findings
- Local squashed PR commit changes
HybridWebView.ts/.js, AndroidMauiHybridWebViewClient, WindowsHybridWebViewHandler.Windows, and WindowsMauiHybridWebView. - Android current PR fix reserves framework bridge endpoints before app
WebResourceRequestedinterception; app-origin normal resources still flow through app interception first. - Windows current PR fix URL-encodes both directions of WebView2 raw string messaging so embedded NUL characters survive the WebView2 string transport.
- Gate was already skipped before this run because no tests were detected in the PR; no gate verification was re-run.
- GitHub CLI auth is unavailable, so issue body, PR comments, inline discussion, and required checks could not be fetched directly during this phase.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 1 | Suggestions: 1
Key code review findings:
⚠️ src/Core/src/Platform/Android/MauiHybridWebViewClient.cs:147— Android now hides_framework/hybridwebview.jsfrom app interception, which is broader than just reserving the JS-to-.NET message/invoke endpoints.- ℹ
src/Core/src/Handlers/HybridWebView/HybridWebViewHandler.Windows.cs:104— consider guarding non-string WebView2 messages before URL decode.
Failure-mode probes:
- App handler throws from
RawMessageReceived: exception now propagates; prior blocking concern is addressed. - Android app intercepts normal app resource: still reaches
WebResourceRequestedunless the reserved framework path predicate matches. - Windows message includes
\0: encode/decode layers appear balanced. - Malformed or non-string WebView2 message: can throw earlier during decode.
Blast radius: platform-specific HybridWebView bridge and resource-interception paths; runs for all Android HybridWebView requests under app origin and all Windows HybridWebView raw message traffic, but has no startup/static-state impact.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #36544 | Reserve Android framework bridge endpoints before app interception; URL-encode WebView2 raw messages both directions on Windows; URL-encode JS raw payloads. | HybridWebView.ts, HybridWebView.js, HybridWebViewHandler.Windows.cs, MauiHybridWebViewClient.cs, MauiHybridWebView.cs |
Original PR; warning about broader Android script bypass and no tests added. |
🔬 Code Review — Deep Analysis
Code Review — PR #36544
Independent Assessment
What this changes: Fixes HybridWebView bridge transport behavior: Android routes reserved framework bridge URLs before app WebResourceRequested interception, and Windows URL-encodes WebView2 string messages to preserve embedded NUL characters.
Inferred motivation: Restore failing HybridWebView interception tests on Android and raw-message special-character round-trip on Windows.
Reconciliation with PR Narrative
Author claims: Android framework bridge requests should no longer reach app interception; Windows should preserve \0 in raw messages.
Agreement/disagreement: Code mostly matches. One PR-description claim is stale: it says Handler.MessageReceived is wrapped in try/catch, but current code intentionally does not wrap it.
Prior Review Reconciliation
| Prior ❌ Error Finding | Source | Status | Evidence |
|---|---|---|---|
Android Handler.MessageReceived wrapper swallowed user RawMessageReceived exceptions |
MauiBot / jonathanpeppers inline comments | ✅ Fixed | Current MauiHybridWebViewClient.cs:218-225 calls Handler.MessageReceived(messageBody) without try/catch. |
Blast Radius Assessment
- Runs for all instances: Yes, for Android HybridWebView app-origin requests and Windows HybridWebView messaging.
- Startup impact: No direct startup impact; affects HybridWebView initialization/resource loading and bridge traffic.
- Static/shared state: No new static mutable state.
CI Status
- Required-check result:
gh pr checks --requiredunavailable due missing auth; public API check runs showmaui-prfailing. - Classification: likely unrelated/base failure for
SimpleTemplateTest.csusingAssert.IsTrue, not touched by this PR; still CI red. - Action taken: invoked
azdo-build-investigator;ci-analysisunavailable; inspected public AzDO timeline. Confidence capped low.
Findings
⚠️ Warning — Android now hides _framework/hybridwebview.js from app interception
src/Core/src/Platform/Android/MauiHybridWebViewClient.cs:147
The new IsFrameworkInternalRequest bypasses app WebResourceRequested for _framework/hybridwebview.js unconditionally. The regression seems caused by __hwvInvokeDotNet / __hwvSendMessage bridge requests reaching app handlers; hiding the bootstrap script too is a broader Android-only behavior change and differs from Windows/iOS, which still invoke app interception first.
💡 Suggestion — Guard non-string WebView2 messages before URL decode
src/Core/src/Handlers/HybridWebView/HybridWebViewHandler.Windows.cs:104
TryGetWebMessageAsString() can return null for non-string messages. Consider preserving the existing MessageReceived validation path by checking for null before Uri.UnescapeDataString.
Failure-Mode Probing
- App handler throws from
RawMessageReceived: exception now propagates; prior ❌ is addressed. - Android app intercepts normal app resource: still reaches
WebResourceRequestedunless reserved bridge path. - Windows message includes
\0: encode/decode layers appear balanced. - Malformed or non-string WebView2 message: can throw earlier during decode.
Verdict: NEEDS_DISCUSSION
Confidence: low — platform bridge code plus red CI/tool-auth limitations.
Summary: The main prior blocking issue is fixed. I’d like human confirmation that Android intentionally stops exposing _framework/hybridwebview.js to app interception, because that broadens the fix beyond the failing bridge endpoints and creates platform divergence.
🛠️ Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix-1 | Inline Android POST bridge guard for __hwvInvokeDotNet/__hwvSendMessage only; keeps _framework/hybridwebview.js visible to app interception; Windows URI encoding with null guard. |
6 files | Narrower than PR's Android script bypass; adds null-safe WebView2 decode. | |
| 2 | try-fix-2 | Move Android raw messages off HTTP interception via native @JavascriptInterface; protect invoke endpoint narrowly; Windows Base64 UTF-8 wrapper instead of URL encoding. |
7 files | Strongest Android root-cause separation, but larger lifecycle/security surface due native JS interface. | |
| 3 | try-fix-3 | Windows structured JSON WebView2 envelope (postMessage object / PostWebMessageAsJson) to avoid string marshalling; scoped Android endpoint precedence without hiding script. |
5 files | Distinct Windows transport strategy; generated JS diff was noisy due TypeScript target rewrite in attempt artifact. | |
| PR | PR #36544 | Android IsFrameworkInternalRequest reserves framework paths before app interception; Windows URL-encodes WebView2 string messages; JS raw payloads encoded. |
5 files | Original PR; pre-flight warning about hiding _framework/hybridwebview.js from app interception. |
Iteration Learnings
| Round | Feedback / Failure | Applied To Next Candidate |
|---|---|---|
| Pre-flight | Current PR may over-broaden Android bypass by hiding _framework/hybridwebview.js; Windows decode should guard null/non-string messages. |
try-fix-1 narrowed Android bypass to POST bridge endpoints and added Windows null guard. |
| try-fix-1 | Build succeeded, but Windows device-test execution produced no TRX/result files. Also, generated JS can become noisy if TypeScript compiler target differs. | try-fix-2 treated identical runner failures as Blocked and preserved minimal JS edits after build. |
| try-fix-2 | Same Windows host blocker; native bridge approach is architecturally distinct but larger than PR and has lifecycle/security review surface. | try-fix-3 explored structured WebView2 JSON messaging instead of another string-encoding wrapper. |
| try-fix-3 | Same Windows result-collection blocker after successful build. No further meaningfully different local approach remains without platform/device validation. | Stop: environment prevents demonstrating a passing candidate. |
Best Fix Assessment
No alternative candidate passed all tests because the Windows HybridWebView device-test runner is blocked in this environment after successful builds. Therefore no candidate is demonstrably better than the PR fix under the requested stop criteria.
If choosing by design only, try-fix-1 is the most conservative alternative: it addresses the Android over-breadth warning by protecting only POST bridge endpoints and keeps the PR's URL-encoding strategy with a null guard. try-fix-2 has the cleanest Android separation but is a larger behavioral and security-surface change. try-fix-3 is the cleanest conceptual WebView2 transport but produced a noisier generated-JS artifact and still needs real WebView2 runtime validation.
Exhausted: Yes — three distinct root-cause strategies were attempted (scoped HTTP guard, native Android bridge + Base64, structured WebView2 JSON). Further local attempts would be trivial variations until Windows or Android device tests can actually run.
Selected Fix: PR #36544 remains the only available checked-in fix; no alternative was validated as better. Recommend adding targeted HybridWebView device tests for raw special-character round-trip and Android framework bridge interception behavior.
📝 Recommended PR Title & Description
Assessment: ✏️ Recommend updating — the current title has a noisy [inflight regression] prefix and the description claims exception isolation / framework shielding details that should be corrected for the winning pr-plus-reviewer fix.
Recommended title
[Android/Windows] HybridWebView: Fix bridge request routing and NUL-character raw messages
Recommended description
### Failed Test
### Android
1. RequestsCanBeInterceptedAndCustomDataReturned
2. RequestsCanBeInterceptedAndAsyncCustomDataReturned
### Windows
1. SendRawMessageRoundTripsSpecialCharacters [InlineData("with\0nul")]
### Root Cause
### Android – Request interception regression
- PR #35850 replaced the native `window.hybridWebViewHost.sendMessage` bridge with an HTTP `fetch()`-based transport that communicates through reserved framework endpoints (`__hwvSendMessage` and `__hwvInvokeDotNet`), which are processed by `MauiHybridWebViewClient.ShouldInterceptRequest`.
- With the bridge now using standard HTTP requests, `ShouldInterceptRequest` invoked the application's `WebResourceRequested` interception before handling the framework's own bridge POST requests. As a result, internal framework requests that were previously invisible to application code became exposed to custom request handlers.
- Applications that assumed every intercepted request contained application-specific query parameters could throw exceptions (for example, `KeyNotFoundException`). Because these exceptions propagated across the JNI boundary on the WebView thread, they could terminate the application.
### Windows – Existing WebView2 NUL-character limitation
HybridWebView communicates through WebView2's `PostWebMessageAsString` and `TryGetWebMessageAsString` APIs, which internally marshal null-terminated native strings (`LPCWSTR`/`LPWSTR`).
Consequently, an embedded NUL (`\0`) character is treated as the end of the string, truncating any remaining content. This limitation existed before PR #35850 and was not introduced by the Android changes. However, the newly added `SendRawMessageRoundTripsSpecialCharacters` device test — specifically the `"with\0nul"` test case — exposed the issue. Other special-character scenarios (Unicode, `%`, and newline) continued to work correctly.
### Description of Change
### Android
The Android bridge routing separates framework-owned bridge POST traffic from application-defined request interception.
- **Corrected request routing:** `ShouldInterceptRequest` handles `__hwvSendMessage` and `__hwvInvokeDotNet` bridge POST requests before invoking the application's `WebResourceRequested` callback. Framework bridge messages are no longer exposed to application interception, restoring the behavior that existed before PR #35850.
- **Scoped framework request validation:** Internal bridge requests are identified using the reserved endpoint paths together with the expected `X-Maui-Invoke-Token` header. The header alone is not treated as a trust boundary.
- **Preserved app interception behavior:** Unless a broader cross-platform shielding model is intentionally implemented for Windows and iOS too, `_framework/hybridwebview.js` should remain observable by app-level request interception before normal framework fallback serves it.
- **Application exception behavior unchanged:** Application-defined request interception and `RawMessageReceived` handlers remain unwrapped, preserving existing app event-handler behavior. Comments should not claim `Handler.MessageReceived` is exception-isolated unless the code actually wraps that dispatch.
- Common URI parsing and application-relative path resolution logic can be shared between framework-route detection and local resource handling.
### Windows
The Windows fix applies URL encoding at the WebView2 transport boundary so embedded NUL characters survive native string marshalling.
- **.NET → JavaScript:** Messages are encoded with `Uri.EscapeDataString` before calling `PostWebMessageAsString`, and JavaScript decodes them using `decodeURIComponent`.
- **JavaScript → .NET:** JavaScript encodes outgoing messages with `encodeURIComponent`, and `OnWebMessageReceived` decodes them using `Uri.UnescapeDataString` before dispatching the message.
- **Shared raw-message path:** The existing shared raw-message pipeline continues to perform a single raw-payload decode in `HybridWebViewHandler.MessageReceived` via `Uri.UnescapeDataString`, while `sendRawMessage` URL-encodes its payload with `encodeURIComponent`.
- **Defensive message handling:** Guard `TryGetWebMessageAsString()` before URI decoding, and guard/catch JavaScript `decodeURIComponent`, so non-string or malformed WebView2 messages do not fail earlier with less useful URI-decoding exceptions.
### Test Coverage Needed
- Android device coverage should verify app `WebResourceRequested` handlers do not receive `__hwvSendMessage` / `__hwvInvokeDotNet` bridge POST requests with expected headers, malformed reserved-route requests do not bypass normal handling, and normal app resources remain interceptable.
- Windows device coverage should verify raw messages containing embedded NUL/control characters round-trip through WebView2.
### Issues Fixed
Fixes the inflight HybridWebView regressions exposed by the Android request interception tests and the Windows embedded-NUL raw-message test.
🏁 Report — Final Recommendation
Comparative Analysis — PR #36544
Candidate ranking
| Rank | Candidate | Test result | Assessment |
|---|---|---|---|
| 1 | pr-plus-reviewer |
Not run locally; sandbox candidate derived from reviewed PR plus expert feedback | Best overall. Preserves the PR's Android bridge-routing and Windows NUL-preservation intent while addressing the expert review's critical/major findings: contradictory Android exception comments, over-broad/Android-only framework shielding, and Windows null/malformed message handling. |
| 2 | try-fix-1 |
Closest alternate to the winning shape: scoped Android POST bridge guard plus URL encoding with a Windows null guard. It does not explicitly address all expert findings, especially JavaScript malformed-decode handling and metadata/comment consistency. | |
| 3 | try-fix-3 |
Structured WebView2 JSON avoids full percent-encoding and keeps Android endpoint precedence scoped, addressing some reviewer concerns. It is a larger transport change and produced noisy generated-JS output in the attempt artifact. | |
| 4 | try-fix-2 |
Native Android @JavascriptInterface removes raw messages from HTTP interception and Base64 avoids percent-decoding semantics. It has the largest lifecycle/security surface and changes more architecture than needed for the regression. |
|
| 5 | pr |
The raw PR fixes the core symptoms but has unresolved expert findings, including one critical finding about contradictory Android exception-safety comments and major Windows null/non-string handling risk. |
No candidate passed regression tests. The three try-fix-* candidates built successfully but were blocked by the Windows device-test host/result-collection environment. The raw PR gate was skipped because no tests were detected. Therefore the winner is selected by design correctness and review risk, not by a passing regression signal.
Key comparison points
pr is not the winner because the expert review found unresolved critical/major issues. The raw Android comments claim exception isolation that the code does not provide, Android shields framework routes differently from other platforms, and Windows assumes every WebView2 message is a valid encoded string.
try-fix-1 is a strong design reference and largely matches the scoped Android + Windows null-guard direction, but it remains an alternate blocked attempt rather than the reviewed PR plus all expert feedback. try-fix-2 and try-fix-3 are valid independent explorations, but both introduce more transport churn than necessary without empirical validation.
Winning candidate
Winner: pr-plus-reviewer
Rationale: It keeps the PR's intended fix while applying the expert review's actionable feedback. That yields the smallest candidate that resolves the critical/major review risks and stays closer to the existing PR than the larger try-fix alternatives.
🧭 Next Steps — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 6 findings
See inline comments for details.
| // handlers (e.g. RawMessageReceived); an exception thrown by app code must be allowed | ||
| // to propagate rather than be swallowed, matching how MAUI treats event handlers such | ||
| // as Button.Click. Developers who want to handle these exceptions can catch them in | ||
| // their own handler. |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[critical] Async/Threading Safety — contradictory JNI exception-safety claims — This Handler.MessageReceived(messageBody) call (which synchronously invokes app-facing handlers such as RawMessageReceived) is explicitly left unwrapped per the comment directly above it ("Do not wrap this in a try/catch ... an exception thrown by app code must be allowed to propagate"). But the comment block added at the top of ShouldInterceptRequest (around line 92) claims the opposite: "Only the framework's own .NET dispatch (Handler.MessageReceived in GetResponse) is exception-isolated, because it runs under a JNI stack where an unhandled throw crashes the native WebView thread." These two added comments directly contradict each other, and the code follows the "do not catch" comment — so the "exception-isolated"/JNI-crash-safety claim is false as shipped. ShouldInterceptRequest is invoked by the native Android WebView engine across a JNI boundary; if an app's RawMessageReceived/message handler throws here, per the PR's own stated risk this can crash the native WebView thread instead of surfacing as a normal managed exception. Either wrap this dispatch (log + isolate, matching the first comment's intent) or correct the misleading first comment to match the actual (propagate) behavior — right now the code's safety characteristics are undocumented/inconsistent and the crash risk the PR itself calls out is left unmitigated for this exact call site.
| // header, so the header alone is never a trust boundary. Before JS -> .NET | ||
| // messages were routed over HTTP they were invisible to app interception, and | ||
| // this preserves that invariant. | ||
| if (IsFrameworkInternalRequest(url, request)) |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[major] Cross-Platform Behavioral Consistency — This new IsFrameworkInternalRequest gate (and its ordering ahead of WebRequestInterceptingWebView.TryInterceptResponseStream) is added only for Android. HybridWebViewHandler.Windows.cs (OnWebResourceRequested, ~line 122) and HybridWebViewHandler.iOS.cs (StartUrlSchemeTask, ~line 186) still call TryInterceptResponseStream (app-level WebResourceRequested/startURLSchemeTask interception) before any check for _framework/hybridwebview.js, __hwvInvokeDotNet, or __hwvSendMessage, so an app's request-interception handler can still see, modify, or short-circuit these framework-internal bridge requests on Windows and iOS/MacCatalyst. If shielding these endpoints from app interception is the security/reliability goal here (per the comment's claim this "preserves [an] invariant"), the same reordering is needed on Windows and iOS; otherwise this is an Android-only fix leaving the other two platforms exposed to the same problem.
| // The JS transport URL-encodes messages so embedded NUL characters survive WebView2's | ||
| // null-terminated string marshalling (TryGetWebMessageAsString returns an LPWSTR). Decode | ||
| // the payload before dispatching it. | ||
| MessageReceived(Uri.UnescapeDataString(args.TryGetWebMessageAsString())); |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[major] Null Safety — malformed/non-string WebView2 message — args.TryGetWebMessageAsString() returns null (not an exception) whenever the posted web message isn't a string (e.g. chrome.webview.postMessage({...}), a number, or any payload posted by other script/code sharing this WebView2 — WebMessageReceived is subscribed for the whole CoreWebView2, not just messages from the HybridWebView bridge script). Uri.UnescapeDataString(null) throws ArgumentNullException immediately, before MessageReceived's own null/empty validation (HybridWebViewHandler.cs ~line 121, which previously produced a clear ArgumentException("The raw message cannot be null or empty.")) ever runs. This replaces a well-defined, descriptive validation exception with an earlier, less-clear ArgumentNullException for any non-string message posted on the same WebView2. Guard for null (e.g. args.TryGetWebMessageAsString() is string s ? Uri.UnescapeDataString(s) : null) before dispatching to MessageReceived.
| // - the message/invoke channels must ALSO carry the protocol marker header, because the | ||
| // header name/value are public and a same-origin script could otherwise set it on an | ||
| // arbitrary URL to bypass interception. | ||
| static bool IsFrameworkInternalRequest(string fullUrl, IWebResourceRequest request) |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Regression Prevention / Test Coverage — No tests accompany this behavior change (request-interception ordering plus new IsFrameworkInternalRequest/TryGetAppRelativePath helpers), nor the Windows NUL-character encoding change. Per the general guidance to test adjacent scenarios and not just the reported one, this area needs coverage for: (1) an app-registered WebResourceRequested handler can still intercept/override normal static-asset and default-document requests, (2) the same handler can no longer intercept _framework/hybridwebview.js, __hwvInvokeDotNet, or __hwvSendMessage requests when the expected headers are present, (3) a request to the reserved __hwvInvokeDotNet/__hwvSendMessage paths that is missing the expected header is not treated as framework-internal (falls through to normal app-interception handling and eventually 400s), and (4) a raw message containing an embedded NUL character round-trips correctly through the Windows SendRawMessage/OnWebMessageReceived path. None of this is exercised by tests in the diff.
| // WebView2's PostWebMessageAsString marshals to a null-terminated LPCWSTR, so any embedded | ||
| // NUL character would truncate the message. URL-encode the payload so it survives; the JS | ||
| // transport decodes it in the WebView2 'message' event listener in hybridwebview.js. | ||
| CoreWebView2.PostWebMessageAsString(Uri.EscapeDataString(rawMessage)); |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Performance-Critical Path — Uri.EscapeDataString now percent-encodes the entire message on every SendRawMessage call (mirrored by encodeURIComponent on every send in HybridWebView.ts/.js), purely to protect against the rare case of an embedded NUL character. Typical payloads here are JSON (quotes, braces, colons, brackets — none of which are RFC3986-unreserved characters) which will be almost entirely percent-escaped, inflating message size up to ~3x on every JS<->.NET round trip through the Windows WebView2 bridge. For a channel that can be used for frequent/streaming JS<->.NET calls, consider only escaping the NUL character itself (or another narrow-scope transform) rather than fully percent-encoding every message.
| // NUL characters survive WebView2's null-terminated string marshalling. Decode here. | ||
| window.chrome.webview.addEventListener('message', (arg: any) => { | ||
| dispatchHybridWebViewMessage(arg.data); | ||
| dispatchHybridWebViewMessage(decodeURIComponent(arg.data)); |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Malformed/non-string WebView2 message handling — decodeURIComponent(arg.data) assumes every chrome.webview 'message' event was posted via the new percent-encoding in MauiHybridWebView.SendRawMessage. But this listener fires for any message posted to this CoreWebView2 (e.g. via PostWebMessageAsJson, a non-string payload, or any other app/component code sharing the same WebView2), where arg.data may not be a string or may not be valid percent-encoding. decodeURIComponent throws URIError: malformed URI sequence for invalid % sequences (and coerces non-string arg.data via implicit toString() first, which can also produce invalid sequences); the resulting unhandled exception aborts dispatchHybridWebViewMessage for that message with no fallback, silently breaking hybrid message dispatch for that event. Consider a typeof arg.data === 'string' guard plus a try/catch around the decode, falling back to the raw value on failure. (Same pattern applies to the generated HybridWebView.js at the equivalent line.)


Failed Test
Android:
Windows:
Root Cause
Android – Request interception regression
window.hybridWebViewHost.sendMessagebridge with an HTTPfetch()-based transport that communicates through reserved framework endpoints (__hwvSendMessageand__hwvInvokeDotNet), which are processed byMauiHybridWebViewClient.ShouldInterceptRequest.ShouldInterceptRequestinvoked the application'sWebResourceRequestedinterception before handling the framework's own bridge requests. As a result, internal framework requests that were previously invisible to application code became exposed to custom request handlers.KeyNotFoundException). Because these exceptions propagated across the JNI boundary on the WebView thread, they could terminate the application.Windows – Existing WebView2 NUL-character limitation
HybridWebView communicates through WebView2's
PostWebMessageAsStringandTryGetWebMessageAsStringAPIs, which internally marshal null-terminated native strings (LPCWSTR/LPWSTR).Consequently, an embedded NUL (
\0) character is treated as the end of the string, truncating any remaining content. This limitation existed before PR #35850 and was not introduced by the Android changes. However, the newly addedSendRawMessageRoundTripsSpecialCharactersdevice test—specifically the"with\0nul"test case—exposed the issue. Other special-character scenarios (Unicode,%, and newline) continued to work correctly.Description of Change
Android
The Android bridge was updated to clearly separate framework-internal bridge traffic from application-defined request interception.
ShouldInterceptRequestnow checksIsFrameworkInternalRequestbefore invoking the application'sWebResourceRequestedcallback. Framework bridge requests are handled entirely within the framework and are no longer exposed to application interception, restoring the behavior that existed before PR Route Android HybridWebView JS->.NET messages through HTTP intercept #35850.X-Maui-Invoke-Tokenheader where applicable, preventing unrelated application requests from being treated as framework traffic.Handler.MessageReceiveddispatch is wrapped in atry/catch, ensuring unexpected framework exceptions cannot escape across the JNI boundary and crash the WebView thread. Application-defined request interception remains unchanged, preserving existing application behavior.TryGetAppRelativePathhelper, reducing duplication and simplifying request processing.Windows
The Windows fix applies URL encoding at the WebView2 transport boundary so embedded NUL characters survive native string marshalling.
Uri.EscapeDataStringbefore callingPostWebMessageAsString, and JavaScript decodes them usingdecodeURIComponent.encodeURIComponent, andOnWebMessageReceiveddecodes them usingUri.UnescapeDataStringbefore dispatching the message.HybridWebViewHandler.MessageReceivedviaUri.UnescapeDataString, whilesendRawMessagenow URL-encodes its payload withencodeURIComponent. This preserves embedded NUL characters and other special characters without changing the shared handler logic or the overall JavaScript bridge design.Validated the behaviour in the following platforms
Issues Fixed:
Fixes #36469
Screenshots
36469_BeforeFix.mov
36469_AfterFix.mov