Improve SimplePie redirects (POST to GET, remove authentication cross-origin)

[Inverle]

Follow-up of #76
FreshRSS/FreshRSS#8400

[Alkarex]

Is it correctly understood that passing CURLOPT_FOLLOWLOCATION as a cURL option will allow skipping this new code?
I am interested in keeping an option to let cURL perform the work rather than using our or SimplePie's re-implementation

[Alkarex]

Although we only use the cURL path in FreshRSS, the fsockopen path further below should probably be updated in the same manner

[Inverle]

Is it correctly understood that passing CURLOPT_FOLLOWLOCATION as a cURL option will allow skipping this new code? I am interested in keeping an option to let cURL perform the work rather than using our or SimplePie's re-implementation

Not yet, because CURLOPT_FOLLOWLOCATION is forced to false:

simplepie/src/File.php

Line 525 in 843cc8b

curl_setopt($fp, CURLOPT_FOLLOWLOCATION, false); // FreshRSS

[Alkarex]

I have tried to apply the same logic to the fsockopen path. But not tested. Checks welcome

[Inverle]

I have tried to apply the same logic to the fsockopen path. But not tested. Checks welcome

Testing with a patch like this in FreshRSS, I can confirm the auth headers are removed correctly after a redirect:

diff --git a/app/Models/SimplePieFetch.php b/app/Models/SimplePieFetch.php
index 1a0737b3d..2e41d87d7 100644
--- a/app/Models/SimplePieFetch.php
+++ b/app/Models/SimplePieFetch.php
@@ -31,7 +31,13 @@ final class FreshRSS_SimplePieFetch extends \SimplePie\File
                } elseif ($redirects < 0) {
                        $redirects = -1; // infinite redirects
                }
-               parent::__construct($url, $timeout, $redirects, $headers, $useragent, $force_fsockopen, $curl_options);
+               if (!isset($GLOBALS['fsockopen_test'])) {
+                       $GLOBALS['fsockopen_test'] = 1;
+                       $headers['Cookie'] = 'test=123';
+                       $headers['Authorization'] = 'test';
+                       $headers['X-SomeOtherHeader'] = '123';
+               }
+               parent::__construct($url, $timeout, $redirects, $headers, $useragent, true, $curl_options);
        }
 
        #[\Override]

[Alkarex]

If possible, I would still like a way, for instance by passing a CURLOPT_FOLLOWLOCATION, to use the native cURL handing of redirections

[Alkarex]

Related:

• SimplePie: Fix HTTP 301 permanent redirection FreshRSS#3180
• Implemented advanced feed settings to allow cookies and HTTP redirects. FreshRSS#4470

[Alkarex]

FYI, I was hopping to propose a little update, but got sidetracked and still have not found an approach I like much. I have a work in progress for probably in a couple of days, but additional inputs welcome, in particular to avoid breaking features as posted just above if possible

[Alkarex]

I have made an attempt. What do you think @Inverle ?

• CURLOPT_FOLLOWLOCATION not set or null: SimplePie PHP handling of HTTP redirections with security checks (default, recommended).
• CURLOPT_FOLLOWLOCATION = truthy: Native cURL handling of HTTP redirections without SimplePie security checks;
• CURLOPT_FOLLOWLOCATION = falsy: No HTTP redirections at all;

And it is up to the client to allow CURLOPT_FOLLOWLOCATION == true or not.

[Inverle]

It seems that original SimplePie does not have this code, even though it should be needed for get_final_requested_uri()?

[Alkarex]

Indeed

[Inverle]

I have made an attempt. What do you think @Inverle ?

* CURLOPT_FOLLOWLOCATION not set or null: SimplePie PHP handling of HTTP redirections with security checks (default, recommended).

* CURLOPT_FOLLOWLOCATION = truthy: Native cURL handling of HTTP redirections _without_ SimplePie security checks;

* CURLOPT_FOLLOWLOCATION = falsy: No HTTP redirections at all;

And it is up to the client to allow CURLOPT_FOLLOWLOCATION == true or not.

It is fine. I have also made this commit: FreshRSS/FreshRSS@e698ae9