Allow for specifying schema in "proxy" #1922
Conversation
sadym-chromium
force-pushed
the
sadym/proxy
branch
from
8b7b4d2 to
5b447f9
Compare
sadym-chromium
changed the title
sadym-chromium
changed the title
| </tr> | ||
|
|
||
| <tr> | ||
| <td><code>socksVersion</code> |
There was a problem hiding this comment.
Is there a reason why you removed the socksVersion field? I thought that for socks proxies it is a mandatory field. Also if clients specify this field we would run into a backward incompatible change.
There was a problem hiding this comment.
The key change is that the "socksProxy" field sets how to proxy ALL the socks connections, not only of the specific version.
There was a problem hiding this comment.
WRT backward compatibility, we can figure things out if we agree on what we want to achieve.
There was a problem hiding this comment.
From the Chromium perspective, the socksVersion does not make actually sense.
There was a problem hiding this comment.
Removing the socks version will cause a backward incompatible change, means it will break clients using that field. We probably should fine a way to deprecate if it is really not needed. But note that when defining a socks proxy in Firefox you need to specify the version. So I don't think that we can get rid of it.
There was a problem hiding this comment.
I see. I re-worked the PR so that the socksVersion can be used together with socks:// scheme.
There was a problem hiding this comment.
We can default socksVersion to 5 to ease the API. @whimboo WDYT?
index.html
Outdated
| undefined. | ||
|
|
||
| <p>A <dfn>proxy schema</dfn> is defined as being one of the following strings: | ||
| "<code>http</code>", "<code>https</code>", "<code>socks4</code>", |
There was a problem hiding this comment.
Neither socks4 nor socks5 is a valid schema as supported by Firefox. For socks proxy there is usually no schema at all when specifying the host. Does Chrome support that? Also note the other comment above regarding the backward incompatible change.
There was a problem hiding this comment.
As documented here, Chrome supports proxy via HTTP, SSL, SOCKS 4 and SOCKS 5:
- http://proxy:8080 uses HTTP.
- https://proxy:8080 uses SSL.
- socks4://proxy:8080 uses SOCKS 4.
- socks5://proxy:8080 uses SOCKS 5.
- socks://proxy:8080 uses SOCKS 5, as schema "socks" is alias for schema "socks5".
There was a problem hiding this comment.
Neither
socks4norsocks5is a valid schema as supported by Firefox. For socks proxy there is usually no schema at all when specifying the host. Does Chrome support that? Also note the other comment above regarding the backward incompatible change.
Does it mean that Firefox cannot proxy SOCKS 5 via SOCKS 4 and vice versa?
|
@whimboo WDYT? |
| </tr> | ||
|
|
||
| <tr> | ||
| <td><code>socksVersion</code> |
There was a problem hiding this comment.
Removing the socks version will cause a backward incompatible change, means it will break clients using that field. We probably should fine a way to deprecate if it is really not needed. But note that when defining a socks proxy in Firefox you need to specify the version. So I don't think that we can get rid of it.
index.html
Outdated
| <var>scheme</var> is omitted, the <var>scheme</var> is implied to be | ||
| "<code>http</code>". If the port is omitted and <var>scheme</var> has a |
There was a problem hiding this comment.
Could we at least try to figure out the right scheme (if not given) in case a default port has been set for the proxy? Using host:443 should most likely be a HTTPS proxy, or? If the port is not given, I assume that we could fallback to HTTP (HTTPS proxies still seem to be used rarely).
Should we add a note to indicate any change to the former version of the spec?
There was a problem hiding this comment.
Should we add a note to indicate any change to the former version of the spec?
Do you have an idea of proper wording?
sadym-chromium
force-pushed
the
sadym/proxy
branch
from
f9c0242 to
7104d77
Compare
|
@whimboo WDYT? |
|
@whimboo could you please take another look? |
whimboo
left a comment
whimboo
left a comment
There was a problem hiding this comment.
Sorry for taking so long with my review. Inline you can find some comments so that we can continue the discussion.
| <td>Defines the <a>SOCKS proxy</a> version | ||
| when the <a><code>proxyType</code></a> is "<code>manual</code>". | ||
| <td>Defines the <a>SOCKS proxy</a> version when the <a>proxy url</a>'s | ||
| <a>proxy scheme</a> is "<code>socks</code>". |
There was a problem hiding this comment.
Similar to my comment above. It would be a backward compatibility breaking change when we now introduce a socks scheme.
There was a problem hiding this comment.
Updated algorithm. The socksVersion is required, if any of the proxies has socks schema, or if the socksProxy don't have schema.
index.html
Outdated
|
|
||
| <li><p>If the <var>scheme</var> is omitted and the <var>port</var> is 443, | ||
| the <var>scheme</var> is implied to be "<code>https</code>". Otherwise, the | ||
| <var>scheme</var> is implied to be "<code>http</code>". |
There was a problem hiding this comment.
This paragraph should only apply to HTTP or HTTPS proxies but not for a proxy URL as specified for a socks proxy.
|
|
||
| <p>A <dfn>proxy scheme</dfn> is defined as being one of the following strings: | ||
| "<code>http</code>", "<code>https</code>", "<code>socks</code>", | ||
| "<code>socks4</code>", "<code>socks5</code>". |
There was a problem hiding this comment.
I still think that is a backward incompatible given that we didn't use a scheme for socks so far and I'm unsure how clients actually specify the socks proxy.
@jgraham maybe you have some additional feedback.
There was a problem hiding this comment.
I updated the algorithm.
- Old users omitted schema in proxy url, and expected the traffic to be proxied to the same protocol.
- Now, the schema defaults to the protocol to proxy. So this the behavior for them is not changed.
However, if the user WANTS to specify schema, now they can do it.
There was a problem hiding this comment.
I know this is incredibly poorly defined to begin with, but I think it's unclear from the specification text what the new semantics actually are.
If we want this to actually work, I think it's worth spending the time to define the right data structures to represent the configuration, even if it's a larger lift.
It seems like we more or less have two things: a source protocol (http, https, etc. or "all") and a destination proxy configuration, which consists of a proxy type, and a host address (host/ip and port) for the server.
I think the assumption that the meaning is clear just from a URL-like serialization of the destination proxy is unfounded. In particular there doesn't seem to be a socks scheme (or socks4 or socks5) at https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml and although it seems to be reasonably common that this serialization as a URL with a socks scheme works, I think we should perhaps be more explicit, unless we have something we can cite.
Co-authored-by: Henrik Skupin <mail@hskupin.info>
sadym-chromium
force-pushed
the
sadym/proxy
branch
from
d4a1533 to
b0a959b
Compare
|
The Browser Testing and Tools Working Group just discussed The full IRC log of that discussion<AutomatedTester> topic: Allow for specifying schema in "proxy"<AutomatedTester> github: https://github.com//pull/1922 <AutomatedTester> sadym: what is specified in the spec isn't what chromium does, chromium does a lot more <AutomatedTester> ... I have reworked the proposal and it is backwards compatiable <AutomatedTester> ... please look and consider this for merging <AutomatedTester> q? |
| <li><p>If <var>proxy</var> contains <a>proxy url</a> with <a>proxy scheme</a> | ||
| "<code>socks</code>" and does not have an <a>own property</a> |
There was a problem hiding this comment.
Is there a reason why you recently replaced has an <a>own property</a> for "<code>socksProxy</code>"? For a socks proxy configuration we should only check the requirement for the socks versionwhen a socks proxy was actually specified. Also to what wouldproxy contains proxy url` point to? It's not a key but a value for some other possible proxy config - a http proxy with socks scheme should not be considered here, right?
There was a problem hiding this comment.
I extended the description of the issue with detailed list of possible capabilities. This line was changed in order to fulfill new scenarios under "socks proxy" section. Otherwise the socks version to be used to connect to the proxy would be undefined. E.g.
httpProxy: socks://proxy.com+socksVersion: 4/5.
If we restrict the scheme to socks, that would restrict all the traffic to be proxied via the same socks protocol version. So you wouldn't be able to proxy http traffic via socks5, while ssl traffic via sock4. Even though this seems quite niche scenario, such a restriction seems quite artificial.
| the <a><code>proxyType</code></a> is "<code>manual</code>". | ||
| <td>A <a>host and optional port</a> for | ||
| scheme "<code>http</code>". | ||
| <td>A <a>proxy url</a> for "<code>http</code>". |
There was a problem hiding this comment.
I think we should call this "proxy specifier" or something, since it isn't necessarily a URL.
| <p>A <dfn>proxy url</dfn> for <a>proxy scheme</a> <var>protocol</var> can be | ||
| either "<code>direct</code>" or a string consists of an optional | ||
| <a>proxy scheme</a> <var>scheme</var> followed by the string | ||
| "<code>://</code>", a valid <a>host</a> <var>host</var>, and optionally a | ||
| colon followed by a valid <a>port</a> <var>port</var>. |
There was a problem hiding this comment.
This is hard to read. Presumably there are three cases:
- The string literal "direct"
- A string containing "://", in which case it has the form
{scheme}://{host}[:{port} - Any other string, in which case it has the form
{host}[:{port}].
I think you need to specify in more detail how to parse the input to get the destination scheme, the destination host and destination port.
| <ol> | ||
| <li><p>If <var>scheme</var> is omitted, let <var>scheme</var> be null. | ||
|
|
||
| <li><p>If <var>scheme</var> is null and <var>port</var> is <code>80</code>, |
There was a problem hiding this comment.
So this seems like potentially a behaviour change, since we're now inferring the destination scheme based on the destination port rather than based on the source scheme. I guess in practice it's likely to be correct since it would be really unusual to run a HTTPS server on 80 or a HTTP server on 443, so I think the risk is acceptable.
| the <var>host</var> may | ||
| <a data-lt="includes credentials">include credentials</a>. | ||
|
|
||
| <li><p>If the <var>port</var> is omitted and <var>scheme</var> has a |
There was a problem hiding this comment.
I know this is copying the existing text, but unless we refer to "implied port" later, this should just be something like "If <var>port</var> is undefined and <var>scheme<var> has a <a>default port</a>, set <var>port</var> to <var>scheme</var>'s <a>default port</a>."
| <td>Defines the proxy <a>host</a> for a <a>SOCKS proxy</a> | ||
| when the <a><code>proxyType</code></a> is "<code>manual</code>". | ||
| <td>A <a>host and optional port</a> with an <a>undefined</a> scheme. | ||
| <td>Defines the proxy <a>host</a> for SOCKS traffic when the |
There was a problem hiding this comment.
I don't think it makes sense to talk about "SOCKS traffic". After establishing the proxy the traffic is just whatever data is being transmitted e.g. HTTP or HTTPS.
| <tr> | ||
| <td><code>httpProxy</code> | ||
| <td>string | ||
| <td>Defines the proxy <a>host</a> for HTTP traffic when |
There was a problem hiding this comment.
In all these cases it seems we're no longer defining a host.
|
|
||
| <p>A <dfn>proxy scheme</dfn> is defined as being one of the following strings: | ||
| "<code>http</code>", "<code>https</code>", "<code>socks</code>", | ||
| "<code>socks4</code>", "<code>socks5</code>". |
There was a problem hiding this comment.
I know this is incredibly poorly defined to begin with, but I think it's unclear from the specification text what the new semantics actually are.
If we want this to actually work, I think it's worth spending the time to define the right data structures to represent the configuration, even if it's a larger lift.
It seems like we more or less have two things: a source protocol (http, https, etc. or "all") and a destination proxy configuration, which consists of a proxy type, and a host address (host/ip and port) for the server.
I think the assumption that the meaning is clear just from a URL-like serialization of the destination proxy is unfounded. In particular there doesn't seem to be a socks scheme (or socks4 or socks5) at https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml and although it seems to be reasonably common that this serialization as a URL with a socks scheme works, I think we should perhaps be more explicit, unless we have something we can cite.
Addressing #1920.
Currently, WebDriver capabilities allow for configuring proxy by protocol. Meaning the protocol traffic can be proxied via the same protocol proxy.
Chromium can proxy traffic via different proxy protocols, and can configure which proxy to use for which protocol. In Chromium, the possible proxy schemas are
http,https,socks4,socks5. Also user can configure separately proxy forHTTP,HTTPSand all other requests (SOCKS).In order to allow for such configurations, we propose the following:
http,https,socks,socks4,socks5) in the proxy url.Here is the details list of options can be provided in the capabilities:
httpproxyThe list of cases, when the proxy is connected with HTTP protocol:
httpProxy: proxy.com. The traffic type is used, as not scheme, nor port were set.httpProxy: proxy.com:123. The traffic type is used, as scheme was not set, and the port is not standard for any supported proxy protocol.sslProxy: http://proxy.com. The scheme has priority over traffic type.socksProxy: http://proxy.com. The scheme has priority over traffic type.socksVersionis not required.sslProxy: http://proxy.com:443. The scheme has priority over port and traffic type.socksProxy: http://proxy.com:443. The scheme has priority over port and traffic type.socksVersionis not required.sslProxy: proxy.com:80. The port has priority over traffic type.socksProxy: proxy.com:80. The port has priority over traffic type.socksVersionis not required.httpsproxyThe list of cases, when the proxy is connected with SSL protocol:
httpsProxy: proxy.com. The traffic type is used, as not scheme, nor port were set.httpsProxy: proxy.com:123. The traffic type is used, as scheme was not set, and the port is not standard for any supported proxy protocol.httpProxy: https://proxy.com. The scheme has priority over traffic type.socksProxy: https://proxy.com. The scheme has priority over traffic type.socksVersionis not required.httpProxy: https://proxy.com:80. The scheme has priority over port and traffic type.socksProxy: https://proxy.com:80. The scheme has priority over port and traffic type.socksVersionis not required.httpProxy: proxy.com:443. The port has priority over traffic type.socksProxy: proxy.com:443. The port has priority over traffic type.socksVersionis not required.socksproxyThe list of cases, when the proxy is connected with SOCKS protocol:
socksProxy: proxy.com+socksVersion: 4/5. The traffic type is used, as not scheme, nor port were set.socksProxy: proxy.com:123+socksVersion: 4/5. The traffic type is used, as scheme was not set, and the port is not standard for any supported proxy protocol.httpProxy: socks://proxy.com+socksVersion: 4/5. The scheme has priority over traffic type.httpProxy: socks4://proxy.com. The scheme has priority over traffic type.socksVersionis part of scheme.httpProxy: socks5://proxy.com. The scheme has priority over traffic type.socksVersionis part of scheme.sslProxy: socks://proxy.com+socksVersion: 4/5. The scheme has priority over traffic type.sslProxy: socks4://proxy.com. The scheme has priority over traffic type.socksVersionis part of scheme.sslProxy: socks5://proxy.com. The scheme has priority over traffic type.socksVersionis part of scheme.httpProxy: socks://proxy.com:80+socksVersion: 4/5. The scheme has priority over port and traffic type.httpProxy: socks4://proxy.com:80. The scheme has priority over port and traffic type.socksVersionis part of scheme.httpProxy: socks5://proxy.com:80. The scheme has priority over port and traffic type.socksVersionis part of scheme.sslProxy: socks://proxy.com:80+socksVersion: 4/5. The scheme has priority over port and traffic type.sslProxy: socks4://proxy.com:80. The scheme has priority over port and traffic type.socksVersionis part of scheme.sslProxy: socks5://proxy.com:80. The scheme has priority over port and traffic type.socksVersionis part of scheme.httpProxy: proxy.com:1080+socksVersion: 4/5. The port has priority over traffic type.sslProxy: proxy.com:1080+socksVersion: 4/5. The port has priority over traffic type.Preview | Diff