Issue 7460 - MOD_REPLACE on groups/link attributes modifies overlap targets

[droideck]

Description: memberof_replace_list and linked_attrs_replace_backpointers sort pre/post with slapi_attr_value_cmp_ext, which returns 0 on match and a negative value or LDAP error code on non-match — not a strict weak ordering. The merge loop's cmp > 0 branch is never taken, so overlap members are deleted and re-added on every MOD_REPLACE, bumping entryUSN.

Switch both comparators to slapi_utf8casecmp on raw bv_val. For that to work, normalize DN-syntax values in str2entry_fast (which did not normalize at all — the old code sat under an unset OBSOLETE_DN_SYNTAX_CHECK) and route str2entry_dupcheck through the same helper. Skip the upgradedn path (SLAPI_STR2ENTRY_USE_OBSOLETE_DNFORMAT) in both, preserving raw bytes for the LMDB import. Fix a stray missing comma in the dupcheck strict-fail slapi_log_err. Drop the now-unused qsortConfig plugin global.

Add Python regression tests for both memberOf and linkedAttrs cases.

Fixes: #7460

Reviewed by: ?

Summary by Sourcery

Ensure DN-valued attributes are normalized consistently so memberOf and linked attributes handle MOD_REPLACE diffs correctly without unnecessarily re-touching overlapping members.

Bug Fixes:

• Fix memberOf group MOD_REPLACE so overlapping members are not removed and re-added, avoiding unnecessary entryUSN bumps.
• Fix linked attributes MOD_REPLACE handling so overlap targets are not spuriously re-modified.
• Correct DN syntax error logging in str2entry_dupcheck to avoid memory leaks when strict DN checking fails.

Enhancements:

• Normalize DN-syntax attribute values during entry parsing and duplicate-checking while preserving raw bytes for legacy upgradedn imports.
• Align memberOf and linked attributes comparator logic to use a strict weak ordering based on UTF-8 case-insensitive comparison of normalized DNs.
• Remove the now-unnecessary memberOf plugin global used to pass config into qsort comparators.

Tests:

• Add regression tests exercising memberOf behavior across overlapping, empty, denormalized, and no-op group MOD_REPLACE scenarios with entryUSN checks.
• Add regression tests verifying DN normalization of imported denormalized member values and subsequent memberOf fixup and MOD_REPLACE behavior.
• Add a linked attributes regression test ensuring MOD_REPLACE does not spuriously update overlap targets’ entryUSN values.

[sourcery-ai]

Hey - I've left some high level feedback:

• The denorm() helper is duplicated in both test_replace_list_diff_correctness and test_replace_list_after_ldif_import; consider factoring this into a shared utility to avoid divergence between the two implementations.
• The new memberOf/linkedAttrs tests rely on fixed time.sleep(delay) calls after operations; polling for the expected state (e.g. memberOf presence or entryUSN change) would make these tests less timing‑sensitive and reduce flakiness on slow or loaded environments.
• In str2entry_fast/str2entry_dupcheck, failures from value_dn_normalize_value are currently ignored; consider at least logging a warning or surfacing an error in strict modes so unexpected normalization failures are more visible during debugging.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `denorm()` helper is duplicated in both `test_replace_list_diff_correctness` and `test_replace_list_after_ldif_import`; consider factoring this into a shared utility to avoid divergence between the two implementations.
- The new memberOf/linkedAttrs tests rely on fixed `time.sleep(delay)` calls after operations; polling for the expected state (e.g. memberOf presence or entryUSN change) would make these tests less timing‑sensitive and reduce flakiness on slow or loaded environments.
- In `str2entry_fast`/`str2entry_dupcheck`, failures from `value_dn_normalize_value` are currently ignored; consider at least logging a warning or surfacing an error in strict modes so unexpected normalization failures are more visible during debugging.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[packit-as-a-service]

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

• sudo dnf install -y 'dnf*-command(copr)'
• dnf copr enable packit/389ds-389-ds-base-7461
• And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

[progier389]

LGTM