Force HTTPS for selected domains#676
Conversation
Some feeds have embeded references using e.g. `//youtube.com/` or `http://youtube.com/` which are increasingly failing as HTTPS is required. This PR allows setting a list of domains for which to force HTTPS with a new function `SimplePie::set_https_domains([])` Downstream PR: FreshRSS/FreshRSS#1087 FreshRSS/FreshRSS#1098
| * @see SimplePie_Misc::https_url() | ||
| * Example array('biz', 'example.com', 'example.org', 'www.example.net'); | ||
| */ | ||
| public function set_https_domains($domains) |
There was a problem hiding this comment.
The list of domains is transformed into a tree for fast lookup and to avoid ambiguities.
Bug from FreshRSS#2136 Related upstream PR simplepie/simplepie#676
Bug from #2136 Related upstream PR simplepie/simplepie#676
|
Well I'm not opposed to this idea in general, I would hate to be on the hook for owning this list. Is there anyway we could source a good list from somebody else who's already maintaining one? The EFF's HTTPS Everywhere extension comes to mind as a possible good source of data. Assuming it's a JSON document, we could periodically fetch the list, transform it into a PHP lookup table, and ship a new release. |
|
My mistake; I was on my phone and didn't see that this is a bring-your-own-list situation. I think this is a good idea. |
|
Not that it should be used, but just as an example, downstream, we use this list by default, customisable by end-users https://github.com/FreshRSS/FreshRSS/blob/edge/force-https.default.txt Our two most important use-cases |
library/SimplePie/Sanitize.php
Outdated
| { | ||
| $value = $this->registry->call('Misc', 'absolutize_url', array($element->getAttribute($attribute), $this->base)); | ||
| if ($value !== false) | ||
| $value = $this->https_url($value); |
There was a problem hiding this comment.
absolutize_url can return false, I don't think you should check the url in that case.
From simplepie#676 A parenthesis was misplaced
From #676 A parenthesis was misplaced
Related to simplepie/simplepie#676 from FreshRSS#1087
Related to simplepie/simplepie#676 from #1087
Some feeds have embedded references using e.g.
//youtube.com/orhttp://youtube.com/which are increasingly failing as HTTPS is required.This PR allows setting a list of domains for which to force HTTPS with a new function
SimplePie::set_https_domains([])Downstream PR: FreshRSS/FreshRSS#1087
FreshRSS/FreshRSS#1098