Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ func main() {
}()

// Run the application. This blocks until the application has been exited.
err := app.Run()
err = app.Run()

// If an error occurred while running the application, log it and exit.
if err != nil {
Expand Down
39 changes: 35 additions & 4 deletions services/httpclient.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package services

import (
"context"
"crypto/tls"
"fmt"
"net"
Expand Down Expand Up @@ -193,17 +194,47 @@ func createSOCKS5ProxyTransport(proxyAddr string) (*http.Transport, error) {
socksAddr = proxyAddr
}

// 创建 SOCKS5 拨号器
dialer, err := proxy.SOCKS5("tcp", socksAddr, nil, proxy.Direct)
// 创建 SOCKS5 拨号器,使用带超时的底层拨号器避免长时间阻塞
baseDialer := &net.Dialer{
Timeout: 30 * time.Second,
KeepAlive: 30 * time.Second,
}
dialer, err := proxy.SOCKS5("tcp", socksAddr, nil, baseDialer)
if err != nil {
return nil, fmt.Errorf("创建 SOCKS5 拨号器失败: %w", err)
}

// 创建使用 SOCKS5 代理的传输层
transport := &http.Transport{
Dial: dialer.Dial,
DialContext: func(ctx interface{}, network, addr string) (net.Conn, error) {
return dialer.Dial(network, addr)
DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
if ctxDialer, ok := dialer.(proxy.ContextDialer); ok {
return ctxDialer.DialContext(ctx, network, addr)
}
type result struct {
conn net.Conn
err error
}
resultCh := make(chan result, 1)
go func() {
if ctxErr := ctx.Err(); ctxErr != nil {
resultCh <- result{conn: nil, err: ctxErr}
return
}
conn, err := dialer.Dial(network, addr)
if ctx.Err() != nil && conn != nil {
_ = conn.Close()
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error returned by conn.Close() is being silently ignored with _. While it's common to ignore close errors in cleanup paths, in this case the connection closure is part of the cancellation handling logic. If the close operation fails, it could lead to resource leaks.

Consider logging the error or at minimum using a comment to document why the error is intentionally ignored.

Copilot uses AI. Check for mistakes.
err = ctx.Err()
conn = nil
}
resultCh <- result{conn: conn, err: err}
}()
select {
case res := <-resultCh:
return res.conn, res.err
case <-ctx.Done():
Comment on lines +220 to +235
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a potential race condition in the context cancellation check. The goroutine checks ctx.Err() at line 220 and then calls dialer.Dial() at line 224, but the context could be canceled between these two checks. Additionally, even if the goroutine detects the context is canceled at line 225, it still sends the result to the channel, which could race with the ctx.Done() case in the select statement.

A cleaner approach would be to handle context cancellation in the select statement and let the goroutine complete naturally. The check at line 220 is also redundant since if the context is already done, the select statement will handle it immediately.

Suggested change
if ctxErr := ctx.Err(); ctxErr != nil {
resultCh <- result{conn: nil, err: ctxErr}
return
}
conn, err := dialer.Dial(network, addr)
if ctx.Err() != nil && conn != nil {
_ = conn.Close()
err = ctx.Err()
conn = nil
}
resultCh <- result{conn: conn, err: err}
}()
select {
case res := <-resultCh:
return res.conn, res.err
case <-ctx.Done():
conn, err := dialer.Dial(network, addr)
resultCh <- result{conn: conn, err: err}
}()
select {
case res := <-resultCh:
// If the context was canceled while dialing, prefer the context error
if ctxErr := ctx.Err(); ctxErr != nil {
if res.conn != nil {
_ = res.conn.Close()
}
return nil, ctxErr
}
return res.conn, res.err
case <-ctx.Done():
// Context canceled before dialing completed; ensure we close any eventual connection.
go func() {
res := <-resultCh
if res.conn != nil {
_ = res.conn.Close()
}
}()

Copilot uses AI. Check for mistakes.
return nil, ctx.Err()
}
Comment on lines +232 to +237
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a potential resource leak when the context is canceled. If the context is canceled after the select statement returns at line 235-236 but before the goroutine completes and sends the result to the channel, the connection opened by dialer.Dial() at line 224 may not be closed. The goroutine would send a result to the buffered channel, but since the main function has already returned, no one would receive it to close the connection.

Consider adding cleanup logic to ensure connections are always closed when the context is canceled, even if the cancellation happens after the function returns.

Copilot uses AI. Check for mistakes.
Comment on lines +210 to +237
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new context-aware SOCKS5 dialing logic lacks test coverage. Given that the repository has comprehensive tests for other services (geminiservice_test.go, providerservice_test.go, etc.), this critical proxy functionality should also have test coverage to verify context cancellation behavior, timeout handling, and connection cleanup work correctly.

Consider adding tests for scenarios such as:

  • Successful dial with context
  • Context canceled before dial completes
  • Context canceled after dial completes
  • Type assertion to ContextDialer succeeds/fails

Copilot uses AI. Check for mistakes.
},
ForceAttemptHTTP2: false, // SOCKS5 通常不支持 HTTP/2
MaxIdleConns: 100,
Expand Down