You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by Emmanuel Lecharny <el...@gmail.com> on 2007/08/15 16:19:48 UTC

[1.5.1] DIRSERVER-832 and how to fix it ...

Hi,

we are trying to cut a 1.5.1 release soon, and we have a couple of
serious issues on our road. DIRSERVER-832 is one of the most serious
one, as it's impacting almost all the code
(https://140.211.11.4/jira/browse/DIRSERVER-832).

The main problem with this bug is that we need to deeply modify the
server to eradicate this potential problem. Here is what we should
check :
- when comparing attributeTypes, we should do it using a case
insensitive comparison (not a big deal !)
- when comparing values, we *must* use the matching rules to be sure
that the comparison is correctly working. This is where we hit the
wall...

For instance, just have a look at the isRemovable() method in the
org.apache.directory.server.core.authz.support.RestrictedByFilter.java
file. (apacheds-core sub-project). What we can find is such a code :

...
                for ( Iterator k = rb.iterator(); k.hasNext(); )
                {
                    RestrictedByItem rbItem = ( RestrictedByItem ) k.next();

                    if ( attrId.equalsIgnoreCase( rbItem.getAttributeType() ) )
                    {
                        Attribute attr = entry.get( rbItem.getValuesIn() );

                        if ( attr == null || !attr.contains( attrValue ) )
                        {
                            return true;
                        }
                    }
                }
...

The last test is badly wrong : attr.contains( attrValue ) will just
compare the value as is, without applying any normalization to the
value. Another problem is that if ( attrId.equalsIgnoreCase(
rbItem.getAttributeType() ) ) won't work if the ACI contains an OID
and the RestrictedByItem contains a name...

We have a couple of helper methods in an AttributeUtils class, like :
public final static boolean containsValue( Attribute attr, Object
compared, AttributeType type ),
but this is only if one can access the registries to grab the
AttributeType. Alas, those registries are not accessible everywhere in
the code...

So now, what next ? I would suggest as a quick fix to add an accessor
to those registries which will be a singleton (I know, Pattern purists
will axe me ...), because it's the fastest way to fix this problem,
and I can't think about some other way (unless we add one more
parameter to the initial method : an accessor to the registry. Yuk
...)

thoughts ?

(Remember, this is a Blocker !)
-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com

Re: [1.5.1] DIRSERVER-832 and how to fix it ...

Posted by Alex Karasulu <ak...@apache.org>.
On 8/15/07, Emmanuel Lecharny <el...@gmail.com> wrote:
>
> Hi Alex,
>
> On 8/15/07, Alex Karasulu <ak...@apache.org> wrote:
> > Just thinking about this issue. Yeah it's a big one but I can knock this
> > out in less than a day.
>
> In my mind, this is a gross under evaluation :). We have to review all
> the code to be sure that we didn't missed any of these comparisons
> (the issue is also a little bit misleading as it make people think
> it's just a problem with RDN).
>
> I would says it will take at least one full week, may be two.


Perhaps but soon as I get cookin I can knock stuff out but might
take longer than I expect agreed.

> I think what is more important is to get the big
> > picture of how many issues we have for 1.5.1 and which are going to
> > take a big effort.
>
> Yes. This is what I'm working on atm.


Cool I'm just trying to get back in the groove with a new machine.

>
> > If there are many hairy big work issues like this one then we can
> postpone
> > this and just get 1.5.1 out the door.  If there are not and we only have
> a
> > few
> > of these to deal with then I can make sure I knock this out in a day.
>
> Your days contains how many hours ?


:)

Ok, just kidding. The idea is to
> get 1.5.1 out, as we have fixed 63 issues since 1.5.0, so whatever the
> number of issues we still have for 1.5.1, I would favor a release by
> the end of this week, so that we can test all the packagers and run a
> certification test on it. Then we will be able to switch to 1.5.2 for
> the next 2 months.
>
> We don't need to have all the bug fixed, we just need to have our
> users pleased with a better version every two months. We don't respect
> the 'release early, release often' principle, and this is not good.
>
> Let's evaluate the remaining bugs and fix as much as we can for
> august, 20, and then release.
>
> WDYT ?


As long as you drive the bug parade then I'm on board.

Alex

Re: [1.5.1] DIRSERVER-832 and how to fix it ...

Posted by Emmanuel Lecharny <el...@gmail.com>.
Hi Alex,

On 8/15/07, Alex Karasulu <ak...@apache.org> wrote:
> Just thinking about this issue. Yeah it's a big one but I can knock this
> out in less than a day.

In my mind, this is a gross under evaluation :). We have to review all
the code to be sure that we didn't missed any of these comparisons
(the issue is also a little bit misleading as it make people think
it's just a problem with RDN).

I would says it will take at least one full week, may be two.

> I think what is more important is to get the big
> picture of how many issues we have for 1.5.1 and which are going to
> take a big effort.

Yes. This is what I'm working on atm.

>
> If there are many hairy big work issues like this one then we can postpone
> this and just get 1.5.1 out the door.  If there are not and we only have a
> few
> of these to deal with then I can make sure I knock this out in a day.

Your days contains how many hours ? Ok, just kidding. The idea is to
get 1.5.1 out, as we have fixed 63 issues since 1.5.0, so whatever the
number of issues we still have for 1.5.1, I would favor a release by
the end of this week, so that we can test all the packagers and run a
certification test on it. Then we will be able to switch to 1.5.2 for
the next 2 months.

We don't need to have all the bug fixed, we just need to have our
users pleased with a better version every two months. We don't respect
the 'release early, release often' principle, and this is not good.

Let's evaluate the remaining bugs and fix as much as we can for
august, 20, and then release.

WDYT ?

-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com

Re: [1.5.1] DIRSERVER-832 and how to fix it ...

Posted by Alex Karasulu <ak...@apache.org>.
Just thinking about this issue. Yeah it's a big one but I can knock this
out in less than a day.  I think what is more important is to get the big
picture of how many issues we have for 1.5.1 and which are going to
take a big effort.

If there are many hairy big work issues like this one then we can postpone
this and just get 1.5.1 out the door.  If there are not and we only have a
few
of these to deal with then I can make sure I knock this out in a day.

WDYT?

Alex

On 8/15/07, Emmanuel Lecharny <el...@gmail.com> wrote:
>
> Hi,
>
> we are trying to cut a 1.5.1 release soon, and we have a couple of
> serious issues on our road. DIRSERVER-832 is one of the most serious
> one, as it's impacting almost all the code
> (https://140.211.11.4/jira/browse/DIRSERVER-832).
>
> The main problem with this bug is that we need to deeply modify the
> server to eradicate this potential problem. Here is what we should
> check :
> - when comparing attributeTypes, we should do it using a case
> insensitive comparison (not a big deal !)
> - when comparing values, we *must* use the matching rules to be sure
> that the comparison is correctly working. This is where we hit the
> wall...
>
> For instance, just have a look at the isRemovable() method in the
> org.apache.directory.server.core.authz.support.RestrictedByFilter.java
> file. (apacheds-core sub-project). What we can find is such a code :
>
> ...
>                 for ( Iterator k = rb.iterator(); k.hasNext(); )
>                 {
>                     RestrictedByItem rbItem = ( RestrictedByItem ) k.next
> ();
>
>                     if ( attrId.equalsIgnoreCase( rbItem.getAttributeType()
> ) )
>                     {
>                         Attribute attr = entry.get( rbItem.getValuesIn()
> );
>
>                         if ( attr == null || !attr.contains( attrValue ) )
>                         {
>                             return true;
>                         }
>                     }
>                 }
> ...
>
> The last test is badly wrong : attr.contains( attrValue ) will just
> compare the value as is, without applying any normalization to the
> value. Another problem is that if ( attrId.equalsIgnoreCase(
> rbItem.getAttributeType() ) ) won't work if the ACI contains an OID
> and the RestrictedByItem contains a name...
>
> We have a couple of helper methods in an AttributeUtils class, like :
> public final static boolean containsValue( Attribute attr, Object
> compared, AttributeType type ),
> but this is only if one can access the registries to grab the
> AttributeType. Alas, those registries are not accessible everywhere in
> the code...
>
> So now, what next ? I would suggest as a quick fix to add an accessor
> to those registries which will be a singleton (I know, Pattern purists
> will axe me ...), because it's the fastest way to fix this problem,
> and I can't think about some other way (unless we add one more
> parameter to the initial method : an accessor to the registry. Yuk
> ...)
>
> thoughts ?
>
> (Remember, this is a Blocker !)
> --
> Regards,
> Cordialement,
> Emmanuel Lécharny
> www.iktek.com
>