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.

Sign up for GitHub

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

Jump to bottom

Add authentication support to Net::HTTP.SOCKSProxy #24

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add authentication support to Net::HTTP.SOCKSProxy #24

wants to merge 2 commits into from

Conversation

ojab
Copy link

@ojab ojab commented Sep 3, 2015

No description provided.

@astro
Copy link
Owner

astro commented Sep 4, 2015

Cool, thank you. Please ping the new maintainer @musybite if nothing happens for a few days.

@ojab
Copy link
Author

ojab commented Nov 30, 2015

ping @musybite

@musybite
Copy link
Collaborator

musybite commented Dec 7, 2015

Thank you for pull request @ojab and sorry for long silence. Everything looks good to me. I'll make release this weekend.

@ojab
Copy link
Author

ojab commented Dec 7, 2015

Thanks!

@ojab ojab force-pushed the master branch 2 times, most recently from 6fa4fcd to f604bcf Compare December 8, 2015 11:19
@ojab
Copy link
Author

ojab commented Dec 8, 2015

I've rebased PR #22 on current master (1.7.0) and put patch for this PR on top, please let me know if something should be changed.

@mikecmpbll mikecmpbll mentioned this pull request Dec 9, 2015
Merged
@mikecmpbll
Copy link

@musybite I wonder if this will be merged & released?

@mikecmpbll mikecmpbll mentioned this pull request Jan 23, 2017
Open
@ajimix
Copy link

ajimix commented Jul 12, 2017 •
edited
Loading

@musybite @astro any way that this can be merged? thanks!

musybite added a commit that referenced this pull request Mar 13, 2018
@musybite
Add authentication support to Net::HTTP.SOCKSProxy
dae6100 
Inspired by #24. This version does not require thread-local variables,
everything kept inside instance and local variables.

Thanks @ojab!
@musybite musybite mentioned this pull request Mar 13, 2018
Open
@musybite
Copy link
Collaborator

@ojab @ajimix @mikecmpbll i'd proposed another implementation in MR #33. API is fully the same. Could you check whether it works for you?

I have certain doubts about switching from class @@-variables to thread-local variables: for example, it will definitely break socksify_ruby script for multi-threaded apps, and could lead to other unintended consequences. From the very beginning, socksify implied "global socksification" of whole application with all its threads, and thread-safe (albeit somewhat ugly) APIs were added only recently. For such old and stable project as socksify, it is important not to break compatibility.

@ojab
Copy link
Author

ojab commented Mar 14, 2018

I don't use socksify any more, so unfortunately I can't test this change.

@MatzFan MatzFan mentioned this pull request Jul 4, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ojab @astro @musybite @mikecmpbll @ajimix @jakemack