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

[connect-tcp] Adjust rules for target_host and target_port #2736

Merged
merged 4 commits into from
Jul 22, 2024

Conversation

bemasc
Copy link
Contributor

@bemasc bemasc commented Feb 14, 2024

Changes:

  • s/tcp_port/target_port/ (aligning with connect-udp)
  • Remove support for multiple IPs

Fixes #2713 and #2714

Changes:

* s/tcp_port/target_port/ (aligning with connect-udp)
* Make IP lists comma-separated instead of RFC 6570 list types.

Fixes #2713 and #2714
@bemasc bemasc added the connect-tcp draft-ietf-httpbis-connect-tcp label Feb 14, 2024
@bemasc bemasc changed the title [connect-tcp] Adjust rules for target_port [connect-tcp] Adjust rules for target_host and target_port Feb 15, 2024
@bemasc
Copy link
Contributor Author

bemasc commented Apr 11, 2024

@MikeBishop The discussion at IETF 119 leaned toward removing the multiple-IP mode entirely. Are you suggesting that we should keep it?

@MikeBishop
Copy link
Contributor

This PR contains two changes; I'm not making a suggestion with regard to multiple-IP mode, but I think aligning the variable names in the template makes sense. Did both get dropped, or is that being discarded because it's in the same PR?

@bemasc
Copy link
Contributor Author

bemasc commented Apr 15, 2024

OK, I've updated this PR to also drop the multiple-IP support entirely.

Comment on lines 54 to 55

Classic HTTP CONNECT proxies can be used to reach a target host that is specified as a domain name or an IP address. However, because only a single target host can be specified, proxy-driven Happy Eyeballs and cross-IP fallback can only be used when the host is a domain name. For IP-targeted requests to succeed, the client must know which address families are supported by the proxy via some out-of-band mechanism, or open multiple independent CONNECT requests and abandon any that prove unnecessary.

## Overview
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that target_host can contain either an IP address or a DNS name, this text still seems relevant to the overall doc. Is the removal deliberate, or was the text relocated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is deliberately removed, as it's no longer a problem that this draft attempts to address.

Copy link
Contributor

@DavidSchinazi DavidSchinazi left a comment

Choose a reason for hiding this comment

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

Thanks for the updates based on the discussion at 119

Comment on lines 194 to 199
A single URI Template can be used for multiple purposes if it includes all the relevant variables. Variables that are not relevant to a particular usage are left undefined, which is permitted when expanding a URI Template.

Multipurpose templates can be useful when a single client may benefit from access to multiple complementary services (e.g., TCP and UDP), or when the proxy is used by a variety of clients with different needs.
Multipurpose templates can be useful when a single client may benefit from access to multiple complementary services (e.g., TCP and UDP), or when the proxy is used by a variety of clients with different needs. The contents of the URI Template are not necessarily sufficient to determine its purpose, so clients must determine this in some other way, such as by probing or via a separate usage indication.

~~~
https://proxy.example/{?target_host,tcp_port,target_port,
target,ipproto,dns}
https://proxy.example/{?target_host,target_port,target,ipproto,dns}
Copy link
Contributor

Choose a reason for hiding this comment

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

@bemasc with the rest of the updates in this PR (which I think are good and reflect the discussion), I'm thinking that it is best to remove this multi-purpose proxies section. I don't think it makes sense to have in this particular document. It doesn't seem specific to connect-tcp, and it explicitly mentions that it isn't sufficient to determine the purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I've removed this section.

@bemasc bemasc merged commit 44a7e4a into main Jul 22, 2024
2 checks passed
@bemasc bemasc deleted the bemasc-target_port branch July 22, 2024 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
connect-tcp draft-ietf-httpbis-connect-tcp
Development

Successfully merging this pull request may close these issues.

connect-tcp: use target_port instead of tcp_port
4 participants