From 1052b70de678cc70d76c065acdd20bc674581dbb Mon Sep 17 00:00:00 2001 From: Tianhu Yang Date: Mon, 22 Jun 2026 20:26:16 +0800 Subject: [PATCH 1/5] fix(http): add comma in Proxy-Authenticate Basic params --- protocol/http/handshake.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocol/http/handshake.go b/protocol/http/handshake.go index 4320061a..e01c13b2 100644 --- a/protocol/http/handshake.go +++ b/protocol/http/handshake.go @@ -57,7 +57,7 @@ func HandleConnectionEx( // Since no one else is using the library, use a fixed realm until rewritten err = responseWith( request, http.StatusProxyAuthRequired, - "Proxy-Authenticate", `Basic realm="sing-box" charset="UTF-8"`, + "Proxy-Authenticate", `Basic realm="sing-box", charset="UTF-8"`, ).Write(conn) if err != nil { return err From 1f0901d7311196ed6b33a58854bc97f3337b542a Mon Sep 17 00:00:00 2001 From: Tianhu Yang Date: Mon, 22 Jun 2026 23:15:38 +0800 Subject: [PATCH 2/5] http: retry missing proxy auth once with timeout --- protocol/http/handshake.go | 76 +++++++- protocol/http/handshake_test.go | 299 ++++++++++++++++++++++++++++++++ 2 files changed, 374 insertions(+), 1 deletion(-) create mode 100644 protocol/http/handshake_test.go diff --git a/protocol/http/handshake.go b/protocol/http/handshake.go index e01c13b2..d571bdaf 100644 --- a/protocol/http/handshake.go +++ b/protocol/http/handshake.go @@ -8,6 +8,8 @@ import ( "net" "net/http" "strings" + "sync/atomic" + "time" "github.com/sagernet/sing/common" "github.com/sagernet/sing/common/auth" @@ -20,6 +22,22 @@ import ( "github.com/sagernet/sing/common/pipe" ) +// proxyAuthRetryTimeoutNanos bounds waiting time for the single auth retry request. +// It remains mutable for tests and uses atomic storage to avoid races. +var proxyAuthRetryTimeoutNanos atomic.Int64 + +func init() { + proxyAuthRetryTimeoutNanos.Store(int64(5 * time.Second)) +} + +func proxyAuthRetryTimeout() time.Duration { + return time.Duration(proxyAuthRetryTimeoutNanos.Load()) +} + +func setProxyAuthRetryTimeout(timeout time.Duration) { + proxyAuthRetryTimeoutNanos.Store(int64(timeout)) +} + func HandleConnectionEx( ctx context.Context, conn net.Conn, @@ -29,11 +47,21 @@ func HandleConnectionEx( source M.Socksaddr, onClose N.CloseHandlerFunc, ) error { + missingProxyAuthorizationRetried := false + waitingRetryProxyAuthentication := false for { request, err := ReadRequest(reader) if err != nil { return E.Cause(err, "read http request") } + if waitingRetryProxyAuthentication { + waitingRetryProxyAuthentication = false + err = conn.SetReadDeadline(time.Time{}) + if err != nil { + return E.Cause(err, "clear retry-proxy-authentication timeout") + } + } + retryMissingProxyAuthorization := shouldRetryMissingProxyAuthorization(request) if authenticator != nil { var ( username string @@ -67,6 +95,15 @@ func HandleConnectionEx( } else if authorization != "" { return E.New("http: authentication failed, Proxy-Authorization=", authorization) } else { + if retryMissingProxyAuthorization && !missingProxyAuthorizationRetried { + missingProxyAuthorizationRetried = true + err = conn.SetReadDeadline(time.Now().Add(proxyAuthRetryTimeout())) + if err != nil { + return E.Cause(err, "set retry-proxy-authentication timeout") + } + waitingRetryProxyAuthentication = true + continue + } return E.New("http: authentication failed, no Proxy-Authorization header") } } @@ -147,7 +184,7 @@ func handleHTTPConnection( conn net.Conn, request *http.Request, source M.Socksaddr, ) error { - keepAlive := !(request.ProtoMajor == 1 && request.ProtoMinor == 0) && strings.TrimSpace(strings.ToLower(request.Header.Get("Proxy-Connection"))) == "keep-alive" + keepAlive := isProxyKeepAlive(request) request.RequestURI = "" removeHopByHopHeaders(request.Header) @@ -212,6 +249,43 @@ func handleHTTPConnection( return nil } +func isProxyKeepAlive(request *http.Request) bool { + connection := request.Header.Get("Connection") + proxyConnection := request.Header.Get("Proxy-Connection") + + if request.ProtoMajor > 1 || (request.ProtoMajor == 1 && request.ProtoMinor >= 1) { + // HTTP/1.1+ connections are persistent unless explicitly closed. + return !hasHeaderToken(connection, "close") && !hasHeaderToken(proxyConnection, "close") + } + + if request.ProtoMajor == 1 && request.ProtoMinor == 0 { + // HTTP/1.0 defaults to close unless keep-alive is requested. + if hasHeaderToken(connection, "close") || hasHeaderToken(proxyConnection, "close") { + return false + } + return hasHeaderToken(connection, "keep-alive") || hasHeaderToken(proxyConnection, "keep-alive") + } + + return false +} + +func hasHeaderToken(headerValue string, token string) bool { + for h := range strings.SplitSeq(headerValue, ",") { + if strings.EqualFold(strings.TrimSpace(h), token) { + return true + } + } + return false +} + +func shouldRetryMissingProxyAuthorization(request *http.Request) bool { + return isProxyKeepAlive(request) && + !request.Close && + !hasHeaderToken(request.Header.Get("Connection"), "upgrade") && + request.ContentLength == 0 && + len(request.TransferEncoding) == 0 +} + func removeHopByHopHeaders(header http.Header) { // Strip hop-by-hop header based on RFC: // http://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.5.1 diff --git a/protocol/http/handshake_test.go b/protocol/http/handshake_test.go new file mode 100644 index 00000000..61c61c22 --- /dev/null +++ b/protocol/http/handshake_test.go @@ -0,0 +1,299 @@ +package http + +import ( + std_bufio "bufio" + "context" + "encoding/base64" + "errors" + "io" + "net" + std_http "net/http" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/sagernet/sing/common/auth" + M "github.com/sagernet/sing/common/metadata" + N "github.com/sagernet/sing/common/network" +) + +type recordingHandler struct { + calls atomic.Int32 +} + +func (h *recordingHandler) NewConnectionEx(_ context.Context, conn net.Conn, _ M.Socksaddr, _ M.Socksaddr, onClose N.CloseHandlerFunc) { + h.calls.Add(1) + go func() { + defer conn.Close() + request, err := std_http.ReadRequest(std_bufio.NewReader(conn)) + if err != nil { + if onClose != nil { + onClose(err) + } + return + } + if request.Body != nil { + _ = request.Body.Close() + } + _, err = io.WriteString(conn, "HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: keep-alive\r\n\r\nOK") + if onClose != nil { + onClose(err) + } + }() +} + +func startHandshake(t *testing.T, authenticator *auth.Authenticator, handler N.TCPConnectionHandlerEx) (net.Conn, *std_bufio.Reader, <-chan error) { + t.Helper() + serverConn, clientConn := net.Pipe() + done := make(chan error, 1) + go func() { + defer serverConn.Close() + done <- HandleConnectionEx(context.Background(), serverConn, std_bufio.NewReader(serverConn), authenticator, handler, M.Socksaddr{}, nil) + }() + return clientConn, std_bufio.NewReader(clientConn), done +} + +func mustWriteRequest(t *testing.T, conn net.Conn, request string) { + t.Helper() + _, err := io.WriteString(conn, request) + if err != nil { + t.Fatalf("write request: %v", err) + } +} + +func mustReadResponse(t *testing.T, reader *std_bufio.Reader) *std_http.Response { + t.Helper() + request, err := std_http.NewRequest("GET", "http://example.com/", nil) + if err != nil { + t.Fatalf("new request: %v", err) + } + response, err := std_http.ReadResponse(reader, request) + if err != nil { + t.Fatalf("read response: %v", err) + } + return response +} + +func waitResult(t *testing.T, done <-chan error) error { + t.Helper() + select { + case err := <-done: + return err + case <-time.After(2 * time.Second): + t.Fatal("timeout waiting for handshake result") + return nil + } +} + +func TestHandleConnectionEx_AuthMissingHeaderRetryThenSuccess(t *testing.T) { + authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) + handler := &recordingHandler{} + clientConn, reader, done := startHandshake(t, authenticator, handler) + defer clientConn.Close() + + mustWriteRequest(t, clientConn, "GET http://example.com/ HTTP/1.1\r\nHost: example.com\r\n\r\n") + firstResponse := mustReadResponse(t, reader) + if firstResponse.StatusCode != std_http.StatusProxyAuthRequired { + t.Fatalf("first response status = %d, want %d", firstResponse.StatusCode, std_http.StatusProxyAuthRequired) + } + _ = firstResponse.Body.Close() + + authorization := base64.StdEncoding.EncodeToString([]byte("user:pass")) + mustWriteRequest(t, clientConn, "GET http://example.com/ HTTP/1.1\r\nHost: example.com\r\nProxy-Authorization: Basic "+authorization+"\r\n\r\n") + secondResponse := mustReadResponse(t, reader) + if secondResponse.StatusCode != std_http.StatusOK { + t.Fatalf("second response status = %d, want %d", secondResponse.StatusCode, std_http.StatusOK) + } + _ = secondResponse.Body.Close() + + if handler.calls.Load() != 1 { + t.Fatalf("handler call count = %d, want 1", handler.calls.Load()) + } + + _ = clientConn.Close() + if err := waitResult(t, done); err == nil { + t.Fatal("expected handshake to end with read error after client close") + } +} + +func TestHandleConnectionEx_AuthMissingHeaderRetryOnlyOnce(t *testing.T) { + authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) + handler := &recordingHandler{} + clientConn, reader, done := startHandshake(t, authenticator, handler) + defer clientConn.Close() + + request := "GET http://example.com/ HTTP/1.1\r\nHost: example.com\r\n\r\n" + mustWriteRequest(t, clientConn, request) + firstResponse := mustReadResponse(t, reader) + if firstResponse.StatusCode != std_http.StatusProxyAuthRequired { + t.Fatalf("first response status = %d, want %d", firstResponse.StatusCode, std_http.StatusProxyAuthRequired) + } + _ = firstResponse.Body.Close() + + mustWriteRequest(t, clientConn, request) + secondResponse := mustReadResponse(t, reader) + if secondResponse.StatusCode != std_http.StatusProxyAuthRequired { + t.Fatalf("second response status = %d, want %d", secondResponse.StatusCode, std_http.StatusProxyAuthRequired) + } + _ = secondResponse.Body.Close() + + err := waitResult(t, done) + if err == nil || !strings.Contains(err.Error(), "no Proxy-Authorization header") { + t.Fatalf("handshake error = %v, want missing Proxy-Authorization header", err) + } + if handler.calls.Load() != 0 { + t.Fatalf("handler call count = %d, want 0", handler.calls.Load()) + } +} + +func TestHandleConnectionEx_AuthMissingHeaderRetryHTTP10KeepAlive(t *testing.T) { + authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) + handler := &recordingHandler{} + clientConn, reader, done := startHandshake(t, authenticator, handler) + defer clientConn.Close() + + mustWriteRequest(t, clientConn, "GET http://example.com/ HTTP/1.0\r\nHost: example.com\r\nConnection: keep-alive\r\n\r\n") + firstResponse := mustReadResponse(t, reader) + if firstResponse.StatusCode != std_http.StatusProxyAuthRequired { + t.Fatalf("first response status = %d, want %d", firstResponse.StatusCode, std_http.StatusProxyAuthRequired) + } + _ = firstResponse.Body.Close() + + authorization := base64.StdEncoding.EncodeToString([]byte("user:pass")) + mustWriteRequest(t, clientConn, "GET http://example.com/ HTTP/1.0\r\nHost: example.com\r\nConnection: keep-alive\r\nProxy-Authorization: Basic "+authorization+"\r\n\r\n") + secondResponse := mustReadResponse(t, reader) + if secondResponse.StatusCode != std_http.StatusOK { + t.Fatalf("second response status = %d, want %d", secondResponse.StatusCode, std_http.StatusOK) + } + _ = secondResponse.Body.Close() + + if handler.calls.Load() != 1 { + t.Fatalf("handler call count = %d, want 1", handler.calls.Load()) + } + + _ = clientConn.Close() + if err := waitResult(t, done); err == nil { + t.Fatal("expected handshake to end with read error after client close") + } +} + +func TestHandleConnectionEx_AuthMissingHeaderNoRetryHTTP10DefaultClose(t *testing.T) { + authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) + handler := &recordingHandler{} + clientConn, reader, done := startHandshake(t, authenticator, handler) + defer clientConn.Close() + + mustWriteRequest(t, clientConn, "GET http://example.com/ HTTP/1.0\r\nHost: example.com\r\n\r\n") + response := mustReadResponse(t, reader) + if response.StatusCode != std_http.StatusProxyAuthRequired { + t.Fatalf("response status = %d, want %d", response.StatusCode, std_http.StatusProxyAuthRequired) + } + _ = response.Body.Close() + + err := waitResult(t, done) + if err == nil || !strings.Contains(err.Error(), "no Proxy-Authorization header") { + t.Fatalf("handshake error = %v, want missing Proxy-Authorization header", err) + } + if handler.calls.Load() != 0 { + t.Fatalf("handler call count = %d, want 0", handler.calls.Load()) + } +} + +func TestHandleConnectionEx_AuthMissingHeaderNoRetryHTTP10CloseWins(t *testing.T) { + authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) + handler := &recordingHandler{} + clientConn, reader, done := startHandshake(t, authenticator, handler) + defer clientConn.Close() + + mustWriteRequest(t, clientConn, "GET http://example.com/ HTTP/1.0\r\nHost: example.com\r\nConnection: keep-alive, close\r\n\r\n") + response := mustReadResponse(t, reader) + if response.StatusCode != std_http.StatusProxyAuthRequired { + t.Fatalf("response status = %d, want %d", response.StatusCode, std_http.StatusProxyAuthRequired) + } + _ = response.Body.Close() + + err := waitResult(t, done) + if err == nil || !strings.Contains(err.Error(), "no Proxy-Authorization header") { + t.Fatalf("handshake error = %v, want missing Proxy-Authorization header", err) + } + if handler.calls.Load() != 0 { + t.Fatalf("handler call count = %d, want 0", handler.calls.Load()) + } +} + +func TestHandleConnectionEx_AuthMissingHeaderNoRetryWithRequestBody(t *testing.T) { + authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) + handler := &recordingHandler{} + clientConn, reader, done := startHandshake(t, authenticator, handler) + defer clientConn.Close() + + mustWriteRequest(t, clientConn, "POST http://example.com/ HTTP/1.1\r\nHost: example.com\r\nProxy-Connection: keep-alive\r\nContent-Length: 4\r\n\r\nping") + response := mustReadResponse(t, reader) + if response.StatusCode != std_http.StatusProxyAuthRequired { + t.Fatalf("response status = %d, want %d", response.StatusCode, std_http.StatusProxyAuthRequired) + } + _ = response.Body.Close() + + err := waitResult(t, done) + if err == nil || !strings.Contains(err.Error(), "no Proxy-Authorization header") { + t.Fatalf("handshake error = %v, want missing Proxy-Authorization header", err) + } + if handler.calls.Load() != 0 { + t.Fatalf("handler call count = %d, want 0", handler.calls.Load()) + } +} + +func TestHandleConnectionEx_AuthMissingHeaderRetryTimeout(t *testing.T) { + oldTimeout := proxyAuthRetryTimeout() + setProxyAuthRetryTimeout(100 * time.Millisecond) + t.Cleanup(func() { + setProxyAuthRetryTimeout(oldTimeout) + }) + + authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) + handler := &recordingHandler{} + clientConn, reader, done := startHandshake(t, authenticator, handler) + defer clientConn.Close() + + mustWriteRequest(t, clientConn, "GET http://example.com/ HTTP/1.1\r\nHost: example.com\r\n\r\n") + response := mustReadResponse(t, reader) + if response.StatusCode != std_http.StatusProxyAuthRequired { + t.Fatalf("response status = %d, want %d", response.StatusCode, std_http.StatusProxyAuthRequired) + } + _ = response.Body.Close() + + err := waitResult(t, done) + if err == nil || !strings.Contains(err.Error(), "read http request") { + t.Fatalf("handshake error = %v, want wrapped read timeout", err) + } + var netErr net.Error + if !errors.As(err, &netErr) || !netErr.Timeout() { + t.Fatalf("handshake error = %v, want net timeout error", err) + } + if handler.calls.Load() != 0 { + t.Fatalf("handler call count = %d, want 0", handler.calls.Load()) + } +} + +func TestHandleConnectionEx_AuthMissingHeaderNoRetryOnUpgrade(t *testing.T) { + authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) + handler := &recordingHandler{} + clientConn, reader, done := startHandshake(t, authenticator, handler) + defer clientConn.Close() + + mustWriteRequest(t, clientConn, "GET http://example.com/ HTTP/1.1\r\nHost: example.com\r\nConnection: upgrade\r\nUpgrade: websocket\r\n\r\n") + response := mustReadResponse(t, reader) + if response.StatusCode != std_http.StatusProxyAuthRequired { + t.Fatalf("response status = %d, want %d", response.StatusCode, std_http.StatusProxyAuthRequired) + } + _ = response.Body.Close() + + err := waitResult(t, done) + if err == nil || !strings.Contains(err.Error(), "no Proxy-Authorization header") { + t.Fatalf("handshake error = %v, want missing Proxy-Authorization header", err) + } + if handler.calls.Load() != 0 { + t.Fatalf("handler call count = %d, want 0", handler.calls.Load()) + } +} From a08f9c928be2ca6a132c9b52468ffa458a3fad00 Mon Sep 17 00:00:00 2001 From: Tianhu Yang Date: Mon, 22 Jun 2026 23:49:25 +0800 Subject: [PATCH 3/5] http: use strings.Split for header token parsing --- protocol/http/handshake.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocol/http/handshake.go b/protocol/http/handshake.go index d571bdaf..0532736a 100644 --- a/protocol/http/handshake.go +++ b/protocol/http/handshake.go @@ -270,7 +270,7 @@ func isProxyKeepAlive(request *http.Request) bool { } func hasHeaderToken(headerValue string, token string) bool { - for h := range strings.SplitSeq(headerValue, ",") { + for _, h := range strings.Split(headerValue, ",") { if strings.EqualFold(strings.TrimSpace(h), token) { return true } From f6be1a78d33023f2baacd0f5a5b7e2dd993faa3b Mon Sep 17 00:00:00 2001 From: Tianhu Yang Date: Tue, 23 Jun 2026 14:55:07 +0800 Subject: [PATCH 4/5] http: use context logger and redact sensitive headers --- protocol/http/handshake.go | 96 +++++++++++++++---- protocol/http/handshake_test.go | 158 +++++++++++++++++++++++++++++--- 2 files changed, 221 insertions(+), 33 deletions(-) diff --git a/protocol/http/handshake.go b/protocol/http/handshake.go index 0532736a..a1a2e78b 100644 --- a/protocol/http/handshake.go +++ b/protocol/http/handshake.go @@ -7,8 +7,8 @@ import ( "io" "net" "net/http" + "sort" "strings" - "sync/atomic" "time" "github.com/sagernet/sing/common" @@ -17,28 +17,42 @@ import ( "github.com/sagernet/sing/common/bufio" E "github.com/sagernet/sing/common/exceptions" F "github.com/sagernet/sing/common/format" + "github.com/sagernet/sing/common/logger" M "github.com/sagernet/sing/common/metadata" N "github.com/sagernet/sing/common/network" "github.com/sagernet/sing/common/pipe" ) -// proxyAuthRetryTimeoutNanos bounds waiting time for the single auth retry request. -// It remains mutable for tests and uses atomic storage to avoid races. -var proxyAuthRetryTimeoutNanos atomic.Int64 +const defaultProxyAuthRetryTimeout = 5 * time.Second -func init() { - proxyAuthRetryTimeoutNanos.Store(int64(5 * time.Second)) +type HTTPServerOptions struct { + ProxyAuthRetryTimeout time.Duration + Logger logger.ContextLogger } -func proxyAuthRetryTimeout() time.Duration { - return time.Duration(proxyAuthRetryTimeoutNanos.Load()) +func normalizeHTTPServerOptions(options HTTPServerOptions) HTTPServerOptions { + if options.ProxyAuthRetryTimeout <= 0 { + options.ProxyAuthRetryTimeout = defaultProxyAuthRetryTimeout + } + if options.Logger == nil { + options.Logger = logger.NOP() + } + return options } -func setProxyAuthRetryTimeout(timeout time.Duration) { - proxyAuthRetryTimeoutNanos.Store(int64(timeout)) +func HandleConnectionEx( + ctx context.Context, + conn net.Conn, + reader *std_bufio.Reader, + authenticator *auth.Authenticator, + handler N.TCPConnectionHandlerEx, + source M.Socksaddr, + onClose N.CloseHandlerFunc, +) error { + return HandleConnectionExWithOptions(ctx, conn, reader, authenticator, handler, source, onClose, HTTPServerOptions{}) } -func HandleConnectionEx( +func HandleConnectionExWithOptions( ctx context.Context, conn net.Conn, reader *std_bufio.Reader, @@ -46,7 +60,9 @@ func HandleConnectionEx( handler N.TCPConnectionHandlerEx, source M.Socksaddr, onClose N.CloseHandlerFunc, + options HTTPServerOptions, ) error { + options = normalizeHTTPServerOptions(options) missingProxyAuthorizationRetried := false waitingRetryProxyAuthentication := false for { @@ -54,6 +70,7 @@ func HandleConnectionEx( if err != nil { return E.Cause(err, "read http request") } + printRequestHeaders(ctx, options.Logger, request) if waitingRetryProxyAuthentication { waitingRetryProxyAuthentication = false err = conn.SetReadDeadline(time.Time{}) @@ -83,10 +100,12 @@ func HandleConnectionEx( } if !authOk { // Since no one else is using the library, use a fixed realm until rewritten - err = responseWith( + proxyAuthRequiredResponse := responseWith( request, http.StatusProxyAuthRequired, "Proxy-Authenticate", `Basic realm="sing-box", charset="UTF-8"`, - ).Write(conn) + ) + printResponseHeaders(ctx, options.Logger, proxyAuthRequiredResponse) + err = proxyAuthRequiredResponse.Write(conn) if err != nil { return err } @@ -97,7 +116,7 @@ func HandleConnectionEx( } else { if retryMissingProxyAuthorization && !missingProxyAuthorizationRetried { missingProxyAuthorizationRetried = true - err = conn.SetReadDeadline(time.Now().Add(proxyAuthRetryTimeout())) + err = conn.SetReadDeadline(time.Now().Add(options.ProxyAuthRetryTimeout)) if err != nil { return E.Cause(err, "set retry-proxy-authentication timeout") } @@ -170,7 +189,7 @@ func HandleConnectionEx( } return bufio.CopyConn(ctx, conn, serverConn) } else { - err = handleHTTPConnection(ctx, handler, conn, request, source) + err = handleHTTPConnection(ctx, handler, conn, request, source, options.Logger) if err != nil { return err } @@ -182,7 +201,9 @@ func handleHTTPConnection( ctx context.Context, handler N.TCPConnectionHandlerEx, conn net.Conn, - request *http.Request, source M.Socksaddr, + request *http.Request, + source M.Socksaddr, + contextLogger logger.ContextLogger, ) error { keepAlive := isProxyKeepAlive(request) request.RequestURI = "" @@ -197,7 +218,9 @@ func handleHTTPConnection( } if request.URL.Scheme == "" || request.URL.Host == "" { - return responseWith(request, http.StatusBadRequest).Write(conn) + badRequestResponse := responseWith(request, http.StatusBadRequest) + printResponseHeaders(ctx, contextLogger, badRequestResponse) + return badRequestResponse.Write(conn) } var innerErr common.TypedValue[error] @@ -223,7 +246,9 @@ func handleHTTPConnection( response, err := httpClient.Do(request.WithContext(requestCtx)) if err != nil { cancel() - return E.Errors(innerErr.Load(), err, responseWith(request, http.StatusBadGateway).Write(conn)) + badGatewayResponse := responseWith(request, http.StatusBadGateway) + printResponseHeaders(ctx, contextLogger, badGatewayResponse) + return E.Errors(innerErr.Load(), err, badGatewayResponse.Write(conn)) } removeHopByHopHeaders(response.Header) @@ -235,6 +260,7 @@ func handleHTTPConnection( } response.Close = !keepAlive + printResponseHeaders(ctx, contextLogger, response) err = response.Write(conn) if err != nil { @@ -286,6 +312,40 @@ func shouldRetryMissingProxyAuthorization(request *http.Request) bool { len(request.TransferEncoding) == 0 } +func printRequestHeaders(ctx context.Context, contextLogger logger.ContextLogger, request *http.Request) { + contextLogger.TraceContext(ctx, "request protocol: ", request.Proto) + printHeaders(ctx, contextLogger, "request", request.Header) +} + +func printResponseHeaders(ctx context.Context, contextLogger logger.ContextLogger, response *http.Response) { + contextLogger.TraceContext(ctx, "response: protocol=", response.Proto, " status=", response.StatusCode) + printHeaders(ctx, contextLogger, "response", response.Header) +} + +func printHeaders(ctx context.Context, contextLogger logger.ContextLogger, kind string, header http.Header) { + keys := make([]string, 0, len(header)) + for key := range header { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + redacted := shouldRedactHeaderValue(key) + for _, value := range header[key] { + if redacted { + value = "[redacted]" + } + contextLogger.TraceContext(ctx, kind, " header: ", key, ": ", value) + } + } +} + +func shouldRedactHeaderValue(headerKey string) bool { + return strings.EqualFold(headerKey, "Authorization") || + strings.EqualFold(headerKey, "Proxy-Authorization") || + strings.EqualFold(headerKey, "Cookie") || + strings.EqualFold(headerKey, "Set-Cookie") +} + func removeHopByHopHeaders(header http.Header) { // Strip hop-by-hop header based on RFC: // http://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.5.1 diff --git a/protocol/http/handshake_test.go b/protocol/http/handshake_test.go index 61c61c22..013aa3e7 100644 --- a/protocol/http/handshake_test.go +++ b/protocol/http/handshake_test.go @@ -5,19 +5,85 @@ import ( "context" "encoding/base64" "errors" + "fmt" "io" "net" std_http "net/http" "strings" + "sync" "sync/atomic" "testing" "time" "github.com/sagernet/sing/common/auth" + "github.com/sagernet/sing/common/logger" M "github.com/sagernet/sing/common/metadata" N "github.com/sagernet/sing/common/network" ) +type testLogger struct { + mu sync.Mutex + messages []string +} + +var _ logger.ContextLogger = (*testLogger)(nil) + +func (l *testLogger) append(args ...any) { + l.mu.Lock() + defer l.mu.Unlock() + l.messages = append(l.messages, fmt.Sprint(args...)) +} + +func (l *testLogger) Messages() []string { + l.mu.Lock() + defer l.mu.Unlock() + return append([]string(nil), l.messages...) +} + +func (l *testLogger) Trace(args ...any) { + l.append(args...) +} + +func (l *testLogger) Debug(args ...any) { +} + +func (l *testLogger) Info(args ...any) { +} + +func (l *testLogger) Warn(args ...any) { +} + +func (l *testLogger) Error(args ...any) { +} + +func (l *testLogger) Fatal(args ...any) { +} + +func (l *testLogger) Panic(args ...any) { +} + +func (l *testLogger) TraceContext(ctx context.Context, args ...any) { + l.append(args...) +} + +func (l *testLogger) DebugContext(ctx context.Context, args ...any) { +} + +func (l *testLogger) InfoContext(ctx context.Context, args ...any) { +} + +func (l *testLogger) WarnContext(ctx context.Context, args ...any) { +} + +func (l *testLogger) ErrorContext(ctx context.Context, args ...any) { +} + +func (l *testLogger) FatalContext(ctx context.Context, args ...any) { +} + +func (l *testLogger) PanicContext(ctx context.Context, args ...any) { +} + type recordingHandler struct { calls atomic.Int32 } @@ -43,13 +109,17 @@ func (h *recordingHandler) NewConnectionEx(_ context.Context, conn net.Conn, _ M }() } -func startHandshake(t *testing.T, authenticator *auth.Authenticator, handler N.TCPConnectionHandlerEx) (net.Conn, *std_bufio.Reader, <-chan error) { +func startHandshake(t *testing.T, authenticator *auth.Authenticator, handler N.TCPConnectionHandlerEx, options *HTTPServerOptions) (net.Conn, *std_bufio.Reader, <-chan error) { t.Helper() serverConn, clientConn := net.Pipe() done := make(chan error, 1) go func() { defer serverConn.Close() - done <- HandleConnectionEx(context.Background(), serverConn, std_bufio.NewReader(serverConn), authenticator, handler, M.Socksaddr{}, nil) + if options == nil { + done <- HandleConnectionEx(context.Background(), serverConn, std_bufio.NewReader(serverConn), authenticator, handler, M.Socksaddr{}, nil) + } else { + done <- HandleConnectionExWithOptions(context.Background(), serverConn, std_bufio.NewReader(serverConn), authenticator, handler, M.Socksaddr{}, nil, *options) + } }() return clientConn, std_bufio.NewReader(clientConn), done } @@ -89,7 +159,7 @@ func waitResult(t *testing.T, done <-chan error) error { func TestHandleConnectionEx_AuthMissingHeaderRetryThenSuccess(t *testing.T) { authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) handler := &recordingHandler{} - clientConn, reader, done := startHandshake(t, authenticator, handler) + clientConn, reader, done := startHandshake(t, authenticator, handler, nil) defer clientConn.Close() mustWriteRequest(t, clientConn, "GET http://example.com/ HTTP/1.1\r\nHost: example.com\r\n\r\n") @@ -120,7 +190,7 @@ func TestHandleConnectionEx_AuthMissingHeaderRetryThenSuccess(t *testing.T) { func TestHandleConnectionEx_AuthMissingHeaderRetryOnlyOnce(t *testing.T) { authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) handler := &recordingHandler{} - clientConn, reader, done := startHandshake(t, authenticator, handler) + clientConn, reader, done := startHandshake(t, authenticator, handler, nil) defer clientConn.Close() request := "GET http://example.com/ HTTP/1.1\r\nHost: example.com\r\n\r\n" @@ -150,7 +220,7 @@ func TestHandleConnectionEx_AuthMissingHeaderRetryOnlyOnce(t *testing.T) { func TestHandleConnectionEx_AuthMissingHeaderRetryHTTP10KeepAlive(t *testing.T) { authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) handler := &recordingHandler{} - clientConn, reader, done := startHandshake(t, authenticator, handler) + clientConn, reader, done := startHandshake(t, authenticator, handler, nil) defer clientConn.Close() mustWriteRequest(t, clientConn, "GET http://example.com/ HTTP/1.0\r\nHost: example.com\r\nConnection: keep-alive\r\n\r\n") @@ -181,7 +251,7 @@ func TestHandleConnectionEx_AuthMissingHeaderRetryHTTP10KeepAlive(t *testing.T) func TestHandleConnectionEx_AuthMissingHeaderNoRetryHTTP10DefaultClose(t *testing.T) { authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) handler := &recordingHandler{} - clientConn, reader, done := startHandshake(t, authenticator, handler) + clientConn, reader, done := startHandshake(t, authenticator, handler, nil) defer clientConn.Close() mustWriteRequest(t, clientConn, "GET http://example.com/ HTTP/1.0\r\nHost: example.com\r\n\r\n") @@ -203,7 +273,7 @@ func TestHandleConnectionEx_AuthMissingHeaderNoRetryHTTP10DefaultClose(t *testin func TestHandleConnectionEx_AuthMissingHeaderNoRetryHTTP10CloseWins(t *testing.T) { authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) handler := &recordingHandler{} - clientConn, reader, done := startHandshake(t, authenticator, handler) + clientConn, reader, done := startHandshake(t, authenticator, handler, nil) defer clientConn.Close() mustWriteRequest(t, clientConn, "GET http://example.com/ HTTP/1.0\r\nHost: example.com\r\nConnection: keep-alive, close\r\n\r\n") @@ -225,7 +295,7 @@ func TestHandleConnectionEx_AuthMissingHeaderNoRetryHTTP10CloseWins(t *testing.T func TestHandleConnectionEx_AuthMissingHeaderNoRetryWithRequestBody(t *testing.T) { authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) handler := &recordingHandler{} - clientConn, reader, done := startHandshake(t, authenticator, handler) + clientConn, reader, done := startHandshake(t, authenticator, handler, nil) defer clientConn.Close() mustWriteRequest(t, clientConn, "POST http://example.com/ HTTP/1.1\r\nHost: example.com\r\nProxy-Connection: keep-alive\r\nContent-Length: 4\r\n\r\nping") @@ -245,15 +315,11 @@ func TestHandleConnectionEx_AuthMissingHeaderNoRetryWithRequestBody(t *testing.T } func TestHandleConnectionEx_AuthMissingHeaderRetryTimeout(t *testing.T) { - oldTimeout := proxyAuthRetryTimeout() - setProxyAuthRetryTimeout(100 * time.Millisecond) - t.Cleanup(func() { - setProxyAuthRetryTimeout(oldTimeout) - }) + options := &HTTPServerOptions{ProxyAuthRetryTimeout: 100 * time.Millisecond} authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) handler := &recordingHandler{} - clientConn, reader, done := startHandshake(t, authenticator, handler) + clientConn, reader, done := startHandshake(t, authenticator, handler, options) defer clientConn.Close() mustWriteRequest(t, clientConn, "GET http://example.com/ HTTP/1.1\r\nHost: example.com\r\n\r\n") @@ -279,7 +345,7 @@ func TestHandleConnectionEx_AuthMissingHeaderRetryTimeout(t *testing.T) { func TestHandleConnectionEx_AuthMissingHeaderNoRetryOnUpgrade(t *testing.T) { authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) handler := &recordingHandler{} - clientConn, reader, done := startHandshake(t, authenticator, handler) + clientConn, reader, done := startHandshake(t, authenticator, handler, nil) defer clientConn.Close() mustWriteRequest(t, clientConn, "GET http://example.com/ HTTP/1.1\r\nHost: example.com\r\nConnection: upgrade\r\nUpgrade: websocket\r\n\r\n") @@ -297,3 +363,65 @@ func TestHandleConnectionEx_AuthMissingHeaderNoRetryOnUpgrade(t *testing.T) { t.Fatalf("handler call count = %d, want 0", handler.calls.Load()) } } + +func TestHandleConnectionEx_LoggerPrintsRequestAndResponse(t *testing.T) { + testLog := &testLogger{} + options := &HTTPServerOptions{Logger: testLog} + authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) + handler := &recordingHandler{} + + clientConn, reader, done := startHandshake(t, authenticator, handler, options) + defer clientConn.Close() + + mustWriteRequest(t, clientConn, "GET http://example.com/ HTTP/1.0\r\nHost: example.com\r\n\r\n") + response := mustReadResponse(t, reader) + if response.StatusCode != std_http.StatusProxyAuthRequired { + t.Fatalf("response status = %d, want %d", response.StatusCode, std_http.StatusProxyAuthRequired) + } + _ = response.Body.Close() + + err := waitResult(t, done) + if err == nil || !strings.Contains(err.Error(), "no Proxy-Authorization header") { + t.Fatalf("handshake error = %v, want missing Proxy-Authorization header", err) + } + + logs := testLog.Messages() + joined := strings.Join(logs, "\n") + if !strings.Contains(joined, "request protocol: HTTP/1.0") { + t.Fatalf("logs missing request protocol line, logs: %s", joined) + } + if !strings.Contains(joined, "response: protocol=HTTP/1.0 status=407") { + t.Fatalf("logs missing response status line, logs: %s", joined) + } +} + +func TestHandleConnectionEx_LoggerRedactsSensitiveHeaders(t *testing.T) { + testLog := &testLogger{} + options := &HTTPServerOptions{Logger: testLog} + authenticator := auth.NewAuthenticator([]auth.User{{Username: "user", Password: "pass"}}) + handler := &recordingHandler{} + + clientConn, reader, done := startHandshake(t, authenticator, handler, options) + defer clientConn.Close() + + authorization := base64.StdEncoding.EncodeToString([]byte("user:pass")) + mustWriteRequest(t, clientConn, "GET http://example.com/ HTTP/1.1\r\nHost: example.com\r\nProxy-Authorization: Basic "+authorization+"\r\n\r\n") + response := mustReadResponse(t, reader) + if response.StatusCode != std_http.StatusOK { + t.Fatalf("response status = %d, want %d", response.StatusCode, std_http.StatusOK) + } + _ = response.Body.Close() + + _ = clientConn.Close() + if err := waitResult(t, done); err == nil { + t.Fatal("expected handshake to end with read error after client close") + } + + joined := strings.Join(testLog.Messages(), "\n") + if !strings.Contains(joined, "request header: Proxy-Authorization: [redacted]") { + t.Fatalf("logs missing redacted proxy authorization header, logs: %s", joined) + } + if strings.Contains(joined, authorization) || strings.Contains(joined, "user:pass") { + t.Fatalf("logs unexpectedly contain sensitive credentials, logs: %s", joined) + } +} From 6c597e4bf5416da36226f02e301316fdd0ace9b7 Mon Sep 17 00:00:00 2001 From: Tianhu Yang Date: Tue, 23 Jun 2026 18:11:40 +0800 Subject: [PATCH 5/5] protocol/http: buffer 407 response and preserve auth retry/logging --- protocol/http/handshake.go | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/protocol/http/handshake.go b/protocol/http/handshake.go index a1a2e78b..e1b003d1 100644 --- a/protocol/http/handshake.go +++ b/protocol/http/handshake.go @@ -2,6 +2,7 @@ package http import ( std_bufio "bufio" + "bytes" "context" "encoding/base64" "io" @@ -105,9 +106,9 @@ func HandleConnectionExWithOptions( "Proxy-Authenticate", `Basic realm="sing-box", charset="UTF-8"`, ) printResponseHeaders(ctx, options.Logger, proxyAuthRequiredResponse) - err = proxyAuthRequiredResponse.Write(conn) + err = writeResponseBuffered(conn, proxyAuthRequiredResponse) if err != nil { - return err + return E.Cause(err, "write proxy authentication required response") } if username != "" { return E.New("http: authentication failed, username=", username, ", password=", password) @@ -386,6 +387,22 @@ func removeExtraHTTPHostPort(req *http.Request) { req.URL.Host = host } +func writeResponseBuffered(conn net.Conn, response *http.Response) error { + var responseBuffer bytes.Buffer + err := response.Write(&responseBuffer) + if err != nil { + return err + } + n, err := conn.Write(responseBuffer.Bytes()) + if err != nil { + return err + } + if n != responseBuffer.Len() { + return io.ErrShortWrite + } + return nil +} + func responseWith(request *http.Request, statusCode int, headers ...string) *http.Response { var header http.Header if len(headers) > 0 {