Skip to content

SimplePie fix merge HTTP headers#8246

Closed
Alkarex wants to merge 2 commits intoFreshRSS:edgefrom
Alkarex:simplepie-fix-merge-http-headers
Closed

SimplePie fix merge HTTP headers#8246
Alkarex wants to merge 2 commits intoFreshRSS:edgefrom
Alkarex:simplepie-fix-merge-http-headers

Conversation

@Alkarex
Copy link
Member

@Alkarex Alkarex commented Nov 24, 2025

HTTP headers were not merged properly, and were also not accounting for possible casing difference.
See #8189 (comment)

Upstream PR:

HTTP headers were not merged properly, and were also not accounting for possible casing difference.
See FreshRSS#8189 (comment)
@Alkarex Alkarex added this to the 1.28.0 milestone Nov 24, 2025
}
if (isset($curl_options[CURLOPT_HTTPHEADER])) {
if (is_array($curl_options[CURLOPT_HTTPHEADER])) {
$headers2 = array_merge($headers2, $curl_options[CURLOPT_HTTPHEADER]);
Copy link
Member Author

Choose a reason for hiding this comment

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

Could not work because those are arrays of strings and not arrays of key-values.

if (!$force_fsockopen && function_exists('curl_exec')) {
$this->method = \SimplePie\SimplePie::FILE_SOURCE_REMOTE | \SimplePie\SimplePie::FILE_SOURCE_CURL;
$fp = curl_init();
$headers2 = [];
Copy link
Member Author

Choose a reason for hiding this comment

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

Renamed as $headers_inlined below

@Alkarex Alkarex added SimplePie 🍰 Bug (confirmed) 🐞 issues that are reproducible labels Nov 24, 2025
}
}
unset($curl_options[CURLOPT_HTTPHEADER]);
// Restore original casing of HTTP headers
Copy link
Member Author

Choose a reason for hiding this comment

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

We could send everything lowercased as required by HTTP/2, but better to keep original casing to avoid being discriminated on that when using HTTP/1.1.

Alkarex added a commit to FreshRSS/simplepie that referenced this pull request Nov 25, 2025
HTTP headers were not merged properly, and were also not accounting for possible casing differences.
Follow-up of simplepie#912
It was for instance not possible to override the `Accept` header from the cURL options.
The existing code could not work properly because we were merging arrays of strings and not arrays of key-values, leading to strange duplicates.

Downstream: FreshRSS/FreshRSS#8246
Alkarex added a commit to Alkarex/simplepie that referenced this pull request Nov 25, 2025
HTTP headers were not merged properly, and were also not accounting for possible casing differences.
Follow-up of simplepie#912
It was for instance not possible to override the `Accept` header from the cURL options.
The existing code could not work properly because we were merging arrays of strings and not arrays of key-values, leading to strange duplicates.

Downstream: FreshRSS/FreshRSS#8246
Upstream: simplepie#956
Alkarex added a commit to FreshRSS/simplepie that referenced this pull request Nov 25, 2025
HTTP headers were not merged properly, and were also not accounting for possible casing differences.
Follow-up of simplepie#912
It was for instance not possible to override the `Accept` header from the cURL options.
The existing code could not work properly because we were merging arrays of strings and not arrays of key-values, leading to strange duplicates.

Downstream:
* FreshRSS/FreshRSS#8246

Upstream:
* simplepie#956
@Alkarex Alkarex marked this pull request as ready for review November 25, 2025 12:58
@Alkarex
Copy link
Member Author

Alkarex commented Nov 25, 2025

I need to rethink that. There are valid reasons for multiple identical HTTP headers such as:

Set-Cookie: a=1
Set-Cookie: b=2

@Alkarex Alkarex closed this Nov 25, 2025
@Alkarex Alkarex deleted the simplepie-fix-merge-http-headers branch November 25, 2025 22:34
Alkarex added a commit to Alkarex/FreshRSS that referenced this pull request Nov 25, 2025
Alkarex added a commit that referenced this pull request Nov 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug (confirmed) 🐞 issues that are reproducible SimplePie 🍰

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant