Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: 修复UDP转发功能 #33

Merged
merged 3 commits into from
Sep 11, 2023
Merged

Conversation

cxz66666
Copy link
Contributor

@cxz66666 cxz66666 commented Sep 2, 2023

#29#1 中提到,RFC标准中的Associate function逻辑和当前实现的有出入,

#1 (comment) 中提到

因为RFC文档里面规定UDP Associate请求所携带的地址应当是client欲发送数据的地址+端口,这一个值可以被服务器用来限制来自其余地址的数据包。
你所说的真实UDP地址是在UDP Associate请求协商完成之后,在client向socks5服务器发送的数据必须加上一个特殊的数据包头,包头中携带真实的目标udp地址,服务器解析后决定将client的请求发往何处

但目前实现逻辑恰好是相反的,因此本提交修复了这个问题,使其符合RFC1928标准

目前可以顺利通过 https://github.com/txthinking/testsocks5https://github.com/semigodking/socks5chk 的udp转发测试,同时正确处理了EOF和连接关闭的情况,可以正常退出协程。

@thinkgos
Copy link
Member

thinkgos commented Sep 2, 2023

你好, ci过不了, 把测试也改一下

@cxz66666
Copy link
Contributor Author

cxz66666 commented Sep 2, 2023

好的,我晚上修一下

@codecov
Copy link

codecov bot commented Sep 2, 2023

Codecov Report

Merging #33 (a447d34) into master (ea3e2a0) will increase coverage by 0.23%.
The diff coverage is 28.88%.

@@            Coverage Diff             @@
##           master      #33      +/-   ##
==========================================
+ Coverage   61.95%   62.19%   +0.23%     
==========================================
  Files          14       14              
  Lines         778      775       -3     
==========================================
  Hits          482      482              
+ Misses        236      233       -3     
  Partials       60       60              
Flag Coverage Δ
unittests 62.19% <28.88%> (+0.23%) ⬆️

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

Files Changed Coverage Δ
statute/auth.go 67.34% <ø> (ø)
statute/datagram.go 97.53% <ø> (ø)
handle.go 46.61% <28.88%> (+0.58%) ⬆️

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

@cxz66666
Copy link
Contributor Author

cxz66666 commented Sep 2, 2023

已修复 @thinkgos

@thinkgos
Copy link
Member

thinkgos commented Sep 3, 2023

@cxz66666 好的, 找个时间我研究一下

@thinkgos thinkgos added enhancement New feature or request good first issue Good for newcomers labels Sep 6, 2023
@thinkgos thinkgos merged commit ef8e9fe into things-go:master Sep 11, 2023
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants