Darshit Shah <dar...@gmail.com> 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
