Add curl user-agent to retrieve favicon#1385
Conversation
Alkarex
changed the title
| curl_setopt($c, CURLOPT_HEADER, false); | ||
| curl_setopt($c, CURLOPT_RETURNTRANSFER, true); | ||
| curl_setopt($c, CURLOPT_BINARYTRANSFER, true); | ||
| curl_setopt($c, CURLOPT_USERAGENT, 'FreshRSS/' . FRESHRSS_VERSION . ' (' . PHP_OS . '; ' . FRESHRSS_WEBSITE . ')'); |
There was a problem hiding this comment.
Probably doesn't hurt, but I am against exposing the operating system of the webserver.
Could that line be in the config file to allow users to overwrite the value?
There was a problem hiding this comment.
Yes, there are some pros and cons. One of the considerations I have is that I see some sites banning some user-agents (e.g. due to abuses) or even IPs, and I hope that having a bit of diversity can help. Same for debugging. It is worth noting that Web browsers also expose the operating system they run on.
But yes, it could indeed be good to make that a constant. There are a couple of different user-agent strings used by FreshRSS, e.g. when going through SimplePie or not.
There was a problem hiding this comment.
Well my comment was indeed without any background knowledge how FreshRSS handles this in all the other requests. Probably that should be streamlined (and configurable for the future).
Actually I just thought I give a little back to a great open source project by doing a code review ...
I know that browsers spread the useragent information even further (that why I use extensions to switch them) but leaking server side information to other sites is probably not the best idea. Every admin/devop I know (me included) tries to hide every possible information about server-side OS and software versions (like type of webserver, php version and so on).
Just my 2 cents - like in every code review - sharing thoughts is the most important part :)
| global $favicons_dir, $default_favicon; | ||
|
|
||
| syslog(LOG_DEBUG, 'FreshRSS Favicon discovery GET ' . $website); | ||
| syslog(LOG_INFO, 'FreshRSS Favicon discovery GET ' . $website); |
There was a problem hiding this comment.
This one could stay debug, as there is already one info log for the same procedure below
There was a problem hiding this comment.
Yes, but not with the same address (two distinct requests) :-)
There was a problem hiding this comment.
I just imagined what happens in your log when you import an OPML with 200+ feeds ;-)
In most cases the favicon will be hosted on the same domain like the feed, so I would say this information is likely redundant - but you are right - these are two different urls. And this action won't happen to often^^
There was a problem hiding this comment.
We should indeed improve the logging options. I think it is important to have logs for the outgoing queries.
2 participants
#1380