[#738] Fix dereferencing an alias that points into another backend#741
Open
vharseko wants to merge 1 commit into
Open
[#738] Fix dereferencing an alias that points into another backend#741vharseko wants to merge 1 commit into
vharseko wants to merge 1 commit into
Conversation
…o another backend LocalBackendSearchOperation resolves the backend once, for the original base DN, and caches it in a field. When the base entry turns out to be an alias, processSearch swaps the base DN and re-enters itself, but never re-resolves the backend, so the recursive call asks the backend of the original base DN for a base DN it does not hold. Aliases whose target lives in the same backend work, which is why this went unnoticed since f771315. Re-resolve the backend for the aliased DN before recursing. When no backend holds the target, the null check already at the top of processSearch reports NO_SUCH_OBJECT for the new base DN. Make both backends honour the LocalBackend.search contract when handed a base DN they do not hold, which is what made this bug so hard to read: MemoryBackend only reported a missing base entry when handlesEntry() was true and otherwise went on to match the filter against a null entry, answering a base search with a NullPointerException (result 80); the pluggable backend reported the internally used UNDEFINED (-1). Both now return NO_SUCH_OBJECT. UNDEFINED stays the default for the other accessBegin() callers: hasSubordinates() relies on it. Add AliasTestCase coverage for an alias crossing a backend boundary and for an alias whose target has no backend at all, a contract test to PluggableBackendImplTestCase, and MemoryBackendTestCase for the memory backend.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #738.
Cause
LocalBackendSearchOperationresolves the backend once, for the original base DN, and caches it in a field. When the base entry turns out to be an alias,processSearchswaps the base DN and re-enters itself, but never re-resolves the backend, so the recursive call asks the backend of the original base DN for a base DN it does not hold. Aliases whose target lives in the same backend work, which is why this went unnoticed since f771315.Note that the base entry itself is always found:
DirectoryServer.getEntry(baseDN)resolves the backend correctly. Only the search that follows goes to the wrong backend — hence the reported oddity that a base search on the alias target succeeds while a search through the alias does not.Fix
Re-resolve the backend for the aliased DN before recursing. When no backend holds the target, the
backend == nullcheck already at the top ofprocessSearchreportsNO_SUCH_OBJECTfor the new base DN.Both backends are also made to honour the
LocalBackend.searchcontract when handed a base DN they do not hold — this is what made the bug so hard to read, since neither returned a usable result code:MemoryBackendonly reported a missing base entry whenhandlesEntry()was true, and otherwise went on to match the filter against anullentry, so a base search answered with aNullPointerExceptionfromSearchFilter(result 80) and a subtree search with zero entries.UNDEFINED(-1), a result code used internally.Both now return
NO_SUCH_OBJECT.UNDEFINEDstays the default for the otheraccessBegin()callers, sincehasSubordinates()relies on it as a sentinel.Tests
AliasTestCase: a second backend (o=test2), an alias crossing the backend boundary (base and subtree scope), and an alias whose target has no backend at all. Only aliases withino=testwere covered so far. All three fail without the fix.PluggableBackendImplTestCase: contract test for a base DN held by no entry container, running for JE, PDB, JDBC and Cassandra.MemoryBackendTestCase: new, the memory backend had no test class of its own.Also run green:
OverlappingBackendTestSuite,LocalBackendWorkflowElementTest,GenericLocalBackendTestCase,LDIFBackendTestCase,SearchOperationTestCase,BackendConfigManagerTestCase,InternalClientConnectionTestCase,InternalSearchOperationTestCase,ChangelogBackendTestCase.