diff --git a/internal/cmd/oohelper/internal/client.go b/internal/cmd/oohelper/internal/client.go index f175d5cb4e..133a521317 100644 --- a/internal/cmd/oohelper/internal/client.go +++ b/internal/cmd/oohelper/internal/client.go @@ -108,7 +108,8 @@ func (oo OOClient) Do(ctx context.Context, config OOConfig) (*CtrlResponse, erro "Accept-Language": {model.HTTPHeaderAcceptLanguage}, "User-Agent": {model.HTTPHeaderUserAgent}, }, - TCPConnect: endpoints, + TCPConnect: endpoints, + XQUICEnabled: true, } data, err := json.Marshal(creq) runtimex.PanicOnError(err, "oohelper: cannot marshal control request") diff --git a/internal/netemx/ooapi.go b/internal/netemx/ooapi.go index f42d48015a..c5653a25ed 100644 --- a/internal/netemx/ooapi.go +++ b/internal/netemx/ooapi.go @@ -35,7 +35,7 @@ func (p *OOAPIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } } -func (p *OOAPIHandler) getApiV1TestHelpers(w http.ResponseWriter, r *http.Request) { +func (p *OOAPIHandler) getApiV1TestHelpers(w http.ResponseWriter, _ *http.Request) { resp := map[string][]model.OOAPIService{ "web-connectivity": { { diff --git a/internal/netemx/oohelperd_test.go b/internal/netemx/oohelperd_test.go index 79aaa4409f..cf24fb3da9 100644 --- a/internal/netemx/oohelperd_test.go +++ b/internal/netemx/oohelperd_test.go @@ -73,13 +73,7 @@ func TestOOHelperDHandler(t *testing.T) { Failure: nil, }, }, - QUICHandshake: map[string]model.THTLSHandshakeResult{ - "93.184.216.34:443": { - ServerName: "www.example.com", - Status: true, - Failure: nil, - }, - }, + QUICHandshake: map[string]model.THTLSHandshakeResult{}, // since https://github.com/ooni/probe-cli/pull/1549 HTTPRequest: model.THHTTPRequestResult{ BodyLength: 1533, DiscoveredH3Endpoint: "www.example.com:443", @@ -93,19 +87,7 @@ func TestOOHelperDHandler(t *testing.T) { }, StatusCode: 200, }, - HTTP3Request: &model.THHTTPRequestResult{ - BodyLength: 1533, - DiscoveredH3Endpoint: "", - Failure: nil, - Title: "Default Web Page", - Headers: map[string]string{ - "Alt-Svc": `h3=":443"`, - "Content-Length": "1533", - "Content-Type": "text/html; charset=utf-8", - "Date": "Thu, 24 Aug 2023 14:35:29 GMT", - }, - StatusCode: 200, - }, + HTTP3Request: nil, // since https://github.com/ooni/probe-cli/pull/1549 DNS: model.THDNSResult{ Failure: nil, Addrs: []string{"93.184.216.34"}, diff --git a/internal/oohelperd/handler.go b/internal/oohelperd/handler.go index dd76c42cdc..ae0ecce64c 100644 --- a/internal/oohelperd/handler.go +++ b/internal/oohelperd/handler.go @@ -11,6 +11,7 @@ import ( "io" "net/http" "net/http/cookiejar" + "os" "strings" "sync/atomic" "time" @@ -31,6 +32,9 @@ const maxAcceptableBodySize = 1 << 24 // // The zero value is invalid; construct using [NewHandler]. type Handler struct { + // EnableQUIC OPTIONALLY enables QUIC. + EnableQUIC bool + // baseLogger is the MANDATORY logger to use. baseLogger model.Logger @@ -69,9 +73,13 @@ type Handler struct { var _ http.Handler = &Handler{} +// enableQUIC allows to control whether to enable QUIC by using environment variables. +var enableQUIC = (os.Getenv("OOHELPERD_ENABLE_QUIC") == "1") + // NewHandler constructs the [handler]. func NewHandler(logger model.Logger, netx *netxlite.Netx) *Handler { return &Handler{ + EnableQUIC: enableQUIC, baseLogger: logger, countRequests: &atomic.Int64{}, indexer: &atomic.Int64{}, diff --git a/internal/oohelperd/handler_test.go b/internal/oohelperd/handler_test.go index 8285e6faae..ce06357922 100644 --- a/internal/oohelperd/handler_test.go +++ b/internal/oohelperd/handler_test.go @@ -254,3 +254,10 @@ func TestHandlerWorkingAsIntended(t *testing.T) { }) } } + +func TestNewHandlerEnableQUIC(t *testing.T) { + handler := NewHandler(log.Log, &netxlite.Netx{Underlying: nil}) + if handler.EnableQUIC != false { + t.Fatal("expected to see false here (is the the environment variable OOHELPERD_ENABLE_QUIC set?!)") + } +} diff --git a/internal/oohelperd/measure.go b/internal/oohelperd/measure.go index 4ed932778a..3ef83c4afc 100644 --- a/internal/oohelperd/measure.go +++ b/internal/oohelperd/measure.go @@ -125,7 +125,7 @@ func measure(ctx context.Context, config *Handler, creq *ctrlRequest) (*ctrlResp // In the v3.17.x and possibly v3.18.x release cycles, QUIC is disabled by // default but clients that know QUIC can enable it. We will eventually remove // this flag and enable QUIC measurements for all clients. - if creq.XQUICEnabled && cresp.HTTPRequest.DiscoveredH3Endpoint != "" { + if config.EnableQUIC && creq.XQUICEnabled && cresp.HTTPRequest.DiscoveredH3Endpoint != "" { // quicconnect: start over all the endpoints for _, endpoint := range endpoints { wg.Add(1) diff --git a/internal/oohelperd/qa_test.go b/internal/oohelperd/qa_test.go new file mode 100644 index 0000000000..51ba666b3f --- /dev/null +++ b/internal/oohelperd/qa_test.go @@ -0,0 +1,137 @@ +package oohelperd_test + +import ( + "bytes" + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/apex/log" + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/must" + "github.com/ooni/probe-cli/v3/internal/netemx" + "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/oohelperd" + "github.com/ooni/probe-cli/v3/internal/optional" + "github.com/ooni/probe-cli/v3/internal/runtimex" +) + +// TestQAEnableDisableQUIC ensures that we can enable and disable QUIC. +func TestQAEnableDisableQUIC(t *testing.T) { + // testcase is a test case for this function + type testcase struct { + name string + enableQUIC optional.Value[bool] + } + + cases := []testcase{{ + name: "with the default settings", + enableQUIC: optional.None[bool](), + }, { + name: "with explicit false", + enableQUIC: optional.Some(false), + }, { + name: "with explicit true", + enableQUIC: optional.Some(true), + }} + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + // create a new testing scenario + env := netemx.MustNewScenario(netemx.InternetScenario) + defer env.Close() + + // create a new handler + handler := oohelperd.NewHandler( + log.Log, + &netxlite.Netx{Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: env.ClientStack}}, + ) + + // optionally and conditionally enable QUIC + if !tc.enableQUIC.IsNone() { + handler.EnableQUIC = tc.enableQUIC.Unwrap() + } + + // create request body + reqbody := &model.THRequest{ + HTTPRequest: "https://www.example.com/", + HTTPRequestHeaders: map[string][]string{ + "Accept-Language": {model.HTTPHeaderAcceptLanguage}, + "Accept": {model.HTTPHeaderAccept}, + "User-Agent": {model.HTTPHeaderUserAgent}, + }, + TCPConnect: []string{netemx.AddressWwwExampleCom}, + XQUICEnabled: true, + } + + // create request + req := runtimex.Try1(http.NewRequest( + "POST", + "http://127.0.0.1:8080/", + bytes.NewReader(must.MarshalJSON(reqbody)), + )) + + // create response recorder + resprec := httptest.NewRecorder() + + // invoke the handler + handler.ServeHTTP(resprec, req) + + // get the response + resp := resprec.Result() + defer resp.Body.Close() + + // make sure the status code indicates success + if resp.StatusCode != 200 { + t.Fatal("expected 200 Ok") + } + + // make sure the content-type is OK + if v := resp.Header.Get("Content-Type"); v != "application/json" { + t.Fatal("unexpected content-type", v) + } + + // read the response body + respbody := runtimex.Try1(netxlite.ReadAllContext(context.Background(), resp.Body)) + + // parse the response body + var jsonresp model.THResponse + must.UnmarshalJSON(respbody, &jsonresp) + + // check whether we have an HTTP3 response + switch { + case !tc.enableQUIC.IsNone() && tc.enableQUIC.Unwrap() && jsonresp.HTTP3Request != nil: + // all good: we have QUIC enabled and we get an HTTP/3 response + + case (tc.enableQUIC.IsNone() || tc.enableQUIC.Unwrap() == false) && jsonresp.HTTP3Request == nil: + // all good: either default behavior or QUIC not enabled and not HTTP/3 response + + default: + t.Fatalf( + "tc.enableQUIC.IsNone() = %v, tc.enableQUIC.UnwrapOr(false) = %v, jsonresp.HTTP3Request = %v", + tc.enableQUIC.IsNone(), + tc.enableQUIC.UnwrapOr(false), + jsonresp.HTTP3Request, + ) + } + + // check whether we have QUIC handshakes + switch { + case !tc.enableQUIC.IsNone() && tc.enableQUIC.Unwrap() && len(jsonresp.QUICHandshake) > 0: + // all good: we have QUIC enabled and we get QUIC handshakes + + case (tc.enableQUIC.IsNone() || tc.enableQUIC.Unwrap() == false) && len(jsonresp.QUICHandshake) <= 0: + // all good: either default behavior or QUIC not enabled and no QUIC handshakes + + default: + t.Fatalf( + "tc.enableQUIC.IsNone() = %v, tc.enableQUIC.UnwrapOr(false) = %v, jsonresp.QUICHandshake = %v", + tc.enableQUIC.IsNone(), + tc.enableQUIC.UnwrapOr(false), + jsonresp.QUICHandshake, + ) + } + }) + } +}