c/ooni-probe-cli
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

refactor(session.go): replace engine/netx with netxlite (#767)

Browse Source 
This diff replaces engine/netx code with netxlite code in
the engine/session.go file. To this end, we needed to move
some code from engine/netx to netxlite. While there, we
did review and improve the unit tests.

A notable change in this diff is (or seems to be) that in
engine/session.go we're not filtering for bogons anymore so
that, in principle, we could believe a resolver returning
to us bogon IP addresses for OONI services. However, I did
not bother with changing bogons filtering because the
sessionresolver package is already filtering for bogons,
so it is actually okay to avoid doing that again the
session.go code. See:

https://github.com/ooni/probe-cli/blob/v3.15.0-alpha.1/internal/engine/internal/sessionresolver/resolvermaker.go#L88

There are two reference issues for this cleanup:

1. https://github.com/ooni/probe/issues/2115

2. https://github.com/ooni/probe/issues/2121
This commit is contained in:
Simone Basso
2022-05-30 22:00:45 +02:00
committed by GitHub
parent 595d0744db
commit 314c3c934d
16 changed files with 466 additions and 347 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/engine/netx/dialer/proxy.go
+2 -53
View File
@@ -1,56 +1,5 @@
package dialer
import (
"context"
"errors"
"net"
"net/url"
import "github.com/ooni/probe-cli/v3/internal/netxlite"
"github.com/ooni/probe-cli/v3/internal/model"
"golang.org/x/net/proxy"
)
// proxyDialer is a dialer that uses a proxy. If the ProxyURL is not configured, this
// dialer is a passthrough for the next Dialer in chain. Otherwise, it will internally
// create a SOCKS5 dialer that will connect to the proxy using the underlying Dialer.
type proxyDialer struct {
model.Dialer
ProxyURL *url.URL
}
// ErrProxyUnsupportedScheme indicates we don't support a protocol scheme.
var ErrProxyUnsupportedScheme = errors.New("proxy: unsupported scheme")
// DialContext implements Dialer.DialContext
func (d *proxyDialer) DialContext(ctx context.Context, network, address string) (net.Conn, error) {
url := d.ProxyURL
if url == nil {
return d.Dialer.DialContext(ctx, network, address)
}
if url.Scheme != "socks5" {
return nil, ErrProxyUnsupportedScheme
}
// the code at proxy/socks5.go never fails; see https://git.io/JfJ4g
child, _ := proxy.SOCKS5(
network, url.Host, nil, &proxyDialerWrapper{d.Dialer})
return d.dial(ctx, child, network, address)
}
func (d *proxyDialer) dial(
ctx context.Context, child proxy.Dialer, network, address string) (net.Conn, error) {
cd := child.(proxy.ContextDialer) // will work
return cd.DialContext(ctx, network, address)
}
// proxyDialerWrapper is required because SOCKS5 expects a Dialer.Dial type but internally
// it checks whether DialContext is available and prefers that. So, we need to use this
// structure to cast our inner Dialer the way in which SOCKS5 likes it.
//
// See https://git.io/JfJ4g.
type proxyDialerWrapper struct {
model.Dialer
}
func (d *proxyDialerWrapper) Dial(network, address string) (net.Conn, error) {
panic(errors.New("proxyDialerWrapper.Dial should not be called directly"))
}
type proxyDialer = netxlite.MaybeProxyDialer
internal/engine/netx/dialer/proxy_test.go
-81
View File
@@ -1,81 +0,0 @@
package dialer
import (
"context"
"errors"
"io"
"net"
"net/url"
"testing"
"github.com/ooni/probe-cli/v3/internal/model/mocks"
)
func TestProxyDialerDialContextNoProxyURL(t *testing.T) {
expected := errors.New("mocked error")
d := &proxyDialer{
Dialer: &mocks.Dialer{
MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) {
return nil, expected
},
},
}
conn, err := d.DialContext(context.Background(), "tcp", "www.google.com:443")
if !errors.Is(err, expected) {
t.Fatal(err)
}
if conn != nil {
t.Fatal("conn is not nil")
}
}
func TestProxyDialerDialContextInvalidScheme(t *testing.T) {
d := &proxyDialer{
ProxyURL: &url.URL{Scheme: "antani"},
}
conn, err := d.DialContext(context.Background(), "tcp", "www.google.com:443")
if !errors.Is(err, ErrProxyUnsupportedScheme) {
t.Fatal("not the error we expected")
}
if conn != nil {
t.Fatal("conn is not nil")
}
}
func TestProxyDialerDialContextWithEOF(t *testing.T) {
const expect = "10.0.0.1:9050"
d := &proxyDialer{
Dialer: &mocks.Dialer{
MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) {
if address != expect {
return nil, errors.New("unexpected address")
}
return nil, io.EOF
},
},
ProxyURL: &url.URL{Scheme: "socks5", Host: expect},
}
conn, err := d.DialContext(context.Background(), "tcp", "www.google.com:443")
if !errors.Is(err, io.EOF) {
t.Fatal("not the error we expected")
}
if conn != nil {
t.Fatal("conn is not nil")
}
}
func TestProxyDialWrapperPanics(t *testing.T) {
d := &proxyDialerWrapper{}
err := func() (rv error) {
defer func() {
if r := recover(); r != nil {
rv = r.(error)
}
}()
d.Dial("tcp", "10.0.0.1:1234")
return
}()
if err.Error() != "proxyDialerWrapper.Dial should not be called directly" {
t.Fatal("unexpected result", err)
}
}
internal/engine/netx/httptransport/bytecounter.go
+2 -71
View File
@@ -1,74 +1,5 @@
package httptransport
import (
"io"
"net/http"
import "github.com/ooni/probe-cli/v3/internal/bytecounter"
"github.com/ooni/probe-cli/v3/internal/bytecounter"
"github.com/ooni/probe-cli/v3/internal/model"
)
// ByteCountingTransport is a RoundTripper that counts bytes.
type ByteCountingTransport struct {
model.HTTPTransport
Counter *bytecounter.Counter
}
// RoundTrip implements RoundTripper.RoundTrip
func (txp ByteCountingTransport) RoundTrip(req *http.Request) (*http.Response, error) {
if req.Body != nil {
req.Body = byteCountingBody{
ReadCloser: req.Body, Account: txp.Counter.CountBytesSent}
}
txp.estimateRequestMetadata(req)
resp, err := txp.HTTPTransport.RoundTrip(req)
if err != nil {
return nil, err
}
txp.estimateResponseMetadata(resp)
resp.Body = byteCountingBody{
ReadCloser: resp.Body, Account: txp.Counter.CountBytesReceived}
return resp, nil
}
func (txp ByteCountingTransport) estimateRequestMetadata(req *http.Request) {
txp.Counter.CountBytesSent(len(req.Method))
txp.Counter.CountBytesSent(len(req.URL.String()))
for key, values := range req.Header {
for _, value := range values {
txp.Counter.CountBytesSent(len(key))
txp.Counter.CountBytesSent(len(": "))
txp.Counter.CountBytesSent(len(value))
txp.Counter.CountBytesSent(len("\r\n"))
}
}
txp.Counter.CountBytesSent(len("\r\n"))
}
func (txp ByteCountingTransport) estimateResponseMetadata(resp *http.Response) {
txp.Counter.CountBytesReceived(len(resp.Status))
for key, values := range resp.Header {
for _, value := range values {
txp.Counter.CountBytesReceived(len(key))
txp.Counter.CountBytesReceived(len(": "))
txp.Counter.CountBytesReceived(len(value))
txp.Counter.CountBytesReceived(len("\r\n"))
}
}
txp.Counter.CountBytesReceived(len("\r\n"))
}
type byteCountingBody struct {
io.ReadCloser
Account func(int)
}
func (r byteCountingBody) Read(p []byte) (int, error) {
count, err := r.ReadCloser.Read(p)
if count > 0 {
r.Account(count)
}
return count, err
}
var _ model.HTTPTransport = ByteCountingTransport{}
type ByteCountingTransport = bytecounter.HTTPTransport
internal/engine/netx/httptransport/bytecounter_test.go
-129
View File
@@ -1,129 +0,0 @@
package httptransport_test
import (
"context"
"errors"
"io"
"net/http"
"strings"
"testing"
"github.com/ooni/probe-cli/v3/internal/bytecounter"
"github.com/ooni/probe-cli/v3/internal/engine/netx/httptransport"
"github.com/ooni/probe-cli/v3/internal/netxlite"
)
func TestByteCounterFailure(t *testing.T) {
counter := bytecounter.New()
txp := httptransport.ByteCountingTransport{
Counter: counter,
HTTPTransport: httptransport.FakeTransport{
Err: io.EOF,
},
}
client := &http.Client{Transport: txp}
req, err := http.NewRequest(
"POST", "https://www.google.com", strings.NewReader("AAAAAA"))
if err != nil {
t.Fatal(err)
}
req.Header.Set("User-Agent", "antani-browser/1.0.0")
resp, err := client.Do(req)
if !errors.Is(err, io.EOF) {
t.Fatal("not the error we expected")
}
if resp != nil {
t.Fatal("expected nil response here")
}
if counter.Sent.Load() != 68 {
t.Fatal("expected around 68 bytes sent")
}
if counter.Received.Load() != 0 {
t.Fatal("expected zero bytes received")
}
}
func TestByteCounterSuccess(t *testing.T) {
counter := bytecounter.New()
txp := httptransport.ByteCountingTransport{
Counter: counter,
HTTPTransport: httptransport.FakeTransport{
Resp: &http.Response{
Body: io.NopCloser(strings.NewReader("1234567")),
Header: http.Header{
"Server": []string{"antani/0.1.0"},
},
Status: "200 OK",
StatusCode: http.StatusOK,
},
},
}
client := &http.Client{Transport: txp}
req, err := http.NewRequest(
"POST", "https://www.google.com", strings.NewReader("AAAAAA"))
if err != nil {
t.Fatal(err)
}
req.Header.Set("User-Agent", "antani-browser/1.0.0")
resp, err := client.Do(req)
if err != nil {
t.Fatal(err)
}
data, err := netxlite.ReadAllContext(context.Background(), resp.Body)
if err != nil {
t.Fatal(err)
}
resp.Body.Close()
if string(data) != "1234567" {
t.Fatal("expected a different body here")
}
if counter.Sent.Load() != 68 {
t.Fatal("expected around 68 bytes sent")
}
if counter.Received.Load() != 37 {
t.Fatal("expected zero around 37 bytes received")
}
}
func TestByteCounterSuccessWithEOF(t *testing.T) {
counter := bytecounter.New()
txp := httptransport.ByteCountingTransport{
Counter: counter,
HTTPTransport: httptransport.FakeTransport{
Resp: &http.Response{
Body: bodyReaderWithEOF{},
Header: http.Header{
"Server": []string{"antani/0.1.0"},
},
Status: "200 OK",
StatusCode: http.StatusOK,
},
},
}
client := &http.Client{Transport: txp}
resp, err := client.Get("https://www.google.com")
if err != nil {
t.Fatal(err)
}
data, err := netxlite.ReadAllContext(context.Background(), resp.Body)
if err != nil {
t.Fatal(err)
}
resp.Body.Close()
if string(data) != "A" {
t.Fatal("expected a different body here")
}
}
type bodyReaderWithEOF struct{}
func (bodyReaderWithEOF) Read(p []byte) (int, error) {
if len(p) < 1 {
panic("should not happen")
}
p[0] = 'A'
return 1, io.EOF // we want code to be robust to this
}
func (bodyReaderWithEOF) Close() error {
return nil
}
internal/engine/netx/netx.go
+1 -1
View File
@@ -202,7 +202,7 @@ func NewHTTPTransport(config Config) model.HTTPTransport {
TLSConfig: config.TLSConfig})
if config.ByteCounter != nil {
txp = httptransport.ByteCountingTransport{
txp = &httptransport.ByteCountingTransport{
Counter: config.ByteCounter, HTTPTransport: txp}
}
if config.Logger != nil {
internal/engine/netx/netx_test.go
+1 -1
View File
@@ -473,7 +473,7 @@ func TestNewWithByteCounter(t *testing.T) {
txp := netx.NewHTTPTransport(netx.Config{
ByteCounter: counter,
})
bctxp, ok := txp.(httptransport.ByteCountingTransport)
bctxp, ok := txp.(*httptransport.ByteCountingTransport)
if !ok {
t.Fatal("not the transport we expected")
}