Extract filename from DisplayName and add extension if missing#35050
Conversation
## Summary
Extends the CI PR review pipeline to support **all test types** (UI
tests, device tests, unit tests, XAML tests) and restructures the review
flow by decoupling the gate from the copilot agent.
### Before
- Gate only supported UI tests (`TestCases.HostApp` /
`TestCases.Shared.Tests`)
- PRs with device tests, unit tests, or XAML tests were **skipped** by
the gate
- Gate ran as Phase 2 inside the copilot agent (4-phase: Pre-Flight →
Gate → Try-Fix → Report)
- Gate results were duplicated across all phase outputs
- AI summary comment included session history merging (841 lines of
code)
### After
- Gate supports **all test types** with auto-detection
- Gate runs as a **standalone script step** before the copilot agent
- Gate posts its own **separate PR comment** (`<!-- AI Gate -->`)
- AI summary is **simplified** (170 lines, always overwrites, no session
history)
- PR review is now 3 phases: Pre-Flight → Try-Fix → Report
## New Scripts
| Script | Purpose |
|--------|---------|
| `Detect-TestsInDiff.ps1` | Analyzes PR files, classifies tests by type
(UITest, DeviceTest, UnitTest, XamlUnitTest), extracts method names from
diffs |
| `post-gate-comment.ps1` | Posts/updates gate result as separate PR
comment |
| `RunTests.ps1` | Unified test runner entry point for all test types |
## Test Detection
```bash
pwsh .github/scripts/shared/Detect-TestsInDiff.ps1 -PRNumber 25129
```
```
📱 [DeviceTest] EditorTests (PlaceholderHorizontalTextAlignment)
Filter: Category=Editor
🖥️ [UITest] Issue10987
Filter: Issue10987
```
## New Review Flow
```
Step 0: Branch setup
Step 1: Gate (verify-tests-fail.ps1 — direct script, no copilot agent)
→ Posts <!-- AI Gate --> comment immediately
Step 2: PR Review (copilot agent — 3 phases: Pre-Flight, Try-Fix, Report)
→ Gate result passed in prompt
Step 3: Post AI Summary (<!-- AI Summary --> comment)
Step 4: Apply labels
```
## PR Comments (Two Separate Comments)
**Gate comment** (`<!-- AI Gate -->`):
```markdown
## 🚦 Gate — Test Verification
► Expand Full Gate — abc1234 · Fix editor alignment
### Gate Result: ✅ PASSED
| Step | Expected | Actual | Result |
| Without fix | FAIL | FAIL | ✅ |
| With fix | PASS | PASS | ✅ |
```
**AI Summary comment** (`<!-- AI Summary -->`):
Pre-Flight, Fix, Report sections only — no gate duplication.
## Key Changes
- **`verify-tests-fail.ps1`**: Auto-detects test type, routes to correct
runner (BuildAndRunHostApp, Run-DeviceTests, dotnet test), iterates over
all detected tests, `-Platform` mandatory
- **`Detect-TestsInDiff.ps1`**: Shared detection engine — reads
`[Category]` attributes for device test filtering, extracts method names
from PR diffs
- **`Review-PR.ps1`**: Gate as Step 1 (script), PR review as Step 2
(copilot), removed PR finalize step
- **`post-ai-summary-comment.ps1`**: Rewritten from 841 → 170 lines,
always overwrites
- **`pr-gate.md`**: Strict output template, no cross-phase duplication
rule
- **`pr-review/SKILL.md`**: 3 phases (removed Gate), no-duplication rule
- **`EstablishBrokenBaseline.ps1`**: Excludes
TestUtils/DeviceTests.Runners from fix file detection
## Verified
- Gate passed locally on Share device tests: without fix=FAIL ✅, with
fix=PASS ✅
- Detection tested on PRs: #25129, #34615, #34598, #31056
- Comments posted to 8 PRs from CI build artifacts
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…, security hardening (#34678) ## Description Overhauls the `copilot-evaluate-tests` gh-aw workflow — switches to on-demand triggers only (`/evaluate-tests` slash command + manual `workflow_dispatch`), adds security hardening, and improves error handling. No auto-runs on PR create/update. ### What Changed **Triggers (on-demand only — no auto-runs)** - Add `slash_command: evaluate-tests` — comment `/evaluate-tests` on a PR to trigger - Keep `workflow_dispatch` — manual trigger from Actions tab with PR number input - Disable `pull_request_target` — no auto-evaluation on PR create/update - Add `bots: ["copilot-swe-agent[bot]"]` — Copilot-authored PRs can be evaluated - Add `labels: ["pr-review", "testing"]` — workflow runs are labeled **Gate step (fast-fail for invalid requests)** - Check PR is OPEN before evaluating (rejects closed/merged PRs with clear message) - Check for test source files in diff before spinning up agent - Fall back to REST API for PRs with 300+ files (where `gh pr diff` returns HTTP 406) - All API errors surfaced with clear messages — no silent masking - `exit 1` stops the workflow immediately — no wasted agent compute **Access gating (`Checkout-GhAwPr.ps1`)** - Reject fork PRs (`isCrossRepository` check) - Verify PR author has write access (admin/write/maintain roles) - Fix `ConvertFrom-Json` ordering — check exit code before JSON parsing - Make infrastructure restore fatal on failure (was soft warning) - Remove pre-delete pattern — `git checkout` overwrites in-place **Workflow improvements** - `hide-older-comments: true` — previous evaluations auto-collapse - `report-as-issue: false` for noop — no issue created when nothing to evaluate - Timeout bumped 15 → 20 minutes - Dry-run mode via `suppress_output` input (workflow_dispatch only) - `Gather-TestContext.ps1` now receives `-PrNumber` parameter **Security documentation (`gh-aw-workflows.instructions.md`)** - Add "Before You Build" anti-patterns table — prefer built-in gh-aw features - Add Security Boundaries section with defense layers table - Add Rules for gh-aw Workflow Authors (DO/DON'T list) - Document `COPILOT_TOKEN` exposure and mitigations - Add `slash_command` to fork behavior table - Update `Checkout-GhAwPr.ps1` description to match current behavior ### Trigger Behavior | Trigger | When it fires | Who can trigger | |---------|---------------|-----------------| | `/evaluate-tests` comment | Comment on a PR | Write-access collaborators + copilot-swe-agent[bot] | | `workflow_dispatch` | Actions tab → "Run workflow" → enter PR number | Write-access collaborators | | ~~`pull_request_target`~~ | ~~Auto on PR create/update~~ | ~~Disabled~~ | ### Security Model Based on [GitHub Security Lab guidance](https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/): - PR contents treated as **passive data** (read/analyze, never built or executed) - Agent runs in **sandboxed container** with `GITHUB_TOKEN` and `gh` CLI scrubbed - Write operations in **separate `safe_outputs` job** (not the agent) - Agent output limited to `max: 1` comment via safe-outputs - `Checkout-GhAwPr.ps1` rejects fork PRs and verifies write access before checkout - Infrastructure restore is fatal on failure — prevents running with untrusted infra ### Validation | Test | PR | Result | |------|-----|--------| | Open PR with tests | #34983 | ✅ Full success (gate → checkout → agent → comment) | | No-test PR | #34876 | ✅ Gate fast-fail ("no test source files") | | Merged PR | #34932 | ✅ Gate fast-fail ("MERGED — skipping") | ### Known Limitations - Fork PRs via `/evaluate-tests` can supply modified `.github/skills/` — accepted residual risk (agent sandboxed, output bounded). Tracked as [gh-aw#18481](github/gh-aw#18481) - `exit 1` in gate step shows ❌ in GitHub checks for no-test/closed PRs — intentional (no built-in "skip" mechanism in gh-aw steps) - `pull_request_target` commented out — can be re-enabled later for auto-evaluation --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
) <!-- 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! ## Summary Adds automated milestone management for PRs and issues. A single workflow handles both automatic milestoning on PR merge and manual tag-based reconciliation after releases ship. ## Problem When PRs merge, they often get milestoned for the wrong service release or not milestoned at all. The actual release a PR ships in depends on which Candidate PR carries the commits and when the SR branch is cut. This creates milestone drift that makes release queries and notes inaccurate. ## Solution ### Files | File | Purpose | |------|---------| | `.github/scripts/Fix-MilestoneDrift.ps1` | Core engine — version detection, milestone mapping, correction logic | | `.github/scripts/Fix-MilestoneDrift.Tests.ps1` | 77 Pester unit tests for all pure functions | | `.github/workflows/fix-milestone-drift.yml` | Single workflow with dual triggers (auto on merge + manual dispatch) | --- ## How It Works ### Version Detection The source of truth is `eng/Versions.props`. For single-PR mode, the script reads `MajorVersion` and `PatchVersion` **at the PR's merge commit SHA** to determine what version the branch was building when the PR merged. ### Milestone Mapping | Patch | Example | Milestone | Rule | |-------|---------|-----------|------| | `0` | `10.0.0` | `.NET 10.0 GA` | Exact zero | | `1-9` | `10.0.5` | `.NET 10.0 SR1` | Early patches | | `N0` | `10.0.60` | `.NET 10 SR6` | patch / 10 | | `N0+x` | `10.0.41` | `.NET 10 SR4.1` | Sub-patch = distinct milestone | Sub-patches are distinct — `.NET 10 SR4.1` != `.NET 10 SR4`. Script warns and skips gracefully if the milestone doesn't exist on GitHub yet. ### Milestone Normalization GitHub milestones have inconsistent naming (`.NET 10.0 SR4` vs `.NET 10 SR4`, `.NET 10.0 GA` vs `.NET 10 GA`). The script normalizes both SR and GA forms as equivalent. ### Branch Ownership Detection Reads `<MajorVersion>` from `eng/Versions.props` on `origin/main`. No hardcoded version numbers — automatically handles version transitions when `main` moves from .NET 10 to .NET 11. ### Merge-Up Protection Checks each PR's `base.ref` to skip PRs from different .NET versions (prevents merge-up commits from causing incorrect milestoning): | `base.ref` | MainBranch=`main` | MainBranch=`net11.0` | |------------|-------------------|---------------------| | `main` | ✅ | ❌ | | `inflight/*`, `darc/*` | ✅ | ❌ | | `net11.0` | ❌ | ✅ | | `release/10.*` | ✅ | ❌ | | `release/11.*` | ❌ | ✅ | ### Linked Issue Detection Scans PR title and body for: - `fix/fixes/fixed/close/closes/closed/resolve/resolves/resolved #N` - Same keywords + `https://github.com/dotnet/maui/issues/N` Bare informational URLs are ignored. Results are deduplicated. --- ## Single Workflow, Two Triggers ### Auto: On PR Merge (`pull_request_target`) Triggers on every PR merge to `main`, `net*.0`, `inflight/*`, or `release/*`. Reads `Versions.props` at the merge commit, sets the milestone on the PR and its linked issues. If the milestone doesn't exist yet on GitHub, warns and skips gracefully (no red CI). ### Manual: Tag Reconciliation (`workflow_dispatch`) | Input | Type | Default | Description | |-------|------|---------|-------------| | `pr_number` | string | _(empty)_ | Single PR to check | | `tag` | string | _(empty)_ | Release tag to audit (e.g. `10.0.60`) | | `apply` | boolean | `false` | Apply changes (default: dry-run) | | `create_issue` | boolean | `false` | Create GitHub issue with report | --- ## Safety | Rule | Purpose | |------|---------| | Dry-run by default (manual) | `workflow_dispatch` requires explicit `apply` checkbox | | Date cutoff (2026-01-01) | Never touches PRs merged before 2026 | | Branch check | Skips merge-up PRs from different .NET versions | | Deduplication | Same issue corrected once even if linked from multiple PRs | | Fixing-keyword URLs | Bare informational issue references ignored | | Milestone must exist | Warns and skips gracefully if milestone not found | | Merge commit fetch | Fetches commit on demand for PRs merged to inflight | | Warning diagnostics | Silent failures in version detection log warnings | | GA tag support | Tag mode handles first release tag (no previous tag) | | Input sanitization | Workflow inputs flow through `env:` vars, not inline interpolation | | Error propagation | `Invoke-ApplyCorrections` throws on failure; CI goes red | | Zero-check guard | Tag mode throws when all PRs fail to fetch (prevents false "all correct") | --- ## Known Limitations - **Preview/RC milestones**: The script currently maps `PatchVersion=0` to `.NET X.0 GA`. It does not read `PreReleaseVersionLabel` or `PreReleaseVersionIteration` from `Versions.props`, so PRs merged to `net11.0` during the preview phase will not be automatically milestoned (the workflow gracefully skips with a warning). Preview milestone support is planned as a follow-up. - **GA tag rate limit**: Running `-Tag X.0.0` (the very first release tag) in manual mode walks the full git history, which could exhaust GitHub API rate limits on large repos. --- ## Test Suite 77 Pester unit tests covering all pure functions: | Block | Tests | Coverage | |-------|-------|----------| | `ConvertTo-Milestone` | 17 | GA, SR1-SR10, sub-patches (SR4.1, SR10.1), early patches, preview/invalid | | `Test-MilestoneMatch` | 12 | Exact, normalization (SR + GA), sub-patch distinction, null/empty | | `Find-MatchingMilestone` | 5 | Direct, normalized, alternate format, non-existent | | `Find-PreviousTag` | 9 | Ordered traversal, boundaries, major-version isolation | | `Get-LinkedIssues` | 11 | Keywords (incl. close/resolve bare), URLs, dedup, case | | `Get-PatchVersion` | 4 | Valid, GA, large, invalid | | `Test-IsReleaseTag` | 5 | Valid release, wrong major, preview, invalid | | `Test-PrBelongsToVersion` | 14 | main/net11.0 contexts, inflight, darc, release, feature | --- ## Local Usage ```powershell # Dry-run: check a single PR ./Fix-MilestoneDrift.ps1 -PrNumber 34620 -RepoPath . -Verbose # Dry-run: audit all PRs in a tagged release ./Fix-MilestoneDrift.ps1 -Tag 10.0.50 -RepoPath . -Verbose # Apply fixes ./Fix-MilestoneDrift.ps1 -PrNumber 34620 -Apply # Full tag audit + apply + report issue ./Fix-MilestoneDrift.ps1 -Tag 10.0.50 -Apply -CreateIssue # Run tests Invoke-Pester ./Fix-MilestoneDrift.Tests.ps1 -Output Detailed ``` ## Validated Results - 77 Pester tests pass - Single-PR: #34047 → `PatchVersion=50` at merge → `.NET 10 SR5` ✅ - Single-PR: #34620 (untagged) → `PatchVersion=60` at merge → `.NET 10 SR6` ✅ - Single-PR: #34553 (merged to inflight) → fetches commit, reads `PatchVersion=60` ✅ - Tag mode: `10.0.50` → 78 PRs checked, 90 corrections, 0 duplicates, 0 false positives ✅ - 50-PR sample against 10.0.60 range → correctly identifies SR7→SR6 drift ✅ - .NET 11 preview PR #34252 → gracefully warns "milestone not found", exits 0 ✅ --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jakub Florkowski <kubaflo123@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
<!-- 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! ## Summary Follow-up to #34686. Adds preview/RC milestone support and release branch detection to the milestone drift fixer. ## Problem PRs merged to \`net11.0\` were not being milestoned because \`Versions.props\` on that branch always has \`PreReleaseVersionIteration=1\` regardless of which preview the PR actually ships in. The iteration is only bumped on release branches. ## Solution ### Release Branch Detection (Primary) New detection step checks release branches first using \`merge-base --is-ancestor\`. For each PR, it finds the **earliest** release branch containing the merge commit: | Release Branch | Milestone | |---|---| | \`release/10.0.1xx\` | \`.NET 10.0 GA\` | | \`release/10.0.1xx-sr5\` | \`.NET 10 SR5\` | | \`release/11.0.1xx-preview1\` | \`.NET 11.0-preview1\` | | \`release/11.0.1xx-preview3\` | \`.NET 11.0-preview3\` | | \`release/12.0.1xx-rc1\` | \`.NET 12.0-rc1\` | ### Detection Order 1. **Explicit \`-Tag\`** — if provided 2. **Release branches** — \`merge-base --is-ancestor\` against \`release/{Major}.0.1xx-*\` branches, earliest match wins 3. **Versions.props** at merge commit — fallback for PRs not yet on any release branch 4. **Tag range search** — last resort ### Preview/RC Milestone Mapping \`ConvertTo-Milestone\` now accepts optional pre-release label and iteration: | Input | Milestone | |---|---| | \`11.0.0 + preview + 3\` | \`.NET 11.0-preview3\` | | \`12.0.0 + rc + 1\` | \`.NET 12.0-rc1\` | | \`10.0.60\` (stable) | \`.NET 10 SR6\` (unchanged) | ## Validated Results | PR | Base | Current Milestone | Script Result | Method | |---|---|---|---|---| | #33524 | net11.0 | .NET 11.0-preview1 | .NET 11.0-preview1 ✅ | Release branch | | #33233 | net11.0 | .NET 11.0-preview1 | .NET 11.0-preview1 ✅ | Release branch | | #30132 | net11.0 | .NET 11.0-preview3 | .NET 11.0-preview3 ✅ | Release branch | | #33834 | net11.0 | .NET 11.0-preview3 | .NET 11.0-preview3 ✅ | Release branch | | #34214 | net11.0 | .NET 11.0-preview2 | .NET 11.0-preview3 ✅ | Release branch (drift caught!) | | #34945 | net11.0 | .NET 11.0-preview4 | preview1 (fallback) | Versions.props (no p4 branch yet) | | #34620 | main | .NET 10 SR6 | .NET 10 SR6 ✅ | Release branch | | #34047 | main | .NET 10 SR4.1 | .NET 10 SR5 ✅ | Release branch (drift caught!) | PR #34214 is a real drift example: milestoned preview2 by a human, but actually on the preview3 release branch. ## Test Suite 88 Pester tests (11 new): - 6 for \`ConvertTo-Milestone\` preview/RC mapping - 5 for \`ConvertBranchToMilestone\` (GA, SR, preview, RC, non-release) --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
<!-- 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 Adds a daily GitHub Actions workflow that generates a prioritized, actionable PR review queue as a GitHub issue -- with intelligent turn-state detection that tells you **whose turn it is** on each PR. ### What it does - Runs weekdays at 8:00 UTC (or on-demand via `workflow_dispatch`) - Queries open PRs across `dotnet/maui` and `dotnet/docs-maui` - Determines actionability for each PR using a layered heuristic: 1. **Bot detection** -- filters out maestro, dependabot, copilot-reviewer, etc. 2. **Blocked states** -- do-not-merge, stale labels 3. **Project board status** -- reads the MAUI SDK Ongoing board (Ready To Review, In Progress, Changes Requested, Todo, Approved) 4. **Review decision** -- APPROVED, CHANGES_REQUESTED with author-response detection 5. **"Who spoke last"** -- compares author activity (issue comments + inline review-thread replies) against reviewer activity timestamps - Creates a GitHub issue organized by actionability: FTE-actionable PRs first, waiting-on-author collapsed, bot PRs collapsed - Auto-closes previous queue issues to avoid clutter - Filters out `stale` and `do-not-merge` PRs ### Implementation Uses a **plain GitHub Action** (not gh-aw) since the task is deterministic formatting -- no AI reasoning needed: - ~30s runtime (vs ~2min with LLM) - Zero LLM cost - No `COPILOT_GITHUB_TOKEN` dependency -- just built-in `GITHUB_TOKEN` - Deterministic output every run - No prompt injection surface ### Changes - **New**: `.github/workflows/pr-review-queue.yml` -- scheduled workflow with `schedule`, `workflow_dispatch`, and `pull_request` (validation only) triggers - **Modified**: `query-reviewable-prs.ps1` -- adds: - `-OutputFormat markdown` option with actionability-grouped output - `Get-ProjectBoardStatuses` -- paginates full project board for all status columns - `Get-TurnState` -- 5-layer decision tree for "whose turn is it" - `Test-IsBot` -- bot account detection - Comment + review-thread analysis for author activity detection - `#Requires -Version 7.0` for null-coalescing operator ### Example output The daily issue shows: - **Actionability Summary** table (FTE-actionable count, waiting on author, backlog, bot, blocked) - **P/0 Priority** PRs with turn state - **Ready to Merge** (approved PRs) - **Needs FTE Review** (milestoned first, then by age, with turn-state detail) - **Waiting on Author** (collapsed `<details>` section) - **Bot / Automated** (collapsed) - **Queue Health** stats --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…34804) ## Problem Android binding builds invoke Gradle to resolve Maven dependencies (e.g., `androidx` packages). When Gradle downloads fail due to transient DNS/network issues on CI agents, the build fails with: ``` gradlew exited with code 1 ``` This is intermittent and often passes on retrigger, but wastes CI capacity and blocks PRs. See #34800 for recent examples. ## Fix Add a shared `cache-gradle.yml` template using the AzDO [`Cache@2`](https://learn.microsoft.com/azure/devops/pipelines/release/caching) task to persist `~/.gradle/caches` between pipeline runs. When the cache is warm, Gradle resolves dependencies locally instead of downloading from Maven Central, eliminating transient network failures. The cache key includes: - `Agent.OS` — separate caches for macOS/Windows/Linux - `gradle-wrapper.properties` — invalidates when Gradle version changes - `build.gradle` files — invalidates when dependencies change The template is called from `provision.yml` so it applies to all pipelines (build, pack, device tests, UI tests). ## Prior art Adapted from the same approach in dotnet/android: dotnet/android@0a68102 (PR #10986) Fixes #34800 --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…commit (#35023) <!-- 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! ## Summary Fixes milestone fallback to use `Versions.props` from `origin/main` instead of the merge commit when no release branch contains the PR. ## Problem When a PR merges to `inflight/current`, the script was reading `Versions.props` at the merge commit. This gives stale values — e.g., PR #34228 merged when `PatchVersion=60` (SR6), but `main` has since moved to `PatchVersion=70` (SR7). The PR will ship in SR7, not SR6. ## Fix When no release branch contains the commit, read `Versions.props` from the development branch HEAD (`origin/main` for .NET 10, `origin/net11.0` for .NET 11) instead of the merge commit. This reflects where the PR is actually heading. Release branch detection still takes priority — PRs already on a release branch get the correct milestone from that branch. ## Validated | PR | Before | After | Method | |---|---|---|---| | #34228 (inflight, stale) | .NET 10 SR6 (wrong) | .NET 10 SR7 (correct) | main's Versions.props | | #34620 (on SR6 branch) | .NET 10 SR6 | .NET 10 SR6 (unchanged) | Release branch | | #30132 (on preview3 branch) | .NET 11.0-preview3 | .NET 11.0-preview3 (unchanged) | Release branch | 91 Pester tests pass. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> 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 -- 35050Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35050" |
6b0b25b to
e6ca5ae
Compare
There was a problem hiding this comment.
Pull request overview
This PR hardens Android CacheContentFile behavior when content providers return a DisplayName that includes directory components, by extracting a shared filename-sanitization helper and adding unit tests around the new behavior.
Changes:
- Add
FileSystemUtils.EnsureFileNamehelper to strip directory components fromDisplayNameand append a known extension when missing. - Update Android
CacheContentFileto useEnsureFileNameinstead of using the rawDisplayName. - Add unit tests covering common and edge-case
DisplayNameinputs and extension handling.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Essentials/src/FileSystem/FileSystemUtils.shared.cs | Introduces EnsureFileName helper for normalizing/sanitizing content provider display names. |
| src/Essentials/src/FileSystem/FileSystemUtils.android.cs | Uses EnsureFileName when constructing the temp file name for cached content URIs. |
| src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs | Adds test coverage for EnsureFileName across path-like display names and extension scenarios. |
e6ca5ae to
89128ff
Compare
- Extract EnsureFileName to FileSystemUtils.shared.cs for cross-platform testability - Strip directory components from DisplayName — some Android content providers (Samsung, Google Drive, third-party file managers) return full paths - Add empty/whitespace guard — falls back to GUID instead of creating dot-files - Add extension when the filename has none and a known extension is available - Add 15 unit tests covering all edge cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
89128ff to
3b2c17d
Compare
🤖 AI Summary
📊 Review Session —
|
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
🧪 FileSystemUtils_Tests FileSystemUtils_Tests |
✅ FAIL — 15s | ✅ PASS — 8s |
🔴 Without fix — 🧪 FileSystemUtils_Tests: FAIL ✅ · 15s
Determining projects to restore...
Restored /home/vsts/work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 712 ms).
Restored /home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj (in 2.54 sec).
Restored /home/vsts/work/1/s/src/Essentials/src/Essentials.csproj (in 5.05 sec).
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13894288
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13894288
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(285,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(295,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(302,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(312,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(319,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(326,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(338,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(348,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(360,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(371,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
🟢 With fix — 🧪 FileSystemUtils_Tests: PASS ✅ · 8s
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13894288
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13894288
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
Essentials.UnitTests -> /home/vsts/work/1/s/artifacts/bin/Essentials.UnitTests/Debug/net10.0/Microsoft.Maui.Essentials.UnitTests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Essentials.UnitTests/Debug/net10.0/Microsoft.Maui.Essentials.UnitTests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)
Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.10] Discovering: Microsoft.Maui.Essentials.UnitTests
[xUnit.net 00:00:00.32] Discovered: Microsoft.Maui.Essentials.UnitTests
[xUnit.net 00:00:00.33] Starting: Microsoft.Maui.Essentials.UnitTests
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_DotDotSegment_ReturnsFalse(path: "../../file.txt") [4 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_DotDotSegment_ReturnsFalse(path: "sub/../../file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_DotDotSegment_ReturnsFalse(path: "sub\\..\\file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_DotDotSegment_ReturnsFalse(path: "..\\file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_DotDotSegment_ReturnsFalse(path: "sub/../file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_DotDotSegment_ReturnsFalse(path: "../file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_RootedPath_ReturnsFalse(path: "///file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_RootedPath_ReturnsFalse(path: "/file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_RootedPath_ReturnsFalse(path: "//file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_SimpleNameWithExtension_ReturnsUnchanged(displayName: "document.pdf", extension: ".pdf", expected: "document.pdf") [2 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_SimpleNameWithExtension_ReturnsUnchanged(displayName: "readme.txt", extension: ".doc", expected: "readme.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_SimpleNameWithExtension_ReturnsUnchanged(displayName: "photo.jpg", extension: null, expected: "photo.jpg") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_ValidRelative_ReturnsTrue(path: "file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_ValidRelative_ReturnsTrue(path: "sub/./file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_ValidRelative_ReturnsTrue(path: "sub/file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_ValidRelative_ReturnsTrue(path: "sub/deep/file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_ValidRelative_ReturnsTrue(path: "./file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_BackslashDotDot_ReturnsNull(relativePath: "..\\readme.txt") [2 ms]
Passed Tests.FileSystemUtils_Tests.Combine_BackslashDotDot_ReturnsNull(relativePath: "subfolder\\..\\..\\readme.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_BackslashDotDot_ReturnsNull(relativePath: "..\\..\\data\\config.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_RelativeRoot_RootedRelative_ReturnsNull(relativePath: "/etc/config.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_RelativeRoot_RootedRelative_ReturnsNull(relativePath: "//images/logo.png") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_EmptyOrWhitespace_GeneratesGuid(displayName: "") [3 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_EmptyOrWhitespace_GeneratesGuid(displayName: " ") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_NullOrEmpty_ReturnsTrue(path: null) [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_NullOrEmpty_ReturnsTrue(path: "") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_AbsoluteRoot_DotDotAboveRoot_ReturnsNull(relativePath: "../file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_AbsoluteRoot_DotDotAboveRoot_ReturnsNull(relativePath: "sub/../../file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_AbsoluteRoot_DotDotAboveRoot_ReturnsNull(relativePath: "../../file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_RelativeRoot_NormalizesSlashes [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_WindowsDriveLetter_ReturnsFalse [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_WindowsDriveLetter_ReturnsNull [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_RelativeRoot_DotDotAboveRoot_ReturnsNull(relativePath: "../../file.txt") [2 ms]
Passed Tests.FileSystemUtils_Tests.Combine_RelativeRoot_DotDotAboveRoot_ReturnsNull(relativePath: "../file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_RelativeRoot_DotDotAboveRoot_ReturnsNull(relativePath: "sub/../../file.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_Null_GeneratesGuid [< 1 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_TrailingSeparator_GeneratesGuid [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_NetworkStylePath_ReturnsNull(relativePath: "///localhost/C$/data.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_NetworkStylePath_ReturnsNull(relativePath: "///127.0.0.1/share/data.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_DotOrDotDot_GeneratesGuid(displayName: "..") [9 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_DotOrDotDot_GeneratesGuid(displayName: ".") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_EmptyExtension_DoesNotAppend [< 1 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_PathWithDirectories_StripsToFilename(displayName: "/data/user/0/com.app/files/doc.pdf", extension: ".pdf", expected: "doc.pdf") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_PathWithDirectories_StripsToFilename(displayName: "some/relative/path/image.png", extension: null, expected: "image.png") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_PathWithDirectories_StripsToFilename(displayName: "/storage/emulated/0/DCIM/photo.jpg", extension: null, expected: "photo.jpg") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_DoubleDotInFilename_ReturnsTrue(path: "a..b/c..d") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_DoubleDotInFilename_ReturnsTrue(path: "image..png") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_DoubleDotInFilename_ReturnsTrue(path: "name...ext") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.IsValidRelativePath_DoubleDotInFilename_ReturnsTrue(path: "foo..bar.js") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_PathWithDirectoriesNoExtension_AppendsExtension [< 1 ms]
Passed Tests.FileSystemUtils_Tests.NormalizePath_ReplacesForwardAndBackSlashes [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_RelativeRoot_ValidRelative_ReturnsRelativePath [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_RelativeRoot_SubPath_PreservesRelativeStructure [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_AbsoluteRoot_EmptyRelative_ReturnsRoot [< 1 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_NoExtension_AppendsExtension(displayName: "screenshot_12345", extension: ".png", expected: "screenshot_12345.png") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_NoExtension_AppendsExtension(displayName: "photo", extension: "jpg", expected: "photo.jpg") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_NoExtension_AppendsExtension(displayName: "download", extension: ".pdf", expected: "download.pdf") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_AbsoluteRoot_WindowsCaseSensitivity [2 ms]
Passed Tests.FileSystemUtils_Tests.Combine_AbsoluteRoot_RootedRelative_ReturnsNull(relativePath: "///data/readme.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_AbsoluteRoot_RootedRelative_ReturnsNull(relativePath: "/etc/config.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_AbsoluteRoot_RootedRelative_ReturnsNull(relativePath: "//images/logo.png") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_DoubleDotInFilename_Succeeds(relativePath: "image..png") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_DoubleDotInFilename_Succeeds(relativePath: "foo..bar.js") [< 1 ms]
[xUnit.net 00:00:00.49] Finished: Microsoft.Maui.Essentials.UnitTests
Passed Tests.FileSystemUtils_Tests.Combine_DoubleDotInFilename_Succeeds(relativePath: "a..b/c..d.txt") [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_AbsoluteRoot_SingleDot_ReturnsPathWithinRoot [< 1 ms]
Passed Tests.FileSystemUtils_Tests.EnsureFileName_NoExtensionAndNoKnownExtension_ReturnsAsIs [< 1 ms]
Passed Tests.FileSystemUtils_Tests.Combine_AbsoluteRoot_ValidRelative_ReturnsFullPath [< 1 ms]
Test Run Successful.
Total tests: 67
Passed: 67
Total time: 1.2347 Seconds
📁 Fix files reverted (3 files)
eng/pipelines/common/provision.ymlsrc/Essentials/src/FileSystem/FileSystemUtils.android.cssrc/Essentials/src/FileSystem/FileSystemUtils.shared.cs
New files (not reverted):
eng/pipelines/common/cache-gradle.yml
🔍 Pre-Flight — Context & Validation
Issue: No linked issue (standalone fix)
PR: #35050 - Extract filename from DisplayName and add extension if missing
Platforms Affected: Android
Files Changed: 2 implementation, 1 test
Key Findings
- Android content providers (Samsung Gallery, Google Drive) sometimes return a full filesystem path (e.g.,
/storage/emulated/0/DCIM/Camera/IMG_20240101.jpg) inMediaStore.IMediaColumns.DisplayNameinstead of a bare filename CacheContentFilepreviously passed this raw value directly toGetTemporaryFile(root, fileName), which callsJava.IO.File(parent, child)— an absolute child path silently ignoresparent; a relative child path attempts to create non-existent nested subdirectories; both result in broken cached file paths- Fix extracts a new
EnsureFileNamehelper inFileSystemUtils.shared.csthat strips directory components viaPath.GetFileName, guards against empty/whitespace/reserved names (.,..), and appends a known extension when missing - 17 new unit tests cover all documented edge cases; 67 total tests pass
- Inline review comment from Copilot about
./..guard was already addressed in the current PR
Code Review Summary
Verdict: LGTM
Confidence: high
Errors: 0 | Warnings: 0 | Suggestions: 2
Key code review findings:
- 💡
EnsureFileNameis inshared.csbut only called from Android — correct placement for testability, but a brief comment noting intended scope would help future readers - 💡 No test for Windows-backslash
DisplayNameinput — behavior is safe (backslash is not a separator on Linux), but the platform assumption is implicit
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #35050 | Extract EnsureFileName helper that strips directory components, guards reserved names, appends extension |
✅ PASSED (Gate) | FileSystemUtils.shared.cs, FileSystemUtils.android.cs |
17 new unit tests |
🔬 Code Review — Deep Analysis
Code Review — PR #35050
Independent Assessment
What this changes: Extracts a new EnsureFileName(string? displayName, string? extension) helper in FileSystemUtils.shared.cs. The helper strips directory components from an Android content-provider DisplayName via Path.GetFileName, guards against empty/whitespace/reserved names (., ..), and appends a known extension when the name has none. The Android CacheContentFile method is simplified to delegate to it. 17 unit tests are added covering the full input space.
Inferred motivation: Some Android content providers (e.g., Samsung Gallery, Google Drive) return a full filesystem path instead of a bare filename in MediaStore.IMediaColumns.DisplayName. Passing that full path directly to Java.IO.File(parent, child) has two broken behaviors: absolute child paths silently ignore parent, and relative child paths attempt to create non-existent nested subdirectories. Both result in broken cached file paths.
Reconciliation with PR Narrative
Author claims: Exactly matches — Samsung and Google Drive return full paths in DisplayName, causing Java.IO.File(parent, child) to misbehave. The PR adds a sanitizing helper and guards against empty, whitespace, ., and .. filenames.
Agreement: Full agreement. The root cause analysis is correct and the code change directly addresses both the absolute-path and the no-extension sub-cases.
Findings
💡 Suggestion — EnsureFileName is in shared.cs but only called from Android
EnsureFileName compiles on all platforms but today is only used from CacheContentFile (Android-only). Placing it in shared.cs is actually the right call — it keeps it out of Android bindings and makes it independently unit-testable — but a brief inline comment noting the intended scope would help future readers understand why a function named with Android-specific semantics lives in the shared file.
💡 Suggestion — No test for a Windows-backslash DisplayName
On Android, Path.GetFileName only treats / as a separator, so an input like "DCIM\\photo.jpg" would be returned unchanged (backslash is not a separator on Linux). A comment in the implementation noting that only forward-slash paths are expected from Android content providers would make this platform assumption explicit. The current behavior (leave it unchanged) is safe since Java.IO.File(parent, "DCIM\\photo.jpg") creates a filename with a literal backslash rather than a path traversal, but it's a latent gotcha.
Devil's Advocate
"Could removing ./.. accidentally drop legitimate filenames like .bashrc?" No — Path.GetFileName(".bashrc") = ".bashrc", which is not equal to "." or "..". Only the exact directory-sentinel values are guarded.
"Could two different URIs map to the same cached filename?" GetTemporaryFile already creates a per-call GUID subdirectory, so even if two URIs produce vacation.jpg, they land in different subdirectories. No collision risk.
"Is assigning Path.GetFileName(filename) back to filename safe under #nullable enable?" Yes. Path.GetFileName carries [return: NotNullIfNotNull("path")], so when the input is guaranteed non-null (which it is at that point), the compiler treats the return as non-null. No warning, no suppression.
"Could this break any pre-existing callers?" The refactoring is transparent: CacheContentFile produced identical results for the happy path (plain filename with extension already present), and the new helper produces the same output for that case. The only behavioral changes are in the bug paths.
"Is CI trustworthy here?" All CI jobs pass. The new unit tests run and pass.
Verdict: LGTM
Confidence: high
Summary: The fix is correct and surgical. EnsureFileName properly isolates the sanitization logic, all edge cases (full absolute paths, relative paths, trailing separators, reserved names, null/empty/whitespace) are handled and tested, and the behavioral change for the existing happy-path callers is zero. The two 💡 suggestions are documentation-only and non-blocking.
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (opus) | LastIndexOfAny('/', '\\') to strip directory separators; handles backslash on Linux too |
✅ PASS | 3 files | Adds backslash edge-case coverage vs PR |
| 2 | try-fix (sonnet) | string.Split('/', '\\') take last segment; FallbackFileName helper |
✅ PASS | 2 files | More verbose than PR; Split pattern consistent with file |
| 3 | try-fix (codex) | NormalizePath() then Path.GetFileName() on normalized string |
✅ PASS | 2 files | Leverages existing helper; also handles backslash |
| 4 | try-fix (gpt-4.1) | Android Java.IO.File.getName() inline in CacheContentFile |
❌ FAIL (infra) | — | Wrong test invocation; code approach not validated |
| PR | PR #35050 | Path.GetFileName() in shared EnsureFileName helper; GUID fallback for empty/./..; append extension |
✅ PASSED (Gate) | 2 files | Simplest; idiomatic .NET; 17 tests |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | No | NO NEW IDEAS |
| gpt-5.3-codex | 2 | Proposed | Use ContentResolver OpenableColumns.DisplayName — not viable (code already uses ContentResolver; problem is the value returned, not how it's fetched) |
Exhausted: Yes — all meaningful path-stripping approaches covered (Path.GetFileName, LastIndexOfAny, Split, NormalizePath+GetFileName)
Selected Fix: PR's fix — Path.GetFileName() in a shared EnsureFileName helper
Reason: Simplest and most idiomatic .NET implementation. All alternatives that passed are functionally equivalent variants of the same core algorithm. The PR's approach uses the BCL's canonical Path.GetFileName() which is already designed for this purpose, requires no additional logic, and is the most readable. The 3 alternatives that handle backslash (attempts 1, 2, 3) add marginal coverage for a case that doesn't occur on Android (content provider paths always use /). The 17 unit tests in the PR comprehensively cover the documented edge cases.
📋 Report — Final Recommendation
✅ Final Recommendation: APPROVE
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | No linked issue; Android-only fix; unit tests |
| Code Review | LGTM (high) | 0 errors, 0 warnings, 2 non-blocking suggestions |
| Gate | ✅ PASSED | android — 67/67 tests pass |
| Try-Fix | ✅ COMPLETE | 4 attempts, 3 passing (1 infra failure) |
| Report | ✅ COMPLETE |
Code Review Impact on Try-Fix
Code review found LGTM with high confidence and no errors or warnings — only two non-blocking 💡 suggestions. The backslash edge-case suggestion directly inspired Attempt 1 (LastIndexOfAny) and Attempt 3 (NormalizePath + Path.GetFileName), both of which passed. Neither added meaningful value over the PR's approach because Android content providers exclusively use forward-slash paths. The code review's LGTM verdict confirmed that no model needed to address correctness concerns, allowing all 4 models to focus on exploring alternative algorithms rather than fixing bugs.
Summary
PR #35050 fixes a real Android bug where content providers (Samsung, Google Drive) return a full filesystem path in MediaStore.IMediaColumns.DisplayName, causing Java.IO.File(parent, child) to either ignore the parent directory or attempt to create non-existent nested subdirectories. The fix is well-scoped, correctly targeted, comprehensively tested with 17 new unit tests, and all Gate + Try-Fix results confirm it works. Three independent alternative implementations also passed tests, all converging on the same core algorithm — confirming the fix's soundness.
Root Cause
CacheContentFile passed the raw DisplayName value directly to GetTemporaryFile(root, fileName). Since DisplayName can contain a full path (e.g., /storage/emulated/0/DCIM/photo.jpg), Java.IO.File(parent, child) received either an absolute child (ignoring parent) or a relative child with non-existent subdirectories. A secondary issue was that display names without extensions would not get the MIME-derived extension appended.
Fix Quality
The fix is clean and surgical: a new EnsureFileName helper in FileSystemUtils.shared.cs centralizes the sanitization logic (strip directory components via Path.GetFileName, GUID fallback for empty/whitespace/./.., append extension when absent). The Android CacheContentFile method delegates to it in 3 lines replacing 4. The helper is placed in the shared file for platform-agnostic unit testability, which is the correct architectural choice. The 17 unit tests cover all documented edge cases, and 3 independently implemented alternatives all passed using equivalent logic — providing strong validation of correctness.
Revert cache-gradle.yml and provision.yml to match inflight/current base branch — these were carried over from main when the PR base was changed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
<!-- 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 When Android content providers return a `DisplayName` that contains directory components (e.g. `/storage/emulated/0/DCIM/Camera/IMG_20240101.jpg` from Samsung, or `/sdcard/.transforms/synthetic/picker/0/.../photo.png` from Google Drive), `CacheContentFile` would pass the full path as a filename to `GetTemporaryFile`. This caused `Java.IO.File(parent, child)` to either ignore the parent directory (absolute paths) or attempt to create non-existent nested directories (relative paths), leading to broken cached file paths. ### Changes - **Extract `EnsureFileName` helper** to `FileSystemUtils.shared.cs` — normalizes a display name by stripping directory components, guarding against empty/whitespace/reserved directory names (`.`, `..`), and appending a known extension when the name has none - **Add empty-string guard** — if `Path.GetFileName` returns empty (e.g. trailing separator `somefolder/`), falls back to a generated GUID name instead of creating a dot-file - **Add 17 unit tests** covering: simple names, paths with directories, null/empty/whitespace display names, trailing separators, `.`/`..` reserved names, extension handling ### Tested scenarios Verified on Android API 36 emulator using a custom `ContentProvider` that injects problematic `DisplayName` values through the full `EnsurePhysicalPath` → `CacheContentFile` → `EnsureFileName` pipeline: | DisplayName | Result | |---|---| | `/storage/emulated/0/DCIM/Camera/IMG_20240101.jpg` | `IMG_20240101.jpg` ✅ | | `/sdcard/.transforms/synthetic/.../photo.png` | `photo.png` ✅ | | `Download/Subdir/document.pdf` | `document.pdf` ✅ | | `somefolder/` | `<guid>.jpg` ✅ | | `""` (empty) | `<guid>.jpg` ✅ | | `" "` (whitespace) | `<guid>.jpg` ✅ | | `screenshot_12345` (no ext) | `screenshot_12345.jpg` ✅ | | `normal_photo.jpg` | `normal_photo.jpg` ✅ | --------- Co-authored-by: Jakub Florkowski <kubaflo123@gmail.com>
<!-- 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 When Android content providers return a `DisplayName` that contains directory components (e.g. `/storage/emulated/0/DCIM/Camera/IMG_20240101.jpg` from Samsung, or `/sdcard/.transforms/synthetic/picker/0/.../photo.png` from Google Drive), `CacheContentFile` would pass the full path as a filename to `GetTemporaryFile`. This caused `Java.IO.File(parent, child)` to either ignore the parent directory (absolute paths) or attempt to create non-existent nested directories (relative paths), leading to broken cached file paths. ### Changes - **Extract `EnsureFileName` helper** to `FileSystemUtils.shared.cs` — normalizes a display name by stripping directory components, guarding against empty/whitespace/reserved directory names (`.`, `..`), and appending a known extension when the name has none - **Add empty-string guard** — if `Path.GetFileName` returns empty (e.g. trailing separator `somefolder/`), falls back to a generated GUID name instead of creating a dot-file - **Add 17 unit tests** covering: simple names, paths with directories, null/empty/whitespace display names, trailing separators, `.`/`..` reserved names, extension handling ### Tested scenarios Verified on Android API 36 emulator using a custom `ContentProvider` that injects problematic `DisplayName` values through the full `EnsurePhysicalPath` → `CacheContentFile` → `EnsureFileName` pipeline: | DisplayName | Result | |---|---| | `/storage/emulated/0/DCIM/Camera/IMG_20240101.jpg` | `IMG_20240101.jpg` ✅ | | `/sdcard/.transforms/synthetic/.../photo.png` | `photo.png` ✅ | | `Download/Subdir/document.pdf` | `document.pdf` ✅ | | `somefolder/` | `<guid>.jpg` ✅ | | `""` (empty) | `<guid>.jpg` ✅ | | `" "` (whitespace) | `<guid>.jpg` ✅ | | `screenshot_12345` (no ext) | `screenshot_12345.jpg` ✅ | | `normal_photo.jpg` | `normal_photo.jpg` ✅ | --------- Co-authored-by: Jakub Florkowski <kubaflo123@gmail.com>
<!-- 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 When Android content providers return a `DisplayName` that contains directory components (e.g. `/storage/emulated/0/DCIM/Camera/IMG_20240101.jpg` from Samsung, or `/sdcard/.transforms/synthetic/picker/0/.../photo.png` from Google Drive), `CacheContentFile` would pass the full path as a filename to `GetTemporaryFile`. This caused `Java.IO.File(parent, child)` to either ignore the parent directory (absolute paths) or attempt to create non-existent nested directories (relative paths), leading to broken cached file paths. ### Changes - **Extract `EnsureFileName` helper** to `FileSystemUtils.shared.cs` — normalizes a display name by stripping directory components, guarding against empty/whitespace/reserved directory names (`.`, `..`), and appending a known extension when the name has none - **Add empty-string guard** — if `Path.GetFileName` returns empty (e.g. trailing separator `somefolder/`), falls back to a generated GUID name instead of creating a dot-file - **Add 17 unit tests** covering: simple names, paths with directories, null/empty/whitespace display names, trailing separators, `.`/`..` reserved names, extension handling ### Tested scenarios Verified on Android API 36 emulator using a custom `ContentProvider` that injects problematic `DisplayName` values through the full `EnsurePhysicalPath` → `CacheContentFile` → `EnsureFileName` pipeline: | DisplayName | Result | |---|---| | `/storage/emulated/0/DCIM/Camera/IMG_20240101.jpg` | `IMG_20240101.jpg` ✅ | | `/sdcard/.transforms/synthetic/.../photo.png` | `photo.png` ✅ | | `Download/Subdir/document.pdf` | `document.pdf` ✅ | | `somefolder/` | `<guid>.jpg` ✅ | | `""` (empty) | `<guid>.jpg` ✅ | | `" "` (whitespace) | `<guid>.jpg` ✅ | | `screenshot_12345` (no ext) | `screenshot_12345.jpg` ✅ | | `normal_photo.jpg` | `normal_photo.jpg` ✅ | --------- Co-authored-by: Jakub Florkowski <kubaflo123@gmail.com>
<!-- 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 When Android content providers return a `DisplayName` that contains directory components (e.g. `/storage/emulated/0/DCIM/Camera/IMG_20240101.jpg` from Samsung, or `/sdcard/.transforms/synthetic/picker/0/.../photo.png` from Google Drive), `CacheContentFile` would pass the full path as a filename to `GetTemporaryFile`. This caused `Java.IO.File(parent, child)` to either ignore the parent directory (absolute paths) or attempt to create non-existent nested directories (relative paths), leading to broken cached file paths. ### Changes - **Extract `EnsureFileName` helper** to `FileSystemUtils.shared.cs` — normalizes a display name by stripping directory components, guarding against empty/whitespace/reserved directory names (`.`, `..`), and appending a known extension when the name has none - **Add empty-string guard** — if `Path.GetFileName` returns empty (e.g. trailing separator `somefolder/`), falls back to a generated GUID name instead of creating a dot-file - **Add 17 unit tests** covering: simple names, paths with directories, null/empty/whitespace display names, trailing separators, `.`/`..` reserved names, extension handling ### Tested scenarios Verified on Android API 36 emulator using a custom `ContentProvider` that injects problematic `DisplayName` values through the full `EnsurePhysicalPath` → `CacheContentFile` → `EnsureFileName` pipeline: | DisplayName | Result | |---|---| | `/storage/emulated/0/DCIM/Camera/IMG_20240101.jpg` | `IMG_20240101.jpg` ✅ | | `/sdcard/.transforms/synthetic/.../photo.png` | `photo.png` ✅ | | `Download/Subdir/document.pdf` | `document.pdf` ✅ | | `somefolder/` | `<guid>.jpg` ✅ | | `""` (empty) | `<guid>.jpg` ✅ | | `" "` (whitespace) | `<guid>.jpg` ✅ | | `screenshot_12345` (no ext) | `screenshot_12345.jpg` ✅ | | `normal_photo.jpg` | `normal_photo.jpg` ✅ | --------- Co-authored-by: Jakub Florkowski <kubaflo123@gmail.com>
<!-- 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 When Android content providers return a `DisplayName` that contains directory components (e.g. `/storage/emulated/0/DCIM/Camera/IMG_20240101.jpg` from Samsung, or `/sdcard/.transforms/synthetic/picker/0/.../photo.png` from Google Drive), `CacheContentFile` would pass the full path as a filename to `GetTemporaryFile`. This caused `Java.IO.File(parent, child)` to either ignore the parent directory (absolute paths) or attempt to create non-existent nested directories (relative paths), leading to broken cached file paths. ### Changes - **Extract `EnsureFileName` helper** to `FileSystemUtils.shared.cs` — normalizes a display name by stripping directory components, guarding against empty/whitespace/reserved directory names (`.`, `..`), and appending a known extension when the name has none - **Add empty-string guard** — if `Path.GetFileName` returns empty (e.g. trailing separator `somefolder/`), falls back to a generated GUID name instead of creating a dot-file - **Add 17 unit tests** covering: simple names, paths with directories, null/empty/whitespace display names, trailing separators, `.`/`..` reserved names, extension handling ### Tested scenarios Verified on Android API 36 emulator using a custom `ContentProvider` that injects problematic `DisplayName` values through the full `EnsurePhysicalPath` → `CacheContentFile` → `EnsureFileName` pipeline: | DisplayName | Result | |---|---| | `/storage/emulated/0/DCIM/Camera/IMG_20240101.jpg` | `IMG_20240101.jpg` ✅ | | `/sdcard/.transforms/synthetic/.../photo.png` | `photo.png` ✅ | | `Download/Subdir/document.pdf` | `document.pdf` ✅ | | `somefolder/` | `<guid>.jpg` ✅ | | `""` (empty) | `<guid>.jpg` ✅ | | `" "` (whitespace) | `<guid>.jpg` ✅ | | `screenshot_12345` (no ext) | `screenshot_12345.jpg` ✅ | | `normal_photo.jpg` | `normal_photo.jpg` ✅ | --------- Co-authored-by: Jakub Florkowski <kubaflo123@gmail.com>
…t#35050) <!-- 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 When Android content providers return a `DisplayName` that contains directory components (e.g. `/storage/emulated/0/DCIM/Camera/IMG_20240101.jpg` from Samsung, or `/sdcard/.transforms/synthetic/picker/0/.../photo.png` from Google Drive), `CacheContentFile` would pass the full path as a filename to `GetTemporaryFile`. This caused `Java.IO.File(parent, child)` to either ignore the parent directory (absolute paths) or attempt to create non-existent nested directories (relative paths), leading to broken cached file paths. ### Changes - **Extract `EnsureFileName` helper** to `FileSystemUtils.shared.cs` — normalizes a display name by stripping directory components, guarding against empty/whitespace/reserved directory names (`.`, `..`), and appending a known extension when the name has none - **Add empty-string guard** — if `Path.GetFileName` returns empty (e.g. trailing separator `somefolder/`), falls back to a generated GUID name instead of creating a dot-file - **Add 17 unit tests** covering: simple names, paths with directories, null/empty/whitespace display names, trailing separators, `.`/`..` reserved names, extension handling ### Tested scenarios Verified on Android API 36 emulator using a custom `ContentProvider` that injects problematic `DisplayName` values through the full `EnsurePhysicalPath` → `CacheContentFile` → `EnsureFileName` pipeline: | DisplayName | Result | |---|---| | `/storage/emulated/0/DCIM/Camera/IMG_20240101.jpg` | `IMG_20240101.jpg` ✅ | | `/sdcard/.transforms/synthetic/.../photo.png` | `photo.png` ✅ | | `Download/Subdir/document.pdf` | `document.pdf` ✅ | | `somefolder/` | `<guid>.jpg` ✅ | | `""` (empty) | `<guid>.jpg` ✅ | | `" "` (whitespace) | `<guid>.jpg` ✅ | | `screenshot_12345` (no ext) | `screenshot_12345.jpg` ✅ | | `normal_photo.jpg` | `normal_photo.jpg` ✅ | --------- Co-authored-by: Jakub Florkowski <kubaflo123@gmail.com>
## What's Coming .NET MAUI inflight/candidate introduces significant improvements across all platforms with focus on quality, performance, and developer experience. This release includes 85 commits with various improvements, bug fixes, and enhancements. ## Button - [Android, iOS] Button: Fix VisualState properties not restored when leaving custom state by @BagavathiPerumal in #33346 <details> <summary>🔧 Fixes</summary> - [Button VisualStates do not work](#19690) </details> ## CollectionView - Fix CollectionView grid spacing updates for first row and column by @KarthikRajaKalaimani in #34527 <details> <summary>🔧 Fixes</summary> - [[MAUI] I2_Vertical grid for horizontal Item Spacing and Vertical Item Spacing - horizontally updating the spacing only applies to the second column](#34257) </details> - CarouselView: Fix cascading PositionChanged/CurrentItemChanged events on collection update by @praveenkumarkarunanithi in #31275 <details> <summary>🔧 Fixes</summary> - [[Windows] CurrentItemChangedEventArgs and PositionChangedEventArgs Not Working Properly in CarouselView](#29529) </details> - [Windows] Fixed ItemSpacing doesn't work in Carousel View by @SubhikshaSf4851 in #30014 <details> <summary>🔧 Fixes</summary> - [ItemSpacing on CarouselView is not applied on Windows.](#29772) </details> - Fix CollectionView not scrolling to top on iOS status bar tap by @jfversluis in #34687 <details> <summary>🔧 Fixes</summary> - [[iOS] UICollectionView ScrollToTop does not work](#19866) </details> - [iOS] Fixed CollectionView Scroll Jitter for TextType HTML Labels by @SubhikshaSf4851 in #34383 <details> <summary>🔧 Fixes</summary> - [CollectionView scrolling is jittery when ItemTemplate contains Label with TextType="Html" in .NET 10](#33065) </details> - Fix CollectionView Header is not visible when ItemsSource is not set and an EmptyView is set in iOS, Mac platform by @KarthikRajaKalaimani in #34989 <details> <summary>🔧 Fixes</summary> - [CollectionView Header is not visible when ItemsSource is not set and EmptyView is set in iOS, Mac platform](#34897) </details> - [Android] Fix CollectionView EmptyView not displayed correctly by @KarthikRajaKalaimani in #34956 <details> <summary>🔧 Fixes</summary> - [[Android] CollectionView - EmptyView not displayed correctly](#34861) </details> - [iOS] Fix CollectionView ScrollOffset not resetting when ItemsSource changes by @SyedAbdulAzeemSF4852 in #34488 <details> <summary>🔧 Fixes</summary> - [[IOS] CollectionView ScrollOffset does not reset when the ItemSource is changed in iOS.](#26366) - [Re-enable Issue7993 test on iOS/Catalyst - CollectionView scroll position not reset when updating ItemsSource](#33500) </details> - [Revert] [iOS] Fixed CollectionView Scroll Jitter for TextType HTML Labels by @SubhikshaSf4851 in #35341 ## Core Lifecycle - [Android] Fix NRE in ContainerView when Android Context is null during lifecycle transition by @rmarinho in #34901 <details> <summary>🔧 Fixes</summary> - [[Android] NullReferenceException in NavigationRootManager.Connect when mapping Window content](#34900) </details> ## DateTimePicker - [Android] Fix for TimePicker Dialog doesn't update the layout when rotating the device with dialog open by @HarishwaranVijayakumar in #31910 <details> <summary>🔧 Fixes</summary> - [[Android] TimePicker Dialog doesn't update the layout when rotating the device with dialog open](#31658) </details> - [Android, iOS] Fixed TimePicker FlowDirection Not Applied Across Platforms by @Dhivya-SF4094 in #30369 <details> <summary>🔧 Fixes</summary> - [TimePicker FlowDirection Not Working on All Platforms](#30192) </details> - [Windows] Fixed TimePicker CharacterSpacing issue by @SubhikshaSf4851 in #30533 <details> <summary>🔧 Fixes</summary> - [[Windows] TimePicker CharacterSpacing Property Not Working on Windows](#30199) </details> - [MacCatalyst] Fix DatePicker Opened/Closed events not being raised by @SubhikshaSf4851 in #34970 <details> <summary>🔧 Fixes</summary> - [[MacCatalyst] DatePicker Opened and Closed events are not raised on Mac platform](#34848) </details> ## Dialogalert - [Android] Fix AlertDialog, ActionSheet, and Prompt render with Material 2 styles when Material 3 is enabled by @HarishwaranVijayakumar in #35121 <details> <summary>🔧 Fixes</summary> - [[Android] AlertDialog, ActionSheet, and Prompt render with Material 2 styles when Material 3 is enabled](#35119) </details> ## Docs - docs: Add UITesting-Guide, ReleasePlanning, and ReleaseProcess to docs/README.md index by @PureWeen in #35195 - docs: Fix hardcoded path and add library overview in Essentials.AI README by @PureWeen in #35194 - docs: Update branch reference from net10.0 to net11.0 in DEVELOPMENT.md by @PureWeen in #35193 ## Drawing - Fix Path Rendering Issue Inside StackLayout When Margin Is Set by @Shalini-Ashokan in #28071 <details> <summary>🔧 Fixes</summary> - [Path does not render if it has Margin](#13801) </details> - Fixed FlowDirection property not working on Drawable control and GraphicsView by @Dhivya-SF4094 in #34557 <details> <summary>🔧 Fixes</summary> - [[Android, Windows, iOS, macOS] FlowDirection property not working on BoxView Control](#34402) </details> - [iOS & Mac] Fix image tile misalignment in GraphicsView ImagePaint by @SubhikshaSf4851 in #34935 <details> <summary>🔧 Fixes</summary> - [[iOS] Image resized with ResizeMode.Fit is not rendered correctly in GraphicsView](#34755) </details> - Fix Shadow does not honour Styles by @KarthikRajaKalaimani in #35081 <details> <summary>🔧 Fixes</summary> - [Shadow does not honour Styles](#19560) </details> ## Entry - [iOS/macCatalyst] Fix Entry and Editor BackgroundColor reset when set to null by @Shalini-Ashokan in #34741 <details> <summary>🔧 Fixes</summary> - [[iOS, Maccatalyst] Entry & Editor BackgroundColor not reset to Null](#34611) </details> - [Windows] Fix password Entry crash when setting text on empty field by @praveenkumarkarunanithi in #33891 <details> <summary>🔧 Fixes</summary> - [[WinUI] Password Obfuscation causes unhandled crash](#33334) </details> ## Essentials - [Essentials] Use mean sea level altitude on Android API 34+ by @KitKeen in #35097 <details> <summary>🔧 Fixes</summary> - [Add support for MslAltitudeMeters in Essentials Geolocation on Android](#27554) </details> ## Flyout - Fixed Flyout Not Displayed on Android When FlyoutWidth Is Set Only for Desktop via OnIdiom by @NanthiniMahalingam in #29028 <details> <summary>🔧 Fixes</summary> - [[Android] FlyoutWidth with OnIdiom shows no flyout](#13243) </details> - Revert "[Windows] Fix Flyout/Locked mode header collapse regression causing UI test failures on candidate branch" by @kubaflo in #35339 - Revert "Revert "[Windows] Fix Flyout/Locked mode header collapse regression causing UI test failures on candidate branch"" by @kubaflo in #35342 ## Flyoutpage - Fix [Android] Title of FlyOutPage is not updating anymore after showing a NonFlyOutPage by @KarthikRajaKalaimani in #34839 <details> <summary>🔧 Fixes</summary> - [[Android] Title of FlyOutPage is not updating anymore after showing a NonFlyOutPage](#33615) </details> ## Label - [iOS] Fix span Tap gesture on wrapped Label lines in iOS 26+ by @SubhikshaSf4851 in #34640 <details> <summary>🔧 Fixes</summary> - [[iOS]Span TapGestureRecognizer does not work on the second line of the span, if the span is wrapped to the next line](#34504) </details> ## Layout - Fixed Stacklayout is not rendered when clip is applied and StackLayout placed child to the Border control in iOS/ Mac platform by @KarthikRajaKalaimani in #33330 <details> <summary>🔧 Fixes</summary> - [[Mac/iOS] StackLayout fails to render content while applying Clip, and the layout is placed inside a Border with Background in .NET MAUI](#33241) </details> ## Map - Fix Changing Location on a Pin does nothing by @NirmalKumarYuvaraj in #30201 <details> <summary>🔧 Fixes</summary> - [[Maps] [Regression from Xamarin.Forms.Maps] Changing Location on a Pin does nothing](#12916) </details> ## Mediapicker - [iOS] Fix HEIC images picked via PickPhotosAsync not displayed by @HarishwaranVijayakumar in #34954 <details> <summary>🔧 Fixes</summary> - [[iOS] [Regression] HEIC images picked via PickPhotosAsync not displayed](#34953) </details> - [Android] Fix MediaPicker.PickPhotosAsync UnauthorizedAccessException on API 28 and below by @HarishwaranVijayakumar in #34981 <details> <summary>🔧 Fixes</summary> - [MediaPicker.PickPhotos fails to modify image, tries to load original source, fails to load source on Android 9.0](#34889) </details> ## Pages - [iOS] Fix ContentPage with ToolbarItem Clicked event leaks when presented as modal page by @devanathan-vaithiyanathan in #35009 <details> <summary>🔧 Fixes</summary> - [ContentPage with ToolbarItem Clicked event leaks when presented as modal page](#34892) </details> ## Platform - [Android] Fix OnBackButtonPressed not invoked for Shell by @Dhivya-SF4094 in #35150 <details> <summary>🔧 Fixes</summary> - [On Screen Back Button Does Not Fire OnBackButtonPressed in Android](#9095) </details> ## RadioButton - Fix RadioButtonGroup not working with ContentView by @Dhivya-SF4094 in #34781 <details> <summary>🔧 Fixes</summary> - [RadioButtonGroup not working with ContentView](#34759) </details> - [Windows] Fix for RadioButton BorderColor and BorderWidth not updated at runtime by @SyedAbdulAzeemSF4852 in #28335 <details> <summary>🔧 Fixes</summary> - [RadioButton Border color not working for focused visual state](#15806) </details> - [iOS] Fix RadioButton BackgroundColor bleeding outside CornerRadius by @SyedAbdulAzeemSF4852 in #34844 <details> <summary>🔧 Fixes</summary> - [[iOS] RadioButton BackgroundColor bleeds outside CornerRadius](#34842) </details> ## SafeArea - [iOS] Fix stale bottom safe area after changing SafeAreaEdges with keyboard open by @praveenkumarkarunanithi in #35083 <details> <summary>🔧 Fixes</summary> - [[iOS] ContentPage bottom has white space after changing SafeAreaEdges while keyboard is open](#34846) </details> ## ScrollView - [Windows] Fix Preserve ScrollView offsets when Orientation changes to Neither by @SubhikshaSf4851 in #34827 <details> <summary>🔧 Fixes</summary> - [[Windows] ScrollView offsets do not preserve when Orientation changes to Neither](#34671) </details> ## Searchbar - [iOS] Fix SearchBar unexpected left margin in iPad windowed mode on 26 Version by @SubhikshaSf4851 in #34704 <details> <summary>🔧 Fixes</summary> - [in iPad windowed mode SearchBar adds left margin equivaltent to SafeAreaInsets when placed inside grid](#34551) </details> ## Shell - [Windows] Fix for Shell.FlyoutBehavior="Flyout" forces the title height space above the tab bar even if the page title is empty by @BagavathiPerumal in #30382 <details> <summary>🔧 Fixes</summary> - [(Windows) Shell.FlyoutBehavior="Flyout" forces the title height space above the tab bar even if the page title is empty](#30254) </details> - Fix Shell flyout items scrolling behind FlyoutHeader on iOS by @Qythyx in #34936 <details> <summary>🔧 Fixes</summary> - [Shell flyout items scroll behind FlyoutHeader on iOS](#34925) </details> - [iOS, Mac] Fix Shell.CurrentState.Location stale in OnNavigated after GoToAsync by @Vignesh-SF3580 in #34880 <details> <summary>🔧 Fixes</summary> - [Shell.OnNavigated not called for route navigation](#34662) </details> - [iOS26]Fix BackButtonBehavior_IsEnabled_False_BackButtonDoesNotNavigate UITest fails by @devanathan-vaithiyanathan in #34890 <details> <summary>🔧 Fixes</summary> - [[iOS 26] BackButtonBehavior_IsEnabled_False_BackButtonDoesNotNavigate test fails with TimeoutException](#34771) </details> - [iOS] Fix Shell page memory leak when using TitleView with x:Name by @Shalini-Ashokan in #35082 <details> <summary>🔧 Fixes</summary> - [[iOS] Title view memory leak](#34975) </details> - [Material 3] Fix Material 2 color flash in AppBar when switching tabs for the first time by @Dhivya-SF4094 in #35117 <details> <summary>🔧 Fixes</summary> - [Material 3: AppBar briefly displays Material 2 colors when switching tabs for the first time](#35116) </details> - [Android] Fix Shell/TabbedPage "More" BottomSheet uses hard-coded M2 colors when Material3 is enabled by @HarishwaranVijayakumar in #35129 <details> <summary>🔧 Fixes</summary> - [[Android] Shell/TabbedPage "More" BottomSheet uses hard-coded M2 colors when Material3 is enabled](#35127) </details> - [Android] Shell: Fix top-tab unselected text visibility in Material 3 light theme by @SyedAbdulAzeemSF4852 in #35128 <details> <summary>🔧 Fixes</summary> - [[Android] Shell top-tab unselected text appears too faint in Material 3 light theme](#35125) </details> - Fix Shell.Items.Clear() memory leak by disconnecting child handlers on removal (#34898) by @Shalini-Ashokan in #35031 <details> <summary>🔧 Fixes</summary> - [Shell.Items.Clear() does not disconnect handlers correctly](#34898) </details> - [iOS&Mac] Fix Shell SearchHandler Query update on Initial load by @SubhikshaSf4851 in #35008 <details> <summary>🔧 Fixes</summary> - [[iOS&Mac] Shell SearchHandler Query not shown in search bar on initial load](#35005) </details> ## SwipeView - [iOS,MacCatalyst] Fix for SwipeView.Open() throwing an ArgumentException on the second programmatic call by @BagavathiPerumal in #34982 <details> <summary>🔧 Fixes</summary> - [[net 11.0][iOS,MacCatalyst] SwipeView.Open() throws ArgumentException on second programmatic call](#34917) </details> - [Android/iOS] Fix SwipeItem visibility change causing double command execution in Execute mode by @praveenkumarkarunanithi in #35087 <details> <summary>🔧 Fixes</summary> - [Changing visibility on an SwipeItem causes multiple items to be executed](#7580) </details> ## Switch - [iOS] Fix Switch ThumbColor reset on iOS 26+ theme changes. by @Shalini-Ashokan in #33953 <details> <summary>🔧 Fixes</summary> - [Switch ThumbColor not Initialized Using VisualStateManager on iOS Device](#33783) - [I9-On macOS 26.2, the "Animate scroll" button is white by default on iOS and Maccatalyst platforms.](#33767) </details> ## TabbedPage - [Windows] TabbedPage: Refresh layout when NavigationView size changes by @BagavathiPerumal in #26217 <details> <summary>🔧 Fixes</summary> - [TabbedPage - ScrollView not allowing scrolling when it should](#26103) - [TabbedPage App on resize hides page bottom content](#11402) - [Grid overflows child ContentPage of parent TabbedPage on initial load and when resizing on Windows](#20028) </details> - [Android] Material 3 Fixed BottomNavigationView overflowing in Tabbed page by @NirmalKumarYuvaraj in #35064 <details> <summary>🔧 Fixes</summary> - [[Android] Material3 - TabbedPage bottom tabs overflowing the contents](#35063) </details> - [Windows] Fix for Multiple Tabs Being Selected in WinUI TabbedPage by @SyedAbdulAzeemSF4852 in #33312 <details> <summary>🔧 Fixes</summary> - [WinUI TabbedPage can have multiple tabs selected](#31799) </details> ## Theming - [iOS] Fix StaticResource Hot Reload crash on iOS by @StephaneDelcroix in #35020 <details> <summary>🔧 Fixes</summary> - [The maui app quit and no errors in error list after editing ResourceDictionary XAML file on iOS Simulator with MAUI SR6 10.0.60](#35018) </details> ## Toolbar - [Windows] Fix for CS1061 build error caused by missing HasMenuBarContent property in MauiToolbar by @BagavathiPerumal in #35040 ## Tooling - Fix VisualStateGroups duplicate name crash with implicit styles (#34716) by @StephaneDelcroix in #34719 <details> <summary>🔧 Fixes</summary> - [SourceGen: VisualStateManager.VisualStateGroups causes 'Names must be unique' at startup](#34716) </details> ## WebView - Refactor the HybridWebView and properly support complex parameters by @mattleibow in #32491 - [Android] Fix WebView scrolling inside ScrollView by @Shalini-Ashokan in #33133 <details> <summary>🔧 Fixes</summary> - [[Android] WebView's content does not scroll when placed inside a ScrollView](#32971) </details> <details> <summary>🔧 Infrastructure (1)</summary> - [Windows] Fix Narrator announcing ContentView children twice when Description is set by @praveenkumarkarunanithi in #33979 <details> <summary>🔧 Fixes</summary> - [[Windows] SemanticProperties.Description announced twice when set on focusable container cell (Label inside)](#33373) </details> </details> <details> <summary>🧪 Testing (14)</summary> - [Testing] SafeArea Feature Matrix Test Cases for ContentPage by @TamilarasanSF4853 in #34877 - [Windows] Fix CollectionView ScrollTo related test cases failed in CI by @HarishwaranVijayakumar in #34907 <details> <summary>🔧 Fixes</summary> - [[Testing][Windows]CollectionView ScrollTo related test cases failed in CI](#34772) </details> - [Testing] Fixed Build error on inflight/ candidate PR 35234 by @HarishKumarSF4517 in #35241 - Fix CI for ValidateKeyboardRuntime_SwitchContainerToSoftInput_WhileKeyboardOpen test failure in May 4th Candidate by @devanathan-vaithiyanathan in #35307 - [Windows] Fix Flyout/Locked mode header collapse regression causing UI test failures on candidate branch by @BagavathiPerumal in #35312 - [iOS/macCatalyst] [Candidate Fix] Editor shadow and theme regression caused by BackgroundColor reset on initial handler connection by @Shalini-Ashokan in #35343 - [Testing] Fixed UI test image failure in PR 35234 - [30/03/2026] Candidate - 1 by @HarishKumarSF4517 in #35325 - [iOS] Fix ShellFeatureMatrix test failures on candidate branch by @Vignesh-SF3580 in #35346 - [Windows] Fix Issue29529VerifyPreviousPositionOnInsert test failure on candidate branch by @praveenkumarkarunanithi in #35398 - [Android] [Candidate Fix] Shell: Fix handler disconnect timing to preserve WebView navigation and memory leak fix by @Shalini-Ashokan in #35417 - [Testing]Revert 'Fix Preserve ScrollView offsets when Orientation changes to Neither' by @TamilarasanSF4853 in #35412 - [Windows] Fix VerifyAllIndicatorDotsShowShadowsWhenIndicatorSize test failure on candidate branch by @praveenkumarkarunanithi in #35458 - [Testing] Fixed test failure in PR 35234 - [05/08/2026] Candidate by @TamilarasanSF4853 in #35362 - [Testing] Fixed test failure in PR 35234 - [05/04/2026] Candidate - 3 by @TamilarasanSF4853 in #35639 </details> <details> <summary>📦 Other (6)</summary> - [UIKit] Avoid useless measure invalidation propagation cycles by @albyrock87 in #33459 - BindableObject property access micro-optimizations by @albyrock87 in #33584 - Extract filename from DisplayName and add extension if missing by @mattleibow in #35050 - [core] Add keyed-DI screenshot extensibility for 3rd-party platform backends by @Redth in #35096 <details> <summary>🔧 Fixes</summary> - [`ViewExtensions.CaptureAsync(IView)` and `IPlatformScreenshot` need extensibility for third-party platform backends](#34266) </details> - Fix MainThread throwing on custom platform backends by @Redth in #35070 <details> <summary>🔧 Fixes</summary> - [`MainThread.BeginInvokeOnMainThread` throws on custom platform backends - Common UI-thread marshaling pattern crashes; `Dispatcher` works but isn't the documented/recommended path](#34101) </details> - Tests: Add 11 missing UnitConverters unit tests by @PureWeen in #35191 </details> <details> <summary>📝 Issue References</summary> Fixes #7580, Fixes #9095, Fixes #11402, Fixes #12916, Fixes #13243, Fixes #13801, Fixes #15806, Fixes #19560, Fixes #19690, Fixes #19866, Fixes #20028, Fixes #26103, Fixes #26366, Fixes #27554, Fixes #29529, Fixes #29772, Fixes #30192, Fixes #30199, Fixes #30254, Fixes #31658, Fixes #31799, Fixes #32971, Fixes #33065, Fixes #33241, Fixes #33334, Fixes #33373, Fixes #33500, Fixes #33615, Fixes #33767, Fixes #33783, Fixes #34101, Fixes #34257, Fixes #34266, Fixes #34402, Fixes #34504, Fixes #34551, Fixes #34611, Fixes #34662, Fixes #34671, Fixes #34716, Fixes #34755, Fixes #34759, Fixes #34771, Fixes #34772, Fixes #34842, Fixes #34846, Fixes #34848, Fixes #34861, Fixes #34889, Fixes #34892, Fixes #34897, Fixes #34898, Fixes #34900, Fixes #34917, Fixes #34925, Fixes #34953, Fixes #34975, Fixes #35005, Fixes #35018, Fixes #35063, Fixes #35116, Fixes #35119, Fixes #35125, Fixes #35127 </details> **Full Changelog**: main...inflight/candidate
…t#35050) <!-- 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 When Android content providers return a `DisplayName` that contains directory components (e.g. `/storage/emulated/0/DCIM/Camera/IMG_20240101.jpg` from Samsung, or `/sdcard/.transforms/synthetic/picker/0/.../photo.png` from Google Drive), `CacheContentFile` would pass the full path as a filename to `GetTemporaryFile`. This caused `Java.IO.File(parent, child)` to either ignore the parent directory (absolute paths) or attempt to create non-existent nested directories (relative paths), leading to broken cached file paths. ### Changes - **Extract `EnsureFileName` helper** to `FileSystemUtils.shared.cs` — normalizes a display name by stripping directory components, guarding against empty/whitespace/reserved directory names (`.`, `..`), and appending a known extension when the name has none - **Add empty-string guard** — if `Path.GetFileName` returns empty (e.g. trailing separator `somefolder/`), falls back to a generated GUID name instead of creating a dot-file - **Add 17 unit tests** covering: simple names, paths with directories, null/empty/whitespace display names, trailing separators, `.`/`..` reserved names, extension handling ### Tested scenarios Verified on Android API 36 emulator using a custom `ContentProvider` that injects problematic `DisplayName` values through the full `EnsurePhysicalPath` → `CacheContentFile` → `EnsureFileName` pipeline: | DisplayName | Result | |---|---| | `/storage/emulated/0/DCIM/Camera/IMG_20240101.jpg` | `IMG_20240101.jpg` ✅ | | `/sdcard/.transforms/synthetic/.../photo.png` | `photo.png` ✅ | | `Download/Subdir/document.pdf` | `document.pdf` ✅ | | `somefolder/` | `<guid>.jpg` ✅ | | `""` (empty) | `<guid>.jpg` ✅ | | `" "` (whitespace) | `<guid>.jpg` ✅ | | `screenshot_12345` (no ext) | `screenshot_12345.jpg` ✅ | | `normal_photo.jpg` | `normal_photo.jpg` ✅ | --------- Co-authored-by: Jakub Florkowski <kubaflo123@gmail.com>
…t#35050) <!-- 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 When Android content providers return a `DisplayName` that contains directory components (e.g. `/storage/emulated/0/DCIM/Camera/IMG_20240101.jpg` from Samsung, or `/sdcard/.transforms/synthetic/picker/0/.../photo.png` from Google Drive), `CacheContentFile` would pass the full path as a filename to `GetTemporaryFile`. This caused `Java.IO.File(parent, child)` to either ignore the parent directory (absolute paths) or attempt to create non-existent nested directories (relative paths), leading to broken cached file paths. ### Changes - **Extract `EnsureFileName` helper** to `FileSystemUtils.shared.cs` — normalizes a display name by stripping directory components, guarding against empty/whitespace/reserved directory names (`.`, `..`), and appending a known extension when the name has none - **Add empty-string guard** — if `Path.GetFileName` returns empty (e.g. trailing separator `somefolder/`), falls back to a generated GUID name instead of creating a dot-file - **Add 17 unit tests** covering: simple names, paths with directories, null/empty/whitespace display names, trailing separators, `.`/`..` reserved names, extension handling ### Tested scenarios Verified on Android API 36 emulator using a custom `ContentProvider` that injects problematic `DisplayName` values through the full `EnsurePhysicalPath` → `CacheContentFile` → `EnsureFileName` pipeline: | DisplayName | Result | |---|---| | `/storage/emulated/0/DCIM/Camera/IMG_20240101.jpg` | `IMG_20240101.jpg` ✅ | | `/sdcard/.transforms/synthetic/.../photo.png` | `photo.png` ✅ | | `Download/Subdir/document.pdf` | `document.pdf` ✅ | | `somefolder/` | `<guid>.jpg` ✅ | | `""` (empty) | `<guid>.jpg` ✅ | | `" "` (whitespace) | `<guid>.jpg` ✅ | | `screenshot_12345` (no ext) | `screenshot_12345.jpg` ✅ | | `normal_photo.jpg` | `normal_photo.jpg` ✅ | --------- Co-authored-by: Jakub Florkowski <kubaflo123@gmail.com>
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
When Android content providers return a
DisplayNamethat contains directory components (e.g./storage/emulated/0/DCIM/Camera/IMG_20240101.jpgfrom Samsung, or/sdcard/.transforms/synthetic/picker/0/.../photo.pngfrom Google Drive),CacheContentFilewould pass the full path as a filename toGetTemporaryFile. This causedJava.IO.File(parent, child)to either ignore the parent directory (absolute paths) or attempt to create non-existent nested directories (relative paths), leading to broken cached file paths.Changes
EnsureFileNamehelper toFileSystemUtils.shared.cs— normalizes a display name by stripping directory components, guarding against empty/whitespace/reserved directory names (.,..), and appending a known extension when the name has nonePath.GetFileNamereturns empty (e.g. trailing separatorsomefolder/), falls back to a generated GUID name instead of creating a dot-file./..reserved names, extension handlingTested scenarios
Verified on Android API 36 emulator using a custom
ContentProviderthat injects problematicDisplayNamevalues through the fullEnsurePhysicalPath→CacheContentFile→EnsureFileNamepipeline:/storage/emulated/0/DCIM/Camera/IMG_20240101.jpgIMG_20240101.jpg✅/sdcard/.transforms/synthetic/.../photo.pngphoto.png✅Download/Subdir/document.pdfdocument.pdf✅somefolder/<guid>.jpg✅""(empty)<guid>.jpg✅" "(whitespace)<guid>.jpg✅screenshot_12345(no ext)screenshot_12345.jpg✅normal_photo.jpgnormal_photo.jpg✅