-
Notifications
You must be signed in to change notification settings - Fork 209
Allow for specifying schema in "proxy" #1922
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
base: master
Are you sure you want to change the base?
Conversation
8b7b4d2
to
5b447f9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
</tr> | ||
|
||
<tr> | ||
<td><code>socksVersion</code> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WRT backward compatibility, we can figure things out if we agree on what we want to achieve.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the Chromium perspective, the socksVersion
does not make actually sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I re-worked the PR so that the socksVersion
can be used together with socks://
scheme.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither
socks4
norsocks5
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a note to indicate any change to the former version of the spec?
Do you have an idea of proper wording?
f9c0242
to
7104d77
Compare
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
,HTTPS
and all other requests (SOCKS
).In order to allow for such configurations, we propose the following:
http
,https
,socks4
,socks5
) in the proxy url.socks
in favor ofother
proxy configuration.Preview | Diff