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

doc(netx): reference issue mentioning future improvements (#802)

Browse Source 
See https://github.com/ooni/probe/issues/2121#issuecomment-1147424810
This commit is contained in:
Simone Basso 2022-06-06 15:16:30 +02:00 committed by GitHub
parent 2502a237fb
commit 57e207e644
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 20 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
internal/engine/netx/dialer.go
View File

@ -13,6 +13,7 @@ import (
// NewDialer creates a new Dialer from the specified config.
func NewDialer(config Config) model.Dialer {
if config.FullResolver == nil {
// TODO(https://github.com/ooni/probe/issues/2121#issuecomment-1147424810)
config.FullResolver = NewResolver(config)
}
logger := model.ValidLoggerOrDefault(config.Logger)

7
internal/engine/netx/dnstransport.go
View File

@ -6,6 +6,7 @@ package netx
// TODO(bassosimone): this code should be refactored to return
// a DNSTransport rather than a model.Resolver. With this in mind,
// I've named this file dnstransport.go.
// TODO(https://github.com/ooni/probe/issues/2121#issuecomment-1147424810)
//
import (
@ -45,6 +46,8 @@ func NewDNSClient(config Config, URL string) (model.Resolver, error) {
// with the option to override the default Hostname and SNI.
func NewDNSClientWithOverrides(config Config, URL, hostOverride, SNIOverride,
TLSVersion string) (model.Resolver, error) {
// We should split this function in smaller and testable units
// TODO(https://github.com/ooni/probe/issues/2121#issuecomment-1147424810)
switch URL {
case "doh://powerdns":
URL = "https://doh.powerdns.org/"
@ -132,6 +135,10 @@ func makeValidEndpoint(URL *url.URL) (string, error) {
if _, _, err := net.SplitHostPort(URL.Host); err == nil {
return URL.Host, nil
}
// Here we should add a test case for when the host is empty
// TODO(https://github.com/ooni/probe/issues/2121#issuecomment-1147424810)
// The second step is to assume that appending the default port
// to a host parsed by url.Parse should be giving us a valid
// endpoint. The possibilities in fact are:

17
internal/engine/netx/http.go
View File

@ -14,12 +14,15 @@ import (
// NewHTTPTransport creates a new HTTPRoundTripper from the given Config.
func NewHTTPTransport(config Config) model.HTTPTransport {
if config.Dialer == nil {
// TODO(https://github.com/ooni/probe/issues/2121#issuecomment-1147424810)
config.Dialer = NewDialer(config)
}
if config.TLSDialer == nil {
// TODO(https://github.com/ooni/probe/issues/2121#issuecomment-1147424810)
config.TLSDialer = NewTLSDialer(config)
}
if config.QUICDialer == nil {
// TODO(https://github.com/ooni/probe/issues/2121#issuecomment-1147424810)
config.QUICDialer = NewQUICDialer(config)
}
tInfo := allTransportsInfo[config.HTTP3Enabled]
@ -30,7 +33,8 @@ func NewHTTPTransport(config Config) model.HTTPTransport {
TLSDialer: config.TLSDialer,
TLSConfig: config.TLSConfig,
})
// TODO(bassosimone): I am not super convinced by this code because it
// TODO(https://github.com/ooni/probe/issues/2121#issuecomment-1147424810): I am
// not super convinced by this code because it
// seems we're currently counting bytes twice in some cases. I think we
// should review how we're counting bytes and using netx currently.
txp = config.ByteCounter.MaybeWrapHTTPTransport(txp) // WAI with ByteCounter == nil
@ -46,7 +50,7 @@ type httpTransportInfo struct {
var allTransportsInfo = map[bool]httpTransportInfo{
false: {
Factory: newSystemTransport,
Factory: newHTTPTransport,
TransportName: "tcp",
},
true: {
@ -56,6 +60,8 @@ var allTransportsInfo = map[bool]httpTransportInfo{
}
// httpTransportConfig contains configuration for constructing an HTTPTransport.
//
// All the fields in this structure MUST be initialized.
type httpTransportConfig struct {
Dialer model.Dialer
Logger model.Logger
@ -66,13 +72,10 @@ type httpTransportConfig struct {
// newHTTP3Transport creates a new HTTP3Transport instance.
func newHTTP3Transport(config httpTransportConfig) model.HTTPTransport {
// 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(config.Logger, config.QUICDialer, config.TLSConfig)
}
// newSystemTransport creates a new "system" HTTP transport. That is a transport
// using the Go standard library with custom dialer and TLS dialer.
func newSystemTransport(config httpTransportConfig) model.HTTPTransport {
// newHTTPTransport creates a new "system" HTTP transport.
func newHTTPTransport(config httpTransportConfig) model.HTTPTransport {
return netxlite.NewHTTPTransport(config.Logger, config.Dialer, config.TLSDialer)
}

3
internal/engine/netx/quic.go
View File

@ -14,7 +14,8 @@ func NewQUICDialer(config Config) model.QUICDialer {
if config.FullResolver == nil {
config.FullResolver = NewResolver(config)
}
// TODO(bassosimone): we should count the bytes consumed by this QUIC dialer
// TODO(https://github.com/ooni/probe/issues/2121#issuecomment-1147424810): we
// should count the bytes consumed by this QUIC dialer
ql := config.ReadWriteSaver.WrapQUICListener(netxlite.NewQUICListener())
logger := model.ValidLoggerOrDefault(config.Logger)
return netxlite.NewQUICDialerWithResolver(ql, logger, config.FullResolver, config.Saver)