Skip to content

Conversation

@Saqib198
Copy link
Contributor

@Saqib198 Saqib198 commented Dec 9, 2025

This PR restores the correct precedence behavior for web development server configuration. Since Flutter 3.38, CLI arguments were being ignored when web_dev_config.yaml was present — specifically for HTTPS and headers.

According to the design and documentation, the precedence should be:

  1. Command-line arguments (highest priority)
  2. web_dev_config.yaml
  3. Built-in defaults (lowest priority)

Root Cause

The regression affected:

  • HTTP headers — merge order was reversed, causing file config to override CLI
  • HTTPS config — CLI TLS arguments were ignored under certain conditions

Note:
--web-port and --web-hostname were already working correctly due to the host ?? this.host and port ?? this.port pattern.
The observed issue was due to headers + HTTPS only.

Changes Made

  • Fixed header merge order so CLI takes precedence
  • Corrected HTTPS config resolution and precedence logic
  • Unified logic across run and drive commands
  • Simplified HTTPS handling using HttpsConfig.parse()
  • Added tests for CLI precedence behavior

Before (broken)

flutter run -d chrome --web-tls-cert-path cert.pem

❌ CLI TLS args ignored when config file existed
❌ headers overridden by file config

After (fixed)

flutter run -d chrome --web-tls-cert-path cert.pem

✅ CLI TLS args respected
✅ headers override config as expected

Tests

  • Added tests verifying CLI takes precedence
  • All existing and new tests pass
  • No breaking changes introduced

Fixes

Fixes: #179014

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

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. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@google-cla
Copy link

google-cla bot commented Dec 9, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Dec 9, 2025
Copy link
Contributor

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

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 adjusts the configuration loading logic to ensure that command-line arguments have precedence over settings in web_dev_config.yaml. The changes affect how HTTPS configuration and web server headers are processed in the drive and run commands. Specifically, the order of merging headers is corrected in WebDevServerConfig.copyWith, and the logic for determining HttpsConfig is updated in both DriveCommand and RunCommand. While the changes are mostly correct, I've identified a logical flaw in drive.dart where CLI arguments for HTTPS are incorrectly ignored if a configuration file is not present, and I've provided a suggestion to align it with the correct implementation in run.dart.

@Saqib198
Copy link
Contributor Author

Thanks for the review!
You're absolutely right — the previous logic in drive.dart was incorrectly depending on fileConfig != null, which caused CLI HTTPS arguments to be ignored when no config file was present.

I've updated the logic to match run.dart so that:

CLI arguments always take precedence

File config works only as a fallback

HTTPS config is correctly applied even without web_dev_config.yaml

Please let me know if anything else is needed.

@bkonyi bkonyi added the team-web Owned by Web platform team label Dec 11, 2025
@bkonyi bkonyi requested review from bkonyi and mdebbar December 11, 2025 16:04
Copy link
Contributor

@bkonyi bkonyi 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 @Saqib198!

This looks good to me in general, with a couple of minor suggestions. Also, this will need a test before it can be submitted.

// Determine HTTPS config with CLI > file precedence
// If CLI provides TLS certs, use them (with file config as fallback for missing values)
// If CLI doesn't provide TLS certs, use file config
final HttpsConfig? httpsConfig;
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider doing this instead (also in drive.dart):

final HttpsConfig? httpsConfig = (fileConfig.https ?? HttpsConfig()).copyWith(
  certPath: stringArg('web-tls-cert-path'),
  certKeyPath: stringArg('web-tls-cert-key-path'),
);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion @bkonyi!

I tried implementing it, but HttpsConfig() cannot be instantiated without arguments since both certPath and certKeyPath are required parameters in the constructor:

const HttpsConfig({required this.certPath, required this.certKeyPath});

Instead, I've simplified the code using HttpsConfig.parse() which handles all the cases cleanly:

final HttpsConfig? httpsConfig = HttpsConfig.parse(
stringArg('web-tls-cert-path') ?? fileConfig.https?.certPath,
stringArg('web-tls-cert-key-path') ?? fileConfig.https?.certKeyPath,
);

This achieves the same simplification (reduced from 15 lines to 4 lines) while:

  • CLI args take precedence over file config
  • Falls back to file config values when CLI args not provided
  • Returns null if neither provides both values
  • Throws ArgumentError if only one path is provided (existing behavior)

I've also added tests for CLI precedence as requested. Let me know if you'd like any changes!

@Saqib198 Saqib198 force-pushed the fix-web-cli-precedence branch from 4521acd to 5f030a9 Compare December 11, 2025 17:49
@github-actions github-actions bot removed the team-web Owned by Web platform team label Dec 11, 2025
}
}

Future<WebDevServerConfig> webDevServerConfigCore() async {
Copy link
Contributor

Choose a reason for hiding this comment

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

@Saqib198 – I moved things into this shared function so we have fewer places to keep this logic up-to-date!

@kevmoo
Copy link
Contributor

kevmoo commented Dec 12, 2025

I took over a bit to fix the analyzer and format issue and to do a tiny bit of cleanup

@Saqib198
Copy link
Contributor Author

Hi @mdebbar 👋
Just a gentle ping on this when you get a chance.
All tests are green and @kevmoo has already applied cleanup/refactors.
Thanks!

@mdebbar
Copy link
Contributor

mdebbar commented Dec 16, 2025

Sorry for the delay here!

Help me understand. The way I'm seeing this PR, it seems to fix the precedence for https and headers. It doesn't change the precedence for the other mentioned arguments: web-port, web-hostname. Are those already working correctly, or am I missing something?

List<ProxyRule>? proxy,
}) {
return WebDevServerConfig(
host: host ?? this.host,
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, these should all be changed, too – right? @Saqib198

RE comments from @mdebbar

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the way they are now is correct. But the issue says web-port's and web-hostname's precedence is incorrect, which is confusing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah!

I get it.

By putting headers after this.headers, it overwrites.

But with port and host, the ?? has things the other way.

So it reads weird, but I think the code as it is now is correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

But I agree, I see no code changes for anything but headers.

So was it really broken? Or are we missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for calling this out — happy to clarify.

You’re right that web-port and web-hostname already had correct CLI > file precedence.
They use the pattern:

host: host ?? this.host,
port: port ?? this.port,

where host / port come from the CLI layer, so CLI values correctly override
web_dev_config.yaml today.

The regression introduced in 3.38 specifically affected:
• headers (due to merge order: {...?headers, ...this.headers})
• HTTPS config (due to conditional logic that ignored CLI TLS args when file config
was present or missing)

That’s why this PR only changes:
• header merge order
• HTTPS config resolution

The issue description mentioned web-port / web-hostname because the symptom users
observed was “CLI args ignored”, but the actual root cause was headers + HTTPS.
Port/hostname were already behaving correctly, so no change was needed there.

Happy to update the PR description or issue wording if that helps reduce confusion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, that makes more sense. Thanks for clarifying!

I would say yes let's update the issue and PR descriptions. If it confused me and @kevmoo, I'm sure it'll confuse more people later when they look at the history of this code.

Copy link
Contributor

@mdebbar mdebbar left a comment

Choose a reason for hiding this comment

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

LGTM

@Saqib198 Saqib198 changed the title Fix: Restore CLI precedence over web_dev_config.yaml Restore CLI precedence for web headers and HTTPS over web_dev_config.yaml Dec 17, 2025
@mdebbar mdebbar added autosubmit Merge PR when tree becomes green via auto submit App and removed autosubmit Merge PR when tree becomes green via auto submit App labels Dec 18, 2025
@mdebbar
Copy link
Contributor

mdebbar commented Dec 18, 2025

@bkonyi does this look good to you? I think we need a 2nd approval to merge.

@Saqib198 Saqib198 requested a review from bkonyi December 19, 2025 17:19
@Saqib198
Copy link
Contributor Author

Hi @bkonyi 👋
All requested changes are addressed, tests are added, and the PR description is updated.
has been clarified per @mdebbar’s feedback.

Whenever you get a chance, please take a look.
Thanks a lot!

@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 6, 2026
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 6, 2026
@auto-submit
Copy link
Contributor

auto-submit bot commented Jan 6, 2026

autosubmit label was removed for flutter/flutter/179639, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR.

@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 6, 2026
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 6, 2026
@auto-submit
Copy link
Contributor

auto-submit bot commented Jan 6, 2026

autosubmit label was removed for flutter/flutter/179639, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 6, 2026
@auto-submit auto-submit bot added this pull request to the merge queue Jan 6, 2026
Merged via the queue into flutter:master with commit fb86531 Jan 6, 2026
143 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 6, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 7, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 7, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 7, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 7, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 7, 2026
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jan 7, 2026
Roll Flutter from 13b2b91 to 01d37bc (74 revisions)

flutter/flutter@13b2b91...01d37bc

2026-01-07 stuartmorgan@google.com Replace Hybrid Composition wiki page with dev-facing content (flutter/flutter#180642)
2026-01-07 116356835+AbdeMohlbi@users.noreply.github.com Improve code quality in `FlutterActivityTest.java` (flutter/flutter#180585)
2026-01-07 iinozemtsev@google.com Roll Dart SDK to 3.11.0-296.1.beta (flutter/flutter#180633)
2026-01-07 vegorov@google.com Bump target Windows version to 10 (flutter/flutter#180624)
2026-01-07 goderbauer@google.com Run hook_user_defines and link_hook integration tests on CI (flutter/flutter#180622)
2026-01-07 iliyazelenkog@gmail.com Fix division by zero in RenderTable intrinsic size methods (flutter/flutter#178217)
2026-01-07 116356835+AbdeMohlbi@users.noreply.github.com Remove more  `requires 24` anotations (flutter/flutter#180116)
2026-01-07 engine-flutter-autoroll@skia.org Roll Skia from 3971dbb85d45 to c5359a4d720e (1 revision) (flutter/flutter#180631)
2026-01-07 huy@nevercode.io Fix TabBar.image does not render at initialIndex for the first time (flutter/flutter#179616)
2026-01-07 engine-flutter-autoroll@skia.org Roll Skia from 1e7ad625f6f7 to 3971dbb85d45 (3 revisions) (flutter/flutter#180627)
2026-01-07 goderbauer@google.com Unpin DDS (flutter/flutter#180571)
2026-01-07 bruno.leroux@gmail.com Fix DropdownMenuEntry.style not resolved when entry is highlighted (flutter/flutter#178873)
2026-01-07 116356835+AbdeMohlbi@users.noreply.github.com Remove unnecessary `@RequiresApi24` annotations from FlutterFragment methods (flutter/flutter#180117)
2026-01-07 engine-flutter-autoroll@skia.org Roll Skia from 7fc63228056b to 1e7ad625f6f7 (1 revision) (flutter/flutter#180616)
2026-01-07 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from QfR2ZFZ5kGTD3raO3... to dTvN_JVSCfGFRasvH... (flutter/flutter#180612)
2026-01-07 bkonyi@google.com [ Widget Preview ] Add support for `dart:ffi` imports (flutter/flutter#180586)
2026-01-07 engine-flutter-autoroll@skia.org Roll Skia from eec90000a899 to 7fc63228056b (2 revisions) (flutter/flutter#180608)
2026-01-07 ulisses.hen@hotmail.com Add --web-define flag for template variable injection in Flutter web builds (flutter/flutter#175805)
2026-01-07 codefu@google.com docs(engine): update rbe notes (flutter/flutter#180599)
2026-01-06 engine-flutter-autoroll@skia.org Roll Skia from b6249496c230 to eec90000a899 (3 revisions) (flutter/flutter#180602)
2026-01-06 146823759+brahim-guaali@users.noreply.github.com Forward proxy 404 responses to client (flutter/flutter#179858)
2026-01-06 engine-flutter-autoroll@skia.org Roll Dart SDK from 40a8c6188f7f to d2e3ce177270 (1 revision) (flutter/flutter#180598)
2026-01-06 104009581+Saqib198@users.noreply.github.com Restore CLI precedence for web headers and HTTPS over web_dev_config.yaml (flutter/flutter#179639)
2026-01-06 engine-flutter-autoroll@skia.org Roll Skia from f5d9da13d56d to b6249496c230 (1 revision) (flutter/flutter#180596)
2026-01-06 dkwingsmt@users.noreply.github.com Enable misc leak tracking (flutter/flutter#176992)
2026-01-06 dacoharkes@google.com [hooks] Don't require NDK for Android targets (flutter/flutter#180594)
2026-01-06 kjlubick@users.noreply.github.com [skia] Disable legacy non-const SkData APIs (flutter/flutter#179684)
2026-01-06 48625061+muradhossin@users.noreply.github.com Fix/ios share context menu (flutter/flutter#176199)
2026-01-06 engine-flutter-autoroll@skia.org Roll Skia from b970aeffa66f to f5d9da13d56d (5 revisions) (flutter/flutter#180588)
2026-01-06 goderbauer@google.com Manually bump dependencies (flutter/flutter#180509)
2026-01-06 engine-flutter-autoroll@skia.org Roll Dart SDK from 8150be8a0e48 to 40a8c6188f7f (2 revisions) (flutter/flutter#180582)
2026-01-06 engine-flutter-autoroll@skia.org Roll Packages from 12eb764 to d3f860d (2 revisions) (flutter/flutter#180581)
2026-01-06 engine-flutter-autoroll@skia.org Roll Fuchsia Test Scripts from MHF-UAfO6sVKqSEYk... to nR2ESa1Gd8yPcWo06... (flutter/flutter#180578)
2026-01-06 sokolovskyi.konstantin@gmail.com Add tooltip support to PlatformMenuItem and PlatformMenu. (flutter/flutter#180069)
2026-01-06 bruno.leroux@gmail.com Add DropdownMenuFormField.errorBuilder (flutter/flutter#179345)
2026-01-06 engine-flutter-autoroll@skia.org Roll Skia from 1845397e11ba to b970aeffa66f (2 revisions) (flutter/flutter#180566)
2026-01-06 oss@simonbinder.eu Don't embed unreferenced assets (flutter/flutter#179251)
2026-01-06 116356835+AbdeMohlbi@users.noreply.github.com Improve documentation about ValueNotifier's behavior (flutter/flutter#179870)
2026-01-06 engine-flutter-autoroll@skia.org Roll Skia from 904ba00331ca to 1845397e11ba (5 revisions) (flutter/flutter#180558)
2026-01-06 engine-flutter-autoroll@skia.org Roll Dart SDK from 2fb9ad834c4d to 8150be8a0e48 (1 revision) (flutter/flutter#180557)
2026-01-06 engine-flutter-autoroll@skia.org Roll Skia from 98c042dde68c to 904ba00331ca (3 revisions) (flutter/flutter#180550)
2026-01-06 engine-flutter-autoroll@skia.org Roll Dart SDK from ba9f7f790966 to 2fb9ad834c4d (2 revisions) (flutter/flutter#180548)
2026-01-06 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from ubBGcRaAKWKihQ4ac... to QfR2ZFZ5kGTD3raO3... (flutter/flutter#180547)
2026-01-06 ahmedsameha1@gmail.com Make sure that a DraggableScrollableSheet doesn't crash in 0x0 enviro… (flutter/flutter#180433)
2026-01-06 ahmedsameha1@gmail.com Make sure that a ColorFiltered doesn't crash 0x0 environment (flutter/flutter#180307)
2026-01-06 ahmedsameha1@gmail.com Make sure that a FadeInImage doesn't crash in 0x0 environment (flutter/flutter#180495)
...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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 3.38 run command line arguments not working for web development config

4 participants