Skip to content

Commit

Permalink
refactor(netxlite): split http code into multiple files (#1267)
Browse files Browse the repository at this point in the history 
This diff splits http.go and http_test.go into multiple files to create
the logical and mental space required to implement new construction
routines suitable for the engine package.

The history of this diff is simple. I started working on forking http.go
for the `./internal/enginenetx` package. Then, I realized it would have
been possible to have those changes directly inside netxlite, if only I
did refactor the http implementation to have a bit more clarity and
file-based modularity. And so I did.

Part of ooni/probe#2531
  • Loading branch information
@bassosimone
bassosimone committed Sep 14, 2023
1 parent cd5715a commit 873b4e1
Show file tree
Hide file tree
Showing 11 changed files with 744 additions and 646 deletions.
229 changes: 0 additions & 229 deletions internal/netxlite/http.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,106 +5,12 @@ package netxlite
//

import (
"context"
"errors"
"net"
"net/http"
"time"

oohttp "github.com/ooni/oohttp"
"github.com/ooni/probe-cli/v3/internal/model"
)

// httpTransportErrWrapper is an HTTPTransport with error wrapping.
type httpTransportErrWrapper struct {
HTTPTransport model.HTTPTransport
}

var _ model.HTTPTransport = &httpTransportErrWrapper{}

func (txp *httpTransportErrWrapper) RoundTrip(req *http.Request) (*http.Response, error) {
resp, err := txp.HTTPTransport.RoundTrip(req)
if err != nil {
return nil, NewTopLevelGenericErrWrapper(err)
}
return resp, nil
}

func (txp *httpTransportErrWrapper) CloseIdleConnections() {
txp.HTTPTransport.CloseIdleConnections()
}

func (txp *httpTransportErrWrapper) Network() string {
return txp.HTTPTransport.Network()
}

// httpTransportLogger is an HTTPTransport with logging.
type httpTransportLogger struct {
// HTTPTransport is the underlying HTTP transport.
HTTPTransport model.HTTPTransport

// Logger is the underlying logger.
Logger model.DebugLogger
}

var _ model.HTTPTransport = &httpTransportLogger{}

func (txp *httpTransportLogger) RoundTrip(req *http.Request) (*http.Response, error) {
txp.Logger.Debugf("> %s %s", req.Method, req.URL.String())
for key, values := range req.Header {
for _, value := range values {
txp.Logger.Debugf("> %s: %s", key, value)
}
}
txp.Logger.Debug(">")
resp, err := txp.HTTPTransport.RoundTrip(req)
if err != nil {
txp.Logger.Debugf("< %s", err)
return nil, err
}
txp.Logger.Debugf("< %d", resp.StatusCode)
for key, values := range resp.Header {
for _, value := range values {
txp.Logger.Debugf("< %s: %s", key, value)
}
}
txp.Logger.Debug("<")
return resp, nil
}

func (txp *httpTransportLogger) CloseIdleConnections() {
txp.HTTPTransport.CloseIdleConnections()
}

func (txp *httpTransportLogger) Network() string {
return txp.HTTPTransport.Network()
}

// httpTransportConnectionsCloser is an HTTPTransport that
// correctly forwards CloseIdleConnections calls.
type httpTransportConnectionsCloser struct {
HTTPTransport model.HTTPTransport
Dialer model.Dialer
TLSDialer model.TLSDialer
}

var _ model.HTTPTransport = &httpTransportConnectionsCloser{}

func (txp *httpTransportConnectionsCloser) RoundTrip(req *http.Request) (*http.Response, error) {
return txp.HTTPTransport.RoundTrip(req)
}

func (txp *httpTransportConnectionsCloser) Network() string {
return txp.HTTPTransport.Network()
}

// CloseIdleConnections forwards the CloseIdleConnections calls.
func (txp *httpTransportConnectionsCloser) CloseIdleConnections() {
txp.HTTPTransport.CloseIdleConnections()
txp.Dialer.CloseIdleConnections()
txp.TLSDialer.CloseIdleConnections()
}

// NewHTTPTransportWithResolver creates a new HTTP transport using
// the stdlib for everything but the given resolver.
func NewHTTPTransportWithResolver(logger model.DebugLogger, reso model.Resolver) model.HTTPTransport {
Expand Down Expand Up @@ -185,26 +91,6 @@ func newOOHTTPBaseTransport(dialer model.Dialer, tlsDialer model.TLSDialer) mode
}
}

// stdlibTransport wraps oohttp.StdlibTransport to add .Network()
type httpTransportStdlib struct {
StdlibTransport *oohttp.StdlibTransport
}

var _ model.HTTPTransport = &httpTransportStdlib{}

func (txp *httpTransportStdlib) CloseIdleConnections() {
txp.StdlibTransport.CloseIdleConnections()
}

func (txp *httpTransportStdlib) RoundTrip(req *http.Request) (*http.Response, error) {
return txp.StdlibTransport.RoundTrip(req)
}

// Network implements HTTPTransport.Network.
func (txp *httpTransportStdlib) Network() string {
return "tcp"
}

// WrapHTTPTransport creates an HTTPTransport using the given logger
// and guarantees that returned errors are wrapped.
//
Expand All @@ -216,105 +102,6 @@ func WrapHTTPTransport(logger model.DebugLogger, txp model.HTTPTransport) model.
}
}

// httpDialerWithReadTimeout enforces a read timeout for all HTTP
// connections. See https://github.com/ooni/probe/issues/1609.
type httpDialerWithReadTimeout struct {
Dialer model.Dialer
}

var _ model.Dialer = &httpDialerWithReadTimeout{}

func (d *httpDialerWithReadTimeout) CloseIdleConnections() {
d.Dialer.CloseIdleConnections()
}

// DialContext implements Dialer.DialContext.
func (d *httpDialerWithReadTimeout) DialContext(
ctx context.Context, network, address string) (net.Conn, error) {
conn, err := d.Dialer.DialContext(ctx, network, address)
if err != nil {
return nil, err
}
return &httpConnWithReadTimeout{conn}, nil
}

// httpTLSDialerWithReadTimeout enforces a read timeout for all HTTP
// connections. See https://github.com/ooni/probe/issues/1609.
type httpTLSDialerWithReadTimeout struct {
TLSDialer model.TLSDialer
}

var _ model.TLSDialer = &httpTLSDialerWithReadTimeout{}

func (d *httpTLSDialerWithReadTimeout) CloseIdleConnections() {
d.TLSDialer.CloseIdleConnections()
}

// ErrNotTLSConn occur when an interface accepts a net.Conn but
// internally needs a TLSConn and you pass a net.Conn that doesn't
// implement TLSConn to such an interface.
var ErrNotTLSConn = errors.New("not a TLSConn")

// DialTLSContext implements TLSDialer's DialTLSContext.
func (d *httpTLSDialerWithReadTimeout) DialTLSContext(
ctx context.Context, network, address string) (net.Conn, error) {
conn, err := d.TLSDialer.DialTLSContext(ctx, network, address)
if err != nil {
return nil, err
}
tconn, okay := conn.(TLSConn) // part of the contract but let's be graceful
if !okay {
conn.Close() // we own the conn here
return nil, ErrNotTLSConn
}
return &httpTLSConnWithReadTimeout{tconn}, nil
}

// httpConnWithReadTimeout enforces a read timeout for all HTTP
// connections. See https://github.com/ooni/probe/issues/1609.
type httpConnWithReadTimeout struct {
net.Conn
}

// httpConnReadTimeout is the read timeout we apply to all HTTP
// conns (see https://github.com/ooni/probe/issues/1609).
//
// This timeout is meant as a fallback mechanism so that a stuck
// connection will _eventually_ fail. This is why it is set to
// a large value (300 seconds when writing this note).
//
// There should be other mechanisms to ensure that the code is
// lively: the context during the RoundTrip and iox.ReadAllContext
// when reading the body. They should kick in earlier. But we
// additionally want to avoid leaking a (parked?) connection and
// the corresponding goroutine, hence this large timeout.
//
// A future @bassosimone may understand this problem even better
// and possibly apply an even better fix to this issue. This
// will happen when we'll be able to further study the anomalies
// described in https://github.com/ooni/probe/issues/1609.
const httpConnReadTimeout = 300 * time.Second

// Read implements Conn.Read.
func (c *httpConnWithReadTimeout) Read(b []byte) (int, error) {
c.Conn.SetReadDeadline(time.Now().Add(httpConnReadTimeout))
defer c.Conn.SetReadDeadline(time.Time{})
return c.Conn.Read(b)
}

// httpTLSConnWithReadTimeout enforces a read timeout for all HTTP
// connections. See https://github.com/ooni/probe/issues/1609.
type httpTLSConnWithReadTimeout struct {
TLSConn
}

// Read implements Conn.Read.
func (c *httpTLSConnWithReadTimeout) Read(b []byte) (int, error) {
c.TLSConn.SetReadDeadline(time.Now().Add(httpConnReadTimeout))
defer c.TLSConn.SetReadDeadline(time.Time{})
return c.TLSConn.Read(b)
}

// NewHTTPTransportStdlib creates a new HTTPTransport using
// the stdlib for DNS resolutions and TLS.
//
Expand Down Expand Up @@ -357,19 +144,3 @@ func NewHTTPClient(txp model.HTTPTransport) model.HTTPClient {
func WrapHTTPClient(clnt model.HTTPClient) model.HTTPClient {
return &httpClientErrWrapper{clnt}
}

type httpClientErrWrapper struct {
HTTPClient model.HTTPClient
}

func (c *httpClientErrWrapper) Do(req *http.Request) (*http.Response, error) {
resp, err := c.HTTPClient.Do(req)
if err != nil {
return nil, NewTopLevelGenericErrWrapper(err)
}
return resp, nil
}

func (c *httpClientErrWrapper) CloseIdleConnections() {
c.HTTPClient.CloseIdleConnections()
}
Loading

0 comments on commit 873b4e1

Please sign in to comment.