From 229ac53f1312fabae39270fd595322406ef8a5d3 Mon Sep 17 00:00:00 2001 From: kelmenhorst <45046038+kelmenhorst@users.noreply.github.com> Date: Fri, 8 Sep 2023 14:23:30 -0300 Subject: [PATCH] fix(webconnectivityqa): Always close the QAEnv (#1245) This diff fixes a bug in the webconnectivityqa tests which we discovered due to the new quic-go API panicking (https://github.com/ooni/probe-cli/pull/1161#issuecomment-1710075811). The bug was that we did not close the `QAEnv` in some of our webconnectivityqa tests. Therefore, the QUIC Server would not close the underlying UDP socket (`net.PacketConn`) and the local address of this socket would not be removed from the QUIC multiplexer. In the subsequent test case, the new quic-go API (see https://github.com/ooni/probe-cli/pull/1161) would panic because the local address of the server would still be registered in the global QUIC multiplexer. --- internal/experiment/webconnectivityqa/badssl_test.go | 2 ++ internal/experiment/webconnectivityqa/control_test.go | 4 ++++ .../experiment/webconnectivityqa/dnsblocking_test.go | 4 ++++ .../experiment/webconnectivityqa/dnshijacking_test.go | 2 ++ internal/experiment/webconnectivityqa/httpdiff_test.go | 4 ++++ internal/experiment/webconnectivityqa/redirect_test.go | 10 ++++++++++ .../experiment/webconnectivityqa/tcpblocking_test.go | 4 ++++ .../experiment/webconnectivityqa/tlsblocking_test.go | 4 ++++ 8 files changed, 34 insertions(+) diff --git a/internal/experiment/webconnectivityqa/badssl_test.go b/internal/experiment/webconnectivityqa/badssl_test.go index 8dc0e27af8..195e45fc01 100644 --- a/internal/experiment/webconnectivityqa/badssl_test.go +++ b/internal/experiment/webconnectivityqa/badssl_test.go @@ -33,6 +33,8 @@ func TestBadSSLConditions(t *testing.T) { for _, tc := range testcases { t.Run(tc.testCase.Name, func(t *testing.T) { env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + tc.testCase.Configure(env) env.Do(func() { diff --git a/internal/experiment/webconnectivityqa/control_test.go b/internal/experiment/webconnectivityqa/control_test.go index 73d91aa0b1..b6bbec240d 100644 --- a/internal/experiment/webconnectivityqa/control_test.go +++ b/internal/experiment/webconnectivityqa/control_test.go @@ -12,6 +12,8 @@ import ( func TestControlFailureWithSuccessfulHTTPWebsite(t *testing.T) { env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + tc := controlFailureWithSuccessfulHTTPWebsite() tc.Configure(env) @@ -33,6 +35,8 @@ func TestControlFailureWithSuccessfulHTTPWebsite(t *testing.T) { func TestControlFailureWithSuccessfulHTTPSWebsite(t *testing.T) { env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + tc := controlFailureWithSuccessfulHTTPSWebsite() tc.Configure(env) diff --git a/internal/experiment/webconnectivityqa/dnsblocking_test.go b/internal/experiment/webconnectivityqa/dnsblocking_test.go index e3948f0941..0f4b6adf6b 100644 --- a/internal/experiment/webconnectivityqa/dnsblocking_test.go +++ b/internal/experiment/webconnectivityqa/dnsblocking_test.go @@ -12,6 +12,8 @@ import ( func TestDNSBlockingAndroidDNSCacheNoData(t *testing.T) { env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + tc := dnsBlockingAndroidDNSCacheNoData() tc.Configure(env) @@ -29,6 +31,8 @@ func TestDNSBlockingAndroidDNSCacheNoData(t *testing.T) { func TestDNSBlockingNXDOMAIN(t *testing.T) { env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + tc := dnsBlockingNXDOMAIN() tc.Configure(env) diff --git a/internal/experiment/webconnectivityqa/dnshijacking_test.go b/internal/experiment/webconnectivityqa/dnshijacking_test.go index d25359c378..f477408d48 100644 --- a/internal/experiment/webconnectivityqa/dnshijacking_test.go +++ b/internal/experiment/webconnectivityqa/dnshijacking_test.go @@ -19,6 +19,8 @@ func TestDNSHijackingTestCases(t *testing.T) { for _, tc := range testcases { t.Run(tc.Name, func(t *testing.T) { env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + tc.Configure(env) env.Do(func() { diff --git a/internal/experiment/webconnectivityqa/httpdiff_test.go b/internal/experiment/webconnectivityqa/httpdiff_test.go index 63295dda6a..54e80e253d 100644 --- a/internal/experiment/webconnectivityqa/httpdiff_test.go +++ b/internal/experiment/webconnectivityqa/httpdiff_test.go @@ -20,6 +20,8 @@ func TestHTTPDiffWithConsistentDNS(t *testing.T) { for _, tc := range testcases { t.Run(tc.Name, func(t *testing.T) { env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + tc.Configure(env) env.Do(func() { @@ -50,6 +52,8 @@ func TestHTTPDiffWithInconsistentDNS(t *testing.T) { for _, tc := range testcases { t.Run(tc.Name, func(t *testing.T) { env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + tc.Configure(env) env.Do(func() { diff --git a/internal/experiment/webconnectivityqa/redirect_test.go b/internal/experiment/webconnectivityqa/redirect_test.go index 288e4d4526..a6418c1375 100644 --- a/internal/experiment/webconnectivityqa/redirect_test.go +++ b/internal/experiment/webconnectivityqa/redirect_test.go @@ -23,6 +23,8 @@ func TestRedirectWithConsistentDNSAndThenConnectionRefused(t *testing.T) { for _, tc := range testcases { t.Run(tc.Name, func(t *testing.T) { env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + tc.Configure(env) env.Do(func() { @@ -55,6 +57,8 @@ func TestRedirectWithConsistentDNSAndThenConnectionReset(t *testing.T) { for _, tc := range testcases { t.Run(tc.Name, func(t *testing.T) { env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + tc.Configure(env) env.Do(func() { @@ -86,6 +90,8 @@ func TestRedirectWithConsistentDNSAndThenNXDOMAIN(t *testing.T) { for _, tc := range testcases { t.Run(tc.Name, func(t *testing.T) { env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + tc.Configure(env) env.Do(func() { @@ -125,6 +131,8 @@ func TestRedirectWithConsistentDNSAndThenEOF(t *testing.T) { for _, tc := range testcases { t.Run(tc.Name, func(t *testing.T) { env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + tc.Configure(env) env.Do(func() { @@ -157,6 +165,8 @@ func TestRedirectWithConsistentDNSAndThenTimeout(t *testing.T) { for _, tc := range testcases { t.Run(tc.Name, func(t *testing.T) { env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + tc.Configure(env) env.Do(func() { diff --git a/internal/experiment/webconnectivityqa/tcpblocking_test.go b/internal/experiment/webconnectivityqa/tcpblocking_test.go index 80930d3a38..5e71802b28 100644 --- a/internal/experiment/webconnectivityqa/tcpblocking_test.go +++ b/internal/experiment/webconnectivityqa/tcpblocking_test.go @@ -12,6 +12,8 @@ import ( func TestTCPBlockingConnectTimeout(t *testing.T) { env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + tc := tcpBlockingConnectTimeout() tc.Configure(env) @@ -30,6 +32,8 @@ func TestTCPBlockingConnectTimeout(t *testing.T) { func TestTCPBlockingConnectionRefusedWithInconsistentDNS(t *testing.T) { env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + tc := tcpBlockingConnectionRefusedWithInconsistentDNS() tc.Configure(env) diff --git a/internal/experiment/webconnectivityqa/tlsblocking_test.go b/internal/experiment/webconnectivityqa/tlsblocking_test.go index fbb8f8c3fd..abd555c91b 100644 --- a/internal/experiment/webconnectivityqa/tlsblocking_test.go +++ b/internal/experiment/webconnectivityqa/tlsblocking_test.go @@ -14,6 +14,8 @@ import ( func TestBlockingTLSConnectionResetWithConsistentDNS(t *testing.T) { env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + tc := tlsBlockingConnectionResetWithConsistentDNS() tc.Configure(env) @@ -40,6 +42,8 @@ func TestBlockingTLSConnectionResetWithConsistentDNS(t *testing.T) { func TestBlockingTLSConnectionResetWithInconsistentDNS(t *testing.T) { env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + tc := tlsBlockingConnectionResetWithInconsistentDNS() tc.Configure(env)