remove the custom dialer - turns out it is not actually needed

Author: peterguy

cmd/src/main.go

@@ -7,7 +7,6 @@ import (
 	"io"
 	"log"
 	"net"
-	"net/http"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -323,14 +322,6 @@ func readConfig() (*config, error) {
 		} else {
 			return nil, errors.Newf("invalid proxy endpoint: %s", proxyStr)
 		}
-	} else {
-		// no SRC_PROXY; check for the standard proxy env variables HTTP_PROXY, HTTPS_PROXY, and NO_PROXY
-		if u, err := http.ProxyFromEnvironment(&http.Request{URL: cfg.endpointURL}); err != nil {
-			// when there's an error, the value for the env variable is not a legit URL
-			return nil, errors.Newf("invalid HTTP_PROXY or HTTPS_PROXY value: %w", err)
-		} else {
-			cfg.proxyURL = u
-		}
 	}
 
 	cfg.additionalHeaders = parseAdditionalHeaders()

internal/api/proxy.go

@@ -1,16 +1,11 @@
 package api
 
 import (
-	"bufio"
 	"context"
 	"crypto/tls"
-	"encoding/base64"
-	"io"
 	"net"
 	"net/http"
 	"net/url"
-
-	"github.com/sourcegraph/sourcegraph/lib/errors"
 )
 
 // proxyDialAddr returns proxyURL.Host with a default port appended if one is
@@ -43,12 +38,6 @@ func withProxyTransport(baseTransport *http.Transport, proxyURL *url.URL, proxyP
 		if cfg.ServerName == "" {
 			cfg.ServerName = host
 		}
-		// Preserve HTTP/2 negotiation to the origin when ForceAttemptHTTP2
-		// is enabled. Without this, the manual TLS handshake would not
-		// advertise h2 via ALPN, silently forcing HTTP/1.1.
-		if baseTransport.ForceAttemptHTTP2 && len(cfg.NextProtos) == 0 {
-			cfg.NextProtos = []string{"h2", "http/1.1"}
-		}
 		tlsConn := tls.Client(conn, cfg)
 		if err := tlsConn.HandshakeContext(ctx); err != nil {
 			tlsConn.Close()
@@ -74,83 +63,7 @@ func withProxyTransport(baseTransport *http.Transport, proxyURL *url.URL, proxyP
 		// clear out any system proxy settings
 		baseTransport.Proxy = nil
 	} else if proxyURL != nil {
-		switch proxyURL.Scheme {
-		case "http", "socks5", "socks5h":
-			// HTTP and SOCKS proxies work out of the box - no need to manually dial
-			baseTransport.Proxy = http.ProxyURL(proxyURL)
-		case "https":
-			dial := func(ctx context.Context, network, addr string) (net.Conn, error) {
-				// Dial the proxy. For TLS-enabled proxies, we TLS-connect to the
-				// proxy itself and force ALPN to HTTP/1.1 to prevent Go from
-				// negotiating HTTP/2 for the CONNECT tunnel. Many proxy servers
-				// don't support HTTP/2 CONNECT, and Go's default Transport.Proxy
-				// would negotiate h2 via ALPN when TLS-connecting to a TLS-enabled
-				// proxy, causing "bogus greeting" errors. For plain HTTP proxies,
-				// CONNECT is always HTTP/1.1 over plain TCP so this isn't needed.
-				// The target connection (e.g. to sourcegraph.com) still negotiates
-				// HTTP/2 normally through the established tunnel.
-				proxyAddr := proxyDialAddr(proxyURL)
-
-				raw, dialErr := (&net.Dialer{}).DialContext(ctx, "tcp", proxyAddr)
-				if dialErr != nil {
-					return nil, dialErr
-				}
-				cfg := baseTransport.TLSClientConfig.Clone()
-				cfg.NextProtos = []string{"http/1.1"}
-				if cfg.ServerName == "" {
-					cfg.ServerName = proxyURL.Hostname()
-				}
-				conn := tls.Client(raw, cfg)
-				if err := conn.HandshakeContext(ctx); err != nil {
-					raw.Close()
-					return nil, err
-				}
-
-				connectReq := &http.Request{
-					Method: "CONNECT",
-					URL:    &url.URL{Opaque: addr},
-					Host:   addr,
-					Header: make(http.Header),
-				}
-				if proxyURL.User != nil {
-					password, _ := proxyURL.User.Password()
-					auth := base64.StdEncoding.EncodeToString([]byte(proxyURL.User.Username() + ":" + password))
-					connectReq.Header.Set("Proxy-Authorization", "Basic "+auth)
-				}
-				if err := connectReq.Write(conn); err != nil {
-					conn.Close()
-					return nil, err
-				}
-
-				resp, err := http.ReadResponse(bufio.NewReader(conn), nil)
-				if err != nil {
-					conn.Close()
-					return nil, err
-				}
-				if resp.StatusCode != http.StatusOK {
-					// For non-200, it's safe/appropriate to close the body (it’s a real response body here).
-					// Try to read a bit (4k bytes) to include in the error message.
-					b, _ := io.ReadAll(io.LimitReader(resp.Body, 4<<10))
-					resp.Body.Close()
-					conn.Close()
-					return nil, errors.Newf("failed to connect to proxy %s: %s: %q", proxyURL.Redacted(), resp.Status, b)
-				}
-				// 200 CONNECT: do NOT resp.Body.Close(); it would interfere with the tunnel.
-				return conn, nil
-			}
-			dialTLS := func(ctx context.Context, network, addr string) (net.Conn, error) {
-				// Dial the underlying connection through the proxy
-				conn, err := dial(ctx, network, addr)
-				if err != nil {
-					return nil, err
-				}
-				return handshakeTLS(ctx, conn, addr)
-			}
-			baseTransport.DialContext = dial
-			baseTransport.DialTLSContext = dialTLS
-			// clear out the system proxy because we're defining our own dialers
-			baseTransport.Proxy = nil
-		}
+		baseTransport.Proxy = http.ProxyURL(proxyURL)
 	}
 
 	return baseTransport

internal/api/proxy_test.go

@@ -376,8 +376,6 @@ func TestWithProxyTransport_ProxyRejectsConnect(t *testing.T) {
 		})
 
 		t.Run("https proxy/"+tt.name, func(t *testing.T) {
-			// The HTTPS proxy path uses a custom dialer with its own error
-			// formatting that includes the status, body, and redacted proxy URL.
 			srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 				http.Error(w, tt.body, tt.statusCode)
 			}))
@@ -393,11 +391,8 @@ func TestWithProxyTransport_ProxyRejectsConnect(t *testing.T) {
 			if err == nil {
 				t.Fatal("expected error, got nil")
 			}
-			if !strings.Contains(err.Error(), fmt.Sprintf("%d", tt.statusCode)) {
-				t.Errorf("error should contain status code %d, got: %v", tt.statusCode, err)
-			}
-			if !strings.Contains(err.Error(), tt.body) {
-				t.Errorf("error should contain body %q, got: %v", tt.body, err)
+			if !strings.Contains(err.Error(), tt.wantErr) {
+				t.Errorf("error should contain %q, got: %v", tt.wantErr, err)
 			}
 		})
 	}