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

Nit on CONNECT tunnel forwarding #170

Closed
tfpauly opened this issue Nov 11, 2018 · 9 comments · Fixed by #349
Closed

Nit on CONNECT tunnel forwarding #170

tfpauly opened this issue Nov 11, 2018 · 9 comments · Fixed by #349

Comments

@tfpauly
Copy link

tfpauly commented Nov 11, 2018

The current section on CONNECT (7.3.6) in the semantics document refers to the proxy forwarding packets:

The CONNECT method requests that the recipient establish a tunnel to the destination 
origin server identified by the request-target and, if successful, thereafter restrict its 
behavior to blind forwarding of packets, in both directions, until the tunnel is closed.

Since the proxy terminates TCP, there is no forwarding of IP packets, but rather the contents of the TCP streams.

@wtarreau
Copy link

Well spotted! I'd even suggest "to blindly forward bytes in both directions ...". Too often I see people who imagine that packet delimitation is respected on both sides of a proxy and I have to tell them that it's just a uninterrupted stream of bytes that each layer decides to delimit wherever it wants.

@LPardue
Copy link
Contributor

LPardue commented Nov 11, 2018

I'd say that 7231 is self-consistent with the use of packet in 7230. If a change is needed, we should change wording consistently. Or, was this issue raised on the WIP core doc?

@tfpauly
Copy link
Author

tfpauly commented Nov 12, 2018

@LPardue this issue is being raised on the WIP core doc. The same text does also exist in 7231, but I consider them to both be in error. While we're updating the text for the core doc, let's fix the text to be clearer.

@LPardue
Copy link
Contributor

LPardue commented Nov 12, 2018

Thanks for the clarification @tfpauly, that sounds good to me.

@LPardue
Copy link
Contributor

LPardue commented Nov 12, 2018

Fwiw, the HiNT draft uses the terminology: blind forwarding of IP payloads, UDP datagram payloads and TCP/IP packet payload.

@MikeBishop
Copy link
Contributor

HTTP/3 says:

All DATA frames on the stream correspond to data sent or received on the TCP
connection. Any DATA frame sent by the client is transmitted by the proxy to the
TCP server; data received from the TCP server is packaged into DATA frames by
the proxy. Note that the size and number of TCP segments is not guaranteed to
map predictably to the size and number of HTTP DATA or QUIC STREAM frames.

@tfpauly
Copy link
Author

tfpauly commented Nov 14, 2018

@MikeBishop that text looks good. Something along those lines, but that also works for HTTP/1.1, would be right for this core doc text.

@reschke
Copy link
Contributor

reschke commented Jan 7, 2020

Maybe just replace "packets" with "data"?

@wtarreau
Copy link

wtarreau commented Mar 8, 2020

So as mentioned above in #327, while having a look at the latest draft I was struck again by this "packets" there, and even forgot about this old issue. Pinging again. I suggest that we change "forwarding of packets" with "forwarding of the byte stream" so that we can close these issues.

@mnot mnot self-assigned this Mar 20, 2020
mnot added a commit that referenced this issue Mar 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

6 participants