Sean Burford wrote:

Hi,

On Fri, May 30, 2008 at 2:40 AM,hyc@symas.com wrote:

...

I wrote the attached patch for the original issue. Unfortunately it asserted right away:

...

...

It was adding the createTimestamp attribute, without providing normalized values. slap_add_opattrs was written before the generalizedTimeNormalize function was written... I suspect there will be a fair number of cases that need to be cleaned up. I don't have time at the moment to chase them all down. Anyone else want to jump in here? If not, we may have to push this back a bit.

I've modified my copy of the source to normalize data where needed. slapd starts up and my test can run. With the old assertion in place it fails exactly as it did before, because the old attr_merge() assertion runs before your new attr_valadd() normalization check.

That aspect of the patch is not optional. The old check is in the wrong place and must be removed. It misses the other places where attrs are being added, while attr_valadd catches all of them.

Removing the old attr_merge() assertion results in your new attr_valadd() check being triggered. Removing the old assertion looks safe, since attr_merge() calls attr_valadd() almost immediately after the assertion anyway.

With these changes, running my test script from the original tarball results in: bdb_modify_internal: add testAttribute attr_valadd: database inconsistent with schema definition of testAttribute, reload the DB bdb_modify_internal: 80 modify/add: testAttribute: merge error (80) hdb_modify: modify failed (80)

I'm working through 'make test' now to find more instances that need fixing. After that I'll send some patches.

...

Note that this patch will probably require a fair number of databases to be reloaded.

I'm not even sure that attributes can be automatically renormalized when a problem is detected, since bugs (eg. with the generation of timestamps, CSNs or referalls) can also trigger the normalization checks.

I think it's important to continue to assert as in my patch, at least at this stage, so we can catch all of those bugs. I also don't think we can silently renormalize just the cases that the "reload the DB" message detects, because that's obviously only going to happen on entries that are explicitly modified, and the rest of the DB will still have problems.