Skip to content

Use single constant for default HTTP Accept header#784

Merged
mblaney merged 3 commits intosimplepie:masterfrom
FreshRSS:http-accept
Feb 13, 2023
Merged

Use single constant for default HTTP Accept header#784
mblaney merged 3 commits intosimplepie:masterfrom
FreshRSS:http-accept

Conversation

@Alkarex
Copy link
Contributor

@Alkarex Alkarex commented Feb 8, 2023

The same string was repeated 5 times.
And add missing headers in SimplePie_Locator::body()
Follow-up of 5d966b9
This prepares for an update of the content of the default HTTP Accept header in another PR.

And add missing headers in `SimplePie_Locator::body()`
Follow-up of simplepie@5d966b9
This prepares for an update of the content of the default HTTP Accept header in another PR.
'Accept' => File::DEFAULT_HTTP_ACCEPT,
];
$feed = $this->registry->create(File::class, [$value, $this->timeout, 5, null, $this->useragent, $this->force_fsockopen, $this->curl_options]);
$feed = $this->registry->create(File::class, [$value, $this->timeout, 5, $headers, $this->useragent, $this->force_fsockopen, $this->curl_options]);
Copy link
Contributor Author

@Alkarex Alkarex Feb 8, 2023

Choose a reason for hiding this comment

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

Likely forgotten in 5d966b9
Otherwise, the $headers variable was not used.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍
Looks like an error to me. I am in favor of fixing it this way.

@Alkarex
Copy link
Contributor Author

Alkarex commented Feb 8, 2023

Downstream PR FreshRSS/FreshRSS#5083

'Accept' => File::DEFAULT_HTTP_ACCEPT,
];
$feed = $this->registry->create(File::class, [$value, $this->timeout, 5, null, $this->useragent, $this->force_fsockopen, $this->curl_options]);
$feed = $this->registry->create(File::class, [$value, $this->timeout, 5, $headers, $this->useragent, $this->force_fsockopen, $this->curl_options]);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍
Looks like an error to me. I am in favor of fixing it this way.

Copy link
Contributor

@Art4 Art4 left a comment

Choose a reason for hiding this comment

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

Thank you 👍

Co-authored-by: Jan Tojnar <jtojnar@gmail.com>
@mblaney mblaney merged commit 7b0901c into simplepie:master Feb 13, 2023
@Alkarex Alkarex deleted the http-accept branch February 24, 2023 12:05
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.

4 participants