From 1eb08826defda73096a759d634b3e4ea7a6a1092 Mon Sep 17 00:00:00 2001 From: Peter Jhong Date: Thu, 9 Jul 2026 15:38:41 +0800 Subject: [PATCH] fix(internet): auto-fill and clamp MTU on connection type switch (#1083) When switching WAN connection type, the MTU field was left in an invalid or empty state: changing to PPPoE kept 1500 (over the 1492 limit), and leaving Bridge mode left the field blank (mtu = 0 = auto). Introduce a single MTU range source of truth on UspWanConnectionType (mtuMin/mtuMax/clampMtu) and normalize MTU in updateConnectionType: keep the current value when it fits the new type's range, otherwise fall back to the type max. Bridge continues to use auto (mtu = 0). The view now references the enum getters instead of duplicating the range logic. Covers issue expectations: over-limit values are clamped (1500 -> 1492), empty/auto values are auto-filled with the type max, and an already-valid value is preserved across the switch. Co-Authored-By: Claude Opus 4.8 --- .../models/usp_wan_connection_type.dart | 18 ++++++ .../usp_internet_settings_notifier.dart | 9 ++- .../views/sections/usp_optional_section.dart | 16 ++--- .../models/usp_wan_connection_type_test.dart | 58 ++++++++++++++++++ .../usp_internet_settings_notifier_test.dart | 61 +++++++++++++++++++ 5 files changed, 149 insertions(+), 13 deletions(-) diff --git a/lib/page/internet_settings/models/usp_wan_connection_type.dart b/lib/page/internet_settings/models/usp_wan_connection_type.dart index 235828312..e96bf3f12 100644 --- a/lib/page/internet_settings/models/usp_wan_connection_type.dart +++ b/lib/page/internet_settings/models/usp_wan_connection_type.dart @@ -66,6 +66,24 @@ enum UspWanConnectionType { bridge => '', // issue #14: empty string = proto=none }; + /// Minimum MTU accepted for this connection type (protocol-independent). + int get mtuMin => 576; + + /// Maximum MTU accepted for this connection type. Varies by protocol + /// overhead: PPPoE reserves 8 bytes (PPP header), PPTP/L2TP reserve 40 bytes + /// (tunnel overhead); the rest use the Ethernet standard 1500. + int get mtuMax => switch (this) { + pppoe => 1492, // 1500 - 8 (PPP header) + pptp || l2tp => 1460, // tunnel overhead + _ => 1500, // Ethernet standard (DHCP, Static, Bridge) + }; + + /// Clamp [mtu] into this type's valid range: values already within + /// `[mtuMin, mtuMax]` are kept; anything outside (too low or too high) falls + /// back to [mtuMax]. Used when switching connection types so a previously + /// valid MTU is preserved when it still fits, and reset to the max otherwise. + int clampMtu(int mtu) => (mtu >= mtuMin && mtu <= mtuMax) ? mtu : mtuMax; + /// Whether this type uses a PPP.Interface (credentials, LowerLayers). bool get isPppBased => this == pppoe || this == pptp || this == l2tp; diff --git a/lib/page/internet_settings/providers/usp_internet_settings_notifier.dart b/lib/page/internet_settings/providers/usp_internet_settings_notifier.dart index 751b178cb..ef16135b1 100644 --- a/lib/page/internet_settings/providers/usp_internet_settings_notifier.dart +++ b/lib/page/internet_settings/providers/usp_internet_settings_notifier.dart @@ -268,9 +268,16 @@ class UspInternetSettingsNotifier final current = state.settings.current; var form = current.form.copyWith(connectionType: type); - // Reset type-specific fields when switching away + // Normalize MTU for the new type (issue #1083). Bridge always uses auto + // (mtu = 0). For every other type, keep the current MTU when it still fits + // the type's range, otherwise fall back to the type's max — this both + // clamps an over-limit value (e.g. 1500 → 1492 on PPPoE) and auto-fills a + // sensible default when the previous value was auto/empty (e.g. leaving + // bridge mode). if (type == UspWanConnectionType.bridge) { form = form.copyWith(mtu: 0); + } else { + form = form.copyWith(mtu: type.clampMtu(form.mtu)); } state = state.copyWith( diff --git a/lib/page/internet_settings/views/sections/usp_optional_section.dart b/lib/page/internet_settings/views/sections/usp_optional_section.dart index 1937906cb..d47a055fb 100644 --- a/lib/page/internet_settings/views/sections/usp_optional_section.dart +++ b/lib/page/internet_settings/views/sections/usp_optional_section.dart @@ -53,18 +53,10 @@ class _UspOptionalSectionState extends ConsumerState { super.dispose(); } - int get _mtuMin => 576; - int get _mtuMax { - final type = widget.state.edited.connectionType; - // MTU max varies by connection type due to protocol overhead - return switch (type) { - UspWanConnectionType.pppoe => 1492, // 1500 - 8 (PPP header) - UspWanConnectionType.pptp || - UspWanConnectionType.l2tp => - 1460, // tunnel overhead - _ => 1500, // Ethernet standard (DHCP, Static, Bridge) - }; - } + // MTU range is owned by [UspWanConnectionType] (single source of truth, + // shared with the notifier's clamp on type switch). + int get _mtuMin => widget.state.edited.connectionType.mtuMin; + int get _mtuMax => widget.state.edited.connectionType.mtuMax; String? _getMtuError(BuildContext context, int mtu) { if (mtu < _mtuMin) return loc(context).mtuMinError(_mtuMin); diff --git a/test/page/internet_settings/models/usp_wan_connection_type_test.dart b/test/page/internet_settings/models/usp_wan_connection_type_test.dart index cee122f6b..b91f75217 100644 --- a/test/page/internet_settings/models/usp_wan_connection_type_test.dart +++ b/test/page/internet_settings/models/usp_wan_connection_type_test.dart @@ -172,5 +172,63 @@ void main() { expect(UspWanConnectionType.dhcp.pppLowerLayers, isNull); }); }); + + group('mtuMax', () { + test('pppoe reserves 8 bytes (1492)', () { + expect(UspWanConnectionType.pppoe.mtuMax, 1492); + }); + + test('pptp reserves tunnel overhead (1460)', () { + expect(UspWanConnectionType.pptp.mtuMax, 1460); + }); + + test('l2tp reserves tunnel overhead (1460)', () { + expect(UspWanConnectionType.l2tp.mtuMax, 1460); + }); + + test('dhcp uses Ethernet standard (1500)', () { + expect(UspWanConnectionType.dhcp.mtuMax, 1500); + }); + + test('staticIp uses Ethernet standard (1500)', () { + expect(UspWanConnectionType.staticIp.mtuMax, 1500); + }); + + test('bridge uses Ethernet standard (1500)', () { + expect(UspWanConnectionType.bridge.mtuMax, 1500); + }); + }); + + group('mtuMin', () { + test('is 576 for every type', () { + for (final type in UspWanConnectionType.values) { + expect(type.mtuMin, 576, reason: '${type.name} mtuMin'); + } + }); + }); + + group('clampMtu', () { + test('keeps an in-range value unchanged', () { + expect(UspWanConnectionType.pppoe.clampMtu(789), 789); + expect(UspWanConnectionType.dhcp.clampMtu(1500), 1500); + expect(UspWanConnectionType.pptp.clampMtu(1460), 1460); + }); + + test('resets an over-max value to the type max', () { + expect(UspWanConnectionType.pppoe.clampMtu(1500), 1492); + expect(UspWanConnectionType.pptp.clampMtu(1500), 1460); + }); + + test('resets a below-min value to the type max', () { + // 0 (auto, e.g. after leaving bridge) and any sub-576 value fall back + // to the max rather than an invalid low value. + expect(UspWanConnectionType.dhcp.clampMtu(0), 1500); + expect(UspWanConnectionType.pppoe.clampMtu(100), 1492); + }); + + test('keeps the min boundary value', () { + expect(UspWanConnectionType.dhcp.clampMtu(576), 576); + }); + }); }); } diff --git a/test/page/internet_settings/providers/usp_internet_settings_notifier_test.dart b/test/page/internet_settings/providers/usp_internet_settings_notifier_test.dart index 0bd89babe..301afeacd 100644 --- a/test/page/internet_settings/providers/usp_internet_settings_notifier_test.dart +++ b/test/page/internet_settings/providers/usp_internet_settings_notifier_test.dart @@ -181,6 +181,67 @@ void main() { container.dispose(); }); + test('updateConnectionType to PPPoE clamps over-limit MTU to 1492', + () async { + when(() => mockService.fetchSettings()) + .thenAnswer((_) async => testFetchResult); + final container = createContainer(); + await Future.delayed(Duration.zero); + + // Initial MTU is 1500 (DHCP), which exceeds PPPoE's 1492 max. + container + .read(uspInternetSettingsProvider.notifier) + .updateConnectionType(UspWanConnectionType.pppoe); + + final form = + container.read(uspInternetSettingsProvider).settings.current.form; + expect(form.connectionType, UspWanConnectionType.pppoe); + expect(form.mtu, 1492); + container.dispose(); + }); + + test('updateConnectionType from bridge auto-fills MTU with type max', + () async { + when(() => mockService.fetchSettings()) + .thenAnswer((_) async => testFetchResult); + final container = createContainer(); + await Future.delayed(Duration.zero); + + final notifier = container.read(uspInternetSettingsProvider.notifier); + // Bridge sets mtu = 0 (auto)... + notifier.updateConnectionType(UspWanConnectionType.bridge); + expect( + container.read(uspInternetSettingsProvider).settings.current.form.mtu, + 0); + + // ...switching back to DHCP must not leave MTU empty; 0 is out of range + // so it falls back to the type max (1500). + notifier.updateConnectionType(UspWanConnectionType.dhcp); + final form = + container.read(uspInternetSettingsProvider).settings.current.form; + expect(form.connectionType, UspWanConnectionType.dhcp); + expect(form.mtu, 1500); + container.dispose(); + }); + + test('updateConnectionType keeps an in-range MTU unchanged', () async { + when(() => mockService.fetchSettings()) + .thenAnswer((_) async => testFetchResult); + final container = createContainer(); + await Future.delayed(Duration.zero); + + final notifier = container.read(uspInternetSettingsProvider.notifier); + // 789 is valid for both DHCP and PPPoE (issue #1083 "last changed value"). + notifier.updateField((f) => f.copyWith(mtu: 789)); + notifier.updateConnectionType(UspWanConnectionType.pppoe); + + final form = + container.read(uspInternetSettingsProvider).settings.current.form; + expect(form.connectionType, UspWanConnectionType.pppoe); + expect(form.mtu, 789); + container.dispose(); + }); + test('isDirty after field update, clean after revert', () async { when(() => mockService.fetchSettings()) .thenAnswer((_) async => testFetchResult);