Closed Bug 7131 Opened 25 years ago Closed 25 years ago

Incorrect handling of setValue() + addValue() on muti-valued attributes

Categories

(Directory :: PerLDAP, defect, P1)

Sun
Solaris
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dg50, Assigned: kmccarth)

References

Details

Attachments

(1 file)

1) Do a search returning an $entry with a multi-valued attribute (like
objectclass)

2) $entry->printLDIF() shows the multivalue is there:

...
objectclass: a
objectclass: b
objectclass: c
...

2) Do $entry->setValue('objectclass', 'foo');

3) Make objectclass multivalued with multiple addValue() calls:
   $entry->addValue('objectclass', 'bar1');
   $entry->addValue('objectclass', 'bar2');
   $entry->addValue('objectclass', 'bar3');

4) $entry->printLDIF(); shows...
...
objectclass: foo
objectclass: bar1
objectcalss: bar2
objectclass: bar3
...
as it should.

5) Do $entry->update(); - it fails

6) Examination of "heavy tracing" server error log shows that instead of

replace: objectclass
objectcalss: foo
objectclass: bar1
objectclass: bar2
objectclass: bar3

being sent to the server, instead

add: objectclass
objectclass: bar1
objectclass: bar2
objectclass: bar3

is.

The "setvalue" of "foo" is getting lost somehow.

This is with PerLDAP 1.22

This bug is blocking important software from being completed.

DG
Priority: P3 → P1
Status: NEW → ASSIGNED
Hi Leif,

Here's a patch to the 1.2 branch of the tree for this:
*** Entry.orig.pm	Fri Jun  4 17:08:22 1999
--- Entry.pm	Fri Jun  4 17:14:10 1999
***************
*** 495,500 ****
--- 495,511 ----
    return 0 unless (defined(@vals) && ($#vals >= $[));
    return 0 unless (defined($attr) && ($attr ne ""));

+   if (defined($self->{$attr}))
+     {
+       $self->{"_self_obj_"}->{"_${attr}_save_"} = [ @{$self->{$attr}} ]
+         unless defined($self->{"_${attr}_save_"});
+     }
+   else
+     {
+       $self->{"_self_obj_"}->{"_${attr}_save_"} = [ ]
+         unless defined($self->{"_${attr}_save_"});
+     }
+
    $self->{"_self_obj_"}->{$attr} = [ @vals ];
    $self->{"_self_obj_"}->{"_${attr}_modified_"} = 1;


I haven't tested this yet.  Another idea would be to set up an
_${attr}_replaced_ attribute.  the Conn->update() could then check the
existance of this attribute and do a "rb" operation on the attribute.  The
current patch will generate an "ab" and a "db" operation.

-Kevin McCarthy
Severity: blocker → normal
Tested with the patched Entry.pm, and the previously deadlocked code now
executes. Everthing appears to be working fine now.

I haven't tested the "heavy tracing" output from the server yet, to see what's
coming over the wire. From your description, it sounds like you're adding and
then deleting. I don't think the bug is fixed "right" until update() does an
atomic, multi-valued ldap_replace. Think of the case in which a multi-valued
attribute with a very large number of values is replaced with a another set of
different (but still many) values.

A definate step forward though. Good work.

DG
This is an old message from Kevin, I'm adding it anyways, even though we have a
patch now.

Hi Leif,

Okay, I think I found the problem with 7131.  Basically, setValue() never
sets _${attr}_save_

This means the very next addValue() sets _${attr}_save_, and sets it to the
value added in by setValue(). (So it's all goofed up!)

As a side note, we may want to introduce an _${attr}_replaced_ attribute so
we can cause a setValue() to do a replace operation.  Then we could change
the $conn->update() to check for the _replaced_ attribute instead of
checking $#attr == $[  .  The current logic will do a 'ab' and a 'db'
instead of a 'rb' when setValue is called.

When you have a moment, maybe you could walk me through the procedure you'd
like me to use for fixing bugs in the future.

Thanks,

-Kevin McCarthy
More old stuff from Kevin:
I'm back from my business trip to the east coast.   Sorry I've been out of
touch - I've been pretty busy since getting back.

Last week, I received confirmation from Carlos that the patch to 3342
works.  Would you like me to generate a patch file against 1.3 for that, or
will the 1.2 patch be okay?

Regarding bug 7131, the patch I submitted works, but the update will
generate 'ab' and 'db'  commands.  Would you like to leave it at that, or
should I work on getting it to do a 'rb'?

It will take me a little time to get my hands on an NT box.  Until then,
I'll have trouble working with the ActiveState/NT problems.  Are there any
other issues you'd like me to work on?

General CVS question: what command do I issue to get a list of all the
branches for PerLDAP?
I've started looking at implementing "rb"  when setValue() is called (bug
7131).  I'll generate some patch files against the 1.3 tree for you to look
at.  Then maybe you can let me know what you think - I'll see if I can get
those done by this weekend.

Oh, one more thing: I posted to the newsgroups about a PerLDAP problem (and
fix) on 6/3/99, Subject: PerLDAP problem (size limits).  It seems that
doing a:
   @{$entry->{'_value_save_'}} = @{$entry->{'value'});
fails when the array gets too big (at least for perl 5.004_04) but
   $entry->{'_value_save_'} = [ $entry->{'value'} ];
works.  Maybe something to do with array copies...  I don't have any Perl
guru's to ask about this, but I'd like to make that change in Entry.pm in
about 5 places.  Do you have any advice on this? Well, I'll submit a bug on
this tomorrow.
*** Bug 11114 has been marked as a duplicate of this bug. ***
Assignee: leif → kmccarth
Status: ASSIGNED → NEW
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
I believe we have fixed this, I'm closing it now, reopen if it's still a
problem. PerLDAP v1.4 will have these fixes.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: