miracle091/dnscrypt-proxy
miracle091
/
dnscrypt-proxy
mirror of https://github.com/DNSCrypt/dnscrypt-proxy.git
1
0
Fork 0
Code Issues Releases Wiki Activity

Fix various timing inconsistencies 

When comparing times in tests, it's necessary to control the `now` value to ensure slow test runs don't fail incorrectly.
Both cache and download code had been using refreshDelay to set the next prefetch delay, which by default meant the 1st prefetch was 3 days after startup - this has now been corrected to match the 1 day expectation.
Enabling some of the cache tests revealed some other incorrect failures in the test that were also fixed.
This commit is contained in:
William Elwood 2019-11-03 06:34:59 +00:00 committed by Frank Denis
parent 03dea47130
commit da0d7fe841
2 changed files with 23 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
dnscrypt-proxy/sources.go
View File

@ -35,6 +35,9 @@ type Source struct {
in string
}
// timeNow can be replaced by tests to provide a static value
var timeNow = time.Now
func fetchFromCache(cacheFile string, refreshDelay time.Duration) (in string, expired bool, delayTillNextUpdate time.Duration, err error) {
expired = false
if refreshDelay < MinSourcesUpdateDelay {
@ -46,10 +49,10 @@ func fetchFromCache(cacheFile string, refreshDelay time.Duration) (in string, ex
delayTillNextUpdate = time.Duration(0)
return
}
elapsed := time.Since(fi.ModTime())
elapsed := timeNow().Sub(fi.ModTime())
if elapsed < refreshDelay {
dlog.Debugf("Cache file [%s] is still fresh", cacheFile)
delayTillNextUpdate = refreshDelay - elapsed
delayTillNextUpdate = MinSourcesUpdateDelay - elapsed
} else {
dlog.Debugf("Cache file [%s] needs to be refreshed", cacheFile)
delayTillNextUpdate = time.Duration(0)
@ -106,7 +109,7 @@ func fetchWithCache(xTransport *XTransport, urlStr string, cacheFile string, ref
err = nil
cached = false
in = string(bin)
delayTillNextUpdate = refreshDelay
delayTillNextUpdate = MinSourcesUpdateDelay
return
}
@ -131,7 +134,7 @@ func NewSource(xTransport *XTransport, urls []string, minisignKeyStr string, cac
if err != nil {
return source, []URLToPrefetch{}, err
}
now := time.Now()
now := timeNow()
urlsToPrefetch := []URLToPrefetch{}
sigCacheFile := cacheFile + ".minisig"
@ -279,6 +282,6 @@ func PrefetchSourceURL(xTransport *XTransport, urlToPrefetch *URLToPrefetch) err
if err == nil && !cached {
AtomicFileWrite(urlToPrefetch.cacheFile, []byte(in))
}
urlToPrefetch.when = time.Now().Add(delayTillNextUpdate)
urlToPrefetch.when = timeNow().Add(delayTillNextUpdate)
return err
}

24
dnscrypt-proxy/sources_test.go
View File

@ -128,7 +128,7 @@ func loadFixtures(t *testing.T, d *SourceTestData) {
for _, source := range d.sources {
for _, suffix := range [...]string{"", ".minisig"} {
file := source + suffix
d.fixtures[TestStateCorrect][file] = SourceFixture{suffix: suffix, content: readFixture(t, filepath.Join("sources", file))}
d.fixtures[TestStateCorrect][file] = SourceFixture{suffix: suffix, content: readFixture(t, filepath.Join("sources", file)), mtime: d.timeNow}
for _, state := range [...]SourceTestState{TestStateExpired, TestStatePartial, TestStateReadErr, TestStateOpenErr,
TestStatePartialSig, TestStateMissingSig, TestStateReadSigErr, TestStateOpenSigErr} {
if _, ok := d.fixtures[state]; !ok {
@ -141,7 +141,7 @@ func loadFixtures(t *testing.T, d *SourceTestData) {
continue
}
}
f := SourceFixture{suffix: suffix}
f := SourceFixture{suffix: suffix, mtime: d.timeNow}
switch state {
case TestStateExpired:
f.content, f.mtime = d.fixtures[TestStateCorrect][file].content, d.timeOld
@ -197,13 +197,13 @@ func checkTestServer(c *check.C, d *SourceTestData) {
func setupSourceTest(t *testing.T) (func(), *SourceTestData) {
d := &SourceTestData{n: -1, xTransport: NewXTransport()}
d.cacheTests = map[string]SourceTestState{ // determines cache files written to disk before each call
//"correct": TestStateCorrect, // TODO: millisecond-level timing inconsistencies should not fail tests
"correct": TestStateCorrect,
//"expired": TestStateExpired, // TODO: an expired cache should be used if no download passes signature verification
//"partial": TestStatePartial, // TODO: failed signature verification should not abort before trying additional URLs and should not destroy the cache
//"partial-sig": TestStatePartialSig, // TODO: failed signature verification should not abort before trying additional URLs and should not destroy the cache
//"missing": TestStateMissing, // TODO: millisecond-level timing inconsistencies should not fail tests
"missing": TestStateMissing,
//"missing-sig": TestStateMissingSig, // TODO: a cache without a signature should cause an attempt to download both files (not just the signature), failed signature verification should not destroy the cache
//"open-err": TestStateOpenErr, // TODO: millisecond-level timing inconsistencies should not fail tests
"open-err": TestStateOpenErr,
//"open-sig-err": TestStateOpenSigErr, // TODO: a cache without a signature should cause an attempt to download both files (not just the signature), failed signature verification should not destroy the cache
}
d.downloadTests = map[string][]SourceTestState{ // determines the list of URLs passed in each call and how they will respond
@ -229,9 +229,10 @@ func setupSourceTest(t *testing.T) (func(), *SourceTestData) {
"no-urls": {},
}
d.xTransport.rebuildTransport()
d.timeNow = time.Now()
d.timeNow = time.Now().AddDate(0, 0, 0)
d.timeOld = d.timeNow.Add(MinSourcesUpdateDelay * -4)
d.timeUpd = d.timeNow.Add(MinSourcesUpdateDelay)
timeNow = func() time.Time { return d.timeNow } // originally defined in sources.go, replaced during testing to ensure consistent results
makeTempDir(t, d)
makeTestServer(t, d)
loadSnakeoil(t, d)
@ -249,7 +250,7 @@ func prepSourceTestCache(t *testing.T, d *SourceTestData, e *SourceTestExpect, s
case TestStateCorrect:
e.in, e.success, e.refresh = string(e.cache[0].content), true, d.timeUpd
case TestStateExpired:
e.in, e.success = string(e.cache[0].content), true
e.in = string(e.cache[0].content)
case TestStatePartial, TestStatePartialSig:
e.err = "signature"
case TestStateMissing, TestStateMissingSig:
@ -264,11 +265,11 @@ func prepSourceTestDownload(t *testing.T, d *SourceTestData, e *SourceTestExpect
if len(downloadTest) > 0 {
for _, state := range downloadTest {
path := "/" + strconv.FormatUint(uint64(state), 10) + "/" + source
if !e.success || !e.refresh.After(d.timeNow) {
if !e.success {
switch state {
case TestStateCorrect:
e.cache = []SourceFixture{d.fixtures[state][source], d.fixtures[state][source+".minisig"]}
e.in, e.success, e.err, e.refresh = string(e.cache[0].content), true, "", d.timeUpd
e.in, e.success, e.refresh = string(e.cache[0].content), true, d.timeUpd
fallthrough
case TestStateMissingSig, TestStatePartial, TestStatePartialSig, TestStateReadSigErr:
d.reqExpect[path+".minisig"]++
@ -297,6 +298,11 @@ func prepSourceTestDownload(t *testing.T, d *SourceTestData, e *SourceTestExpect
e.prefetchUrls = append(e.prefetchUrls, URLToPrefetch{d.server.URL + path + ".minisig", e.cachePath + ".minisig", e.refresh})
}
}
if e.success {
e.err = ""
}
} else if !e.success {
e.err = "no URL"
}
}