Skip to content

Conversation

@Alkarex
Copy link
Contributor

@Alkarex Alkarex commented Sep 16, 2020

Using cURL CURLOPT_FOLLOWLOCATION instead of the manual method in SimplePie hides the list of HTTP redirects along the way, and prevents the distinction of e.g. 301 vs. 302 redirects needed for simplepie->subscribe_url(true).

This patch disables CURLOPT_FOLLOWLOCATION in SimplePie, and fixes the manual method at the same time (curl_options were not given to following redirection requests).
The use of CURLOPT_FOLLOWLOCATION was nevertheless not systematic (only when open_basedir was not set), so now there is only one logic independent of open_basedir.

Using cURL CURLOPT_FOLLOWLOCATION instead of the manual method in
SimplePie hides the list of HTTP redirects along the way, and prevents
the distinction of e.g. 301 vs. 302 redirects needed for
`simplepie->subscribe_url(true)`.

This patch disables CURLOPT_FOLLOWLOCATION in SimplePie, and fixes the
manual method at the same time (curl_options where not given to
following redirection requests).
The use of CURLOPT_FOLLOWLOCATION was nevertheless not systematic (only
when open_basedir was not set), so now there is only one logic
independent of open_basedir.
curl_setopt($fp, CURLOPT_REFERER, $url);
curl_setopt($fp, CURLOPT_USERAGENT, $useragent);
curl_setopt($fp, CURLOPT_HTTPHEADER, $headers2);
if (!ini_get('open_basedir') && version_compare(SimplePie_Misc::get_curl_version(), '7.15.2', '>='))
Copy link
Contributor Author

@Alkarex Alkarex Sep 16, 2020

Choose a reason for hiding this comment

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

Those cURL options were only used in some configurations, and were problematic

$this->redirects++;
$location = SimplePie_Misc::absolutize_url($this->headers['location'], $url);
$previousStatusCode = $this->status_code;
$this->__construct($location, $timeout, $redirects, $headers, $useragent, $force_fsockopen);
Copy link
Contributor Author

@Alkarex Alkarex Sep 16, 2020

Choose a reason for hiding this comment

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

For configurations using the manual redirect method, the cURL options were lost.

@Alkarex
Copy link
Contributor Author

Alkarex commented Sep 16, 2020

FreshRSS/FreshRSS#3180

@Alkarex
Copy link
Contributor Author

Alkarex commented Sep 16, 2020

Original code for HTTP 301 Moved Permanently b3e7227

@mblaney
Copy link
Member

mblaney commented Sep 17, 2020

thanks @Alkarex looks good to me.

@Alkarex Alkarex deleted the fix_301_permanent_url branch September 17, 2020 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants