Skip to content

Commit

Permalink
fix(oohelperd): disable QUIC by default (#1549)
Browse files Browse the repository at this point in the history
  • Loading branch information
bassosimone authored Apr 12, 2024
1 parent d08ce02 commit 2211b97
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 23 deletions.
3 changes: 2 additions & 1 deletion internal/cmd/oohelper/internal/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion internal/netemx/ooapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
{
Expand Down
22 changes: 2 additions & 20 deletions internal/netemx/oohelperd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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"},
Expand Down
8 changes: 8 additions & 0 deletions internal/oohelperd/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"io"
"net/http"
"net/http/cookiejar"
"os"
"strings"
"sync/atomic"
"time"
Expand All @@ -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

Expand Down Expand Up @@ -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{},
Expand Down
7 changes: 7 additions & 0 deletions internal/oohelperd/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,3 +262,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?!)")
}
}
2 changes: 1 addition & 1 deletion internal/oohelperd/measure.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
137 changes: 137 additions & 0 deletions internal/oohelperd/qa_test.go
Original file line number Diff line number Diff line change
@@ -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,
)
}
})
}
}

0 comments on commit 2211b97

Please sign in to comment.