Skip to content

File: Various clean-ups#945

Merged
jtojnar merged 6 commits intomasterfrom
wip/jtojnar/file-cleanups
Sep 29, 2025
Merged

File: Various clean-ups#945
jtojnar merged 6 commits intomasterfrom
wip/jtojnar/file-cleanups

Conversation

@jtojnar
Copy link
Member

@jtojnar jtojnar commented Sep 21, 2025

  • Remove redundant redirect count check: CURLINFO_REDIRECT_COUNT is only non-zero when CURLOPT_FOLLOWLOCATION is enabled but we track redirects ourselves since 692e8bc.

  • Remove redundant url setting: Since starting to handle redirects ourselves in 692e8bc, url info item (aka CURLINFO_EFFECTIVE_URL) will be the same as the initial URI.

  • Remove redundant status_code setting: In fact, when the server sends interim responses, this was counterproductive – it would overwrite the final response code returned by CURLINFO_HTTP_CODE with the interim one (e.g. 100). Though, the parser will still include the headers of all but the first response in the body.

  • Avoid truthiness conversion of curl_errno: Explicit comparison is clearer than relying on CURLE_OK being falsey (0).

  • Replace redundant string cast with assertion: This is only needed for PHPStan. Making the type assertion explicit and moving it earlier will allow us to make the prepareHeaders call conditional without having to add more casts.

  • Make “header preparation” conditional: The method call was introduced in 4b1fc33 to support tunneling through HTTP proxy using CONNECT method, whose response curl includes in the curl_exec result. Ideally, we would prevent that with CURLOPT_SUPPRESS_CONNECT_HEADERS but that is only available in PHP 7.3.

jtojnar and others added 5 commits September 21, 2025 17:46
`CURLINFO_REDIRECT_COUNT` is only non-zero when `CURLOPT_FOLLOWLOCATION` is enabled
but we track redirects ourselves since 692e8bc.

https://curl.se/libcurl/c/CURLOPT_FOLLOWLOCATION.html
https://curl.se/libcurl/c/CURLINFO_REDIRECT_COUNT.html
Since starting to handle redirects ourselves in 692e8bc, `url` info item (aka `CURLINFO_EFFECTIVE_URL`) will be the same as the initial URI.
In fact, when the server sends interim responses, this was counterproductive – it would overwrite the final response code returned by `CURLINFO_HTTP_CODE` with the interim one (e.g. 100).
Though, the parser will still include the headers of all but the first response in the body.
Explicit comparison is clearer than relying on `CURLE_OK` being falsey (`0`).
This is only needed for PHPStan.

Making the type assertion explicit and moving it earlier will allow us to make the `prepareHeaders` call conditional without having to add more casts.
The method call was introduced in 4b1fc33 to support tunneling through HTTP proxy using CONNECT method, whose response curl includes in the `curl_exec` result.

Ideally, we would prevent that with `CURLOPT_SUPPRESS_CONNECT_HEADERS` but that is only available in PHP 7.3:
https://www.php.net/manual/en/curl.constants.php#constant.curlopt-suppress-connect-headers
@jtojnar jtojnar requested a review from Art4 September 28, 2025 09:46
@Art4 Art4 added this to the 1.10.0 milestone Sep 29, 2025
@jtojnar jtojnar merged commit a3ea016 into master Sep 29, 2025
20 checks passed
@jtojnar jtojnar deleted the wip/jtojnar/file-cleanups branch September 29, 2025 09:43
This was referenced Sep 30, 2025
Alkarex added a commit to FreshRSS/simplepie that referenced this pull request Sep 30, 2025
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.

3 participants