fix(netxlite): http3 transport needs logging by default (#492)
Adapt other places where it was not using a logger to either choose a reasonable logger or disable logging for backwards compat. See https://github.com/ooni/probe/issues/1591
This commit is contained in:
		
							parent
							
								
									18b2eb37ff
								
							
						
					
					
						commit
						e68adec9a5
					
				| @ -8,6 +8,7 @@ import ( | |||||||
| 	"sort" | 	"sort" | ||||||
| 	"strings" | 	"strings" | ||||||
| 
 | 
 | ||||||
|  | 	"github.com/apex/log" | ||||||
| 	"github.com/ooni/probe-cli/v3/internal/engine/experiment/websteps" | 	"github.com/ooni/probe-cli/v3/internal/engine/experiment/websteps" | ||||||
| 	"github.com/ooni/probe-cli/v3/internal/netxlite" | 	"github.com/ooni/probe-cli/v3/internal/netxlite" | ||||||
| 	"github.com/ooni/probe-cli/v3/internal/runtimex" | 	"github.com/ooni/probe-cli/v3/internal/runtimex" | ||||||
| @ -135,7 +136,9 @@ func (e *DefaultExplorer) getH3(h3URL *h3URL, headers map[string][]string) (*htt | |||||||
| 	tlsConf := &tls.Config{ | 	tlsConf := &tls.Config{ | ||||||
| 		NextProtos: []string{h3URL.proto}, | 		NextProtos: []string{h3URL.proto}, | ||||||
| 	} | 	} | ||||||
| 	transport := netxlite.NewHTTP3Transport( | 	// Rationale for using log.Log here: we're already using log.Log | ||||||
|  | 	// in this package, so it seems fair to use it also here | ||||||
|  | 	transport := netxlite.NewHTTP3Transport(log.Log, | ||||||
| 		netxlite.NewQUICDialerFromContextDialerAdapter(dialer), tlsConf) | 		netxlite.NewQUICDialerFromContextDialerAdapter(dialer), tlsConf) | ||||||
| 	// TODO(bassosimone): here we should use runtimex.PanicOnError | 	// TODO(bassosimone): here we should use runtimex.PanicOnError | ||||||
| 	jarjar, _ := cookiejar.New(nil) | 	jarjar, _ := cookiejar.New(nil) | ||||||
|  | |||||||
| @ -10,7 +10,15 @@ import ( | |||||||
| // | // | ||||||
| // New code should use netxlite.NewHTTP3Transport instead. | // New code should use netxlite.NewHTTP3Transport instead. | ||||||
| func NewHTTP3Transport(config Config) RoundTripper { | func NewHTTP3Transport(config Config) RoundTripper { | ||||||
| 	return netxlite.NewHTTP3Transport( | 	// Rationale for using NoLogger here: previously this code did | ||||||
|  | 	// not use a logger as well, so it's fine to keep it as is. | ||||||
|  | 	return netxlite.NewHTTP3Transport(&NoLogger{}, | ||||||
| 		netxlite.NewQUICDialerFromContextDialerAdapter(config.QUICDialer), | 		netxlite.NewQUICDialerFromContextDialerAdapter(config.QUICDialer), | ||||||
| 		config.TLSConfig) | 		config.TLSConfig) | ||||||
| } | } | ||||||
|  | 
 | ||||||
|  | type NoLogger struct{} | ||||||
|  | 
 | ||||||
|  | func (*NoLogger) Debug(message string) {} | ||||||
|  | 
 | ||||||
|  | func (*NoLogger) Debugf(format string, v ...interface{}) {} | ||||||
|  | |||||||
| @ -53,17 +53,20 @@ func (txp *http3Transport) CloseIdleConnections() { | |||||||
| // dialer argument MUST NOT be nil. If the tlsConfig argument is nil, | // dialer argument MUST NOT be nil. If the tlsConfig argument is nil, | ||||||
| // then the code will use the default TLS configuration. | // then the code will use the default TLS configuration. | ||||||
| func NewHTTP3Transport( | func NewHTTP3Transport( | ||||||
| 	dialer QUICDialer, tlsConfig *tls.Config) HTTPTransport { | 	logger Logger, dialer QUICDialer, tlsConfig *tls.Config) HTTPTransport { | ||||||
| 	return &http3Transport{ | 	return &httpTransportLogger{ | ||||||
| 		child: &http3.RoundTripper{ | 		HTTPTransport: &http3Transport{ | ||||||
| 			Dial: (&http3Dialer{dialer}).dial, | 			child: &http3.RoundTripper{ | ||||||
| 			// The following (1) reduces the number of headers that Go will | 				Dial: (&http3Dialer{dialer}).dial, | ||||||
| 			// automatically send for us and (2) ensures that we always receive | 				// The following (1) reduces the number of headers that Go will | ||||||
| 			// back the true headers, such as Content-Length. This change is | 				// automatically send for us and (2) ensures that we always receive | ||||||
| 			// functional to OONI's goal of observing the network. | 				// back the true headers, such as Content-Length. This change is | ||||||
| 			DisableCompression: true, | 				// functional to OONI's goal of observing the network. | ||||||
| 			TLSClientConfig:    tlsConfig, | 				DisableCompression: true, | ||||||
|  | 				TLSClientConfig:    tlsConfig, | ||||||
|  | 			}, | ||||||
|  | 			dialer: dialer, | ||||||
| 		}, | 		}, | ||||||
| 		dialer: dialer, | 		Logger: logger, | ||||||
| 	} | 	} | ||||||
| } | } | ||||||
|  | |||||||
| @ -7,6 +7,7 @@ import ( | |||||||
| 	"net/http" | 	"net/http" | ||||||
| 	"testing" | 	"testing" | ||||||
| 
 | 
 | ||||||
|  | 	"github.com/apex/log" | ||||||
| 	"github.com/lucas-clemente/quic-go" | 	"github.com/lucas-clemente/quic-go" | ||||||
| 	"github.com/lucas-clemente/quic-go/http3" | 	"github.com/lucas-clemente/quic-go/http3" | ||||||
| 	"github.com/ooni/probe-cli/v3/internal/netxlite/mocks" | 	"github.com/ooni/probe-cli/v3/internal/netxlite/mocks" | ||||||
| @ -80,8 +81,12 @@ func TestNewHTTP3Transport(t *testing.T) { | |||||||
| 	t.Run("creates the correct type chain", func(t *testing.T) { | 	t.Run("creates the correct type chain", func(t *testing.T) { | ||||||
| 		qd := &mocks.QUICDialer{} | 		qd := &mocks.QUICDialer{} | ||||||
| 		config := &tls.Config{} | 		config := &tls.Config{} | ||||||
| 		txp := NewHTTP3Transport(qd, config) | 		txp := NewHTTP3Transport(log.Log, qd, config) | ||||||
| 		h3txp := txp.(*http3Transport) | 		logger := txp.(*httpTransportLogger) | ||||||
|  | 		if logger.Logger != log.Log { | ||||||
|  | 			t.Fatal("invalid logger") | ||||||
|  | 		} | ||||||
|  | 		h3txp := logger.HTTPTransport.(*http3Transport) | ||||||
| 		if h3txp.dialer != qd { | 		if h3txp.dialer != qd { | ||||||
| 			t.Fatal("invalid dialer") | 			t.Fatal("invalid dialer") | ||||||
| 		} | 		} | ||||||
|  | |||||||
| @ -42,7 +42,7 @@ func TestHTTP3Transport(t *testing.T) { | |||||||
| 			log.Log, | 			log.Log, | ||||||
| 			netxlite.NewResolverSystem(log.Log), | 			netxlite.NewResolverSystem(log.Log), | ||||||
| 		) | 		) | ||||||
| 		txp := netxlite.NewHTTP3Transport(d, &tls.Config{}) | 		txp := netxlite.NewHTTP3Transport(log.Log, d, &tls.Config{}) | ||||||
| 		client := &http.Client{Transport: txp} | 		client := &http.Client{Transport: txp} | ||||||
| 		resp, err := client.Get("https://www.google.com/robots.txt") | 		resp, err := client.Get("https://www.google.com/robots.txt") | ||||||
| 		if err != nil { | 		if err != nil { | ||||||
|  | |||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user