Skip to content

Commit

Permalink
BREAKING CHANGE: feat: move StreamAllContext to netxlite (#1490)
Browse files Browse the repository at this point in the history
This diff moves the `StreamAllContext` implementation from
`webconnectivitylte` to `netxlite`.

We flagged this diff as BREAKING CHANGE because we also fixed a bug
where, in case of context timeout, `StreamAllContext` was suppressing
the error rather than reporting it. Re-reading the code, that felt
incorrect because it did not allow us to clearly know that we timed out
reading the body, which could be caused by throttling.

The new behavior seems more accurate.

Because of this change, I also felt it was time to add explicit tests
for cases where we download fails because it takes too much time for us
to fetch the whole response body. I am not 100% sure we are correctly
flagging this case, but an `http-failure` probably seems fine at the
moment and we can always revisit this as we learn more about throttling.

Part of ooni/probe#2654.
  • Loading branch information
bassosimone committed Feb 5, 2024
1 parent 6ebf265 commit 8b13815
Show file tree
Hide file tree
Showing 19 changed files with 6,094 additions and 89 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ require (
github.com/miekg/dns v1.1.57
github.com/mitchellh/go-wordwrap v1.0.1
github.com/montanaflynn/stats v0.7.1
github.com/ooni/netem v0.0.0-20231213064826-3cc1ea5e942c
github.com/ooni/netem v0.0.0-20240205182847-14e4ce92d41e
github.com/ooni/oocrypto v0.5.7
github.com/ooni/oohttp v0.6.7
github.com/ooni/probe-assets v0.21.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -356,8 +356,8 @@ github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7J
github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
github.com/onsi/gomega v1.17.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY=
github.com/onsi/gomega v1.29.0 h1:KIA/t2t5UBzoirT4H9tsML45GEbo3ouUnBHsCfD2tVg=
github.com/ooni/netem v0.0.0-20231213064826-3cc1ea5e942c h1:T6Li/1zerzLDRaST/B2TQlmKE1XIdqeewQtd9c59t9U=
github.com/ooni/netem v0.0.0-20231213064826-3cc1ea5e942c/go.mod h1:b/wAvTR5n92Vk2b0SBmuMU0xO4ZGVrsXtU7zjTby7vw=
github.com/ooni/netem v0.0.0-20240205182847-14e4ce92d41e h1:OwDVOPs8NBLb1yEZXnn7rbfbBQJVOpIgEZsqq6QpqG8=
github.com/ooni/netem v0.0.0-20240205182847-14e4ce92d41e/go.mod h1:b/wAvTR5n92Vk2b0SBmuMU0xO4ZGVrsXtU7zjTby7vw=
github.com/ooni/oocrypto v0.5.7 h1:QEb1KTh5gZ9s1IQjk7rNF076YVwit+2sDKNbo39IEa8=
github.com/ooni/oocrypto v0.5.7/go.mod h1:HjEQ5pQBl6btcWgAsKKq1tFo8CfBrZu63C/vPAUGIDk=
github.com/ooni/oohttp v0.6.7 h1:wmCjx9+gzx7p1xc/kMAmgXSgXKu7G8CAmil4Zii3g10=
Expand Down
2 changes: 1 addition & 1 deletion internal/experiment/webconnectivitylte/cleartextflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ func (t *CleartextFlow) httpTransaction(ctx context.Context, network, address, a
t.CookieJar.SetCookies(req.URL, cookies)
}
reader := io.LimitReader(resp.Body, maxbody)
body, err = StreamAllContext(ctx, reader)
body, err = netxlite.StreamAllContext(ctx, reader)
}
if err == nil && httpRedirectIsRedirect(resp) {
err = httpValidateRedirect(resp)
Expand Down
81 changes: 0 additions & 81 deletions internal/experiment/webconnectivitylte/iox.go

This file was deleted.

2 changes: 1 addition & 1 deletion internal/experiment/webconnectivitylte/secureflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ func (t *SecureFlow) httpTransaction(ctx context.Context, network, address, alpn
t.CookieJar.SetCookies(req.URL, cookies)
}
reader := io.LimitReader(resp.Body, maxbody)
body, err = StreamAllContext(ctx, reader)
body, err = netxlite.StreamAllContext(ctx, reader)
}
if err == nil && httpRedirectIsRedirect(resp) {
err = httpValidateRedirect(resp)
Expand Down
3 changes: 3 additions & 0 deletions internal/experiment/webconnectivityqa/testcase.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ func AllTestCases() []*TestCase {
tcpBlockingConnectTimeout(),
tcpBlockingConnectionRefusedWithInconsistentDNS(),

throttlingWithHTTP(),
throttlingWithHTTPS(),

tlsBlockingConnectionResetWithConsistentDNS(),
tlsBlockingConnectionResetWithInconsistentDNS(),

Expand Down
72 changes: 72 additions & 0 deletions internal/experiment/webconnectivityqa/throttling.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package webconnectivityqa

import (
"time"

"github.com/apex/log"
"github.com/ooni/netem"
"github.com/ooni/probe-cli/v3/internal/netemx"
)

// throttlingWithHTTP is the case where an HTTP website has throttling configured for it.
func throttlingWithHTTP() *TestCase {
return &TestCase{
Name: "throttlingWithHTTP",
Flags: TestCaseFlagNoV04,
Input: "http://largefile.com/",
Configure: func(env *netemx.QAEnv) {

env.DPIEngine().AddRule(&netem.DPIThrottleTrafficForTCPEndpoint{
Delay: 300 * time.Millisecond,
Logger: log.Log,
PLR: 0.1,
ServerIPAddress: netemx.AddressLargeFileCom1,
ServerPort: 80,
})

env.DPIEngine().AddRule(&netem.DPIThrottleTrafficForTCPEndpoint{
Delay: 300 * time.Millisecond,
Logger: log.Log,
PLR: 0.1,
ServerIPAddress: netemx.AddressLargeFileCom2,
ServerPort: 80,
})

},
ExpectErr: false,
ExpectTestKeys: &testKeys{
DNSConsistency: "consistent",
HTTPExperimentFailure: "generic_timeout_error",
XBlockingFlags: 8, // AnalysisBlockingFlagHTTPBlocking
Accessible: false,
Blocking: "http-failure",
},
}
}

// throttlingWithHTTPS is the case where an HTTPS website has throttling configured for it.
func throttlingWithHTTPS() *TestCase {
return &TestCase{
Name: "throttlingWithHTTPS",
Flags: TestCaseFlagNoV04,
Input: "https://largefile.com/",
Configure: func(env *netemx.QAEnv) {

env.DPIEngine().AddRule(&netem.DPIThrottleTrafficForTLSSNI{
Delay: 300 * time.Millisecond,
Logger: log.Log,
PLR: 0.1,
SNI: "largefile.com",
})

},
ExpectErr: false,
ExpectTestKeys: &testKeys{
DNSConsistency: "consistent",
HTTPExperimentFailure: "generic_timeout_error",
XBlockingFlags: 8, // AnalysisBlockingFlagHTTPBlocking
Accessible: false,
Blocking: "http-failure",
},
}
}
Loading

0 comments on commit 8b13815

Please sign in to comment.