From c71d6ed4334287b01b5c41db7597d4e5f060e06a Mon Sep 17 00:00:00 2001 From: Andrey Meshkov Date: Mon, 25 Feb 2019 18:56:51 +0300 Subject: [PATCH] Fix race in safesearch tests --- dnsfilter/dnsfilter_test.go | 15 ++++++++++++--- dnsfilter/safesearch.go | 10 +++++----- dnsforward/dnsforward_test.go | 8 ++++---- dnsforward/stats.go | 5 +++++ 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/dnsfilter/dnsfilter_test.go b/dnsfilter/dnsfilter_test.go index bbb01ac0..b617dd41 100644 --- a/dnsfilter/dnsfilter_test.go +++ b/dnsfilter/dnsfilter_test.go @@ -654,13 +654,19 @@ func TestCheckHostSafeSearchGoogle(t *testing.T) { } } -func TestSafeSearchCacheYandex (t *testing.T) { +func TestSafeSearchCacheYandex(t *testing.T) { d := NewForTest() defer d.Destroy() domain := "yandex.ru" + var result Result + var err error + // Check host with disabled safesearch - result, err := d.CheckHost(domain) + result, err = d.CheckHost(domain) + if err != nil { + t.Fatalf("Cannot check host due to %s", err) + } if result.IP != nil { t.Fatalf("SafeSearch is not enabled but there is an answer for `%s` !", domain) } @@ -693,11 +699,14 @@ func TestSafeSearchCacheYandex (t *testing.T) { } } -func TestSafeSearchCacheGoogle (t *testing.T) { +func TestSafeSearchCacheGoogle(t *testing.T) { d := NewForTest() defer d.Destroy() domain := "www.google.ru" result, err := d.CheckHost(domain) + if err != nil { + t.Fatalf("Cannot check host due to %s", err) + } if result.IP != nil { t.Fatalf("SafeSearch is not enabled but there is an answer!") } diff --git a/dnsfilter/safesearch.go b/dnsfilter/safesearch.go index c4834b2f..d7f4ce07 100644 --- a/dnsfilter/safesearch.go +++ b/dnsfilter/safesearch.go @@ -1,11 +1,11 @@ package dnsfilter var safeSearchDomains = map[string]string{ - "yandex.com": "213.180.193.56", - "yandex.ru": "213.180.193.56", - "yandex.ua": "213.180.193.56", - "yandex.by": "213.180.193.56", - "yandex.kz": "213.180.193.56", + "yandex.com": "213.180.193.56", + "yandex.ru": "213.180.193.56", + "yandex.ua": "213.180.193.56", + "yandex.by": "213.180.193.56", + "yandex.kz": "213.180.193.56", "www.yandex.com": "213.180.193.56", "www.yandex.ru": "213.180.193.56", "www.yandex.ua": "213.180.193.56", diff --git a/dnsforward/dnsforward_test.go b/dnsforward/dnsforward_test.go index df14b8ae..c068d0ef 100644 --- a/dnsforward/dnsforward_test.go +++ b/dnsforward/dnsforward_test.go @@ -157,7 +157,7 @@ func TestSafeSearch(t *testing.T) { client := dns.Client{Net: "udp"} yandexDomains := []string{"yandex.com.", "yandex.by.", "yandex.kz.", "yandex.ru.", "yandex.com."} for _, host := range yandexDomains { - exchangeAndAssertResponse(t, client, addr, host, "213.180.193.56") + exchangeAndAssertResponse(t, &client, addr, host, "213.180.193.56") } // Check aggregated stats @@ -182,7 +182,7 @@ func TestSafeSearch(t *testing.T) { // Test safe search for google. googleDomains := []string{"www.google.com.", "www.google.com.af.", "www.google.be.", "www.google.by."} for _, host := range googleDomains { - exchangeAndAssertResponse(t, client, addr, host, ip.String()) + exchangeAndAssertResponse(t, &client, addr, host, ip.String()) } // Check aggregated stats @@ -191,7 +191,7 @@ func TestSafeSearch(t *testing.T) { assert.Equal(t, s.GetAggregatedStats()["dns_queries"], float64(len(yandexDomains)+len(googleDomains))) // Do one more exchange - exchangeAndAssertResponse(t, client, addr, "google-public-dns-a.google.com.", "8.8.8.8") + exchangeAndAssertResponse(t, &client, addr, "google-public-dns-a.google.com.", "8.8.8.8") // Check aggregated stats assert.Equal(t, s.GetAggregatedStats()["replaced_safesearch"], float64(len(yandexDomains)+len(googleDomains))) @@ -524,7 +524,7 @@ func sendTestMessages(t *testing.T, conn *dns.Conn) { } } -func exchangeAndAssertResponse(t *testing.T, client dns.Client, addr net.Addr, host, ip string) { +func exchangeAndAssertResponse(t *testing.T, client *dns.Client, addr net.Addr, host, ip string) { req := createTestMessage(host) reply, _, err := client.Exchange(req, addr.String()) if err != nil { diff --git a/dnsforward/stats.go b/dnsforward/stats.go index 705a250f..62565a98 100644 --- a/dnsforward/stats.go +++ b/dnsforward/stats.go @@ -61,9 +61,11 @@ func newStats() *stats { } func initPeriodicStats(periodic *periodicStats, period time.Duration) { + periodic.Lock() periodic.entries = statsEntries{} periodic.lastRotate = time.Now() periodic.period = period + periodic.Unlock() } func (s *stats) purgeStats() { @@ -253,6 +255,9 @@ func (s *stats) getAggregatedStats() map[string]interface{} { } func (s *stats) generateMapFromStats(stats *periodicStats, start int, end int) map[string]interface{} { + stats.RLock() + defer stats.RUnlock() + // clamp start = clamp(start, 0, statsHistoryElements) end = clamp(end, 0, statsHistoryElements)