Re: [Bug-wget] [Bug-Wget] Misc. patches

[Giuseppe Scrivano]

Darshit Shah <address@hidden> writes:

>>>  static bool
>>>  check_domain_match (const char *cookie_domain, const char *host)
>>> @@ -509,6 +519,7 @@ check_domain_match (const char *cookie_domain, const 
>>> char *host)
>>>
>>>  #ifdef HAVE_LIBPSL
>>>    DEBUGP (("cdm: 1"));
>>> +  char * cookie_domain_lower, * host_lower;
>>
>> please initialize them to NULL and format like char
>> *cookie_domain_lower, *host_lower (no space between * and the variable
>> name), otherwise...
>>
>>>    const psl_ctx_t *psl;
>>>    int is_acceptable;
>>>
>>> @@ -519,7 +530,18 @@ check_domain_match (const char *cookie_domain, const 
>>> char *host)
>>>        goto no_psl;
>>>      }
>>>
>>> -  is_acceptable = psl_is_cookie_domain_acceptable (psl, host, 
>>> cookie_domain);
>>> +  if (psl_str_to_utf8lower (cookie_domain, NULL, NULL, 
>>> &cookie_domain_lower) != PSL_SUCCESS ||
>>> +      psl_str_to_utf8lower (host, NULL, NULL, &host_lower) != PSL_SUCCESS)
>>
>> ...if the first "psl_str_to_utf8lower" fails then "host_lower" keeps
>> some bogus value...
>>
>>> +    {
>>> +        DEBUGP (("libpsl unable to parse domain name. "
>>> +                 "Falling back to simple heuristics.\n"));
>>> +        goto no_psl;
>>> +    }
>>> +
>>> +  is_acceptable = psl_is_cookie_domain_acceptable (psl, host_lower, 
>>> cookie_domain_lower);
>>> +  xfree (cookie_domain_lower);
>>> +  xfree (host_lower);
>>
>> ...and *boom* here.
>>
> Aah! I somehow managed not to get any "boom"s despite having a test
> that saw psl_str_to_utf8lower() fail. However, your comment is correct
> and I'll fix that. The general idea was that if the function fails, it
> will fail on both the calls

I somehow misread the patch and the position of the no_psl label.  We
should move the two xfree in the cleanup block, after "no_psl", to avoid
a potential memory leak.

Regards,
Giuseppe