pr review changes

internal/restrictednet/client.go

@@ -41,7 +41,8 @@ func New(settings Settings) *Client {
 }
 
 // OpenHTTPSByHostname opens an https connection through the firewall,
-// valid for up to one second, to the hostname which in the format `host:port`.
+// to the hostname which in the format `host:port`. The returned cleanup
+// function must be called to remove the temporary firewall rule and close connections.
 // It first resolves the domain in hostname using DNS over HTTPS and then opens
 // the restricted HTTPS connection to the resolved IP.
 func (c *Client) OpenHTTPSByHostname(ctx context.Context, hostname string) (

internal/restrictednet/https.go

@@ -16,6 +16,8 @@ import (
 )
 
 // OpenHTTPS opens temporary restrictive firewall output for one HTTPS destination.
+// The returned [*http.Client] must be used sequentially only, and each request must
+// have its response body fully read/discarded and then closed.
 // The returned cleanup function must be called to remove the temporary firewall rule and close connections.
 func (c *Client) OpenHTTPS(ctx context.Context, destinationTLSName string, destinationAddrPort netip.AddrPort,
 ) (httpClient *http.Client, cleanup func() error, err error) {

internal/restrictednet/https_test.go

@@ -1,8 +1,11 @@
+//go:build integration
+
 package restrictednet
 
 import (
 	"context"
 	"fmt"
+	"io"
 	"net/http"
 	"net/netip"
 	"testing"
@@ -94,16 +97,20 @@ func Test_Client_OpenHTTPS(t *testing.T) {
 	require.NotNil(t, httpClient)
 	require.NotNil(t, cleanup)
 
-	request, err := http.NewRequestWithContext(ctx, http.MethodGet, "https://"+destinationTLSName, nil)
-	require.NoError(t, err)
+	const requests = 2
 
-	response, err := httpClient.Do(request)
-	require.NoError(t, err)
-	t.Cleanup(func() {
-		_ = response.Body.Close()
-	})
+	for range requests {
+		request, err := http.NewRequestWithContext(ctx, http.MethodGet, "https://"+destinationTLSName, nil)
+		require.NoError(t, err)
 
-	assert.Equal(t, http.StatusOK, response.StatusCode)
+		response, err := httpClient.Do(request)
+		require.NoError(t, err)
+		_, err = io.Copy(io.Discard, response.Body)
+		require.NoError(t, err)
+		err = response.Body.Close()
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusOK, response.StatusCode)
+	}
 
 	err = cleanup()
 	require.NoError(t, err)

internal/restrictednet/resolve.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"net/netip"
 	"net/url"
+	"strconv"
 
 	"github.com/miekg/dns"
 )
@@ -76,8 +77,16 @@ func (c *Client) resolveOneQuestionType(ctx context.Context,
 		dohServerIPs = append(dohServerIPs, dohServer.IPv4...)
 
 		for _, dohServerIP := range dohServerIPs {
-			const defaultDoHPort = 443
-			dohServerAddrPort := netip.AddrPortFrom(dohServerIP, defaultDoHPort)
+			const defaultDoHPort uint16 = 443
+			port := defaultDoHPort
+			if portStr := dohURL.Port(); portStr != "" {
+				port, err = parseDestinationPort(portStr)
+				if err != nil {
+					errs = append(errs, fmt.Errorf("parsing DoH server port: %w", err))
+					continue
+				}
+			}
+			dohServerAddrPort := netip.AddrPortFrom(dohServerIP, port)
 			responseMessage, err := c.doHQuery(ctx, queryWire, dohURL, dohServerAddrPort)
 			switch {
 			case err != nil:
@@ -178,3 +187,19 @@ func answersToNetipAddrs(message *dns.Msg) (addresses []netip.Addr) {
 	}
 	return addresses
 }
+
+func parseDestinationPort(portStr string) (port uint16, err error) {
+	portUint, err := strconv.ParseUint(portStr, 10, 16)
+	if err != nil {
+		return 0, err
+	}
+
+	const maxPortUint = 65535
+	switch {
+	case portUint == 0:
+		return 0, errors.New("port cannot be 0")
+	case portUint > maxPortUint:
+		return 0, fmt.Errorf("port cannot be greater than %d", maxPortUint)
+	}
+	return uint16(portUint), nil
+}

internal/restrictednet/resolve_test.go

@@ -1,3 +1,5 @@
+//go:build integration
+
 package restrictednet
 
 import (

internal/restrictednet/unix.go

@@ -1,4 +1,4 @@
-//go:build unix
+//go:build !windows
 
 package restrictednet