Skip to content

Conversation

@aran
Copy link
Contributor

@aran aran commented Oct 10, 2023

Adds support for a new --web-header option to flutter run.

Creates a workaround for #127902

This PR allows adding additional headers for the flutter run web server. This is useful to add headers like Cross-Origin-Embedder-Policy and Cross-Origin-Opener-Policy without the use of a proxy server. These headers are required enable advanced web features. This approach provides flexibility to the developer to make use of the feature as they see fit and is backward-compatible. One tradeoff is that it increases the surface area to support for future changes to the flutter web server.

#127902 is not fully addressed by this change. The solution for that task will be more opinionated. This PR creates a general-purpose workaround for anyone who needs a solution sooner while the bigger solution is developed.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie or stuartmorgan on the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Oct 10, 2023
@michaelnew
Copy link

For whatever it's worth, I just tested this out with --web-header=Cross-Origin-Opener-Policy=same-origin --web-header=Cross-Origin-Embedder-Policy=require-corp, and that's allowed me to use sqlite3 wasm with the OPFS to read/write to a sqlite database in the browser. Previously I had to build for web and then host with my own server that set those headers, but now I can run with the debugger attached.

Huge win for me, so thank you for this PR. I hope it gets merged soon!

@andrewkolos andrewkolos added the fyi-web For the attention of Web platform team label Oct 13, 2023
Copy link
Contributor

@andrewkolos andrewkolos left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

The intent of this looks good to me from a CLI tool perspective. Please update this test (edit: I realized that test is specifically for flags, not options) and add a test for a case where the user provides an invalid header and a tool exit gets thrown. I will be adding another reviewer shortly

@andrewkolos
Copy link
Contributor

@eyebrowsoffire Can you review this from a web perspective and make sure this is an appropriate resolution for #127902 ? Also, if you happen to know of an existing integration test that this feature could be added to, please let us know; it would be nice to have this under test end-to-end.

Copy link
Contributor

@eyebrowsoffire eyebrowsoffire left a comment

Choose a reason for hiding this comment

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

Looks good to me. As discussed on Discord, I don't believe this is a full resolution to #127902 and we should not close that issue based on just this PR. I think we need more specific and turnkey handling of the CORS headers needed for crossOriginIsolated rather than a generic mechanism like this. Nonetheless, this change seems good to me and will be generally useful for end users.

Moves tests into their own "--web-header" group. Also updates the invalid input tests to check against the exception message to make sure we are throwing a ToolExit with the correct message. After doing this, I noticed tests were passing despite the thrown error message having nothing to do with the --web-header option, so I fixed them.
@andrewkolos
Copy link
Contributor

Thanks @eyebrowsoffire!

@aran, I've pushed some changes which simply move and rename the tests to be more consistent with the general pattern found throughout the rest of the CLI tool test code.

I also made some of the throwsToolExit checks more strict by adding error message checks. Upon doing this, I noticed some of the ToolExits being thrown weren't the ones we were actually testing for, so I tweaked some of the test setup to resolve this. Feel free to make additional tweaks.

Copy link
Contributor

@andrewkolos andrewkolos left a comment

Choose a reason for hiding this comment

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

LGTM. I will merge this after CI checks pass and @aran has had ample time to respond to my latest commits should they choose to do so

@aran
Copy link
Contributor Author

aran commented Oct 17, 2023

Great collaborating with you on this, thanks for all the help and for taking this change.

@andrewkolos
Copy link
Contributor

Great collaborating with you on this, thanks for all the help and for taking this change.

Likewise; thanks for your efforts and contribution!

@andrewkolos andrewkolos added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 17, 2023
@auto-submit auto-submit bot merged commit 48eee14 into flutter:master Oct 17, 2023
@aran aran deleted the headers branch October 17, 2023 21:53
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Oct 19, 2023
Manual roll Flutter from 6eea6e2 to 189196d (44 revisions)

Manual roll requested by [email protected]

flutter/flutter@6eea6e2...189196d

2023-10-18 [email protected] Roll Flutter Engine from ab86c53c19cd to 6caee3236d37 (2 revisions) (flutter/flutter#136834)
2023-10-18 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Reland] Skip injecting Bonjour settings when port publication is disabled" (flutter/flutter#136839)
2023-10-18 [email protected] Convert menus to use OverlayPortal (flutter/flutter#130534)
2023-10-18 [email protected] [Reland] Skip injecting Bonjour settings when port publication is disabled (flutter/flutter#136751)
2023-10-18 [email protected] Roll Flutter Engine from 0eff5d191856 to ab86c53c19cd (1 revision) (flutter/flutter#136832)
2023-10-18 [email protected] Fixes ability to call nextFocus() on a node to focus its descendant (flutter/flutter#136773)
2023-10-18 [email protected] Roll Flutter Engine from b778a07f8ae9 to 0eff5d191856 (6 revisions) (flutter/flutter#136829)
2023-10-18 [email protected] Implement GApplication:shutdown so a Flutter developer knows where to put code that should occur on application shutdown. (flutter/flutter#136780)
2023-10-18 [email protected] Roll Flutter Engine from 7f37c9b181af to b778a07f8ae9 (1 revision) (flutter/flutter#136818)
2023-10-18 [email protected] Fix `Slider` `onChanged` callback order & never calls `onChangeStart` on  `SliderInteraction.slideOnly` allowed interaction (flutter/flutter#136720)
2023-10-18 [email protected] Roll Flutter Engine from 78026b4003fe to 7f37c9b181af (1 revision) (flutter/flutter#136817)
2023-10-18 [email protected] [Feat] Stroke color for Slider value indicator (flutter/flutter#135986)
2023-10-18 [email protected] Fixed : Empty Rows shown at last page in Paginated data table (flutter/flutter#132646)
2023-10-18 [email protected] Roll Flutter Engine from 46923fd39032 to 78026b4003fe (1 revision) (flutter/flutter#136814)
2023-10-18 [email protected] Roll Packages from d439062 to 14aa69e (1 revision) (flutter/flutter#136813)
2023-10-18 [email protected] Don't build native assets in `flutter build bundle` (flutter/flutter#136641)
2023-10-18 [email protected] Roll Flutter Engine from c9c9684e03a3 to 46923fd39032 (1 revision) (flutter/flutter#136795)
2023-10-18 [email protected] Roll Flutter Engine from 0c1c29271e8b to c9c9684e03a3 (1 revision) (flutter/flutter#136792)
2023-10-18 [email protected] Roll Flutter Engine from 1de09d13e708 to 0c1c29271e8b (3 revisions) (flutter/flutter#136789)
2023-10-18 [email protected] Roll Flutter Engine from 6fc36e61a99a to 1de09d13e708 (1 revision) (flutter/flutter#136785)
2023-10-18 [email protected] Roll Flutter Engine from 3f818efff3c5 to 6fc36e61a99a (1 revision) (flutter/flutter#136781)
2023-10-18 [email protected] Roll Flutter Engine from 2eef9b43cfb4 to 3f818efff3c5 (3 revisions) (flutter/flutter#136777)
2023-10-18 [email protected] Roll Flutter Engine from 5df7af34a718 to 2eef9b43cfb4 (4 revisions) (flutter/flutter#136774)
2023-10-18 [email protected] Flutter preview device (flutter/flutter#135639)
2023-10-18 [email protected] Add findChildIndexCallback examples (flutter/flutter#133469)
2023-10-18 [email protected] Adds API for performing semantics actions in tests (flutter/flutter#132598)
2023-10-18 [email protected] Roll Flutter Engine from e57b5bac4244 to 5df7af34a718 (2 revisions) (flutter/flutter#136772)
2023-10-17 [email protected] Roll Flutter Engine from f9f937e51080 to e57b5bac4244 (3 revisions) (flutter/flutter#136768)
2023-10-17 [email protected] clean up `--dart-define-from-file` option tests (flutter/flutter#135980)
2023-10-17 [email protected] GoldenFileComparators should dispose created Image objects.  (flutter/flutter#136716)
2023-10-17 [email protected] make integration_test_driver_extended.dart support writeResponseData--(done) (flutter/flutter#128382)
2023-10-17 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 3.6.0 to 4.1.1 (flutter/flutter#136762)
2023-10-17 [email protected] Roll Flutter Engine from 289f29b1ad00 to f9f937e51080 (1 revision) (flutter/flutter#136755)
2023-10-17 [email protected] Roll Flutter Engine from 659e68a097b5 to 289f29b1ad00 (1 revision) (flutter/flutter#136752)
2023-10-17 [email protected] Support --web-header option for flutter run (flutter/flutter#136297)
2023-10-17 [email protected] Add Android 14 physical devices to firebase tests (flutter/flutter#136736)
2023-10-17 [email protected] Roll Flutter Engine from 62a90a91cee3 to 659e68a097b5 (3 revisions) (flutter/flutter#136746)
2023-10-17 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Skip injecting Bonjour settings when port publication is disabled" (flutter/flutter#136750)
2023-10-17 [email protected] Revert "[SingleChildScrollView] Correct the offset pixels if it is out of range during layout" (flutter/flutter#136744)
2023-10-17 [email protected] Skip injecting Bonjour settings when port publication is disabled (flutter/flutter#136562)
2023-10-17 [email protected] Roll Flutter Engine from 3ecbe924a598 to 62a90a91cee3 (2 revisions) (flutter/flutter#136739)
2023-10-17 [email protected] Roll Flutter Engine from 0a4d8b99a95b to 3ecbe924a598 (3 revisions) (flutter/flutter#136732)
2023-10-17 [email protected] Reenable NexusLowRes API 29 (flutter/flutter#136686)
2023-10-17 [email protected] Reenable the nexus 6p tests (flutter/flutter#136689)
...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App fyi-web For the attention of Web platform team tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants