things-go / go-socks5

socks5 server in pure Golang with much custom optional. Full TCP/UDP and IPv4/IPv6 support.
MIT License
368 stars 59 forks source link

feat: use common net.Conn and add EOF check #12

Closed cxz66666 closed 1 year ago

cxz66666 commented 1 year ago
  1. net.UDPConn is not a good type because it implements lots of functions, e.g.: if dial call gonet.DialUDP and returns a gonet.UDPConn type, it will cause a type cast error! So change all the net.UDPConn types to net.Conn

  2. Add check EOF error in io.Read . Without it in most situations, it will cause infinite loops!

codecov[bot] commented 1 year ago

Codecov Report

Merging #12 (7bfa01d) into master (da64a46) will decrease coverage by 0.24%. The diff coverage is 36.36%.

@@            Coverage Diff             @@
##           master      #12      +/-   ##
==========================================
- Coverage   62.19%   61.95%   -0.24%     
==========================================
  Files          14       14              
  Lines         775      778       +3     
==========================================
  Hits          482      482              
- Misses        232      236       +4     
+ Partials       61       60       -1     
Flag Coverage Δ
unittests 61.95% <36.36%> (-0.24%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
option.go 14.81% <ø> (ø)
statute/auth.go 67.34% <ø> (ø)
statute/message.go 66.66% <ø> (ø)
statute/method.go 73.52% <ø> (ø)
handle.go 46.02% <26.31%> (-0.59%) :arrow_down:
bufferpool/pool.go 100.00% <100.00%> (ø)
server.go 61.53% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

cxz66666 commented 1 year ago

ping @thinkgos, any update?

thinkgos commented 1 year ago

@cxz66666 改成 net.Conn 是因为要使用 其它网络库么? 可以考虑用 userAssociateHandle, 自定义一个解析 . 目前CI mac go1.15版本测试失败

cxz66666 commented 1 year ago

@thinkgos

  1. 一方面是我需要使用gvisor中的gonet库,另一方面是感觉使用net.Conn接口通用性更好,现在的功能也完全可以用net.Conn代替
  2. 我尝试回退到最新的提交#50a7e68 版本跑了下ci,得到了和现在完全一样的结果,都是go1.15 macos 在TestDNSResolver中报错Attempt to use unknown class,使用1.16 macos则可以跑通。或许您可以更新下ci的go版本 https://github.com/cxz66666/go-socks5/actions/runs/4172609361/jobs/7223925635
  3. 还有eof的检查需要额外注意一下,现在的代码中如果https://github.com/things-go/go-socks5/blob/50a7e687253fd65b0e4f103ae42ed85f2aeb080a/handle.go#L280-L288 err返回err.EOF,则会导致无限循环 导致 cpu吃满,关联此issue https://github.com/things-go/go-socks5/issues/6
thinkgos commented 1 year ago

@cxz66666 这样, 你把所有检查eof 的返回明确返回nil, 把go.mod 版本提到1.18, ci 提到 1.19和1.20

cxz66666 commented 1 year ago

@thinkgos ok,稍后我会在此分支中更新

cxz66666 commented 1 year ago

@thinkgos 已经更新若干文件

  1. go mod升级1.18,更新go.mod与go.sum
  2. github action全部升级latest版本,ci使用1.18和1.19进行测试,cilint 的action已经可以通过
  3. 根据golangci-lint修改若干文件
thinkgos commented 1 year ago

@cxz66666 感谢, 我顺便加了dependabot. 那个golangci-lint 可以不用太关心,我没有实时跟进这些规则.