Skip to content

Commit

Permalink
cleanup: use testingx.NewHTTPProxyHandler as proxy (ooni#1277)
Browse files Browse the repository at this point in the history 
This diff replaces `testingx.HTTPHandlerProxy` with
`testingx.NewHTTPProxyHandler` as the proxy used for implementing netemx
scenarios and removes `testingx.HTTPHandlerProxy`.

We introduced `testingx.NewHTTPProxyHandler` in
ooni#1274. It is a more comprehensive
proxy because it supports both proxying via HTTP header and CONNECT
proxying.

While there, emit more clear debug messages during the TLS handshake.

While there, fix how we're skipping tests in `testingx` and
`testingproxy` because otherwise we end up skipping the netem tests that
we should not be skipping. (Noticed of this because the coverage
dropped!)

Reference issue: ooni/probe#2531

Overall objective: good testing for proxying for when we introduce
beacons.
  • Loading branch information
@bassosimone @Murphy-OrangeMud
bassosimone authored and Murphy-OrangeMud committed Feb 13, 2024
1 parent ad0e1b0 commit 09bf5d8
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 279 deletions.
2 changes: 1 addition & 1 deletion internal/netemx/scenario.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func MustNewScenario(config []*ScenarioDomainAddresses) *QAEnv {
opts = append(opts, QAEnvOptionNetStack(addr,
&HTTPCleartextServerFactory{
Factory: HTTPHandlerFactoryFunc(func(env NetStackServerFactoryEnv, stack *netem.UNetStack) http.Handler {
return testingx.HTTPHandlerProxy(env.Logger(), &netxlite.Netx{
return testingx.NewHTTPProxyHandler(env.Logger(), &netxlite.Netx{
Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: stack}})
}),
Ports: []int{80},
Expand Down
6 changes: 3 additions & 3 deletions internal/netxlite/tls.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,18 +255,18 @@ func (h *tlsHandshakerLogger) Handshake(
ctx context.Context, conn net.Conn, config *tls.Config,
) (net.Conn, tls.ConnectionState, error) {
h.DebugLogger.Debugf(
"tls {sni=%s next=%+v}...", config.ServerName, config.NextProtos)
"tls_handshake {sni=%s next=%+v}...", config.ServerName, config.NextProtos)
start := time.Now()
tlsconn, state, err := h.TLSHandshaker.Handshake(ctx, conn, config)
elapsed := time.Since(start)
if err != nil {
h.DebugLogger.Debugf(
"tls {sni=%s next=%+v}... %s in %s", config.ServerName,
"tls_handshake {sni=%s next=%+v}... %s in %s", config.ServerName,
config.NextProtos, err, elapsed)
return nil, tls.ConnectionState{}, err
}
h.DebugLogger.Debugf(
"tls {sni=%s next=%+v}... ok in %s {next=%s cipher=%s v=%s}",
"tls_handshake {sni=%s next=%+v}... ok in %s {next=%s cipher=%s v=%s}",
config.ServerName, config.NextProtos, elapsed, state.NegotiatedProtocol,
TLSCipherSuiteString(state.CipherSuite),
TLSVersionString(state.Version))
Expand Down
8 changes: 4 additions & 4 deletions internal/testingproxy/qa_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import (

func TestWorkingAsIntended(t *testing.T) {
for _, testCase := range testingproxy.AllTestCases {
short := testCase.Short()
if !short && testing.Short() {
t.Skip("skip test in short mode")
}
t.Run(testCase.Name(), func(t *testing.T) {
short := testCase.Short()
if !short && testing.Short() {
t.Skip("skip test in short mode")
}
testCase.Run(t)
})
}
Expand Down
8 changes: 4 additions & 4 deletions internal/testingx/httpproxy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ import (

func TestHTTPProxyHandler(t *testing.T) {
for _, testCase := range testingproxy.AllTestCases {
short := testCase.Short()
if !short && testing.Short() {
t.Skip("skip test in short mode")
}
t.Run(testCase.Name(), func(t *testing.T) {
short := testCase.Short()
if !short && testing.Short() {
t.Skip("skip test in short mode")
}
testCase.Run(t)
})
}
Expand Down
69 changes: 0 additions & 69 deletions internal/testingx/httptestx.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@ package testingx
import (
"crypto/tls"
"crypto/x509"
"io"
"net"
"net/http"
"net/url"

"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/optional"
"github.com/ooni/probe-cli/v3/internal/runtimex"
)
Expand Down Expand Up @@ -176,70 +174,3 @@ func httpHandlerHijack(w http.ResponseWriter, r *http.Request, policy string) {
// nothing
}
}

// TODO(bassosimone): eventually we may want to have a model type
// that models the equivalent of [netxlite.Netx].

// HTTPHandlerProxyNetx is [netxlite.Netx] as seen by [HTTPHandlerProxy].
type HTTPHandlerProxyNetx interface {
NewHTTPTransportStdlib(logger model.DebugLogger) model.HTTPTransport
}

// HTTPHandlerProxy is a handler implementing an HTTP proxy using the host header
// to determine who to connect to. We additionally use the via header to avoid sending
// requests to ourself. Please, note that we designed this proxy ONLY to be used for
// testing purposes and that it's rather simplistic.
func HTTPHandlerProxy(logger model.Logger, netx HTTPHandlerProxyNetx) http.Handler {
return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
// reject requests that already visited the proxy and requests we cannot route
if req.Host == "" || req.Header.Get("Via") != "" {
rw.WriteHeader(http.StatusBadRequest)
return
}

// be explicit about not supporting request bodies
if req.Method != http.MethodGet {
rw.WriteHeader(http.StatusNotImplemented)
return
}

// clone the request before modifying it
req = req.Clone(req.Context())

// include proxy header to prevent sending requests to ourself
req.Header.Add("Via", "testingx/0.1.0")

// fix: "http: Request.RequestURI can't be set in client requests"
req.RequestURI = ""

// fix: `http: unsupported protocol scheme ""`
req.URL.Host = req.Host

// fix: "http: no Host in request URL"
req.URL.Scheme = "http"

logger.Debugf("PROXY: sending request: %s", req)

// create HTTP client using netx
txp := netx.NewHTTPTransportStdlib(logger)

// obtain response
resp, err := txp.RoundTrip(req)
if err != nil {
logger.Warnf("PROXY: request failed: %s", err.Error())
rw.WriteHeader(http.StatusBadGateway)
return
}

// write response
rw.WriteHeader(resp.StatusCode)
for key, values := range resp.Header {
for _, value := range values {
rw.Header().Add(key, value)
}
}

// write response body
_, _ = io.Copy(rw, resp.Body)
})
}
198 changes: 0 additions & 198 deletions internal/testingx/httptestx_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,12 @@ import (
"io"
"net"
"net/http"
"net/http/httptest"
"net/url"
"testing"
"time"

"github.com/apex/log"
"github.com/google/go-cmp/cmp"
"github.com/ooni/netem"
"github.com/ooni/probe-cli/v3/internal/mocks"
"github.com/ooni/probe-cli/v3/internal/netxlite"
"github.com/ooni/probe-cli/v3/internal/runtimex"
"github.com/ooni/probe-cli/v3/internal/testingx"
Expand Down Expand Up @@ -476,198 +473,3 @@ func TestHTTPTestxWithNetem(t *testing.T) {
})
}
}

func TestHTTPHandlerProxy(t *testing.T) {
expectedBody := []byte("Google is built by a large team of engineers, designers, researchers, robots, and others in many different sites across the globe. It is updated continuously, and built with more tools and technologies than we can shake a stick at. If you'd like to help us out, see careers.google.com.\n")

type testcase struct {
name string
construct func() (*netxlite.Netx, string, []io.Closer)
short bool
}

testcases := []testcase{
{
name: "using the real network",
construct: func() (*netxlite.Netx, string, []io.Closer) {
var closers []io.Closer

netx := &netxlite.Netx{
Underlying: nil, // so we're using the real network
}

proxyServer := testingx.MustNewHTTPServer(testingx.HTTPHandlerProxy(log.Log, netx))
closers = append(closers, proxyServer)

return netx, proxyServer.URL, closers
},
short: false,
},

{
name: "using netem",
construct: func() (*netxlite.Netx, string, []io.Closer) {
var closers []io.Closer

topology := runtimex.Try1(netem.NewStarTopology(log.Log))
closers = append(closers, topology)

wwwStack := runtimex.Try1(topology.AddHost("142.251.209.14", "142.251.209.14", &netem.LinkConfig{}))
proxyStack := runtimex.Try1(topology.AddHost("10.0.0.1", "142.251.209.14", &netem.LinkConfig{}))
clientStack := runtimex.Try1(topology.AddHost("10.0.0.2", "142.251.209.14", &netem.LinkConfig{}))

dnsConfig := netem.NewDNSConfig()
dnsConfig.AddRecord("www.google.com", "", "142.251.209.14")
dnsServer := runtimex.Try1(netem.NewDNSServer(log.Log, wwwStack, "142.251.209.14", dnsConfig))
closers = append(closers, dnsServer)

wwwServer := testingx.MustNewHTTPServerEx(
&net.TCPAddr{IP: net.IPv4(142, 251, 209, 14), Port: 80},
wwwStack,
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Write(expectedBody)
}),
)
closers = append(closers, wwwServer)

proxyServer := testingx.MustNewHTTPServerEx(
&net.TCPAddr{IP: net.IPv4(10, 0, 0, 1), Port: 80},
proxyStack,
testingx.HTTPHandlerProxy(log.Log, &netxlite.Netx{
Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: proxyStack},
}),
)
closers = append(closers, proxyServer)

clientNet := &netxlite.Netx{Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: clientStack}}
return clientNet, proxyServer.URL, closers
},
short: true,
}}

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
if !tc.short && testing.Short() {
t.Skip("skip test in short mode")
}

netx, proxyURL, closers := tc.construct()
defer func() {
for _, closer := range closers {
closer.Close()
}
}()

URL := runtimex.Try1(url.Parse(proxyURL))
URL.Path = "/humans.txt"

req := runtimex.Try1(http.NewRequest("GET", URL.String(), nil))
req.Host = "www.google.com"

//log.SetLevel(log.DebugLevel)

txp := netx.NewHTTPTransportStdlib(log.Log)
client := netxlite.NewHTTPClient(txp)

resp, err := client.Do(req)
if err != nil {
t.Fatal(err)
}
defer resp.Body.Close()

if resp.StatusCode != 200 {
t.Fatal("expected to see 200, got", resp.StatusCode)
}

t.Logf("%+v", resp)

body, err := netxlite.ReadAllContext(req.Context(), resp.Body)
if err != nil {
t.Fatal(err)
}

t.Logf("%s", string(body))

if diff := cmp.Diff(expectedBody, body); diff != "" {
t.Fatal(diff)
}
})
}

t.Run("rejects requests without a host header", func(t *testing.T) {
rr := httptest.NewRecorder()
netx := &netxlite.Netx{Underlying: &mocks.UnderlyingNetwork{
// all nil: panic if we hit the network
}}
handler := testingx.HTTPHandlerProxy(log.Log, netx)
req := &http.Request{
Host: "", // explicitly empty
}
handler.ServeHTTP(rr, req)
res := rr.Result()
if res.StatusCode != http.StatusBadRequest {
t.Fatal("unexpected status code", res.StatusCode)
}
})

t.Run("rejects requests with a via header", func(t *testing.T) {
rr := httptest.NewRecorder()
netx := &netxlite.Netx{Underlying: &mocks.UnderlyingNetwork{
// all nil: panic if we hit the network
}}
handler := testingx.HTTPHandlerProxy(log.Log, netx)
req := &http.Request{
Host: "www.example.com",
Header: http.Header{
"Via": {"antani/0.1.0"},
},
}
handler.ServeHTTP(rr, req)
res := rr.Result()
if res.StatusCode != http.StatusBadRequest {
t.Fatal("unexpected status code", res.StatusCode)
}
})

t.Run("rejects requests with a POST method", func(t *testing.T) {
rr := httptest.NewRecorder()
netx := &netxlite.Netx{Underlying: &mocks.UnderlyingNetwork{
// all nil: panic if we hit the network
}}
handler := testingx.HTTPHandlerProxy(log.Log, netx)
req := &http.Request{
Host: "www.example.com",
Header: http.Header{},
Method: http.MethodPost,
}
handler.ServeHTTP(rr, req)
res := rr.Result()
if res.StatusCode != http.StatusNotImplemented {
t.Fatal("unexpected status code", res.StatusCode)
}
})

t.Run("returns 502 when the round trip fails", func(t *testing.T) {
rr := httptest.NewRecorder()
netx := &netxlite.Netx{Underlying: &mocks.UnderlyingNetwork{
MockGetaddrinfoLookupANY: func(ctx context.Context, domain string) ([]string, string, error) {
return nil, "", errors.New("mocked error")
},
MockGetaddrinfoResolverNetwork: func() string {
return "antani"
},
}}
handler := testingx.HTTPHandlerProxy(log.Log, netx)
req := &http.Request{
Host: "www.example.com",
Header: http.Header{},
Method: http.MethodGet,
URL: &url.URL{},
}
handler.ServeHTTP(rr, req)
res := rr.Result()
if res.StatusCode != http.StatusBadGateway {
t.Fatal("unexpected status code", res.StatusCode)
}
})
}

0 comments on commit 09bf5d8

Please sign in to comment.