Skip to content

[flutter_tools] Fix local CanvasKit asset paths on Windows - #193776

Merged
auto-submit[bot] merged 2 commits into
flutter:masterfrom
molchanovia:fix/canvaskit-windows-path-193505
Oct 29, 2026
Merged

auto-submit[bot] merged 2 commits into
flutter:masterfrom
molchanovia:fix/canvaskit-windows-path-193505

Conversation

@molchanovia

@molchanovia molchanovia commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

On Windows, converting a CanvasKit request URI to a filesystem path before matching the canvaskit/ prefix causes valid asset requests to return HTTP 404. Match the URI path segments first, then build the asset path with the filesystem's path context.

Adds hermetic regression coverage for POSIX and Windows paths, including root and Chromium JavaScript/Wasm assets, response bodies and headers, and rejection of invalid CanvasKit paths.

Fixes #193505.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

@github-actions github-actions Bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Oct 3, 2026
@molchanovia
molchanovia marked this pull request as ready for review October 3, 2026 22:52

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the local CanvasKit file handler in flutter_web_platform.dart to resolve paths using request.url.pathSegments and introduces a new test suite to verify CanvasKit asset serving. Feedback on these changes identifies a potential directory traversal vulnerability when constructing file paths from URL segments without validation, and suggests rejecting path segments containing .. or . along with adding corresponding test cases.

@flutter-zl
flutter-zl requested a review from mdebbar October 21, 2026 17:22
@flutter-zl flutter-zl added the CICD Run CI/CD label Oct 21, 2026
@flutter-dashboard flutter-dashboard Bot removed the CICD Run CI/CD label Oct 21, 2026
@flutter-zl flutter-zl added the CICD Run CI/CD label Oct 21, 2026
Match CanvasKit requests by URI path segments before constructing the
filesystem path. This fixes valid asset requests returning 404 on
Windows, where path.fromUri produces backslashes that do not match
the canvaskit/ prefix.

Only serve existing files whose resolved paths stay within the
CanvasKit directory.

Add hermetic tests for POSIX and Windows paths, covering root and
Chromium JavaScript/Wasm assets, response bodies and headers, and
rejection of missing files, directories, and paths outside CanvasKit.

Related to flutter#193505.
@molchanovia
molchanovia force-pushed the fix/canvaskit-windows-path-193505 branch from e0fc9a9 to 7257aee Compare October 22, 2026 19:06
@flutter-dashboard flutter-dashboard Bot removed the CICD Run CI/CD label Oct 22, 2026
@mdebbar mdebbar added the CICD Run CI/CD label Oct 27, 2026

@mdebbar mdebbar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix, this LGTM!

@mdebbar
mdebbar requested a review from bkonyi October 27, 2026 22:26
@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 29, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Oct 29, 2026
Merged via the queue into flutter:master with commit 91ffada Oct 29, 2026
29 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 29, 2026
@hellohuanlin

Copy link
Copy Markdown
Contributor

Reason for revert:

This is the first commit that breaks the tree:

Screenshot 2026-10-29 at 3 04 35 PM

@hellohuanlin hellohuanlin added the revert Autorevert PR (with "Reason for revert:" comment) label Oct 29, 2026
@flutteractionsbot

Copy link
Copy Markdown
Contributor

Successfully created revert PR: #195635

@flutteractionsbot flutteractionsbot removed the revert Autorevert PR (with "Reason for revert:" comment) label Oct 29, 2026
kevmoo pushed a commit to kevmoo/flutter that referenced this pull request Oct 29, 2026
…lutter#195635)

Reverts: [[flutter_tools] Fix local CanvasKit asset paths on
Windows](flutter#193776)

Initiated by: @hellohuanlin

Reason for reverting: This is the first commit that breaks the tree

Original PR Author: @meg4cyberc4t

Reviewed By: @mdebbar

The original PR description is provided below:

On Windows, converting a CanvasKit request URI to a filesystem path
before matching the `canvaskit/` prefix causes valid asset requests to
return HTTP 404. Match the URI path segments first, then build the asset
path with the filesystem's path context.

Adds hermetic regression coverage for POSIX and Windows paths, including
root and Chromium JavaScript/Wasm assets, response bodies and headers,
and rejection of invalid CanvasKit paths.

Fixes flutter#193505.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [AI contribution guidelines] and understand my
responsibilities, or I am not using AI tools.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant in-code documentation (doc comments with
`///`).
- [x] If this PR introduces a new feature or capability, I created and
linked a website documentation issue or PR in [flutter/website] (or
verified none is needed).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

If this change needs to override an active code freeze, provide a
comment explaining why. The code freeze workflow can be overridden by
code reviewers. See pinned issues for any active code freezes with
guidance.

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[AI contribution guidelines]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[flutter/website]: https://github.com/flutter/website
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
molchanovia added a commit to molchanovia/flutter that referenced this pull request Nov 1, 2026
Reland flutter#193776, reverted by flutter#195635. Match CanvasKit requests by URI path
segments before constructing filesystem paths, preserving containment
checks and POSIX/Windows regression coverage.

Pass MustacheTemplateRenderer to FlutterWebPlatform.start in the regression
tests to satisfy the required templateRenderer parameter added by flutter#194062.
jpohhhh pushed a commit to Telosnex/flutter that referenced this pull request Nov 5, 2026
…flutter#195770)

<!--
Thanks for filing a pull request!
Reviewers are typically assigned within a week of filing a request.
To learn more about code review, see our documentation on Tree Hygiene:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
-->

Relands flutter#193776, reverted by flutter#195635.

On Windows, converting a CanvasKit request URI to a filesystem path
before matching `canvaskit/` causes valid asset requests to return HTTP
404. Match URI path segments first, then construct and validate the
filesystem path. Restore regression coverage for POSIX and Windows
paths, including JavaScript/Wasm assets, response bodies and headers,
and invalid paths.

The regression tests now pass `MustacheTemplateRenderer` to
`FlutterWebPlatform.start`, addressing the required `templateRenderer`
parameter introduced by flutter#194062 that caused the original tree breakage.

Fixes flutter#193505.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [AI contribution guidelines] and understand my
responsibilities, or I am not using AI tools.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant in-code documentation (doc comments with
`///`).
- [x] If this PR introduces a new feature or capability, I created and
linked a website documentation issue or PR in [flutter/website] (or
verified none is needed).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

If this change needs to override an active code freeze, provide a
comment explaining why. The code freeze workflow can be overridden by
code reviewers. See pinned issues for any active code freezes with
guidance.

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[AI contribution guidelines]:
https://github.com/flutter/flutter/blob/main/CONTRIBUTING.ai.md
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[flutter/website]: https://github.com/flutter/website
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flutter test --platform chrome hangs/fails on Windows: CanvasKit assets 404 due to backslash path from path.fromUri

6 participants