-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
SOCKS proxy support #9287
SOCKS proxy support #9287
Conversation
Update: direct connection to yt-dl.org in Jython also fails:
From Wireshark it's shown a |
Thanks a bunch for this. Since I'm in China, don't have much choice but to run this branch. Hope it gets integrated soon. EDIT: specifying |
In my codes the default port for SOCKS proxy is 1080. Can you paste the full verbose result from the following command?
By the way, don't edit comments except minor changes (typo etc.). The buggy Github does not send notifications about edited comments. |
Thanks, I'll keep in mind not to use the edit feature for adding anything important in the future. Here is the output. I get other errors when I specify port 1080, but at least it gets past this one.
|
For reference, I get stuck here when I add the port, and can see in my socks server logs that youtube-dl does not attempt to use the socks proxy at all.
|
Can you run the following command:
And upload strace.txt? This file may contain sensitive data (your IP, username etc.). Check its contents before uploading. |
return struct.unpack(spec, *args) | ||
else: | ||
struct_pack = struct.pack | ||
struct_unpack = struct.unpack |
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 moving to compat.py
why not prefix with conventional compat_
?
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.
Maybe shlex_quote
and subprocess_check_output
should be renamed, too?
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.
Maybe but not as part of this PR. subprocess_check_output
also seems to be unused at all.
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.
Here is the strace you asked for. I've never used strace to debug, so if you have time (or links to good articles) I'd love to see what parts of it you'll be looking at for your debugging. |
@OliverUv From your logs there are at least two youtube-dl installations. One is at |
Very sorry for the noise @yan12125 - your suggestion solved everything. I was unaware that python would reach out to the globally installed instance when I tried to run (the non-built) local instance. I've now also learned that I need to |
Use srelay to test socks.py with local servers. Let's see the CI result. |
self.close() | ||
raise Socks5Error(Socks5Error.ERR_GENERAL_FAILURE) | ||
elif method == Socks5Auth.AUTH_NONE: | ||
pass |
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 guess this elif
clause can be safely omitted.
I've not had a chance to take a deep look yet but I guess it can be merged for now. Documentation for |
Seems on Travis CI, ports in the original range are often used.
Some of test_socks failed on Travis CI. I guess some ports are used, so I use a different range in
Done. |
Rebased and pushed to master. |
Given example of
|
Sorry it's a bug caused by me. Could you try version 2016.05.10? |
Yeah, 2016.5.10 works fine. |
Originally reported at #9287 (comment)
Thanks for testing. Should be fixed in cdd94c2. |
Implements #402.
I adapt the public domain version of socks.py proposed by @bluec0re at #305 (comment).
To run
test_socks.py
, you need to run two SOCKS server on two different public IPs. For example, createtest/local_parameters.json
with the following contents:And then run two SSH servers:
ssh server1_ip -D 1080
,ssh server2_ip -D 1090
(The step numbers are going wrong - blame Github Flavored Markdown :) )
And my test results:
HTTPS over SOCKS fails on Jython. I don't know why. Direct connection to https://yt-dl.org/ip is OK in Jython. I'll open a new issue and mark it as "known bugs" if this PR get merged.