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

proxy/http: Avoid getting stuck when using server-first protocols #1517

Merged
merged 2 commits into from Jun 25, 2022

Conversation

lrh2000
Copy link
Contributor

@lrh2000 lrh2000 commented Jan 3, 2022

PR #99 reduces an extra RTT when setting up the CONNECT tunnel
via http proxy, however, server-first protocols (such as MySQL)
will be unusable as v2ray keeps to wait for the client request.
This commit solves the problem by limiting the maximum waiting
time to 50 ms.

Copy link
Contributor

@database64128 database64128 left a comment

Choose a reason for hiding this comment

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

firstPayload = make([]byte, 0) is not needed, because you already had a nil slice with var firstPayload []byte.

@codecov-commenter
Copy link

codecov-commenter commented Jan 3, 2022

Codecov Report

Merging #1517 (31c1a00) into master (a9bf783) will increase coverage by 0.10%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1517      +/-   ##
==========================================
+ Coverage   39.27%   39.37%   +0.10%     
==========================================
  Files         604      616      +12     
  Lines       35510    37097    +1587     
==========================================
+ Hits        13946    14608     +662     
- Misses      20026    20868     +842     
- Partials     1538     1621      +83     
Impacted Files Coverage Δ
proxy/http/client.go 0.58% <0.00%> (-0.01%) ⬇️
proxy/shadowsocks/client.go 0.86% <ø> (ø)
proxy/trojan/client.go 1.36% <0.00%> (ø)
proxy/vless/outbound/outbound.go 1.08% <0.00%> (ø)
proxy/vmess/outbound/outbound.go 61.78% <0.00%> (ø)
app/router/router.go 27.02% <0.00%> (-2.18%) ⬇️
transport/internet/headers/http/http.go 83.54% <0.00%> (-1.90%) ⬇️
transport/internet/quic/config.pb.go 37.89% <0.00%> (-1.24%) ⬇️
proxy/shadowsocks/simplified/config.go 3.03% <0.00%> (-0.20%) ⬇️
proxy/trojan/server.go 0.33% <0.00%> (-0.06%) ⬇️
... and 43 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d13894e...31c1a00. Read the comment docs.

@database64128
Copy link
Contributor

Also, #1292 uses 100ms for shadowsocks outbound. Maybe we should define a constant and use the same wait time for all types of outbound?

@lrh2000
Copy link
Contributor Author

lrh2000 commented Jan 3, 2022

firstPayload = make([]byte, 0) is not needed, because you already had a nil slice with var firstPayload []byte.

Also, #1292 uses 100ms for shadowsocks outbound. Maybe we should define a constant and use the same wait time for all types of outbound?

Thanks for your review. Actually I'm not familiar with go... and also the codebase... Any suggestions on where should I put the constant?

@database64128
Copy link
Contributor

Actually I'm not familiar with go... and also the codebase...

You did a great job already! Feel free to ask if you have any questions.

Any suggestions on where should I put the constant?

proxy/proxy.go might be a good place.

@lrh2000
Copy link
Contributor Author

lrh2000 commented Jan 4, 2022

Thanks @database64128 , for your nice words and kind help! I've made the constant and you may have a look at it again.

@github-actions github-actions bot added the Stale label May 5, 2022
@github-actions github-actions bot closed this May 11, 2022
@lrh2000
Copy link
Contributor Author

lrh2000 commented May 13, 2022

I do not think this problem has been solved. Why did this PR get closed? It seems that I cannot even reopen it.

@xiaokangwang xiaokangwang reopened this May 13, 2022
@xiaokangwang
Copy link
Contributor

xiaokangwang commented May 13, 2022

I will have a look at this pr and try to merge it before next release.

@lrh2000
Copy link
Contributor Author

lrh2000 commented May 13, 2022

@xiaokangwang Thanks! I need this feature and now I have to manually apply the patch, which is really annoying. If any changes are needed, feel free to comment.

@github-actions github-actions bot removed the Stale label May 14, 2022
PR v2fly#99 reduces an extra RTT when setting up the CONNECT tunnel
via http proxy, however, server-first protocols (such as MySQL)
will be unusable as v2ray keeps to wait for the client request.
This commit solves the problem by limiting the maximum waiting
time to 50 ms.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants