doc(netxlite): revamp the documentation (ooni#523)

Part of ooni#506. In parallel with
tutorials, we also need to make sure we have good documentation.

internal/README.md

@@ -1,3 +1,19 @@
 # Directory github.com/ooni/probe-cli/internal
 
 This directory contains private Go packages.
+
+As a reminder, you can always check the Go documentation of
+a package by using
+
+```bash
+go doc -all ./internal/$package
+```
+
+where `$package` is the name of the package.
+
+Some notable packages:
+
+- [netxlite](netxlite) is the underlying networking library;
+
+- [tutorial](tutorial) contains tutorials on writing new experiments,
+using measurements libraries, and networking code.

internal/netxlite/classify.go

@@ -11,28 +11,29 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/scrubber"
 )
 
-// ClassifyGenericError is the generic classifier mapping an error
-// occurred during an operation to an OONI failure string.
+// ClassifyGenericError is maps an error occurred during an operation
+// to an OONI failure string. This specific classifier is the most
+// generic one. You usually use it when mapping I/O errors. You should
+// check whether there is a specific classifier for more specific
+// operations (e.g., DNS resolution, TLS handshake).
 //
-// If the input error is already an ErrWrapper we don't perform
-// the classification again and we return its Failure to the caller.
-//
-// Classification rules
+// If the input error is an *ErrWrapper we don't perform
+// the classification again and we return its Failure.
 //
 // We put inside this classifier:
 //
-// - system call errors
+// - system call errors;
 //
-// - generic errors that can occur in multiple places
+// - generic errors that can occur in multiple places;
 //
-// - all the errors that depend on strings
+// - all the errors that depend on strings.
 //
 // The more specific classifiers will call this classifier if
 // they fail to find a mapping for the input error.
 //
 // If everything else fails, this classifier returns a string
 // like "unknown_failure: XXX" where XXX has been scrubbed
-// so to remove any network endpoints from its value.
+// so to remove any network endpoints from the original error string.
 func ClassifyGenericError(err error) string {
 	// The list returned here matches the values used by MK unless
 	// explicitly noted otherwise with a comment.
@@ -133,14 +134,14 @@ const (
 	quicTLSUnrecognizedName = 112
 )
 
-// ClassifyQUICHandshakeError maps an error occurred during the QUIC
-// handshake to an OONI failure string.
+// ClassifyQUICHandshakeError maps errors during a QUIC
+// handshake to OONI failure strings.
 //
-// If the input error is already an ErrWrapper we don't perform
-// the classification again and we return its Failure to the caller.
+// If the input error is an *ErrWrapper we don't perform
+// the classification again and we return its Failure.
 //
-// If this classifier fails, it calls ClassifyGenericError and
-// returns to the caller its return value.
+// If this classifier fails, it calls ClassifyGenericError
+// and returns to the caller its return value.
 func ClassifyQUICHandshakeError(err error) string {
 	var errwrapper *ErrWrapper
 	if errors.As(err, &errwrapper) {
@@ -229,26 +230,29 @@ func quicIsCertificateError(alert uint8) bool {
 // filters for DNS bogons MUST use this error.
 var ErrDNSBogon = errors.New("dns: detected bogon address")
 
-// These strings are same as the standard library.
+// We use these strings to string-match errors in the standard library
+// and map such errors to OONI failures.
 const (
 	DNSNoSuchHostSuffix        = "no such host"
 	DNSServerMisbehavingSuffix = "server misbehaving"
 	DNSNoAnswerSuffix          = "no answer from DNS server"
 )
 
-// These errors are returned by the decoder and/or the serial resolver.
+// These errors are returned by custom DNSTransport instances (e.g.,
+// DNSOverHTTPS and DNSOverUDP). Their suffix matches the equivalent
+// unexported errors used by the Go standard library.
 var (
 	ErrOODNSNoSuchHost  = fmt.Errorf("ooniresolver: %s", DNSNoSuchHostSuffix)
 	ErrOODNSRefused     = errors.New("ooniresolver: refused")
 	ErrOODNSMisbehaving = fmt.Errorf("ooniresolver: %s", DNSServerMisbehavingSuffix)
 	ErrOODNSNoAnswer    = fmt.Errorf("ooniresolver: %s", DNSNoAnswerSuffix)
 )
 
-// ClassifyResolverError maps an error occurred during a domain name
-// resolution to the corresponding OONI failure string.
+// ClassifyResolverError maps DNS resolution errors to
+// OONI failure strings.
 //
-// If the input error is already an ErrWrapper we don't perform
-// the classification again and we return its Failure to the caller.
+// If the input error is an *ErrWrapper we don't perform
+// the classification again and we return its Failure.
 //
 // If this classifier fails, it calls ClassifyGenericError and
 // returns to the caller its return value.
@@ -271,8 +275,8 @@ func ClassifyResolverError(err error) string {
 // ClassifyTLSHandshakeError maps an error occurred during the TLS
 // handshake to an OONI failure string.
 //
-// If the input error is already an ErrWrapper we don't perform
-// the classification again and we return its Failure to the caller.
+// If the input error is an *ErrWrapper we don't perform
+// the classification again and we return its Failure.
 //
 // If this classifier fails, it calls ClassifyGenericError and
 // returns to the caller its return value.

internal/netxlite/dialer.go

@@ -17,8 +17,7 @@ type Dialer interface {
 	CloseIdleConnections()
 }
 
-// NewDialerWithResolver is a convenience factory that calls
-// WrapDialer for a stdlib dialer type.
+// NewDialerWithResolver calls WrapDialer for the stdlib dialer.
 func NewDialerWithResolver(logger Logger, resolver Resolver) Dialer {
 	return WrapDialer(logger, resolver, &dialerSystem{})
 }
@@ -30,14 +29,14 @@ func NewDialerWithResolver(logger Logger, resolver Resolver) Dialer {
 //
 // 2. resolves domain names using the givern resolver;
 //
-// 3. when using a resolver, each available enpoint is tried
+// 3. when the resolver is not a "null" resolver,
+// each available enpoint is tried
 // sequentially. On error, the code will return what it believes
 // to be the most representative error in the pack. Most often,
-// such an error is the first one that occurred. Choosing the
+// the first error that occurred. Choosing the
 // error to return using this logic is a QUIRK that we owe
 // to the original implementation of netx. We cannot change
-// this behavior until all the legacy code that relies on
-// it has been migrated to more sane patterns.
+// this behavior until we refactor legacy code using it.
 //
 // Removing this quirk from the codebase is documented as
 // TODO(https://github.com/ooni/probe/issues/1779).
@@ -49,6 +48,9 @@ func NewDialerWithResolver(logger Logger, resolver Resolver) Dialer {
 // 6. if a dialer wraps a resolver, the dialer will forward
 // the CloseIdleConnection call to its resolver (which is
 // instrumental to manage a DoH resolver connections properly).
+//
+// In general, do not use WrapDialer directly but try to use
+// more high-level factories, e.g., NewDialerWithResolver.
 func WrapDialer(logger Logger, resolver Resolver, dialer Dialer) Dialer {
 	return &dialerLogger{
 		Dialer: &dialerResolver{
@@ -65,8 +67,9 @@ func WrapDialer(logger Logger, resolver Resolver, dialer Dialer) Dialer {
 	}
 }
 
-// NewDialerWithoutResolver is like NewDialerWithResolver except that
-// it will fail with ErrNoResolver if passed a domain name.
+// NewDialerWithoutResolver calls NewDialerWithResolver with a "null" resolver.
+//
+// The returned dialer fails with ErrNoResolver if passed a domain name.
 func NewDialerWithoutResolver(logger Logger) Dialer {
 	return NewDialerWithResolver(logger, &nullResolver{})
 }
@@ -183,12 +186,15 @@ func (d *dialerLogger) CloseIdleConnections() {
 	d.Dialer.CloseIdleConnections()
 }
 
-// ErrNoConnReuse indicates we cannot reuse the connection provided
-// to a single use (possibly TLS) dialer.
+// ErrNoConnReuse is the type of error returned when you create a
+// "single use" dialer or a "single use" TLS dialer and you dial
+// more than once, which is not supported by such a dialer.
 var ErrNoConnReuse = errors.New("cannot reuse connection")
 
-// NewSingleUseDialer returns a dialer that returns the given connection once
-// and after that always fails with the ErrNoConnReuse error.
+// NewSingleUseDialer returns a "single use" dialer. The first
+// dial will succed and return conn regardless of the network
+// and address arguments passed to DialContext. Any subsequent
+// dial returns ErrNoConnReuse.
 func NewSingleUseDialer(conn net.Conn) Dialer {
 	return &dialerSingleUse{conn: conn}
 }
@@ -263,10 +269,11 @@ func (c *dialerErrWrapperConn) Close() error {
 	return nil
 }
 
-// ErrNoDialer indicates that no dialer is configured.
+// ErrNoDialer is the type of error returned by "null" dialers
+// when you attempt to dial with them.
 var ErrNoDialer = errors.New("no configured dialer")
 
-// NewNullDialer returns a dialer that always fails.
+// NewNullDialer returns a dialer that always fails with ErrNoDialer.
 func NewNullDialer() Dialer {
 	return &nullDialer{}
 }

internal/netxlite/dnsdecoder.go

@@ -5,9 +5,34 @@ import "github.com/miekg/dns"
 // The DNSDecoder decodes DNS replies.
 type DNSDecoder interface {
 	// DecodeLookupHost decodes an A or AAAA reply.
+	//
+	// Arguments:
+	//
+	// - qtype is the query type (e.g., dns.TypeAAAA)
+	//
+	// - data contains the reply bytes read from a DNSTransport
+	//
+	// Returns:
+	//
+	// - on success, a list of IP addrs inside the reply and a nil error
+	//
+	// - on failure, a nil list and an error.
+	//
+	// Note that this function will return an error if there is no
+	// IP address inside of the reply.
 	DecodeLookupHost(qtype uint16, data []byte) ([]string, error)
 
 	// DecodeHTTPS decodes an HTTPS reply.
+	//
+	// The argument is the reply as read by the DNSTransport.
+	//
+	// On success, this function returns an HTTPSSvc structure and
+	// a nil error. On failure, the HTTPSSvc pointer is nil and
+	// the error points to the error that occurred.
+	//
+	// This function will return an error if the HTTPS reply does not
+	// contain at least a valid ALPN entry. It will not return
+	// an error, though, when there are no IPv4/IPv6 hints in the reply.
 	DecodeHTTPS(data []byte) (*HTTPSSvc, error)
 }
 

internal/netxlite/dnsencoder.go

@@ -4,6 +4,18 @@ import "github.com/miekg/dns"
 
 // The DNSEncoder encodes DNS queries to bytes
 type DNSEncoder interface {
+	// Encode transforms its arguments into a serialized DNS query.
+	//
+	// Arguments:
+	//
+	// - domain is the domain for the query (e.g., x.org);
+	//
+	// - qtype is the query type (e.g., dns.TypeA);
+	//
+	// - padding is whether to add padding to the query.
+	//
+	// On success, this function returns a valid byte array and
+	// a nil error. On failure, we have an error and the byte array is nil.
 	Encode(domain string, qtype uint16, padding bool) ([]byte, error)
 }
 
@@ -21,7 +33,6 @@ const (
 	dnsDNSSECEnabled = true
 )
 
-// Encode implements Encoder.Encode
 func (e *DNSEncoderMiekg) Encode(domain string, qtype uint16, padding bool) ([]byte, error) {
 	question := dns.Question{
 		Name:   dns.Fqdn(domain),

internal/netxlite/dnsoverhttps.go

@@ -10,34 +10,44 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/engine/httpheader"
 )
 
-// HTTPClient is the HTTP client expected by DNSOverHTTPS.
+// HTTPClient is an http.Client-like interface.
 type HTTPClient interface {
 	Do(req *http.Request) (*http.Response, error)
 	CloseIdleConnections()
 }
 
-// DNSOverHTTPS is a DNS over HTTPS RoundTripper. Requests are submitted over
-// an HTTP/HTTPS channel provided by URL using the Do function.
+// DNSOverHTTPS is a DNS-over-HTTPS DNSTransport.
 type DNSOverHTTPS struct {
-	Client       HTTPClient
-	URL          string
+	// Client is the MANDATORY http client to use.
+	Client HTTPClient
+
+	// URL is the MANDATORY URL of the DNS-over-HTTPS server.
+	URL string
+
+	// HostOverride is OPTIONAL and allows to override the
+	// Host header sent in every request.
 	HostOverride string
 }
 
-// NewDNSOverHTTPS creates a new DNSOverHTTP instance from the
-// specified http.Client and URL, as a convenience.
+// NewDNSOverHTTPS creates a new DNSOverHTTPS instance.
+//
+// Arguments:
+//
+// - client in http.Client-like type (e.g., http.DefaultClient);
+//
+// - URL is the DoH resolver URL (e.g., https://1.1.1.1/dns-query).
 func NewDNSOverHTTPS(client HTTPClient, URL string) *DNSOverHTTPS {
 	return NewDNSOverHTTPSWithHostOverride(client, URL, "")
 }
 
-// NewDNSOverHTTPSWithHostOverride is like NewDNSOverHTTPS except that
-// it's creating a resolver where we use the specified host.
+// NewDNSOverHTTPSWithHostOverride creates a new DNSOverHTTPS
+// with the given Host header override.
 func NewDNSOverHTTPSWithHostOverride(
 	client HTTPClient, URL, hostOverride string) *DNSOverHTTPS {
 	return &DNSOverHTTPS{Client: client, URL: URL, HostOverride: hostOverride}
 }
 
-// RoundTrip implements RoundTripper.RoundTrip.
+// RoundTrip sends a query and receives a reply.
 func (t *DNSOverHTTPS) RoundTrip(ctx context.Context, query []byte) ([]byte, error) {
 	ctx, cancel := context.WithTimeout(ctx, 45*time.Second)
 	defer cancel()
@@ -65,22 +75,22 @@ func (t *DNSOverHTTPS) RoundTrip(ctx context.Context, query []byte) ([]byte, err
 	return ReadAllContext(ctx, resp.Body)
 }
 
-// RequiresPadding returns true for DoH according to RFC8467
+// RequiresPadding returns true for DoH according to RFC8467.
 func (t *DNSOverHTTPS) RequiresPadding() bool {
 	return true
 }
 
-// Network returns the transport network (e.g., doh, dot)
+// Network returns the transport network, i.e., "doh".
 func (t *DNSOverHTTPS) Network() string {
 	return "doh"
 }
 
-// Address returns the upstream server address.
+// Address returns the URL we're using for the DoH server.
 func (t *DNSOverHTTPS) Address() string {
 	return t.URL
 }
 
-// CloseIdleConnections closes idle connections.
+// CloseIdleConnections closes idle connections, if any.
 func (t *DNSOverHTTPS) CloseIdleConnections() {
 	t.Client.CloseIdleConnections()
 }