FileClient: Merge HTTP headers from cURL options#912
Merged
jtojnar merged 1 commit intosimplepie:masterfrom Mar 15, 2025
Merged
FileClient: Merge HTTP headers from cURL options#912jtojnar merged 1 commit intosimplepie:masterfrom
jtojnar merged 1 commit intosimplepie:masterfrom
Conversation
The `$curl_options` parameter could override the `CURLOPT_HTTPHEADER` option, so make sure to merge the HTTP headers properly
Alkarex
added a commit
to Alkarex/FreshRSS
that referenced
this pull request
Mar 5, 2025
fix FreshRSS#6712 (comment) We would sometimes wrongly override the default HTTP headers of SimplePie FreshRSS/simplepie#33 simplepie/simplepie#912
Alkarex
added a commit
to FreshRSS/FreshRSS
that referenced
this pull request
Mar 5, 2025
* Fix regression cURL HTTP headers fix #6712 (comment) We would sometimes wrongly override the default HTTP headers of SimplePie FreshRSS/simplepie#33 simplepie/simplepie#912 * Sync SimplePie FreshRSS/simplepie#33
jtojnar
approved these changes
Mar 15, 2025
Member
jtojnar
left a comment
There was a problem hiding this comment.
Thanks.
This makes sense to me: AFAICT, set_curl_options is the only way besides subclassing to pass extra headers with requests, if a bit indiscriminate, so we should support it. And we probably do not want to discard headers set by SimplePie, as it should, in theory, know what it is doing.
Generally, it might be desirable to provide a mechanism for overriding the headers set by SimplePie (the way that can be used for removing headers set by cURL – setting a header to an empty value – does not work, duplicate headers from $headers and $curl_options will be sent), but since the only SimplePie only ever sets one of four headers (Accept, if-modified-since, if-none-match, X-FORWARDED-FOR), implementing it would probably not be worth it. Especially when the FileClient is meant to away eventually.
Comparison of the behavior
Test script
<?php
require __DIR__ . '/vendor/autoload.php';
$file = new SimplePie\File(
'https://httpbin.org/headers',
headers: [
'X-Both-Args' => 'Headers',
'X-Headers-Arg' => '1',
'X-Headers-Arg-Hopefully-Shadowed' => 'Headers',
],
curl_options: [
\CURLOPT_HTTPHEADER => [
'X-Both-Args: Options',
'X-Curl-Options-Arg: 1',
'X-Headers-Arg-Hopefully-Shadowed:',
'X-Headers-Arg-Hopefully-Shadowed: Options',
]
]
);
echo $file->get_body_content();master
{
"headers": {
"Accept": "*/*",
"Accept-Encoding": "deflate, gzip, br, zstd",
"Host": "httpbin.org",
"Referer": "https://httpbin.org/headers",
"X-Amzn-Trace-Id": "Root=1-67d54198-20e1c4c154bbe72c0db9357d",
"X-Both-Args": "Options",
"X-Curl-Options-Arg": "1",
"X-Headers-Arg-Hopefully-Shadowed": "Options"
}
}This PR
{
"headers": {
"Accept": "*/*",
"Accept-Encoding": "deflate, gzip, br, zstd",
"Host": "httpbin.org",
"Referer": "https://httpbin.org/headers",
"X-Amzn-Trace-Id": "Root=1-67d54136-2cf0ca7c20c8e2cd2ae221cc",
"X-Both-Args": "Headers,Options",
"X-Curl-Options-Arg": "1",
"X-Headers-Arg": "1",
"X-Headers-Arg-Hopefully-Shadowed": "Headers,Options"
}
}
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
commented
Nov 25, 2025
| } | ||
| if (isset($curl_options[CURLOPT_HTTPHEADER])) { | ||
| if (is_array($curl_options[CURLOPT_HTTPHEADER])) { | ||
| $headers2 = array_merge($headers2, $curl_options[CURLOPT_HTTPHEADER]); |
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
$curl_optionsparameter could override theCURLOPT_HTTPHEADERoption, so make sure to merge the HTTP headers properly.Before this patch, providing an additional HTTP header via
$curl_options[CURLOPT_HTTPHEADER]would remove all essential headers such as Accept, if-modified-since, etc.