Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions lib/page/internet_settings/models/usp_wan_connection_type.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,18 +53,10 @@ class _UspOptionalSectionState extends ConsumerState<UspOptionalSection> {
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading