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

WIP: Add curl library to support multiple proxies #477

Closed
wants to merge 9 commits into from

Conversation

pellaeon
Copy link
Contributor

  • Use curl to support proxies, it will also be easier to add cookie support.
  • Switch FacebookBridge to use the new curl wrapper.
  • The curl wrapper will search all files in proxylist/ for proxies, and randomly picks one to use. For the proxy URL string syntax, refer to PROXY PROTOCOL PREFIXES section in curl manpage. Example: 1.2.3.4:8080 is a HTTP proxy, curl also supports SOCKS.

This PR contains some commented out code, might be useful for future enhancements, so I lefted it there.
Also, if the maintainers would like to split these features into multiple PRs, I would also be happy to do that.

pellaeon and others added 9 commits February 21, 2017 10:38
cookiejar: /tmp/rssbridge-fb-cookies.txt , make sure it is writable by
web server process
Unfortunately, after I implemented cookies, I found that with
_fb_noscript=1, the captcha will only work for a single time. ie. the
next request will require a new captcha.
@logmanoriginal
Copy link
Contributor

logmanoriginal commented Mar 19, 2017

Great work!

The usage of multiple bridges proxies was also proposed in #460 (also related to Facebook), so I would like to give it some attention. (I think this can be made a bit more generic by abstracting getSimpleHTMLDOM and getSimpleHTMLDOM based on your implementation)

Can you please split all changes done to FacebookBridge into a separate PR?
I see you have already done that with a portion in #468

@logmanoriginal logmanoriginal added the New-Feature This is a new feature label Mar 19, 2017
Copy link
Contributor

@logmanoriginal logmanoriginal left a comment

Choose a reason for hiding this comment

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

This is very nice!
With some slight adjustments and a check regarding coding styles this is good to go.

All changes in the FacebookBridge should be split into a separate PR.

See my comments for some questions.

$proxies_str = file_get_contents(__DIR__.'/../proxylist/'.$file);
$proxies = array_merge($proxies, explode("\n", $proxies_str, -1));
}
$proxy = $proxies[array_rand($proxies)];
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this work if no proxy is specified (no files in the 'proxylist/' directory)?

, $stripRN
, $defaultBRText
, $defaultSpanText),
$info, $header, $proxy);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any need to know $info, $header, $proxy?

, $defaultSpanText),
$info, $header, $proxy);
}
?>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please have a look at the Travis-CI results. Also checkout the new contributions guidelines regarding coding style.

}

return parent::getName();
return isset($this->extraInfos['name']) ? $this->extraInfos['name'] : $this->authorName.' - Facebook Bridge';
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please split this file into a separate PR

@pellaeon pellaeon changed the title Add curl library to support multiple proxies WIP: Add curl library to support multiple proxies May 7, 2017
@pellaeon
Copy link
Contributor Author

pellaeon commented May 7, 2017

Hey I opened a new pull request #531 that only contains contents_curl.php, and fixes are applied there.

@pellaeon pellaeon closed this May 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New-Feature This is a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants