From b2ecc45133eb091966d1e87c401f2d350e16256c Mon Sep 17 00:00:00 2001 From: William Elwood Date: Thu, 31 Oct 2019 05:53:16 +0000 Subject: [PATCH] Treat each list and signature pair as a single unit When a list fails to download, there's no point trying to download the signature. Code duplication moved to where it's easier to refactor away. Enabled a few more tests. --- dnscrypt-proxy/sources.go | 46 ++++++++++++++++++---------------- dnscrypt-proxy/sources_test.go | 5 ++-- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/dnscrypt-proxy/sources.go b/dnscrypt-proxy/sources.go index 13774613..790f11e9 100644 --- a/dnscrypt-proxy/sources.go +++ b/dnscrypt-proxy/sources.go @@ -49,7 +49,7 @@ func (source *Source) checkSignature(bin, sig []byte) (err error) { // timeNow can be replaced by tests to provide a static value var timeNow = time.Now -func fetchFromCache(cacheFile string, refreshDelay time.Duration) (bin []byte, delayTillNextUpdate time.Duration, err error) { +func fetchFromCache(cacheFile string, refreshDelay time.Duration) (bin, sig []byte, delayTillNextUpdate time.Duration, err error) { delayTillNextUpdate = time.Duration(0) if refreshDelay < DefaultPrefetchDelay { refreshDelay = DefaultPrefetchDelay @@ -61,6 +61,9 @@ func fetchFromCache(cacheFile string, refreshDelay time.Duration) (bin []byte, d if bin, err = ioutil.ReadFile(cacheFile); err != nil { return } + if sig, err = ioutil.ReadFile(cacheFile + ".minisig"); err != nil { + return + } if elapsed := timeNow().Sub(fi.ModTime()); elapsed < refreshDelay { dlog.Debugf("Cache file [%s] is still fresh", cacheFile) delayTillNextUpdate = DefaultPrefetchDelay - elapsed @@ -79,8 +82,8 @@ func fetchFromURL(xTransport *XTransport, u *url.URL) (bin []byte, err error) { return } -func fetchWithCache(xTransport *XTransport, urlStr string, cacheFile string, refreshDelay time.Duration) (bin []byte, delayTillNextUpdate time.Duration, err error) { - if bin, delayTillNextUpdate, err = fetchFromCache(cacheFile, refreshDelay); err != nil { +func fetchWithCache(xTransport *XTransport, urlStr string, cacheFile string, refreshDelay time.Duration) (bin, sig []byte, delayTillNextUpdate time.Duration, err error) { + if bin, sig, delayTillNextUpdate, err = fetchFromCache(cacheFile, refreshDelay); err != nil { if len(urlStr) == 0 { dlog.Errorf("Cache file [%s] not present and no URL given to retrieve it", cacheFile) return @@ -94,11 +97,17 @@ func fetchWithCache(xTransport *XTransport, urlStr string, cacheFile string, ref dlog.Infof("Loading source information from URL [%s]", urlStr) - var u *url.URL - if u, err = url.Parse(urlStr); err != nil { + var srcURL, sigURL *url.URL + if srcURL, err = url.Parse(urlStr); err != nil { return } - if bin, err = fetchFromURL(xTransport, u); err != nil { + if sigURL, err = url.Parse(urlStr + ".minisig"); err != nil { + return + } + if bin, err = fetchFromURL(xTransport, srcURL); err != nil { + return + } + if sig, err = fetchFromURL(xTransport, sigURL); err != nil { return } if err = AtomicFileWrite(cacheFile, bin); err != nil { @@ -106,6 +115,11 @@ func fetchWithCache(xTransport *XTransport, urlStr string, cacheFile string, ref dlog.Warnf("%s: %s", absPath, err) } } + if err = AtomicFileWrite(cacheFile+".minisig", sig); err != nil { + if absPath, err2 := filepath.Abs(cacheFile + ".minisig"); err2 == nil { + dlog.Warnf("%s: %s", absPath, err) + } + } delayTillNextUpdate = DefaultPrefetchDelay return } @@ -133,23 +147,18 @@ func NewSource(xTransport *XTransport, urls []string, minisignKeyStr string, cac return source, err } now := timeNow() - sigCacheFile := cacheFile + ".minisig" source.prefetch = []*URLToPrefetch{} var bin, sig []byte - var delayTillNextUpdate, sigDelayTillNextUpdate time.Duration - var sigErr error + var delayTillNextUpdate time.Duration var preloadURL string if len(urls) <= 0 { - bin, delayTillNextUpdate, err = fetchWithCache(xTransport, "", cacheFile, refreshDelay) - sig, sigDelayTillNextUpdate, sigErr = fetchWithCache(xTransport, "", sigCacheFile, refreshDelay) + bin, sig, delayTillNextUpdate, err = fetchWithCache(xTransport, "", cacheFile, refreshDelay) } else { preloadURL = urls[0] for _, url := range urls { - sigURL := url + ".minisig" - bin, delayTillNextUpdate, err = fetchWithCache(xTransport, url, cacheFile, refreshDelay) - sig, sigDelayTillNextUpdate, sigErr = fetchWithCache(xTransport, sigURL, sigCacheFile, refreshDelay) - if err == nil && sigErr == nil { + bin, sig, delayTillNextUpdate, err = fetchWithCache(xTransport, url, cacheFile, refreshDelay) + if err == nil { preloadURL = url break } @@ -158,12 +167,7 @@ func NewSource(xTransport *XTransport, urls []string, minisignKeyStr string, cac } if len(preloadURL) > 0 { url := preloadURL - sigURL := url + ".minisig" 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 } if err != nil { return @@ -274,7 +278,7 @@ PartsLoop: } func PrefetchSourceURL(xTransport *XTransport, urlToPrefetch *URLToPrefetch) error { - _, delayTillNextUpdate, err := fetchWithCache(xTransport, urlToPrefetch.url, urlToPrefetch.cacheFile, DefaultPrefetchDelay) + _, _, delayTillNextUpdate, err := fetchWithCache(xTransport, urlToPrefetch.url, urlToPrefetch.cacheFile, DefaultPrefetchDelay) urlToPrefetch.when = timeNow().Add(delayTillNextUpdate) return err } diff --git a/dnscrypt-proxy/sources_test.go b/dnscrypt-proxy/sources_test.go index 12205200..a0d468e7 100644 --- a/dnscrypt-proxy/sources_test.go +++ b/dnscrypt-proxy/sources_test.go @@ -208,9 +208,9 @@ func setupSourceTest(t *testing.T) (func(), *SourceTestData) { "correct": {TestStateCorrect}, //"partial": {TestStatePartial}, // TODO: failed signature verification should not count as successfully prefetched //"partial-sig": {TestStatePartialSig}, // TODO: failed signature verification should not count as successfully prefetched - //"missing": {TestStateMissing}, // TODO: a list download failure should not cause an attempt to download the signature for that URL + "missing": {TestStateMissing}, //"missing-sig": {TestStateMissingSig}, // TODO: failed signature verification should not count as successfully prefetched - //"read-err": {TestStateReadErr}, // TODO: a list download failure should not cause an attempt to download the signature for that URL + "read-err": {TestStateReadErr}, //"read-sig-err": {TestStateReadSigErr}, // TODO: failed signature verification should not count as successfully prefetched "open-err": {TestStateOpenErr}, //"open-sig-err": {TestStateOpenSigErr}, // TODO: failed signature verification should not count as successfully prefetched @@ -293,7 +293,6 @@ func prepSourceTestDownload(t *testing.T, d *SourceTestData, e *SourceTestExpect e.Source.urls = append(e.Source.urls, d.server.URL+path) if state != TestStatePathErr { 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 {