Re: Fix for bug in ldapServiceLookup in libpq

From: "Albe Laurenz" <laurenz(dot)albe(at)wien(dot)gv(dot)at>
To: "Tom Lane *EXTERN*" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "pgsql-hackers" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix for bug in ldapServiceLookup in libpq
Date: 2011-05-12 08:22:23
Message-ID: D960CB61B694CF459DCFB4B0128514C20668FBB6@exadv11.host.magwien.gv.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Tom Lane wrote:
>> I have found a small but annoying bug in libpq where
>> connection parameters are resolved via LDAP.

> Hmm ... that's a bug all right, but why have the null-termination
> inside the loop at all? Seems like it should look like
>
> for (p = result, i = 0; values[i] != NULL; ++i)
> {
> strncpy(p, values[i]->bv_val, values[i]->bv_len);
> p += values[i]->bv_len;
> *(p++) = '\n';
> }
> *p = '\0';

Yes, that's better.

> ... btw, shouldn't this function free the "result" string when it's
done
> with it? AFAICS that string is not returned to the caller, it's just
> being leaked.

Oops, yes it should.

> (I'll refrain from asking why it's creating the string in the first
> place rather than parsing ldap_get_values_len's output as-is ...)

So that I can close the LDAP connection as soon as feasible, but of
course
that's not absolutely necessary.

I have attached a new version of the patch that should address all known
problems.

Yours,
Laurenz Albe

Attachment Content-Type Size
ldapServiceLookup.patch application/octet-stream 1.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kohei Kaigai 2011-05-12 08:35:23 Re: [v9.2] Leaky view and RLS
Previous Message Dean Rasheed 2011-05-12 07:42:22 Re: Tables cannot have INSTEAD OF triggers