Skip to content
This repository has been archived by the owner on Jun 2, 2019. It is now read-only.

Socks proxy support #355

Open
2i3r opened this issue Jan 9, 2018 · 10 comments
Open

Socks proxy support #355

2i3r opened this issue Jan 9, 2018 · 10 comments

Comments

@2i3r
Copy link

2i3r commented Jan 9, 2018

While http proxy works well in both traditional and async way of connections, socks proxy is not available yet.
I worked on and implemented Socks4/5 in async version by using aiosocks module, socks5 tested and seems to work. ready for pull request.
usage is just the same as before, call set_proxy in start:
telepot.aio.api.set_proxy("socks5://server.addr:port", ("username", "password"));
or
telepot.aio.api.set_proxy("socks4://server.addr:port", ("username"));

Not sure how to add similar support for urllib3.
Should I request a pull at this point?

Thank you for telepot.

@nickoala
Copy link
Owner

Two questions:

  1. Does that mean you have made the underlying layers aiohttp and aiosocks swappable?
  2. Are you relying on the protocol specifier of the URL (the stuff before ://) to decide which layer to swap in? This may seem obvious, but I am just curious and want to make sure.

Thank you, in any case. Don't worry about urllib3. I am happy for this feature to be asymmetrical, if I decide to include it at all.

@2i3r
Copy link
Author

2i3r commented Jan 10, 2018

You can find my commit in aio/api.py.

q2: Newly added commit looks for application protocol in start of url, i.e. before :// and decide which type of protocol is going to be used. further more aiosocks also relies of url of format socks5://... or socks4://... or http://... to decide for the format of connection.

q1: In set_proxy() if it find the type of protocol is not http then it would import aiosocks and it's sub-modules, then it will rebuild the default in _pools with a new aiohttp.ClientSession() made by aiosocks's connector and request_class. also commit take care of _create_onetime_pool and authentication methods.

@das7pad
Copy link
Contributor

das7pad commented Jan 11, 2018

Great idea to add socks proxy support @2i3r !

Please note that the package aiosocks is not optimized for pip. The requested aiohttp version is checked on import instead of before installing the package - the latter one would request a different version in case of a version mismatch. I could file a PR to fix this upstream.
In addition a PR here would cause a huge version drop of aiohttp from 2.0.0 - 2.3.2. Projects depending on aiohttp<2.3.2 - which was just released on 2017-11-01 - could not use telepot anymore. The problem here: we can not simply add aiosocks to the requirements and let pip choose a version that works with aiohttp<2.3.2 for the older projects as aiosocks does not specify the required version in its setup.

@2i3r
Copy link
Author

2i3r commented Jan 11, 2018

@das7pad really good points.
It seems we need an upstream PR for this.
When I look at aiohttp releases after 2.0.0.0 they fixed lots of bugs. it must be some how backward compatible, isn't it?
In addition, I'm not sure if conditional import of aiosocks as implemented in my commit, is a policy to follow here.

Bests,

@2i3r 2i3r closed this as completed Jan 11, 2018
@2i3r 2i3r reopened this Jan 11, 2018
@2i3r
Copy link
Author

2i3r commented Jan 11, 2018

Sorry, it was a mistake.

@das7pad
Copy link
Contributor

das7pad commented Jan 11, 2018

Upstream PR: nibrag/aiosocks#27

I just read their setup in detail and we would drop support for python3.5.0, python3.5.1 and python3.5.2. Only the latter one could be an issue as it is still the default python3 version for ubuntu LTS.

@2i3r
Copy link
Author

2i3r commented Jan 11, 2018

Thank you @das7pad, really good point, I'm learning new stuff. I think it may be a big limitation, to drop support for python3.5.3- versions

In aiosocks/setup.py the python 3.5.3+ requirement added on efd1ff commit when New starttls implemented. It seems this requirement is because of asyncio.sslproto usage?

@nickoala
Copy link
Owner

nickoala commented Jan 11, 2018

@das7pad and @2i3r, thanks for all the work and discussions.

Supporting more types of proxies sounds like a good idea. However, as I overheard your conversations, something makes me uneasy. Should we make telepot depend on one more library just because we want to support one more type of proxies? What if someday someone comes up with an SSH tunneling proxy and telepot wants to support that, leading to dependence on an SSH library? How about a VPN proxy? Should telepot include a VPN library too? You get the idea ...

Being a library solely for Telegram Bot API, telepot should depend on one thing only: HTTP libraries. Other communication requirements should fall on the application. Reviewing the module telepot.api and telepot.aio.api and considering how proxy is supported, reveals a pattern. It's all about supplying an appropriate Connection Provider:

  • By default, telepot uses a plain HTTP connection provider.
  • In addition, telepot provides an HTTP proxy connection provider (because HTTP proxies are common and it's easy to integrate with the HTTP libraries telepot already uses).
  • If other types of proxies are required, custom connection providers should be implemented and plugged in by developers.

This "Pluggable Connection Provider" architecture is not yet implemented by telepot. I will start to think about it. Hopefully it will be included in the near future. By that time, for some common types of non-HTTP proxies, e.g. SOCKS, their connection providers may be posted as examples to help others who share the same concerns.

In conclusion, SOCKS proxy will not become part of telepot. I still want to thank @2i3r and @das7pad for prompting me to think of a better way 😊

@2i3r
Copy link
Author

2i3r commented Jan 11, 2018

I think this is certainly the best choice. Thank you @nickoala for following.
In mean time to an example be posted, anyone wants socks4/5 proxy may clone and use my unofficial repo from here, it will be fall behind the official telepot very soon.

@botfarmer
Copy link

What if someday someone comes up with an SSH tunneling proxy and telepot wants to support that, leading to dependence on an SSH library? How about a VPN proxy?

But it's all about socks5 proxy. To connect via ssh tunneling proxy (ssh -D 1080 -f -q -N user@my.example.com) or vpn proxy or tor - you just need socks5 suppport.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants