From b32821a5868df473bb24cf954448a3ace1c59c3c Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Tue, 26 Nov 2019 17:07:25 +0100 Subject: [PATCH] lib/config, lib/connections: Remove ListenAddresses hack (#6188) --- lib/config/config.go | 10 ++--- lib/config/config_test.go | 44 +++++++++---------- lib/config/migrations.go | 26 +++++------ lib/config/optionsconfiguration.go | 69 +++++++++++++++++++++++++++--- lib/config/wrapper.go | 60 -------------------------- lib/connections/service.go | 2 +- lib/stun/stun.go | 2 +- lib/syncthing/syncthing.go | 2 +- lib/ur/usage_report.go | 4 +- 9 files changed, 107 insertions(+), 112 deletions(-) diff --git a/lib/config/config.go b/lib/config/config.go index 92faa393..85dadd91 100644 --- a/lib/config/config.go +++ b/lib/config/config.go @@ -130,9 +130,9 @@ func NewWithFreePorts(myID protocol.DeviceID) (Configuration, error) { return Configuration{}, errors.Wrap(err, "get free port (BEP)") } if port == DefaultTCPPort { - cfg.Options.ListenAddresses = []string{"default"} + cfg.Options.RawListenAddresses = []string{"default"} } else { - cfg.Options.ListenAddresses = []string{ + cfg.Options.RawListenAddresses = []string{ fmt.Sprintf("tcp://%s", net.JoinHostPort("0.0.0.0", strconv.Itoa(port))), "dynamic+https://relays.syncthing.net/endpoint", } @@ -305,8 +305,8 @@ func (cfg *Configuration) clean() error { existingFolders[folder.ID] = folder } - cfg.Options.ListenAddresses = util.UniqueTrimmedStrings(cfg.Options.ListenAddresses) - cfg.Options.GlobalAnnServers = util.UniqueTrimmedStrings(cfg.Options.GlobalAnnServers) + cfg.Options.RawListenAddresses = util.UniqueTrimmedStrings(cfg.Options.RawListenAddresses) + cfg.Options.RawGlobalAnnServers = util.UniqueTrimmedStrings(cfg.Options.RawGlobalAnnServers) if cfg.Version > 0 && cfg.Version < OldestHandledVersion { l.Warnf("Configuration version %d is deprecated. Attempting best effort conversion, but please verify manually.", cfg.Version) @@ -396,7 +396,7 @@ nextPendingDevice: // Deprecated protocols are removed from the list of listeners and // device addresses. So far just kcp*. for _, prefix := range []string{"kcp"} { - cfg.Options.ListenAddresses = filterURLSchemePrefix(cfg.Options.ListenAddresses, prefix) + cfg.Options.RawListenAddresses = filterURLSchemePrefix(cfg.Options.RawListenAddresses, prefix) for i := range cfg.Devices { dev := &cfg.Devices[i] dev.Addresses = filterURLSchemePrefix(dev.Addresses, prefix) diff --git a/lib/config/config_test.go b/lib/config/config_test.go index 82cf57b9..1a424d73 100644 --- a/lib/config/config_test.go +++ b/lib/config/config_test.go @@ -37,8 +37,8 @@ func init() { func TestDefaultValues(t *testing.T) { expected := OptionsConfiguration{ - ListenAddresses: []string{"default"}, - GlobalAnnServers: []string{"default"}, + RawListenAddresses: []string{"default"}, + RawGlobalAnnServers: []string{"default"}, GlobalAnnEnabled: true, LocalAnnEnabled: true, LocalAnnPort: 21027, @@ -74,7 +74,7 @@ func TestDefaultValues(t *testing.T) { CREnabled: true, StunKeepaliveStartS: 180, StunKeepaliveMinS: 20, - StunServers: []string{"default"}, + RawStunServers: []string{"default"}, } cfg := New(device1) @@ -175,16 +175,16 @@ func TestNoListenAddresses(t *testing.T) { } expected := []string{""} - actual := cfg.Options().ListenAddresses + actual := cfg.Options().RawListenAddresses if diff, equal := messagediff.PrettyDiff(expected, actual); !equal { - t.Errorf("Unexpected ListenAddresses. Diff:\n%s", diff) + t.Errorf("Unexpected RawListenAddresses. Diff:\n%s", diff) } } func TestOverriddenValues(t *testing.T) { expected := OptionsConfiguration{ - ListenAddresses: []string{"tcp://:23000"}, - GlobalAnnServers: []string{"udp4://syncthing.nym.se:22026"}, + RawListenAddresses: []string{"tcp://:23000"}, + RawGlobalAnnServers: []string{"udp4://syncthing.nym.se:22026"}, GlobalAnnEnabled: false, LocalAnnEnabled: false, LocalAnnPort: 42123, @@ -222,7 +222,7 @@ func TestOverriddenValues(t *testing.T) { CREnabled: false, StunKeepaliveStartS: 9000, StunKeepaliveMinS: 900, - StunServers: []string{"foo"}, + RawStunServers: []string{"foo"}, } os.Unsetenv("STNOUPGRADE") @@ -424,20 +424,20 @@ func TestIssue1750(t *testing.T) { t.Fatal(err) } - if cfg.Options().ListenAddresses[0] != "tcp://:23000" { - t.Errorf("%q != %q", cfg.Options().ListenAddresses[0], "tcp://:23000") + if cfg.Options().RawListenAddresses[0] != "tcp://:23000" { + t.Errorf("%q != %q", cfg.Options().RawListenAddresses[0], "tcp://:23000") } - if cfg.Options().ListenAddresses[1] != "tcp://:23001" { - t.Errorf("%q != %q", cfg.Options().ListenAddresses[1], "tcp://:23001") + if cfg.Options().RawListenAddresses[1] != "tcp://:23001" { + t.Errorf("%q != %q", cfg.Options().RawListenAddresses[1], "tcp://:23001") } - if cfg.Options().GlobalAnnServers[0] != "udp4://syncthing.nym.se:22026" { - t.Errorf("%q != %q", cfg.Options().GlobalAnnServers[0], "udp4://syncthing.nym.se:22026") + if cfg.Options().RawGlobalAnnServers[0] != "udp4://syncthing.nym.se:22026" { + t.Errorf("%q != %q", cfg.Options().RawGlobalAnnServers[0], "udp4://syncthing.nym.se:22026") } - if cfg.Options().GlobalAnnServers[1] != "udp4://syncthing.nym.se:22027" { - t.Errorf("%q != %q", cfg.Options().GlobalAnnServers[1], "udp4://syncthing.nym.se:22027") + if cfg.Options().RawGlobalAnnServers[1] != "udp4://syncthing.nym.se:22027" { + t.Errorf("%q != %q", cfg.Options().RawGlobalAnnServers[1], "udp4://syncthing.nym.se:22027") } } @@ -553,13 +553,13 @@ func TestNewSaveLoad(t *testing.T) { func TestPrepare(t *testing.T) { var cfg Configuration - if cfg.Folders != nil || cfg.Devices != nil || cfg.Options.ListenAddresses != nil { + if cfg.Folders != nil || cfg.Devices != nil || cfg.Options.RawListenAddresses != nil { t.Error("Expected nil") } cfg.prepare(device1) - if cfg.Folders == nil || cfg.Devices == nil || cfg.Options.ListenAddresses == nil { + if cfg.Folders == nil || cfg.Devices == nil || cfg.Options.RawListenAddresses == nil { t.Error("Unexpected nil") } } @@ -580,7 +580,7 @@ func TestCopy(t *testing.T) { cfg.Devices[0].Addresses[0] = "wrong" cfg.Folders[0].Devices[0].DeviceID = protocol.DeviceID{0, 1, 2, 3} - cfg.Options.ListenAddresses[0] = "wrong" + cfg.Options.RawListenAddresses[0] = "wrong" cfg.GUI.APIKey = "wrong" bsChanged, err := json.MarshalIndent(cfg, "", " ") @@ -771,7 +771,7 @@ func TestV14ListenAddressesMigration(t *testing.T) { cfg := Configuration{ Version: 13, Options: OptionsConfiguration{ - ListenAddresses: tc[0], + RawListenAddresses: tc[0], DeprecatedRelayServers: tc[1], }, } @@ -781,8 +781,8 @@ func TestV14ListenAddressesMigration(t *testing.T) { } sort.Strings(tc[2]) - if !reflect.DeepEqual(cfg.Options.ListenAddresses, tc[2]) { - t.Errorf("Migration error; actual %#v != expected %#v", cfg.Options.ListenAddresses, tc[2]) + if !reflect.DeepEqual(cfg.Options.RawListenAddresses, tc[2]) { + t.Errorf("Migration error; actual %#v != expected %#v", cfg.Options.RawListenAddresses, tc[2]) } } } diff --git a/lib/config/migrations.go b/lib/config/migrations.go index c2d0e9fb..1bcbd90c 100644 --- a/lib/config/migrations.go +++ b/lib/config/migrations.go @@ -216,12 +216,12 @@ func migrateToConfigV18(cfg *Configuration) { func migrateToConfigV15(cfg *Configuration) { // Undo v0.13.0 broken migration - for i, addr := range cfg.Options.GlobalAnnServers { + for i, addr := range cfg.Options.RawGlobalAnnServers { switch addr { case "default-v4v2/": - cfg.Options.GlobalAnnServers[i] = "default-v4" + cfg.Options.RawGlobalAnnServers[i] = "default-v4" case "default-v6v2/": - cfg.Options.GlobalAnnServers[i] = "default-v6" + cfg.Options.RawGlobalAnnServers[i] = "default-v6" } } } @@ -248,9 +248,9 @@ func migrateToConfigV14(cfg *Configuration) { hasDefault := false for _, raddr := range cfg.Options.DeprecatedRelayServers { if raddr == "dynamic+https://relays.syncthing.net/endpoint" { - for i, addr := range cfg.Options.ListenAddresses { + for i, addr := range cfg.Options.RawListenAddresses { if addr == "tcp://0.0.0.0:22000" { - cfg.Options.ListenAddresses[i] = "default" + cfg.Options.RawListenAddresses[i] = "default" hasDefault = true break } @@ -269,16 +269,16 @@ func migrateToConfigV14(cfg *Configuration) { if addr == "" { continue } - cfg.Options.ListenAddresses = append(cfg.Options.ListenAddresses, addr) + cfg.Options.RawListenAddresses = append(cfg.Options.RawListenAddresses, addr) } cfg.Options.DeprecatedRelayServers = nil // For consistency - sort.Strings(cfg.Options.ListenAddresses) + sort.Strings(cfg.Options.RawListenAddresses) var newAddrs []string - for _, addr := range cfg.Options.GlobalAnnServers { + for _, addr := range cfg.Options.RawGlobalAnnServers { uri, err := url.Parse(addr) if err != nil { // That's odd. Skip the broken address. @@ -291,7 +291,7 @@ func migrateToConfigV14(cfg *Configuration) { newAddrs = append(newAddrs, addr) } - cfg.Options.GlobalAnnServers = newAddrs + cfg.Options.RawGlobalAnnServers = newAddrs for i, fcfg := range cfg.Folders { if fcfg.DeprecatedReadOnly { @@ -315,9 +315,9 @@ func migrateToConfigV13(cfg *Configuration) { func migrateToConfigV12(cfg *Configuration) { // Change listen address schema - for i, addr := range cfg.Options.ListenAddresses { + for i, addr := range cfg.Options.RawListenAddresses { if len(addr) > 0 && !strings.HasPrefix(addr, "tcp://") { - cfg.Options.ListenAddresses[i] = util.Address("tcp", addr) + cfg.Options.RawListenAddresses[i] = util.Address("tcp", addr) } } @@ -332,7 +332,7 @@ func migrateToConfigV12(cfg *Configuration) { // Use new discovery server var newDiscoServers []string var useDefault bool - for _, addr := range cfg.Options.GlobalAnnServers { + for _, addr := range cfg.Options.RawGlobalAnnServers { if addr == "udp4://announce.syncthing.net:22026" { useDefault = true } else if addr == "udp6://announce-v6.syncthing.net:22026" { @@ -344,7 +344,7 @@ func migrateToConfigV12(cfg *Configuration) { if useDefault { newDiscoServers = append(newDiscoServers, "default") } - cfg.Options.GlobalAnnServers = newDiscoServers + cfg.Options.RawGlobalAnnServers = newDiscoServers // Use new multicast group if cfg.Options.LocalAnnMCAddr == "[ff32::5222]:21026" { diff --git a/lib/config/optionsconfiguration.go b/lib/config/optionsconfiguration.go index b9b4b140..f5b0b8ad 100644 --- a/lib/config/optionsconfiguration.go +++ b/lib/config/optionsconfiguration.go @@ -9,12 +9,13 @@ package config import ( "fmt" + "github.com/syncthing/syncthing/lib/rand" "github.com/syncthing/syncthing/lib/util" ) type OptionsConfiguration struct { - ListenAddresses []string `xml:"listenAddress" json:"listenAddresses" default:"default"` - GlobalAnnServers []string `xml:"globalAnnounceServer" json:"globalAnnounceServers" default:"default" restart:"true"` + RawListenAddresses []string `xml:"listenAddress" json:"listenAddresses" default:"default"` + RawGlobalAnnServers []string `xml:"globalAnnounceServer" json:"globalAnnounceServers" default:"default" restart:"true"` GlobalAnnEnabled bool `xml:"globalAnnounceEnabled" json:"globalAnnounceEnabled" default:"true" restart:"true"` LocalAnnEnabled bool `xml:"localAnnounceEnabled" json:"localAnnounceEnabled" default:"true" restart:"true"` LocalAnnPort int `xml:"localAnnouncePort" json:"localAnnouncePort" default:"21027" restart:"true"` @@ -56,7 +57,7 @@ type OptionsConfiguration struct { CREnabled bool `xml:"crashReportingEnabled" json:"crashReportingEnabled" default:"true" restart:"true"` StunKeepaliveStartS int `xml:"stunKeepaliveStartS" json:"stunKeepaliveStartS" default:"180"` // 0 for off StunKeepaliveMinS int `xml:"stunKeepaliveMinS" json:"stunKeepaliveMinS" default:"20"` // 0 for off - StunServers []string `xml:"stunServer" json:"stunServers" default:"default"` + RawStunServers []string `xml:"stunServer" json:"stunServers" default:"default"` DatabaseTuning Tuning `xml:"databaseTuning" json:"databaseTuning" restart:"true"` DeprecatedUPnPEnabled bool `xml:"upnpEnabled,omitempty" json:"-"` @@ -69,10 +70,10 @@ type OptionsConfiguration struct { func (opts OptionsConfiguration) Copy() OptionsConfiguration { optsCopy := opts - optsCopy.ListenAddresses = make([]string, len(opts.ListenAddresses)) - copy(optsCopy.ListenAddresses, opts.ListenAddresses) - optsCopy.GlobalAnnServers = make([]string, len(opts.GlobalAnnServers)) - copy(optsCopy.GlobalAnnServers, opts.GlobalAnnServers) + optsCopy.RawListenAddresses = make([]string, len(opts.RawListenAddresses)) + copy(optsCopy.RawListenAddresses, opts.RawListenAddresses) + optsCopy.RawGlobalAnnServers = make([]string, len(opts.RawGlobalAnnServers)) + copy(optsCopy.RawGlobalAnnServers, opts.RawGlobalAnnServers) optsCopy.AlwaysLocalNets = make([]string, len(opts.AlwaysLocalNets)) copy(optsCopy.AlwaysLocalNets, opts.AlwaysLocalNets) optsCopy.UnackedNotificationIDs = make([]string, len(opts.UnackedNotificationIDs)) @@ -97,3 +98,57 @@ func (opts OptionsConfiguration) RequiresRestartOnly() OptionsConfiguration { func (opts OptionsConfiguration) IsStunDisabled() bool { return opts.StunKeepaliveMinS < 1 || opts.StunKeepaliveStartS < 1 || !opts.NATEnabled } + +func (opts OptionsConfiguration) ListenAddresses() []string { + var addresses []string + for _, addr := range opts.RawListenAddresses { + switch addr { + case "default": + addresses = append(addresses, DefaultListenAddresses...) + default: + addresses = append(addresses, addr) + } + } + return util.UniqueTrimmedStrings(addresses) +} + +func (opts OptionsConfiguration) StunServers() []string { + var addresses []string + for _, addr := range opts.RawStunServers { + switch addr { + case "default": + defaultPrimaryAddresses := make([]string, len(DefaultPrimaryStunServers)) + copy(defaultPrimaryAddresses, DefaultPrimaryStunServers) + rand.Shuffle(defaultPrimaryAddresses) + addresses = append(addresses, defaultPrimaryAddresses...) + + defaultSecondaryAddresses := make([]string, len(DefaultSecondaryStunServers)) + copy(defaultSecondaryAddresses, DefaultSecondaryStunServers) + rand.Shuffle(defaultSecondaryAddresses) + addresses = append(addresses, defaultSecondaryAddresses...) + default: + addresses = append(addresses, addr) + } + } + + addresses = util.UniqueTrimmedStrings(addresses) + + return addresses +} + +func (opts OptionsConfiguration) GlobalDiscoveryServers() []string { + var servers []string + for _, srv := range opts.RawGlobalAnnServers { + switch srv { + case "default": + servers = append(servers, DefaultDiscoveryServers...) + case "default-v4": + servers = append(servers, DefaultDiscoveryServersV4...) + case "default-v6": + servers = append(servers, DefaultDiscoveryServersV6...) + default: + servers = append(servers, srv) + } + } + return util.UniqueTrimmedStrings(servers) +} diff --git a/lib/config/wrapper.go b/lib/config/wrapper.go index 1082b74b..85631d30 100644 --- a/lib/config/wrapper.go +++ b/lib/config/wrapper.go @@ -14,9 +14,7 @@ import ( "github.com/syncthing/syncthing/lib/events" "github.com/syncthing/syncthing/lib/osutil" "github.com/syncthing/syncthing/lib/protocol" - "github.com/syncthing/syncthing/lib/rand" "github.com/syncthing/syncthing/lib/sync" - "github.com/syncthing/syncthing/lib/util" ) // The Committer interface is implemented by objects that need to know about @@ -87,10 +85,6 @@ type Wrapper interface { IgnoredDevice(id protocol.DeviceID) bool IgnoredFolder(device protocol.DeviceID, folder string) bool - ListenAddresses() []string - GlobalDiscoveryServers() []string - StunServers() []string - Subscribe(c Committer) Unsubscribe(c Committer) } @@ -108,30 +102,6 @@ type wrapper struct { requiresRestart uint32 // an atomic bool } -func (w *wrapper) StunServers() []string { - var addresses []string - for _, addr := range w.cfg.Options.StunServers { - switch addr { - case "default": - defaultPrimaryAddresses := make([]string, len(DefaultPrimaryStunServers)) - copy(defaultPrimaryAddresses, DefaultPrimaryStunServers) - rand.Shuffle(defaultPrimaryAddresses) - addresses = append(addresses, defaultPrimaryAddresses...) - - defaultSecondaryAddresses := make([]string, len(DefaultSecondaryStunServers)) - copy(defaultSecondaryAddresses, DefaultSecondaryStunServers) - rand.Shuffle(defaultSecondaryAddresses) - addresses = append(addresses, defaultSecondaryAddresses...) - default: - addresses = append(addresses, addr) - } - } - - addresses = util.UniqueTrimmedStrings(addresses) - - return addresses -} - // Wrap wraps an existing Configuration structure and ties it to a file on // disk. func Wrap(path string, cfg Configuration, evLogger events.Logger) Wrapper { @@ -456,36 +426,6 @@ func (w *wrapper) Save() error { return nil } -func (w *wrapper) GlobalDiscoveryServers() []string { - var servers []string - for _, srv := range w.Options().GlobalAnnServers { - switch srv { - case "default": - servers = append(servers, DefaultDiscoveryServers...) - case "default-v4": - servers = append(servers, DefaultDiscoveryServersV4...) - case "default-v6": - servers = append(servers, DefaultDiscoveryServersV6...) - default: - servers = append(servers, srv) - } - } - return util.UniqueTrimmedStrings(servers) -} - -func (w *wrapper) ListenAddresses() []string { - var addresses []string - for _, addr := range w.Options().ListenAddresses { - switch addr { - case "default": - addresses = append(addresses, DefaultListenAddresses...) - default: - addresses = append(addresses, addr) - } - } - return util.UniqueTrimmedStrings(addresses) -} - func (w *wrapper) RequiresRestart() bool { return atomic.LoadUint32(&w.requiresRestart) != 0 } diff --git a/lib/connections/service.go b/lib/connections/service.go index 3b5e2c86..4b880d13 100644 --- a/lib/connections/service.go +++ b/lib/connections/service.go @@ -582,7 +582,7 @@ func (s *service) CommitConfiguration(from, to config.Configuration) bool { s.listenersMut.Lock() seen := make(map[string]struct{}) - for _, addr := range config.Wrap("", to, s.evLogger).ListenAddresses() { + for _, addr := range to.Options.ListenAddresses() { if addr == "" { // We can get an empty address if there is an empty listener // element in the config, indicating no listeners should be diff --git a/lib/stun/stun.go b/lib/stun/stun.go index 0b27f809..9ff0c14d 100644 --- a/lib/stun/stun.go +++ b/lib/stun/stun.go @@ -131,7 +131,7 @@ func (s *Service) serve(ctx context.Context) { l.Debugf("Starting stun for %s", s) - for _, addr := range s.cfg.StunServers() { + for _, addr := range s.cfg.Options().StunServers() { // This blocks until we hit an exit condition or there are issues with the STUN server. // This returns a boolean signifying if a different STUN server should be tried (oppose to the whole thing // shutting down and this winding itself down. diff --git a/lib/syncthing/syncthing.go b/lib/syncthing/syncthing.go index e0b93500..071142b9 100644 --- a/lib/syncthing/syncthing.go +++ b/lib/syncthing/syncthing.go @@ -262,7 +262,7 @@ func (a *App) startup() error { a.mainService.Add(connectionsService) if a.cfg.Options().GlobalAnnEnabled { - for _, srv := range a.cfg.GlobalDiscoveryServers() { + for _, srv := range a.cfg.Options().GlobalDiscoveryServers() { l.Infoln("Using discovery server", srv) gd, err := discover.NewGlobal(srv, a.cert, connectionsService, a.evLogger) if err != nil { diff --git a/lib/ur/usage_report.go b/lib/ur/usage_report.go index 9f3cff3b..f12c4169 100644 --- a/lib/ur/usage_report.go +++ b/lib/ur/usage_report.go @@ -192,7 +192,7 @@ func (s *Service) reportData(urVersion int, preview bool) map[string]interface{} res["deviceUses"] = deviceUses defaultAnnounceServersDNS, defaultAnnounceServersIP, otherAnnounceServers := 0, 0, 0 - for _, addr := range opts.GlobalAnnServers { + for _, addr := range opts.RawGlobalAnnServers { if addr == "default" || addr == "default-v4" || addr == "default-v6" { defaultAnnounceServersDNS++ } else { @@ -208,7 +208,7 @@ func (s *Service) reportData(urVersion int, preview bool) map[string]interface{} } defaultRelayServers, otherRelayServers := 0, 0 - for _, addr := range s.cfg.ListenAddresses() { + for _, addr := range s.cfg.Options().ListenAddresses() { switch { case addr == "dynamic+https://relays.syncthing.net/endpoint": defaultRelayServers++