[PATCHES] LDAP: Sanitize group dn before using in filter

[Lukas Slebodnik]

ehlo,

attached is alternative version for ticket 2744.
The unit test passed but there is a performance improvement.

If we compare 3rd patch to current master than
there is a 10 less invocation of malloc (an also other functions) per group.
and we saved allocation of 1091 per group.

There was a comment[1] in old thread about a possibility of double sanitized dn.
I was not able to find such case. I would be glad if someone
can extend unit test which will pass with master and fail with attached patch.

LS

[1] https://lists.fedorahosted.org/pipermail/sssd-devel/2015-August/024346.html

[Lukas Slebodnik]

On (01/09/15 11:06), Lukas Slebodnik wrote:
> ehlo,
>
> attached is alternative version for ticket 2744.
> The unit test passed but there is a performance improvement.
>
> If we compare 3rd patch to current master than
> there is a 10 less invocation of malloc (an also other functions) per group.
> and we saved allocation of 1091 per group.
>
> There was a comment[1] in old thread about a possibility of double sanitized dn.
> I was not able to find such case. I would be glad if someone
> can extend unit test which will pass with master and fail with attached patch.
>
> LS
>
> [1] https://lists.fedorahosted.org/pipermail/sssd-devel/2015-August/024346.html

Bump.

LS

[Jakub Hrozek]

On Fri, Sep 18, 2015 at 11:40:20AM +0200, Lukas Slebodnik wrote:
> On (01/09/15 11:06), Lukas Slebodnik wrote:
> > ehlo,
> >
> > attached is alternative version for ticket 2744.
> > The unit test passed but there is a performance improvement.
> >
> > If we compare 3rd patch to current master than
> > there is a 10 less invocation of malloc (an also other functions) per group.
> > and we saved allocation of 1091 per group.
> >
> > There was a comment[1] in old thread about a possibility of double sanitized dn.
> > I was not able to find such case. I would be glad if someone
> > can extend unit test which will pass with master and fail with attached patch.
> >
> > LS
> >
> > [1] https://lists.fedorahosted.org/pipermail/sssd-devel/2015-August/024346.html
>
> Bump.
>
> LS

Pavel, can you review this patchset? You know the code.

[Pavel Březina]

On 09/01/2015 11:06 AM, Lukas Slebodnik wrote:
> ehlo,
>
> attached is alternative version for ticket 2744.
> The unit test passed but there is a performance improvement.
>
> If we compare 3rd patch to current master than
> there is a 10 less invocation of malloc (an also other functions) per group.
> and we saved allocation of 1091 per group.
>
> There was a comment[1] in old thread about a possibility of double sanitized dn.
> I was not able to find such case. I would be glad if someone
> can extend unit test which will pass with master and fail with attached patch.
>
> LS
>
> [1] https://lists.fedorahosted.org/pipermail/sssd-devel/2015-August/024346.html

Looks like I misunderstood your original proposal. This looks indeed better.

I see now why double-sanitization is not an issue.

Ack.

[Jakub Hrozek]

On Fri, Sep 18, 2015 at 03:39:50PM +0200, Pavel Březina wrote:
> On 09/01/2015 11:06 AM, Lukas Slebodnik wrote:
> > ehlo,
> >
> > attached is alternative version for ticket 2744.
> > The unit test passed but there is a performance improvement.
> >
> > If we compare 3rd patch to current master than
> > there is a 10 less invocation of malloc (an also other functions) per group.
> > and we saved allocation of 1091 per group.
> >
> > There was a comment[1] in old thread about a possibility of double sanitized dn.
> > I was not able to find such case. I would be glad if someone
> > can extend unit test which will pass with master and fail with attached patch.
> >
> > LS
> >
> > [1] https://lists.fedorahosted.org/pipermail/sssd-devel/2015-August/024346.html
>
> Looks like I misunderstood your original proposal. This looks indeed better.
>
> I see now why double-sanitization is not an issue.
>
> Ack.

CI: http://sssd-ci.duckdns.org/logs/job/26/93/summary.html

master:
 * 2cec08a3174bff951c048c57b4b0e4517ad6b7b1
 * 6cb5bad3c8e2f35ca9dce1800a506d626f90c079
 * 3d8b576bf49a79d5776574b96c6ef9535bbc46ac
 * 9c563db822758732b25a3c8c61ffac90a7deffc3

[Lukas Slebodnik]

On (21/09/15 17:00), Jakub Hrozek wrote:
> On Fri, Sep 18, 2015 at 03:39:50PM +0200, Pavel Březina wrote:
> > On 09/01/2015 11:06 AM, Lukas Slebodnik wrote:
> > > ehlo,
> > >
> > > attached is alternative version for ticket 2744.
> > > The unit test passed but there is a performance improvement.
> > >
> > > If we compare 3rd patch to current master than
> > > there is a 10 less invocation of malloc (an also other functions) per group.
> > > and we saved allocation of 1091 per group.
> > >
> > > There was a comment[1] in old thread about a possibility of double sanitized dn.
> > > I was not able to find such case. I would be glad if someone
> > > can extend unit test which will pass with master and fail with attached patch.
> > >
> > > LS
> > >
> > > [1] https://lists.fedorahosted.org/pipermail/sssd-devel/2015-August/024346.html
> >
> > Looks like I misunderstood your original proposal. This looks indeed better.
> >
> > I see now why double-sanitization is not an issue.
> >
> > Ack.
>
> CI: http://sssd-ci.duckdns.org/logs/job/26/93/summary.html
>
> master:
> 2cec08a3174bff951c048c57b4b0e4517ad6b7b1
> 6cb5bad3c8e2f35ca9dce1800a506d626f90c079
> 3d8b576bf49a79d5776574b96c6ef9535bbc46ac
> 9c563db822758732b25a3c8c61ffac90a7deffc3

Attached are patches for 1.12 branch

LS

[Pavel Březina]

On 09/21/2015 07:45 PM, Lukas Slebodnik wrote:
> On (21/09/15 17:00), Jakub Hrozek wrote:
> > On Fri, Sep 18, 2015 at 03:39:50PM +0200, Pavel Březina wrote:
> > > On 09/01/2015 11:06 AM, Lukas Slebodnik wrote:
> > > > ehlo,
> > > >
> > > > attached is alternative version for ticket 2744.
> > > > The unit test passed but there is a performance improvement.
> > > >
> > > > If we compare 3rd patch to current master than
> > > > there is a 10 less invocation of malloc (an also other functions) per group.
> > > > and we saved allocation of 1091 per group.
> > > >
> > > > There was a comment[1] in old thread about a possibility of double sanitized dn.
> > > > I was not able to find such case. I would be glad if someone
> > > > can extend unit test which will pass with master and fail with attached patch.
> > > >
> > > > LS
> > > >
> > > > [1] https://lists.fedorahosted.org/pipermail/sssd-devel/2015-August/024346.html
> > >
> > > Looks like I misunderstood your original proposal. This looks indeed better.
> > >
> > > I see now why double-sanitization is not an issue.
> > >
> > > Ack.
> >
> > CI: http://sssd-ci.duckdns.org/logs/job/26/93/summary.html
> >
> > master:
> > 2cec08a3174bff951c048c57b4b0e4517ad6b7b1
> > 6cb5bad3c8e2f35ca9dce1800a506d626f90c079
> > 3d8b576bf49a79d5776574b96c6ef9535bbc46ac
> > 9c563db822758732b25a3c8c61ffac90a7deffc3
>
> Attached are patches for 1.12 branch
>
> LS

Ack.

[Jakub Hrozek]

On Tue, Sep 22, 2015 at 10:35:09AM +0200, Pavel Březina wrote:
> > LS
>
> Ack.

CI on sssd-1-12 passed:
http://sssd-ci.duckdns.org/logs/job/27/63/summary.html

(Debian test failure is unrelated, we only fixed the dyndns test in
master)

Pushed to sssd-1-12 as:
    28dff998717b8bc70e28f39a8c7e3720c04305b7
    f2520325ef26def6680552da58b4cb3abe4523e1