gaffling / PHP-Grab-Favicon

🖼 Saves the favicon of the given URL and returns the image path.
http://suchmaschine.biz
MIT License
26 stars 6 forks source link

cURL Timeout Not Working Properly #10

Open LeeThompson opened 1 year ago

LeeThompson commented 1 year ago

There is a bug (my fault) where the curl timeout isn't being set properly, this will cause it to hang instead of timing out.

I have fixed it locally but I've changed other things as well and not ready for a PR.

Quick fix is to insert the following two lines in the loadfunction

    $timeOut = getGlobal('curl_timeout');
    if (!isset($timeOut )) { $timeOut = 60; }

Current:

function load($url, $DEBUG, $consoleMode = false, $timeOut = 60) {
  if (function_exists('curl_version')) {
    $ch = curl_init($url);
    curl_setopt($ch, CURLOPT_USERAGENT, getGlobal('curl_useragent'));
    curl_setopt($ch, CURLOPT_VERBOSE, getGlobal('curl_verbose'));
    curl_setopt($ch, CURLOPT_TIMEOUT, $timeOut);

Hot Fix:

function load($url, $DEBUG, $consoleMode = false, $timeOut = 60) {
  if (function_exists('curl_version')) {
    $timeOut = getGlobal('curl_timeout');
    if (!isset($timeOut )) { $timeOut = 60; }
    $ch = curl_init($url);
    curl_setopt($ch, CURLOPT_USERAGENT, getGlobal('curl_useragent'));
    curl_setopt($ch, CURLOPT_VERBOSE, getGlobal('curl_verbose'));
    curl_setopt($ch, CURLOPT_TIMEOUT, $timeOut);

(It's handled much better in the newer code)

gaffling commented 1 year ago

If you use it like this:

function load($url, $DEBUG, $consoleMode = false, $timeOut = 60) {
  if (function_exists('curl_version')) {
    $timeOut = getGlobal('curl_timeout');
    if (!isset($timeOut )) { $timeOut = 60; }

$timeOut = getGlobal('curl_timeout'); is already set before the function call, so I would implement it like this:

function load($url, $DEBUG, $consoleMode = false, $timeOut = 60) {
  if (function_exists('curl_version')) {
    if (!isset($timeOut )) { $timeOut = 60; }
LeeThompson commented 1 year ago

It's being used by the fallback (non curl) as well, but I'm restructuring it a bit. The above is just a quick fix.

gaffling commented 1 year ago

I will be happy with every update-request and promise to merge it ASAP ;-)

LeeThompson commented 1 year ago

I'll see if I can do a special branch for this hotfix. My main branch is in the middle of some major rework (including supporting a config file).

LeeThompson commented 1 year ago

Nevermind, it looks like you got it, that should be enough for now! :)

gaffling commented 1 year ago

No Stress, we can do it all in one Step if you like

LeeThompson commented 1 year ago

I'm using #9 for my working on list.