From 2f5596503192a28240327eb409b0e4e82074e436 Mon Sep 17 00:00:00 2001 From: eYey <64911790+eYey343@users.noreply.github.com> Date: Wed, 2 Sep 2026 10:00:00 +0200 Subject: [PATCH] [client, android] Type the split tunnelling mode instead of storing a string (#7387) gomobile carries only basic types, so the typed constants stay unexported and the exported SplitTunnelMode* ints are what the Java side gets. --- client/android/split_tunnel.go | 47 +++++++++++++------ client/android/split_tunnel_test.go | 70 +++++++++++++++++++++++------ 2 files changed, 89 insertions(+), 28 deletions(-) diff --git a/client/android/split_tunnel.go b/client/android/split_tunnel.go index 59ac539f9..60e1ceeb8 100644 --- a/client/android/split_tunnel.go +++ b/client/android/split_tunnel.go @@ -1,17 +1,29 @@ package android -// Split tunnelling modes, stored as strings so an unknown value written by a -// newer build degrades to "off" rather than to some other mode's behaviour. +// SplitTunnelMode is which of the two selections, if either, the tunnel applies. +// Its values land in the profile's stored preferences, so the constants below +// are append-only and must never be reordered. +type SplitTunnelMode int + const ( - SplitTunnelModeOff = "off" - SplitTunnelModeExclude = "exclude" - SplitTunnelModeInclude = "include" + modeOff SplitTunnelMode = iota + modeExclude + modeInclude +) + +// The same modes as basic ints. gomobile drops a constant whose type is not a +// basic one, so these are what reaches the generated Java bindings, and they +// keep the Android side tied to the values above instead of repeating 0, 1, 2. +const ( + SplitTunnelModeOff = int(modeOff) + SplitTunnelModeExclude = int(modeExclude) + SplitTunnelModeInclude = int(modeInclude) ) type splitTunnelSection struct { - Mode string `json:"mode"` - Excluded []string `json:"excluded"` - Included []string `json:"included"` + Mode SplitTunnelMode `json:"mode"` + Excluded []string `json:"excluded"` + Included []string `json:"included"` } // PackageList wraps []string for gomobile compatibility. @@ -49,8 +61,12 @@ func (l *PackageList) Get(i int) string { // carries. The two selections are kept apart because the platform applies one // or the other and never both, and so that switching mode does not throw away // the picks made in the other one. +// +// Mode is an int rather than a SplitTunnelMode because gomobile carries only +// basic types across the binding. It holds one of the SplitTunnelMode* +// constants. type SplitTunnelSettings struct { - Mode string + Mode int Excluded *PackageList Included *PackageList } @@ -73,18 +89,21 @@ func packagesOf(list *PackageList) []string { return out } -func normalizeSplitTunnelMode(mode string) string { +// normalizeSplitTunnelMode maps anything outside the known set to off, so a mode +// written by a newer build degrades to carrying every application rather than to +// some other mode's behaviour. +func normalizeSplitTunnelMode(mode SplitTunnelMode) SplitTunnelMode { switch mode { - case SplitTunnelModeExclude, SplitTunnelModeInclude: + case modeExclude, modeInclude: return mode default: - return SplitTunnelModeOff + return modeOff } } func settingsFromSection(section splitTunnelSection) *SplitTunnelSettings { out := NewSplitTunnelSettings() - out.Mode = normalizeSplitTunnelMode(section.Mode) + out.Mode = int(normalizeSplitTunnelMode(section.Mode)) for _, pkg := range section.Excluded { out.Excluded.Add(pkg) } @@ -99,7 +118,7 @@ func sectionFromSettings(settings *SplitTunnelSettings) splitTunnelSection { settings = NewSplitTunnelSettings() } return splitTunnelSection{ - Mode: normalizeSplitTunnelMode(settings.Mode), + Mode: normalizeSplitTunnelMode(SplitTunnelMode(settings.Mode)), Excluded: packagesOf(settings.Excluded), Included: packagesOf(settings.Included), } diff --git a/client/android/split_tunnel_test.go b/client/android/split_tunnel_test.go index b8465e8ef..bf6e0267e 100644 --- a/client/android/split_tunnel_test.go +++ b/client/android/split_tunnel_test.go @@ -1,6 +1,7 @@ package android import ( + "encoding/json" "reflect" "testing" ) @@ -8,34 +9,48 @@ import ( func TestNormalizeSplitTunnelMode(t *testing.T) { tests := []struct { name string - mode string - want string + mode SplitTunnelMode + want SplitTunnelMode }{ - {name: "exclude is kept", mode: SplitTunnelModeExclude, want: SplitTunnelModeExclude}, - {name: "include is kept", mode: SplitTunnelModeInclude, want: SplitTunnelModeInclude}, - {name: "off is kept", mode: SplitTunnelModeOff, want: SplitTunnelModeOff}, - {name: "empty falls back to off", mode: "", want: SplitTunnelModeOff}, - {name: "a mode from a newer build falls back to off", mode: "only-work-apps", want: SplitTunnelModeOff}, + {name: "exclude is kept", mode: modeExclude, want: modeExclude}, + {name: "include is kept", mode: modeInclude, want: modeInclude}, + {name: "off is kept", mode: modeOff, want: modeOff}, + {name: "a mode from a newer build falls back to off", mode: SplitTunnelMode(7), want: modeOff}, + {name: "a negative mode falls back to off", mode: SplitTunnelMode(-1), want: modeOff}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if got := normalizeSplitTunnelMode(tt.mode); got != tt.want { - t.Errorf("normalizeSplitTunnelMode(%q) = %q, want %q", tt.mode, got, tt.want) + t.Errorf("normalizeSplitTunnelMode(%d) = %d, want %d", tt.mode, got, tt.want) } }) } } +// The constants the Android side reads must stay the values the store writes: +// gomobile carries the ints below, not the typed constants they mirror. +func TestSplitTunnelModeConstantsMirrorTheTypedOnes(t *testing.T) { + if SplitTunnelModeOff != int(modeOff) { + t.Errorf("off = %d, want %d", SplitTunnelModeOff, modeOff) + } + if SplitTunnelModeExclude != int(modeExclude) { + t.Errorf("exclude = %d, want %d", SplitTunnelModeExclude, modeExclude) + } + if SplitTunnelModeInclude != int(modeInclude) { + t.Errorf("include = %d, want %d", SplitTunnelModeInclude, modeInclude) + } +} + func TestSettingsFromSection(t *testing.T) { got := settingsFromSection(splitTunnelSection{ - Mode: SplitTunnelModeExclude, + Mode: modeExclude, Excluded: []string{"com.example.a", "com.example.b"}, Included: []string{"com.example.c"}, }) if got.Mode != SplitTunnelModeExclude { - t.Errorf("mode = %q, want %q", got.Mode, SplitTunnelModeExclude) + t.Errorf("mode = %d, want %d", got.Mode, SplitTunnelModeExclude) } if got.Excluded.Size() != 2 || got.Excluded.Get(0) != "com.example.a" { t.Errorf("excluded = %v, want the two stored packages", packagesOf(got.Excluded)) @@ -52,7 +67,7 @@ func TestSettingsFromEmptySectionCarriesEverything(t *testing.T) { got := settingsFromSection(splitTunnelSection{}) if got.Mode != SplitTunnelModeOff { - t.Errorf("mode = %q, want %q", got.Mode, SplitTunnelModeOff) + t.Errorf("mode = %d, want %d", got.Mode, SplitTunnelModeOff) } if got.Excluded == nil || got.Included == nil { t.Fatal("both selections must be usable lists, not nil") @@ -62,6 +77,23 @@ func TestSettingsFromEmptySectionCarriesEverything(t *testing.T) { } } +// The section is what the profile's preference file holds, so the mode has to +// survive a JSON round trip as the number the constants name. +func TestSectionEncodesTheModeAsItsNumber(t *testing.T) { + raw, err := json.Marshal(sectionFromSettings(&SplitTunnelSettings{Mode: SplitTunnelModeInclude})) + if err != nil { + t.Fatalf("marshal section: %v", err) + } + + var back splitTunnelSection + if err := json.Unmarshal(raw, &back); err != nil { + t.Fatalf("unmarshal section: %v", err) + } + if back.Mode != modeInclude { + t.Errorf("mode = %d, want %d, from %s", back.Mode, modeInclude, raw) + } +} + func TestSectionFromSettingsRoundTrip(t *testing.T) { settings := NewSplitTunnelSettings() settings.Mode = SplitTunnelModeInclude @@ -72,7 +104,7 @@ func TestSectionFromSettingsRoundTrip(t *testing.T) { back := settingsFromSection(section) if back.Mode != SplitTunnelModeInclude { - t.Errorf("mode = %q, want %q", back.Mode, SplitTunnelModeInclude) + t.Errorf("mode = %d, want %d", back.Mode, SplitTunnelModeInclude) } if !reflect.DeepEqual(packagesOf(back.Included), []string{"com.example.a"}) { t.Errorf("included = %v, want [com.example.a]", packagesOf(back.Included)) @@ -84,11 +116,21 @@ func TestSectionFromSettingsRoundTrip(t *testing.T) { } } +// A mode the Java side never sets, such as one left by a newer build, must not +// reach the stored section either. +func TestSectionFromSettingsNormalizesAnUnknownMode(t *testing.T) { + section := sectionFromSettings(&SplitTunnelSettings{Mode: 7}) + + if section.Mode != modeOff { + t.Errorf("mode = %d, want %d", section.Mode, modeOff) + } +} + func TestSectionFromNilSettings(t *testing.T) { section := sectionFromSettings(nil) - if section.Mode != SplitTunnelModeOff { - t.Errorf("mode = %q, want %q", section.Mode, SplitTunnelModeOff) + if section.Mode != modeOff { + t.Errorf("mode = %d, want %d", section.Mode, modeOff) } if len(section.Excluded) != 0 || len(section.Included) != 0 { t.Errorf("selections = %v/%v, want both empty", section.Excluded, section.Included)