Skip to content

Commit

Permalink
fix(oohelperd,netemx): construct equivalent HTTPTransports (ooni#1464)
Browse files Browse the repository at this point in the history
This diff modifies oohelperd and netemx to ensure we construct
equivalent HTTPTransports.

Let's show that this is actually the case.

Let's start from the HTTP/1.1 and HTTP2 transport.

This is what `oohelperd` does after this diff:

```Go
		NewHTTPClient: func(logger model.Logger) model.HTTPClient {
			// TODO(ooni/probe#2534): the NewHTTPTransportWithResolver has QUIRKS and
			// we should evaluate whether we can avoid using it here
			return NewHTTPClientWithTransportFactory(
				logger,
				netxlite.NewHTTPTransportWithResolver,
			)
		},
```

This is what `netemx` does after this diff:

```Go
	handler.NewHTTPClient = func(logger model.Logger) model.HTTPClient {
		return oohelperd.NewHTTPClientWithTransportFactory(
			logger,
			func(dl model.DebugLogger, r model.Resolver) model.HTTPTransport {
				dialer := netx.NewDialerWithResolver(dl, r)
				tlsDialer := netxlite.NewTLSDialer(dialer, netx.NewTLSHandshakerStdlib(dl))
				// TODO(ooni/probe#2534): NewHTTPTransport is QUIRKY but
				// we probably don't care about using a QUIRKY function here
				return netxlite.NewHTTPTransport(dl, dialer, tlsDialer)
			},
		)
	}
```

We're using the same (now public) `NewHTTPClientWithTransportFactory`
function. So, what remains to be done to reach a QED is to show that
this code called by `oohelperd`:

```Go
			return NewHTTPClientWithTransportFactory(
				logger,
				netxlite.NewHTTPTransportWithResolver,
			)
```

is equivalent to this code called by `netemx`:

```Go
			func(dl model.DebugLogger, r model.Resolver) model.HTTPTransport {
				dialer := netx.NewDialerWithResolver(dl, r)
				tlsDialer := netxlite.NewTLSDialer(dialer, netx.NewTLSHandshakerStdlib(dl))
				// TODO(ooni/probe#2534): NewHTTPTransport is QUIRKY but
				// we probably don't care about using a QUIRKY function here
				return netxlite.NewHTTPTransport(dl, dialer, tlsDialer)
			},
```

This is evident if we expand `netxlite.NewHTTPTransportWithResolver`,
whose implementation is:

```Go
func NewHTTPTransportWithResolver(logger model.DebugLogger, reso model.Resolver) model.HTTPTransport {
	dialer := NewDialerWithResolver(logger, reso)
	thx := NewTLSHandshakerStdlib(logger)
	tlsDialer := NewTLSDialer(dialer, thx)
	return NewHTTPTransport(logger, dialer, tlsDialer)
}
```

in fact, the following lines of code called from `oohelperd`:

```Go
	dialer := NewDialerWithResolver(logger, reso)
	thx := NewTLSHandshakerStdlib(logger)
	tlsDialer := NewTLSDialer(dialer, thx)
	return NewHTTPTransport(logger, dialer, tlsDialer)
```

are equivalent to this `netemx` code:

```Go
				dialer := netx.NewDialerWithResolver(dl, r)
				tlsDialer := netxlite.NewTLSDialer(dialer, netx.NewTLSHandshakerStdlib(dl))
				// TODO(ooni/probe#2534): NewHTTPTransport is QUIRKY but
				// we probably don't care about using a QUIRKY function here
				return netxlite.NewHTTPTransport(dl, dialer, tlsDialer)
```

Modulo the fact that `netemx` code is using methods of the
`*netxlite.Netx` structure rather than bare functions.

Let's now inspect how we construct HTTP3.

This is what `oohelperd` does:

```Go
		NewHTTP3Client: func(logger model.Logger) model.HTTPClient {
			return NewHTTPClientWithTransportFactory(
				logger,
				netxlite.NewHTTP3TransportWithResolver,
			)
		},
```

This is what `netemx` does:

```Go
	handler.NewHTTP3Client = func(logger model.Logger) model.HTTPClient {
		return oohelperd.NewHTTPClientWithTransportFactory(
			logger,
			func(dl model.DebugLogger, r model.Resolver) model.HTTPTransport {
				qd := netx.NewQUICDialerWithResolver(netx.NewUDPListener(), dl, r)
				return netxlite.NewHTTP3Transport(dl, qd, nil)
			},
		)
	}
```

Because we're using the same `NewHTTPClientWithTransportFactory`
factory, we need to show that `oohelperd`'s

```Go
			return NewHTTPClientWithTransportFactory(
				logger,
				netxlite.NewHTTP3TransportWithResolver,
			)
```

is equivalent to `netemx`'s

```Go
		return oohelperd.NewHTTPClientWithTransportFactory(
			logger,
			func(dl model.DebugLogger, r model.Resolver) model.HTTPTransport {
				qd := netx.NewQUICDialerWithResolver(netx.NewUDPListener(), dl, r)
				return netxlite.NewHTTP3Transport(dl, qd, nil)
			},
		)
```

To show that we need to expand `NewHTTP3TransportWithResolver`, which
reads:

```Go
func NewHTTP3TransportWithResolver(logger model.DebugLogger, reso model.Resolver) model.HTTPTransport {
	qd := NewQUICDialerWithResolver(NewUDPListener(), logger, reso)
	return NewHTTP3Transport(logger, qd, nil)
}
```

And then we can conclude that we're good because the code invoked by
`oohelperd`:

```Go
	qd := NewQUICDialerWithResolver(NewUDPListener(), logger, reso)
	return NewHTTP3Transport(logger, qd, nil)
```

is equivalent to the code invoked by `netemx`:

```Go
				qd := netx.NewQUICDialerWithResolver(netx.NewUDPListener(), dl, r)
				return netxlite.NewHTTP3Transport(dl, qd, nil)
```

modulo the fact that `netemx` is using methods defined by the `netx`
object.

Extracted from ooni#1462.
    
Part of ooni/probe#2652.
    
Part of ooni/probe#1517.
  • Loading branch information
bassosimone authored and Murphy-OrangeMud committed Feb 13, 2024
1 parent a30eb55 commit 3ace615
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 27 deletions.
41 changes: 18 additions & 23 deletions internal/netemx/oohelperd.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@ package netemx
import (
"fmt"
"net/http"
"net/http/cookiejar"

"github.com/ooni/netem"
"github.com/ooni/probe-cli/v3/internal/logx"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/netxlite"
"github.com/ooni/probe-cli/v3/internal/oohelperd"
"golang.org/x/net/publicsuffix"
)

// OOHelperDFactory is the factory to create an [http.Handler] implementing the OONI Web Connectivity
Expand All @@ -21,7 +19,7 @@ var _ HTTPHandlerFactory = &OOHelperDFactory{}

// NewHandler implements QAEnvHTTPHandlerFactory.NewHandler.
func (f *OOHelperDFactory) NewHandler(env NetStackServerFactoryEnv, unet *netem.UNetStack) http.Handler {
netx := netxlite.Netx{Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: unet}}
netx := &netxlite.Netx{Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: unet}}
handler := oohelperd.NewHandler()

handler.BaseLogger = &logx.PrefixLogger{
Expand All @@ -46,29 +44,26 @@ func (f *OOHelperDFactory) NewHandler(env NetStackServerFactoryEnv, unet *netem.
}

handler.NewHTTPClient = func(logger model.Logger) model.HTTPClient {
cookieJar, _ := cookiejar.New(&cookiejar.Options{
PublicSuffixList: publicsuffix.List,
})
// TODO(https://github.com/ooni/probe/issues/2534): NewHTTPTransportStdlib is QUIRKY but we probably
// don't care about using a QUIRKY function here
return &http.Client{
Transport: netx.NewHTTPTransportStdlib(logger),
CheckRedirect: nil,
Jar: cookieJar,
Timeout: 0,
}
return oohelperd.NewHTTPClientWithTransportFactory(
logger,
func(dl model.DebugLogger, r model.Resolver) model.HTTPTransport {
dialer := netx.NewDialerWithResolver(dl, r)
tlsDialer := netxlite.NewTLSDialer(dialer, netx.NewTLSHandshakerStdlib(dl))
// TODO(https://github.com/ooni/probe/issues/2534): NewHTTPTransport is QUIRKY but
// we probably don't care about using a QUIRKY function here
return netxlite.NewHTTPTransport(dl, dialer, tlsDialer)
},
)
}

handler.NewHTTP3Client = func(logger model.Logger) model.HTTPClient {
cookieJar, _ := cookiejar.New(&cookiejar.Options{
PublicSuffixList: publicsuffix.List,
})
return &http.Client{
Transport: netx.NewHTTP3TransportStdlib(logger),
CheckRedirect: nil,
Jar: cookieJar,
Timeout: 0,
}
return oohelperd.NewHTTPClientWithTransportFactory(
logger,
func(dl model.DebugLogger, r model.Resolver) model.HTTPTransport {
qd := netx.NewQUICDialerWithResolver(netx.NewUDPListener(), dl, r)
return netxlite.NewHTTP3Transport(dl, qd, nil)
},
)
}

return handler
Expand Down
9 changes: 5 additions & 4 deletions internal/oohelperd/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,14 @@ func NewHandler() *Handler {
NewHTTPClient: func(logger model.Logger) model.HTTPClient {
// TODO(https://github.com/ooni/probe/issues/2534): the NewHTTPTransportWithResolver has QUIRKS and
// we should evaluate whether we can avoid using it here
return newHTTPClientWithTransportFactory(
return NewHTTPClientWithTransportFactory(
logger,
netxlite.NewHTTPTransportWithResolver,
)
},

NewHTTP3Client: func(logger model.Logger) model.HTTPClient {
return newHTTPClientWithTransportFactory(
return NewHTTPClientWithTransportFactory(
logger,
netxlite.NewHTTP3TransportWithResolver,
)
Expand Down Expand Up @@ -218,8 +218,9 @@ func newCookieJar() *cookiejar.Jar {
}))
}

// newHTTPClientWithTransportFactory creates a new HTTP client.
func newHTTPClientWithTransportFactory(
// NewHTTPClientWithTransportFactory creates a new HTTP client
// using the given [model.HTTPTransport] factory.
func NewHTTPClientWithTransportFactory(
logger model.Logger,
txpFactory func(model.DebugLogger, model.Resolver) model.HTTPTransport,
) model.HTTPClient {
Expand Down

0 comments on commit 3ace615

Please sign in to comment.