From 78f2dead79a96aebf33012178f9e37c8f9db8a45 Mon Sep 17 00:00:00 2001 From: William Elwood Date: Thu, 31 Oct 2019 01:04:08 +0000 Subject: [PATCH] Move prefetch URLs onto Source struct This is mostly in preparation for further refactoring, but does reduce the number of return values from `NewSource()` too. --- dnscrypt-proxy/config.go | 4 ++-- dnscrypt-proxy/proxy.go | 9 ++++---- dnscrypt-proxy/sources.go | 13 ++++++----- dnscrypt-proxy/sources_test.go | 42 +++++++++++++++------------------- 4 files changed, 31 insertions(+), 37 deletions(-) diff --git a/dnscrypt-proxy/config.go b/dnscrypt-proxy/config.go index b3a07865..9e287f11 100644 --- a/dnscrypt-proxy/config.go +++ b/dnscrypt-proxy/config.go @@ -613,12 +613,12 @@ func (config *Config) loadSource(proxy *Proxy, requiredProps stamps.ServerInform if cfgSource.RefreshDelay <= 0 { cfgSource.RefreshDelay = 72 } - source, sourceUrlsToPrefetch, err := NewSource(proxy.xTransport, cfgSource.URLs, cfgSource.MinisignKeyStr, cfgSource.CacheFile, cfgSource.FormatStr, time.Duration(cfgSource.RefreshDelay)*time.Hour) - proxy.urlsToPrefetch = append(proxy.urlsToPrefetch, sourceUrlsToPrefetch...) + source, err := NewSource(proxy.xTransport, cfgSource.URLs, cfgSource.MinisignKeyStr, cfgSource.CacheFile, cfgSource.FormatStr, time.Duration(cfgSource.RefreshDelay)*time.Hour) if err != nil { dlog.Criticalf("Unable to retrieve source [%s]: [%s]", cfgSourceName, err) return err } + proxy.urlsToPrefetch = append(proxy.urlsToPrefetch, source.prefetch...) registeredServers, err := source.Parse(cfgSource.Prefix) if err != nil { if len(registeredServers) == 0 { diff --git a/dnscrypt-proxy/proxy.go b/dnscrypt-proxy/proxy.go index a1a6f6ef..aaa9b1a6 100644 --- a/dnscrypt-proxy/proxy.go +++ b/dnscrypt-proxy/proxy.go @@ -60,7 +60,7 @@ type Proxy struct { forwardFile string cloakFile string pluginsGlobals PluginsGlobals - urlsToPrefetch []URLToPrefetch + urlsToPrefetch []*URLToPrefetch clientsCount uint32 maxClients uint32 xTransport *XTransport @@ -177,7 +177,7 @@ func (proxy *Proxy) StartProxy() { dlog.Error(err) dlog.Notice("dnscrypt-proxy is waiting for at least one server to be reachable") } - proxy.prefetcher(&proxy.urlsToPrefetch) + proxy.prefetcher(proxy.urlsToPrefetch) if len(proxy.serversInfo.registeredServers) > 0 { go func() { for { @@ -195,12 +195,11 @@ func (proxy *Proxy) StartProxy() { } } -func (proxy *Proxy) prefetcher(urlsToPrefetch *[]URLToPrefetch) { +func (proxy *Proxy) prefetcher(urlsToPrefetch []*URLToPrefetch) { go func() { for { now := time.Now() - for i := range *urlsToPrefetch { - urlToPrefetch := &(*urlsToPrefetch)[i] + for _, urlToPrefetch := range urlsToPrefetch { if now.After(urlToPrefetch.when) { dlog.Debugf("Prefetching [%s]", urlToPrefetch.url) if err := PrefetchSourceURL(proxy.xTransport, urlToPrefetch); err != nil { diff --git a/dnscrypt-proxy/sources.go b/dnscrypt-proxy/sources.go index 13ea2816..cef399e9 100644 --- a/dnscrypt-proxy/sources.go +++ b/dnscrypt-proxy/sources.go @@ -31,6 +31,7 @@ const ( type Source struct { urls []string + prefetch []*URLToPrefetch format SourceFormat in []byte minisignKey *minisign.PublicKey @@ -118,21 +119,21 @@ type URLToPrefetch struct { when time.Time } -func NewSource(xTransport *XTransport, urls []string, minisignKeyStr string, cacheFile string, formatStr string, refreshDelay time.Duration) (source Source, urlsToPrefetch []URLToPrefetch, err error) { +func NewSource(xTransport *XTransport, urls []string, minisignKeyStr string, cacheFile string, formatStr string, refreshDelay time.Duration) (source Source, err error) { source = Source{urls: urls} - urlsToPrefetch = []URLToPrefetch{} if formatStr == "v2" { source.format = SourceFormatV2 } else { - return source, []URLToPrefetch{}, fmt.Errorf("Unsupported source format: [%s]", formatStr) + return source, fmt.Errorf("Unsupported source format: [%s]", formatStr) } if minisignKey, err := minisign.NewPublicKey(minisignKeyStr); err == nil { source.minisignKey = &minisignKey } else { - return source, urlsToPrefetch, err + return source, err } now := timeNow() sigCacheFile := cacheFile + ".minisig" + source.prefetch = []*URLToPrefetch{} var bin, sig []byte var delayTillNextUpdate, sigDelayTillNextUpdate time.Duration @@ -157,8 +158,8 @@ func NewSource(xTransport *XTransport, urls []string, minisignKeyStr string, cac if len(preloadURL) > 0 { url := preloadURL sigURL := url + ".minisig" - urlsToPrefetch = append(urlsToPrefetch, URLToPrefetch{url: url, cacheFile: cacheFile, when: now.Add(delayTillNextUpdate)}) - urlsToPrefetch = append(urlsToPrefetch, URLToPrefetch{url: sigURL, cacheFile: sigCacheFile, when: now.Add(sigDelayTillNextUpdate)}) + source.prefetch = append(source.prefetch, &URLToPrefetch{url: url, cacheFile: cacheFile, when: now.Add(delayTillNextUpdate)}) + source.prefetch = append(source.prefetch, &URLToPrefetch{url: sigURL, cacheFile: sigCacheFile, when: now.Add(sigDelayTillNextUpdate)}) } if sigErr != nil && err == nil { err = sigErr diff --git a/dnscrypt-proxy/sources_test.go b/dnscrypt-proxy/sources_test.go index 6cf178fb..8193c940 100644 --- a/dnscrypt-proxy/sources_test.go +++ b/dnscrypt-proxy/sources_test.go @@ -55,12 +55,9 @@ type SourceTestData struct { type SourceTestExpect struct { success, download bool - in []byte cachePath string cache []SourceFixture refresh time.Time - urls []string - prefetchUrls []URLToPrefetch Source *Source err string } @@ -249,9 +246,9 @@ func prepSourceTestCache(t *testing.T, d *SourceTestData, e *SourceTestExpect, s e.cache = []SourceFixture{d.fixtures[state][source], d.fixtures[state][source+".minisig"]} switch state { case TestStateCorrect: - e.in, e.success, e.refresh = e.cache[0].content, true, d.timeUpd + e.Source.in, e.success, e.refresh = e.cache[0].content, true, d.timeUpd case TestStateExpired: - e.in = e.cache[0].content + e.Source.in = e.cache[0].content case TestStatePartial, TestStatePartialSig: e.err = "signature" case TestStateMissing, TestStateMissingSig: @@ -270,7 +267,7 @@ func prepSourceTestDownload(t *testing.T, d *SourceTestData, e *SourceTestExpect switch state { case TestStateCorrect: e.cache = []SourceFixture{d.fixtures[state][source], d.fixtures[state][source+".minisig"]} - e.in, e.success, e.refresh = e.cache[0].content, true, d.timeUpd + e.Source.in, e.success, e.refresh = e.cache[0].content, true, d.timeUpd fallthrough case TestStateMissingSig, TestStatePartial, TestStatePartialSig, TestStateReadSigErr: d.reqExpect[path+".minisig"]++ @@ -293,10 +290,10 @@ func prepSourceTestDownload(t *testing.T, d *SourceTestData, e *SourceTestExpect path = "..." + path // non-numeric port fails URL parsing e.err = "parse" } - e.urls = append(e.urls, d.server.URL+path) + e.Source.urls = append(e.Source.urls, d.server.URL+path) if state != TestStatePathErr { - e.prefetchUrls = append(e.prefetchUrls, URLToPrefetch{d.server.URL + path, e.cachePath, e.refresh}) - e.prefetchUrls = append(e.prefetchUrls, URLToPrefetch{d.server.URL + path + ".minisig", e.cachePath + ".minisig", e.refresh}) + e.Source.prefetch = append(e.Source.prefetch, &URLToPrefetch{d.server.URL + path, e.cachePath, e.refresh}) + e.Source.prefetch = append(e.Source.prefetch, &URLToPrefetch{d.server.URL + path + ".minisig", e.cachePath + ".minisig", e.refresh}) } } if e.success { @@ -309,32 +306,29 @@ func setupSourceTestCase(t *testing.T, d *SourceTestData, i int, cacheTest *SourceTestState, downloadTest []SourceTestState) (id string, e *SourceTestExpect) { id = strconv.Itoa(d.n) + "-" + strconv.Itoa(i) e = &SourceTestExpect{ - cachePath: filepath.Join(d.tempDir, id), - refresh: d.timeNow, - urls: []string{}, - prefetchUrls: []URLToPrefetch{}, + cachePath: filepath.Join(d.tempDir, id), + refresh: d.timeNow, + Source: &Source{urls: []string{}, prefetch: []*URLToPrefetch{}, format: SourceFormatV2, minisignKey: d.key}, } if cacheTest != nil { prepSourceTestCache(t, d, e, d.sources[i], *cacheTest) i = (i + 1) % len(d.sources) // make the cached and downloaded fixtures different } prepSourceTestDownload(t, d, e, d.sources[i], downloadTest) - e.Source = &Source{e.urls, SourceFormatV2, e.in, d.key} return } func TestNewSource(t *testing.T) { teardown, d := setupSourceTest(t) defer teardown() - doTest := func(t *testing.T, e *SourceTestExpect, got Source, urls []URLToPrefetch, err error) { + doTest := func(t *testing.T, e *SourceTestExpect, got Source, err error) { c := check.T(t) if len(e.err) > 0 { c.Match(err, e.err, "Unexpected error") } else { c.Nil(err, "Unexpected error") } - c.DeepEqual(got, *e.Source, "Unexpected return Source") - c.DeepEqual(urls, e.prefetchUrls, "Unexpected return prefetch URLs") + c.DeepEqual(got, *e.Source, "Unexpected return") checkTestServer(c, d) checkSourceCache(c, e.cachePath, e.cache) } @@ -345,13 +339,13 @@ func TestNewSource(t *testing.T) { e *SourceTestExpect }{ {"old format", d.keyStr, "v1", MinSourcesUpdateDelay * 3, &SourceTestExpect{ - Source: &Source{urls: nil}, prefetchUrls: []URLToPrefetch{}, err: "Unsupported source format"}}, + Source: &Source{}, err: "Unsupported source format"}}, {"invalid public key", "", "v2", MinSourcesUpdateDelay * 3, &SourceTestExpect{ - Source: &Source{urls: nil}, prefetchUrls: []URLToPrefetch{}, err: "Invalid encoded public key"}}, + Source: &Source{}, err: "Invalid encoded public key"}}, } { t.Run(tt.name, func(t *testing.T) { - got, urls, err := NewSource(d.xTransport, tt.e.urls, tt.key, tt.e.cachePath, tt.v, tt.refresh) - doTest(t, tt.e, got, urls, err) + got, err := NewSource(d.xTransport, tt.e.Source.urls, tt.key, tt.e.cachePath, tt.v, tt.refresh) + doTest(t, tt.e, got, err) }) } for cacheTestName, cacheTest := range d.cacheTests { @@ -360,8 +354,8 @@ func TestNewSource(t *testing.T) { for i := range d.sources { id, e := setupSourceTestCase(t, d, i, &cacheTest, downloadTest) t.Run("cache "+cacheTestName+", download "+downloadTestName+"/"+id, func(t *testing.T) { - got, urls, err := NewSource(d.xTransport, e.urls, d.keyStr, e.cachePath, "v2", MinSourcesUpdateDelay*3) - doTest(t, e, got, urls, err) + got, err := NewSource(d.xTransport, e.Source.urls, d.keyStr, e.cachePath, "v2", MinSourcesUpdateDelay*3) + doTest(t, e, got, err) }) } } @@ -374,7 +368,7 @@ func TestPrefetchSourceURL(t *testing.T) { doTest := func(t *testing.T, expects []*SourceTestExpect) { c := check.T(t) for _, e := range expects { - for _, url := range e.urls { + for _, url := range e.Source.urls { for _, suffix := range []string{"", ".minisig"} { pf := &URLToPrefetch{url + suffix, e.cachePath + suffix, d.timeOld} PrefetchSourceURL(d.xTransport, pf)