Skip to content
This repository has been archived by the owner on Oct 30, 2023. It is now read-only.

TRACE header support #71

Closed
wants to merge 3 commits into from
Closed

TRACE header support #71

wants to merge 3 commits into from

Conversation

zdmytriv
Copy link

@zdmytriv zdmytriv commented Apr 3, 2020

For requests through forward proxy we can get additional (not required) TRACE header. For example:

PROXY TCP4 172.17.0.1 172.17.0.2 54636 8080
TRACE 1b458c481644a39176ba72dc6ddbf766
CONNECT httpbin.org:443 HTTP/1.1
Host: httpbin.org:443
User-Agent: curl/7.64.1
Proxy-Connection: Keep-Alive
...

Format: TRACE <32HEX>\r\n

It ALWAYS goes after PROXY header. No PROXY header ====> no TRACE for sure.

  1. If there are no protocol headers (like PROXY or TRACE) => proxy will accept request and process
  2. If there is just PROXY header => proxy will accept request and process
  3. If there is PROXY and TRACE headers => proxy will accept request and process
  4. If there is just TRACE header => we accept request BUT it will fail because CONNECT is expected to be the first word.

Related PR:

Fixes: https://app.clubhouse.io/vgs/story/67682/fix-tracing-for-forward-proxy

Copy link

@mjallday mjallday left a comment

Choose a reason for hiding this comment

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

@viacheslav-fomin-main
Copy link
Collaborator

do not forget, that lp contains not yet used in proxy code, @k-sever is on it.

@zdmytriv
Copy link
Author

zdmytriv commented Apr 6, 2020

do not forget, that lp contains not yet used in proxy code, @k-sever is on it.

Sorry I didn't get what "IP contains not yet used"?

@osklyarenko
Copy link

[ch67682]

Automatically linked to https://app.clubhouse.io/vgs/story/67682

@zdmytriv
Copy link
Author

zdmytriv commented Apr 8, 2020

Closing in favor of #72

@zdmytriv zdmytriv closed this Apr 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants