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

Add socks5 proxy #240

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add socks5 proxy #240

wants to merge 1 commit into from

Conversation

alchen
Copy link

@alchen alchen commented Feb 12, 2016

I needed to use socks5 proxy for my Twitter requests, so I made some modifications to the module:

  • Added a new request.js to encapsulate request with relevant proxy configs.
  • Added a new dependency [mscdex/socksv5](https://github.com/mscdex/socksv5) to use request with socks5. There is only a handful of socks agents and this one seems to be still being maintained and also most up-to-date.
  • Moved trusted certificate checking logic into req.on('response', ... because of request 2.65.0 getPeerCertificate() always returns null on node 4.2.1, works on 0.12.7 nodejs/node#3545 (test cases was failing).
  • Changed a couple of test cases to accommodate above changes.

The proxy option is enabled by adding something along this line into Twit config: proxy: { host: '127.0.0.1', port: '1080' }

I haven't had a chance to try HTTP proxies. I wonder how well they work with Twitter's https endpoints.

@ttezel
Copy link
Owner

ttezel commented May 31, 2018

@alchen could you rebase these changes off latest master branch?

Copy link
Owner

@ttezel ttezel left a comment

Choose a reason for hiding this comment

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

Could you rebase these changes off latest master so I can re-review?

@alchen
Copy link
Author

alchen commented Jun 2, 2018

@ttezel Thanks for the interest of incorporating this. I am however no longer familiar with the changes that I wrote -- I quickly rebased everything and someone else might be able to pick up from here.

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

Successfully merging this pull request may close these issues.

2 participants