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

Reduce http proxy client overhead by 1RTT #2157

Merged
merged 5 commits into from
Feb 18, 2020

Conversation

Anonymous-Someneese
Copy link
Contributor

In standard HTTP proxy, client will wait until proxy connects to the destination and send 200 OK back. This introduce 1 RTT overhead. If we send the content right after writing the CONNECT command, this could be eliminated.
As for compatibility, a standard http proxy should be able to separate the command and content by \r\n\r\n. In my test, most proxy server can handle this behavior.
With this optimization, HTTPS proxy can have the same overhead as other TLS based protocols.

Anonymous-Someneese added 4 commits January 6, 2020 06:28
@Anonymous-Someneese
Copy link
Contributor Author

Can anyone review this?

@kslr
Copy link
Contributor

kslr commented Jan 7, 2020

cc @vcptr @xiaokangwang

Copy link
Contributor

@vcptr vcptr left a comment

Choose a reason for hiding this comment

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

Code looks good. But the changes in app/reverse/bridge.go seems unrelated to this PR.
Can you put them in another PR?

@Anonymous-Someneese
Copy link
Contributor Author

Code looks good. But the changes in app/reverse/bridge.go seems unrelated to this PR.
Can you put them in another PR?

The HTTP changes would require connection initiator to send first or wait 5ms for HTTP CONNECT to send. The behaviour of bridge would be affected.

@kslr kslr merged commit 2c0d55e into v2ray:master Feb 18, 2020
@kslr
Copy link
Contributor

kslr commented Mar 19, 2020

Hi
according to MZZ findings, if the request is sent before it returns 200, problems will occur
I need to undo this merges first

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

3 participants