From 0983b06b4b308be5e0bfd16f2b101114d9008d56 Mon Sep 17 00:00:00 2001 From: favonia Date: Sun, 7 Jul 2024 13:37:16 +0300 Subject: [PATCH] feat: reprobe 1.1.1.1 and 1.0.0.1 when probing fails (#788) --- cmd/ddns/ddns.go | 2 +- internal/config/config.go | 4 ++-- internal/config/network_probe.go | 29 +++++++++++++++++---------- internal/config/network_probe_test.go | 9 +++++---- internal/updater/updater.go | 3 ++- internal/updater/updater_test.go | 18 +++++++++++------ 6 files changed, 40 insertions(+), 25 deletions(-) diff --git a/cmd/ddns/ddns.go b/cmd/ddns/ddns.go index 6be4c6b2..ac3f645b 100644 --- a/cmd/ddns/ddns.go +++ b/cmd/ddns/ddns.go @@ -32,7 +32,7 @@ func initConfig(ctx context.Context, ppfmt pp.PP) (*config.Config, setter.Setter c := config.Default() // Read the config - if !c.ReadEnv(ppfmt) || !c.NormalizeConfig(ppfmt) || !c.ShouldWeUse1001(ctx, ppfmt) { + if !c.ReadEnv(ppfmt) || !c.NormalizeConfig(ppfmt) { return c, nil, false } diff --git a/internal/config/config.go b/internal/config/config.go index 757862fc..579141c8 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -18,7 +18,7 @@ import ( type Config struct { Auth api.Auth Provider map[ipnet.Type]provider.Provider - Use1001 bool + ShouldWeUse1001 *bool Domains map[ipnet.Type][]domain.Domain UpdateCron cron.Schedule UpdateOnStart bool @@ -42,7 +42,7 @@ func Default() *Config { ipnet.IP4: provider.NewCloudflareTrace(), ipnet.IP6: provider.NewCloudflareTrace(), }, - Use1001: false, + ShouldWeUse1001: nil, Domains: map[ipnet.Type][]domain.Domain{ ipnet.IP4: nil, ipnet.IP6: nil, diff --git a/internal/config/network_probe.go b/internal/config/network_probe.go index cab1fef3..500343df 100644 --- a/internal/config/network_probe.go +++ b/internal/config/network_probe.go @@ -27,14 +27,15 @@ func ProbeURL(ctx context.Context, url string) bool { return err == nil } -// ShouldWeUse1001 quickly checks 1.1.1.1 and 1.0.0.1 and notes whether 1.0.0.1 should be used. +// ShouldWeUse1001Now quickly checks 1.1.1.1 and 1.0.0.1 and notes whether 1.0.0.1 should be used. // -// Note that the return value is about whether the detection is successfully done, not that -// whether we should use 1.0.0.1. The function will update the field [Config.Use1001] directly. -func (c *Config) ShouldWeUse1001(ctx context.Context, ppfmt pp.PP) bool { - c.Use1001 = false +// [Config.ShouldWeUse1001] remembers the results. +func (c *Config) ShouldWeUse1001Now(ctx context.Context, ppfmt pp.PP) bool { + if c.ShouldWeUse1001 != nil { + return *c.ShouldWeUse1001 + } if c.Provider[ipnet.IP4] == nil || !c.Provider[ipnet.IP4].ShouldWeCheck1111() { - return true + return false // any answer would work } if ppfmt.IsEnabledFor(pp.Info) { @@ -44,16 +45,22 @@ func (c *Config) ShouldWeUse1001(ctx context.Context, ppfmt pp.PP) bool { if ProbeURL(ctx, "https://1.1.1.1") { ppfmt.Infof(pp.EmojiGood, "1.1.1.1 is working. Great!") + + res := false + c.ShouldWeUse1001 = &res + return false } else { if ProbeURL(ctx, "https://1.0.0.1") { ppfmt.Warningf(pp.EmojiError, "1.1.1.1 is not working, but 1.0.0.1 is; using 1.0.0.1") ppfmt.Infof(pp.EmojiHint, "1.1.1.1 is probably blocked or hijacked by your router or ISP") - c.Use1001 = true + + res := true + c.ShouldWeUse1001 = &res + return true } else { - ppfmt.Warningf(pp.EmojiError, "Both 1.1.1.1 and 1.0.0.1 are not working; sticking to 1.1.1.1") - ppfmt.Infof(pp.EmojiHint, "The network might be temporarily down, or has not been set up yet") - ppfmt.Infof(pp.EmojiHint, "If you start this tool during booting, make sure the network is already up") + ppfmt.Warningf(pp.EmojiError, "Both 1.1.1.1 and 1.0.0.1 are not working; sticking to 1.1.1.1 now") + ppfmt.Infof(pp.EmojiHint, "The network might be temporarily down; will redo probing later") + return false } } - return true } diff --git a/internal/config/network_probe_test.go b/internal/config/network_probe_test.go index 2d036dbf..a0195faf 100644 --- a/internal/config/network_probe_test.go +++ b/internal/config/network_probe_test.go @@ -48,8 +48,9 @@ func TestProbeCloudflareIPs(t *testing.T) { ) c := config.Default() // config.ShouldWeUse1001 must return false on GitHub. - require.True(t, c.ShouldWeUse1001(context.Background(), mockPP)) - require.False(t, c.Use1001) + require.False(t, c.ShouldWeUse1001Now(context.Background(), mockPP)) + require.NotNil(t, c.ShouldWeUse1001) + require.False(t, *c.ShouldWeUse1001) } func TestProbeCloudflareIPsNoIP4(t *testing.T) { @@ -58,6 +59,6 @@ func TestProbeCloudflareIPsNoIP4(t *testing.T) { mockPP := mocks.NewMockPP(mockCtrl) c := config.Default() c.Provider[ipnet.IP4] = nil - require.True(t, c.ShouldWeUse1001(context.Background(), mockPP)) - require.False(t, c.Use1001) + require.False(t, c.ShouldWeUse1001Now(context.Background(), mockPP)) + require.Nil(t, c.ShouldWeUse1001) } diff --git a/internal/updater/updater.go b/internal/updater/updater.go index ad8475ff..2cd95a01 100644 --- a/internal/updater/updater.go +++ b/internal/updater/updater.go @@ -141,7 +141,8 @@ func UpdateIPs(ctx context.Context, ppfmt pp.PP, c *config.Config, s setter.Sett for _, ipNet := range [...]ipnet.Type{ipnet.IP4, ipnet.IP6} { if c.Provider[ipNet] != nil { - ip, msg := detectIP(ctx, ppfmt, c, ipNet, c.Use1001) + use1001 := c.ShouldWeUse1001Now(ctx, ppfmt) + ip, msg := detectIP(ctx, ppfmt, c, ipNet, use1001) msgs = append(msgs, msg) // Note: If we can't detect the new IP address, diff --git a/internal/updater/updater_test.go b/internal/updater/updater_test.go index f680feb3..d9c8f254 100644 --- a/internal/updater/updater_test.go +++ b/internal/updater/updater_test.go @@ -103,7 +103,8 @@ func TestUpdateIPsMultiple(t *testing.T) { domain4_4: false, } conf.RecordComment = RecordComment - conf.Use1001 = true + use1001 := true + conf.ShouldWeUse1001 = &use1001 conf.DetectionTimeout = time.Second conf.UpdateTimeout = time.Second mockPP := mocks.NewMockPP(mockCtrl) @@ -203,7 +204,8 @@ func TestDeleteIPsMultiple(t *testing.T) { domain4_4: false, } conf.RecordComment = RecordComment - conf.Use1001 = true + use1001 := true + conf.ShouldWeUse1001 = &use1001 conf.DetectionTimeout = time.Second conf.UpdateTimeout = time.Second mockPP := mocks.NewMockPP(mockCtrl) @@ -280,7 +282,8 @@ func TestUpdateIPsUninitializedProxied(t *testing.T) { conf.TTL = api.TTLAuto conf.Proxied = map[domain.Domain]bool{} conf.RecordComment = RecordComment - conf.Use1001 = true + use1001 := true + conf.ShouldWeUse1001 = &use1001 conf.DetectionTimeout = time.Second conf.UpdateTimeout = time.Second mockPP := mocks.NewMockPP(mockCtrl) @@ -369,7 +372,8 @@ func TestUpdateIPsHints(t *testing.T) { conf.TTL = api.TTLAuto conf.Proxied = map[domain.Domain]bool{domain4: false, domain6: false} conf.RecordComment = RecordComment - conf.Use1001 = true + use1001 := true + conf.ShouldWeUse1001 = &use1001 conf.DetectionTimeout = time.Second conf.UpdateTimeout = time.Second mockPP := mocks.NewMockPP(mockCtrl) @@ -657,7 +661,8 @@ func TestUpdateIPs(t *testing.T) { conf.TTL = api.TTLAuto conf.Proxied = map[domain.Domain]bool{domain4: false, domain6: false} conf.RecordComment = RecordComment - conf.Use1001 = true + use1001 := true + conf.ShouldWeUse1001 = &use1001 conf.DetectionTimeout = time.Second conf.UpdateTimeout = time.Second mockPP := mocks.NewMockPP(mockCtrl) @@ -823,7 +828,8 @@ func TestDeleteIPs(t *testing.T) { conf.TTL = api.TTLAuto conf.Proxied = map[domain.Domain]bool{domain4: false, domain6: false} conf.RecordComment = RecordComment - conf.Use1001 = true + use1001 := true + conf.ShouldWeUse1001 = &use1001 conf.DetectionTimeout = time.Second conf.UpdateTimeout = time.Second mockPP := mocks.NewMockPP(mockCtrl)