fix(ss): address review findings for PR #96 — nil guards, resource leaks, dead code
- Bump go-shadowsocks2 to v0.1.3 (new API symbols) - Return error instead of nil when Target() is empty in TCP handler - Reset wbuf unconditionally on write error to prevent unbounded growth - Add nil guard on targetAddr before WriteTo to prevent panic - Remove duplicate NewClientConfig call and dead ClientConfig literal - Remove unused noDelay metadata field - Fix buffer-size guard to catch negative values (len(b)==0 → bufSize<=0) - Add address context to parse/session error messages
This commit is contained in:
@@ -10,19 +10,16 @@ import (
|
|||||||
type metadata struct {
|
type metadata struct {
|
||||||
key string
|
key string
|
||||||
connectTimeout time.Duration
|
connectTimeout time.Duration
|
||||||
noDelay bool
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func (c *ssConnector) parseMetadata(md mdata.Metadata) (err error) {
|
func (c *ssConnector) parseMetadata(md mdata.Metadata) (err error) {
|
||||||
const (
|
const (
|
||||||
key = "key"
|
key = "key"
|
||||||
connectTimeout = "timeout"
|
connectTimeout = "timeout"
|
||||||
noDelay = "nodelay"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
c.md.key = mdutil.GetString(md, key)
|
c.md.key = mdutil.GetString(md, key)
|
||||||
c.md.connectTimeout = mdutil.GetDuration(md, connectTimeout)
|
c.md.connectTimeout = mdutil.GetDuration(md, connectTimeout)
|
||||||
c.md.noDelay = mdutil.GetBool(md, noDelay)
|
|
||||||
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -58,13 +58,8 @@ func (c *ssuConnector) Init(md md.Metadata) (err error) {
|
|||||||
}
|
}
|
||||||
clientConfig.UDPTimeout = time.Minute
|
clientConfig.UDPTimeout = time.Minute
|
||||||
|
|
||||||
tcpClientConfig, err := utils.NewClientConfig(method, password)
|
|
||||||
if err != nil {
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
|
|
||||||
c.clientCfg = clientConfig
|
c.clientCfg = clientConfig
|
||||||
c.tcpClient = core.NewTCPClient(tcpClientConfig)
|
c.tcpClient = core.NewTCPClient(clientConfig)
|
||||||
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -98,15 +93,11 @@ func (c *ssuConnector) Connect(ctx context.Context, conn net.Conn, network, addr
|
|||||||
}
|
}
|
||||||
serverAddr, err := netip.ParseAddrPort(conn.RemoteAddr().String())
|
serverAddr, err := netip.ParseAddrPort(conn.RemoteAddr().String())
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, fmt.Errorf("ss: parse remote addr %q: %w", conn.RemoteAddr().String(), err)
|
||||||
}
|
}
|
||||||
clientCfg := c.clientCfg
|
clientCfg := c.clientCfg
|
||||||
clientCfg.ServerAddr = serverAddr
|
clientCfg.ServerAddr = serverAddr
|
||||||
client := core.NewUDPClient(core.ClientConfig{
|
client := core.NewUDPClient(clientCfg)
|
||||||
Cipher: clientCfg.Cipher,
|
|
||||||
ServerAddr: serverAddr,
|
|
||||||
UDPTimeout: time.Minute,
|
|
||||||
})
|
|
||||||
if err := client.Init(); err != nil {
|
if err := client.Init(); err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -8,7 +8,7 @@ require (
|
|||||||
github.com/gin-contrib/cors v1.7.2
|
github.com/gin-contrib/cors v1.7.2
|
||||||
github.com/gin-gonic/gin v1.10.1
|
github.com/gin-gonic/gin v1.10.1
|
||||||
github.com/go-gost/core v0.4.0
|
github.com/go-gost/core v0.4.0
|
||||||
github.com/go-gost/go-shadowsocks2 v0.1.2
|
github.com/go-gost/go-shadowsocks2 v0.1.3
|
||||||
github.com/go-gost/gosocks4 v0.1.0
|
github.com/go-gost/gosocks4 v0.1.0
|
||||||
github.com/go-gost/gosocks5 v0.5.0
|
github.com/go-gost/gosocks5 v0.5.0
|
||||||
github.com/go-gost/plugin v0.3.0
|
github.com/go-gost/plugin v0.3.0
|
||||||
|
|||||||
@@ -51,8 +51,8 @@ github.com/gin-gonic/gin v1.10.1 h1:T0ujvqyCSqRopADpgPgiTT63DUQVSfojyME59Ei63pQ=
|
|||||||
github.com/gin-gonic/gin v1.10.1/go.mod h1:4PMNQiOhvDRa013RKVbsiNwoyezlm2rm0uX/T7kzp5Y=
|
github.com/gin-gonic/gin v1.10.1/go.mod h1:4PMNQiOhvDRa013RKVbsiNwoyezlm2rm0uX/T7kzp5Y=
|
||||||
github.com/go-gost/core v0.4.0 h1:jLuIMO7ZMJX/BDCYLrYXAmIykT+HkqEuzaTeLMdrY1c=
|
github.com/go-gost/core v0.4.0 h1:jLuIMO7ZMJX/BDCYLrYXAmIykT+HkqEuzaTeLMdrY1c=
|
||||||
github.com/go-gost/core v0.4.0/go.mod h1:WGI43jOka7FAsSAwi/fSMaqxdR+E339ycb4NBGlFr6A=
|
github.com/go-gost/core v0.4.0/go.mod h1:WGI43jOka7FAsSAwi/fSMaqxdR+E339ycb4NBGlFr6A=
|
||||||
github.com/go-gost/go-shadowsocks2 v0.1.2 h1:qgTUQS6aCGzsFArjiKjLP0a7zEVtEYVCRVc5LjOsaVk=
|
github.com/go-gost/go-shadowsocks2 v0.1.3 h1:6CUZLp+mTWXnKP2aK8/Z9ZP+ERMX9gSbywmPu4kGX/A=
|
||||||
github.com/go-gost/go-shadowsocks2 v0.1.2/go.mod h1:866zFNNI3He6Wef1M/IvAjTal74WhcfKfBgRpTlkKys=
|
github.com/go-gost/go-shadowsocks2 v0.1.3/go.mod h1:866zFNNI3He6Wef1M/IvAjTal74WhcfKfBgRpTlkKys=
|
||||||
github.com/go-gost/gosocks4 v0.1.0 h1:eAzev6qw4fzkFQKC9uCHLVNnnPdHyqCggbnfNN80Pmk=
|
github.com/go-gost/gosocks4 v0.1.0 h1:eAzev6qw4fzkFQKC9uCHLVNnnPdHyqCggbnfNN80Pmk=
|
||||||
github.com/go-gost/gosocks4 v0.1.0/go.mod h1:hzVjwijJuZR1pp3GqpTj+AKcSGrx68RlWTrQMFMYBP0=
|
github.com/go-gost/gosocks4 v0.1.0/go.mod h1:hzVjwijJuZR1pp3GqpTj+AKcSGrx68RlWTrQMFMYBP0=
|
||||||
github.com/go-gost/gosocks5 v0.5.0 h1:YE37l1MJwde8diIQdynStqogMotG5enoTdborhA5yic=
|
github.com/go-gost/gosocks5 v0.5.0 h1:YE37l1MJwde8diIQdynStqogMotG5enoTdborhA5yic=
|
||||||
|
|||||||
@@ -149,7 +149,7 @@ func (h *ssHandler) Handle(ctx context.Context, conn net.Conn, opts ...handler.H
|
|||||||
target = tc.Target()
|
target = tc.Target()
|
||||||
}
|
}
|
||||||
if len(target) == 0 {
|
if len(target) == 0 {
|
||||||
return
|
return errors.New("ss: target not available")
|
||||||
}
|
}
|
||||||
conn = wrappedConn
|
conn = wrappedConn
|
||||||
|
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"bytes"
|
"bytes"
|
||||||
"context"
|
"context"
|
||||||
"errors"
|
"errors"
|
||||||
|
"fmt"
|
||||||
"net"
|
"net"
|
||||||
"sync"
|
"sync"
|
||||||
"time"
|
"time"
|
||||||
@@ -191,7 +192,7 @@ func (h *ssuHandler) relayPacketUDP(ctx context.Context, src net.PacketConn, ro
|
|||||||
TargetAddr() net.Addr
|
TargetAddr() net.Addr
|
||||||
})
|
})
|
||||||
if !ok {
|
if !ok {
|
||||||
return errors.New("ss: missing udp session context")
|
return fmt.Errorf("ss: missing udp session context for addr %v", addr)
|
||||||
}
|
}
|
||||||
|
|
||||||
targetAddr := ctxAddr.TargetAddr()
|
targetAddr := ctxAddr.TargetAddr()
|
||||||
@@ -204,9 +205,13 @@ func (h *ssuHandler) relayPacketUDP(ctx context.Context, src net.PacketConn, ro
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if targetAddr == nil {
|
||||||
|
return errors.New("ss: target address not available")
|
||||||
|
}
|
||||||
|
|
||||||
session := ctxAddr.Session()
|
session := ctxAddr.Session()
|
||||||
if session == nil {
|
if session == nil {
|
||||||
return errors.New("ss: udp session not found")
|
return fmt.Errorf("ss: udp session not found for addr %v", addr)
|
||||||
}
|
}
|
||||||
|
|
||||||
dstConn, err := h.packetConnForSession(ctx, src, session, ro, log)
|
dstConn, err := h.packetConnForSession(ctx, src, session, ro, log)
|
||||||
@@ -255,10 +260,11 @@ func (h *ssuHandler) packetConnForSession(ctx context.Context, src net.PacketCon
|
|||||||
h.connMap.Delete(session.Hash())
|
h.connMap.Delete(session.Hash())
|
||||||
}()
|
}()
|
||||||
|
|
||||||
b := make([]byte, h.md.udpBufferSize)
|
bufSize := h.md.udpBufferSize
|
||||||
if len(b) == 0 {
|
if bufSize <= 0 {
|
||||||
b = make([]byte, defaultBufferSize)
|
bufSize = defaultBufferSize
|
||||||
}
|
}
|
||||||
|
b := make([]byte, bufSize)
|
||||||
|
|
||||||
for {
|
for {
|
||||||
n, raddr, err := cc.ReadFrom(b)
|
n, raddr, err := cc.ReadFrom(b)
|
||||||
|
|||||||
@@ -36,10 +36,7 @@ func (c *shadowConn) Write(b []byte) (n int, err error) {
|
|||||||
c.wbuf.Write(b) // append the data to the cached header
|
c.wbuf.Write(b) // append the data to the cached header
|
||||||
written, err := c.Conn.Write(c.wbuf.Bytes())
|
written, err := c.Conn.Write(c.wbuf.Bytes())
|
||||||
if err != nil {
|
if err != nil {
|
||||||
// If short write, keep unwritten bytes
|
c.wbuf.Reset()
|
||||||
if written > 0 && written < c.wbuf.Len() {
|
|
||||||
c.wbuf.Next(written)
|
|
||||||
}
|
|
||||||
return written, err
|
return written, err
|
||||||
}
|
}
|
||||||
c.wbuf.Reset()
|
c.wbuf.Reset()
|
||||||
|
|||||||
Reference in New Issue
Block a user