From 017e4f00facdde323db4e3667bedd7d5a7c2d456 Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Wed, 12 Nov 2025 20:20:27 -0800 Subject: [PATCH] [tool] clean up https cert configuration handling (#178139) It's only valid if both the cert and the cert key are set. Makes the code a lot cleaner, too. --- packages/flutter_tools/lib/src/device.dart | 8 +-- .../lib/src/isolated/web_asset_server.dart | 11 ++-- .../lib/src/web/devfs_config.dart | 45 +++++++++---- .../general.shard/web/https_config_test.dart | 63 +++++++++++++++++++ 4 files changed, 103 insertions(+), 24 deletions(-) create mode 100644 packages/flutter_tools/test/general.shard/web/https_config_test.dart diff --git a/packages/flutter_tools/lib/src/device.dart b/packages/flutter_tools/lib/src/device.dart index 17da79eb74d..cce79aeb867 100644 --- a/packages/flutter_tools/lib/src/device.dart +++ b/packages/flutter_tools/lib/src/device.dart @@ -1374,13 +1374,7 @@ class DebuggingOptions { webDevServerConfig: WebDevServerConfig( port: json['port'] is int ? json['port']! as int : 8080, host: json['hostname'] is String ? json['hostname']! as String : 'localhost', - - https: (json['tlsCertPath'] != null || json['tlsCertKeyPath'] != null) - ? HttpsConfig( - certPath: json['tlsCertPath'] as String?, - certKeyPath: json['tlsCertKeyPath'] as String?, - ) - : null, + https: HttpsConfig.parse(json['tlsCertPath'], json['tlsCertKeyPath']), headers: (json['webHeaders']! as Map).cast(), ), ); diff --git a/packages/flutter_tools/lib/src/isolated/web_asset_server.dart b/packages/flutter_tools/lib/src/isolated/web_asset_server.dart index 971eca0acc6..cc491784b49 100644 --- a/packages/flutter_tools/lib/src/isolated/web_asset_server.dart +++ b/packages/flutter_tools/lib/src/isolated/web_asset_server.dart @@ -197,8 +197,7 @@ class WebAssetServer implements AssetReader { }) async { final String hostname = webDevServerConfig.host; final int port = webDevServerConfig.port; - final String? tlsCertPath = webDevServerConfig.https?.certPath; - final String? tlsCertKeyPath = webDevServerConfig.https?.certKeyPath; + final HttpsConfig? httpsConfig = webDevServerConfig.https; final Map extraHeaders = webDevServerConfig.headers; final List proxy = webDevServerConfig.proxy; @@ -217,10 +216,10 @@ class WebAssetServer implements AssetReader { const kMaxRetries = 4; for (var i = 0; i <= kMaxRetries; i++) { try { - if (tlsCertPath != null && tlsCertKeyPath != null) { + if (httpsConfig != null) { final serverContext = SecurityContext() - ..useCertificateChain(tlsCertPath) - ..usePrivateKey(tlsCertKeyPath); + ..useCertificateChain(httpsConfig.certPath) + ..usePrivateKey(httpsConfig.certKeyPath); httpServer = await HttpServer.bindSecure(address, port, serverContext); } else { httpServer = await HttpServer.bind(address, port); @@ -260,7 +259,7 @@ class WebAssetServer implements AssetReader { final int selectedPort = server.selectedPort; final cleanHost = hostname == webDevAnyHostDefault ? 'localhost' : hostname; - final scheme = tlsCertPath != null && tlsCertKeyPath != null ? 'https' : 'http'; + final scheme = httpsConfig != null ? 'https' : 'http'; server._baseUri = Uri( scheme: scheme, host: cleanHost, diff --git a/packages/flutter_tools/lib/src/web/devfs_config.dart b/packages/flutter_tools/lib/src/web/devfs_config.dart index abcfad477ab..7b4de5b2af4 100644 --- a/packages/flutter_tools/lib/src/web/devfs_config.dart +++ b/packages/flutter_tools/lib/src/web/devfs_config.dart @@ -189,28 +189,51 @@ WebDevServerConfig: /// Represents the [HttpsConfig] for the web dev server @immutable class HttpsConfig { - const HttpsConfig({this.certPath, this.certKeyPath}); - + const HttpsConfig({required this.certPath, required this.certKeyPath}); factory HttpsConfig.fromYaml(YamlMap yaml) { final String? certPath = _validateType(value: yaml[_kCertPath], fieldName: _kCertPath); + if (certPath == null) { + throw ArgumentError.value(yaml, 'yaml', '"$_kCertPath" must be defined'); + } + final String? certKeyPath = _validateType( value: yaml[_kCertKeyPath], fieldName: _kCertKeyPath, ); + if (certKeyPath == null) { + throw ArgumentError.value(yaml, 'yaml', '"$_kCertKeyPath" must be defined'); + } return HttpsConfig(certPath: certPath, certKeyPath: certKeyPath); } - /// Creates a copy of this [HttpsConfig] with optional overrides. - HttpsConfig copyWith({String? certPath, String? certKeyPath}) { - return HttpsConfig( - certPath: certPath ?? this.certPath, - certKeyPath: certKeyPath ?? this.certKeyPath, - ); - } + /// If [tlsCertPath] and [tlsCertKeyPath] are both [String] return an instance. + /// + /// If they are both `null`, return `null`. + /// + /// Otherwise, throw an [Exception]. + static HttpsConfig? parse(Object? tlsCertPath, Object? tlsCertKeyPath) => + switch ((tlsCertPath, tlsCertKeyPath)) { + (final String certPath, final String certKeyPath) => HttpsConfig( + certPath: certPath, + certKeyPath: certKeyPath, + ), + (null, null) => null, + (final Object? certPath, final Object? certKeyPath) => throw ArgumentError( + 'When providing TLS certificates, both `tlsCertPath` and ' + '`tlsCertKeyPath` must be provided as strings. ' + 'Found: tlsCertPath: ${certPath ?? 'null'}, tlsCertKeyPath: ${certKeyPath ?? 'null'}', + ), + }; - final String? certPath; - final String? certKeyPath; + /// Creates a copy of this [HttpsConfig] with optional overrides. + HttpsConfig copyWith({String? certPath, String? certKeyPath}) => HttpsConfig( + certPath: certPath ?? this.certPath, + certKeyPath: certKeyPath ?? this.certKeyPath, + ); + + final String certPath; + final String certKeyPath; @override String toString() { diff --git a/packages/flutter_tools/test/general.shard/web/https_config_test.dart b/packages/flutter_tools/test/general.shard/web/https_config_test.dart new file mode 100644 index 00000000000..2c2dfb7a9fa --- /dev/null +++ b/packages/flutter_tools/test/general.shard/web/https_config_test.dart @@ -0,0 +1,63 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:flutter_tools/src/web/devfs_config.dart'; +import 'package:test/test.dart'; +import 'package:yaml/yaml.dart'; + +void main() { + group('parse', () { + test('returns HttpsConfig when both paths are provided', () { + final HttpsConfig result = HttpsConfig.parse('/path/to/cert', '/path/to/key')!; + expect(result.certPath, '/path/to/cert'); + expect(result.certKeyPath, '/path/to/key'); + }); + + test('returns null when both paths are null', () { + final HttpsConfig? result = HttpsConfig.parse(null, null); + expect(result, isNull); + }); + + test('throws ArgumentError when only one field is provided', () { + expect(() => HttpsConfig.parse('/path/to/cert', null), throwsArgumentError); + expect(() => HttpsConfig.parse(null, '/path/to/key'), throwsArgumentError); + }); + + test('throws ArgumentError when the field is the wrong type', () { + expect(() => HttpsConfig.parse(1, '/path/to/key'), throwsArgumentError); + expect(() => HttpsConfig.parse('/path/to/cert', 1), throwsArgumentError); + }); + }); + + group('fromYaml', () { + test('fromYaml throws an ArgumentError if cert-path is not defined', () { + expect( + () => HttpsConfig.fromYaml(loadYaml('cert-key-path: /path/to/key') as YamlMap), + throwsArgumentError, + ); + }); + + test('fromYaml throws an ArgumentError if cert-key-path is not defined', () { + expect( + () => HttpsConfig.fromYaml(loadYaml('cert-path: /path/to/cert') as YamlMap), + throwsArgumentError, + ); + }); + + test( + 'fromYaml creates an HttpsConfig object when both certificate and key paths are provided', + () { + final https = HttpsConfig.fromYaml( + loadYaml(''' +cert-path: /path/to/cert +cert-key-path: /path/to/key''') + as YamlMap, + ); + + expect(https.certPath, '/path/to/cert'); + expect(https.certKeyPath, '/path/to/key'); + }, + ); + }); +}