diff --git a/src/FreeDSx/Ldap/Search/Filter/ApproximateFilter.php b/src/FreeDSx/Ldap/Search/Filter/ApproximateFilter.php index 5ff7acd0..240497f8 100644 --- a/src/FreeDSx/Ldap/Search/Filter/ApproximateFilter.php +++ b/src/FreeDSx/Ldap/Search/Filter/ApproximateFilter.php @@ -20,7 +20,7 @@ * * @author Chad Sikorra */ -class ApproximateFilter implements FilterInterface, Stringable +class ApproximateFilter implements FilterInterface, FilterAttributeInterface, Stringable { use AttributeValueAssertionTrait; diff --git a/src/FreeDSx/Ldap/Search/Filter/EqualityFilter.php b/src/FreeDSx/Ldap/Search/Filter/EqualityFilter.php index 16c0eab2..84a5da34 100644 --- a/src/FreeDSx/Ldap/Search/Filter/EqualityFilter.php +++ b/src/FreeDSx/Ldap/Search/Filter/EqualityFilter.php @@ -20,7 +20,7 @@ * * @author Chad Sikorra */ -class EqualityFilter implements FilterInterface, Stringable +class EqualityFilter implements FilterInterface, FilterAttributeInterface, Stringable { use AttributeValueAssertionTrait; diff --git a/src/FreeDSx/Ldap/Search/Filter/FilterAttributeInterface.php b/src/FreeDSx/Ldap/Search/Filter/FilterAttributeInterface.php new file mode 100644 index 00000000..12072961 --- /dev/null +++ b/src/FreeDSx/Ldap/Search/Filter/FilterAttributeInterface.php @@ -0,0 +1,25 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Search\Filter; + +/** + * A filter that asserts against a single attribute type. + */ +interface FilterAttributeInterface +{ + /** + * The attribute type the filter references, or null when it is unspecified (an extensibleMatch without one). + */ + public function getAttribute(): ?string; +} diff --git a/src/FreeDSx/Ldap/Search/Filter/FilterAttributes.php b/src/FreeDSx/Ldap/Search/Filter/FilterAttributes.php new file mode 100644 index 00000000..959a22d3 --- /dev/null +++ b/src/FreeDSx/Ldap/Search/Filter/FilterAttributes.php @@ -0,0 +1,93 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Search\Filter; + +use function array_merge; +use function array_unique; +use function array_values; +use function strtok; +use function strtolower; + +/** + * Collects the base attribute names a filter references, so a reader can materialize just those for evaluation. + */ +final class FilterAttributes +{ + /** + * Lowercased base attribute names the filter references, or null when the set is indeterminate (an extensibleMatch + * without an attribute, or an unrecognized filter type) and every attribute must therefore be materialized. + * + * @return list|null + */ + public static function referenced(FilterInterface $filter): ?array + { + $names = self::walk($filter); + + if ($names === null) { + return null; + } + + return array_values(array_unique($names)); + } + + /** + * @return list|null + */ + private static function walk(FilterInterface $filter): ?array + { + if ($filter instanceof FilterContainerInterface) { + return self::walkChildren($filter->get()); + } + + if ($filter instanceof NotFilter) { + return self::walk($filter->get()); + } + + if (!$filter instanceof FilterAttributeInterface) { + return null; + } + + $attribute = $filter->getAttribute(); + + if ($attribute === null) { + return null; + } + + return [strtolower((string) strtok($attribute, ';'))]; + } + + /** + * @param array $children + * @return list|null + */ + private static function walkChildren(array $children): ?array + { + $names = []; + + foreach ($children as $child) { + $childNames = self::walk($child); + + if ($childNames === null) { + return null; + } + + $names = array_merge( + $names, + $childNames, + ); + } + + return $names; + } +} diff --git a/src/FreeDSx/Ldap/Search/Filter/GreaterThanOrEqualFilter.php b/src/FreeDSx/Ldap/Search/Filter/GreaterThanOrEqualFilter.php index a2c1bdf0..bc4d15f8 100644 --- a/src/FreeDSx/Ldap/Search/Filter/GreaterThanOrEqualFilter.php +++ b/src/FreeDSx/Ldap/Search/Filter/GreaterThanOrEqualFilter.php @@ -20,7 +20,7 @@ * * @author Chad Sikorra */ -class GreaterThanOrEqualFilter implements FilterInterface, Stringable +class GreaterThanOrEqualFilter implements FilterInterface, FilterAttributeInterface, Stringable { use AttributeValueAssertionTrait; diff --git a/src/FreeDSx/Ldap/Search/Filter/LessThanOrEqualFilter.php b/src/FreeDSx/Ldap/Search/Filter/LessThanOrEqualFilter.php index 0459fdd0..bafdafe2 100644 --- a/src/FreeDSx/Ldap/Search/Filter/LessThanOrEqualFilter.php +++ b/src/FreeDSx/Ldap/Search/Filter/LessThanOrEqualFilter.php @@ -20,7 +20,7 @@ * * @author Chad Sikorra */ -class LessThanOrEqualFilter implements FilterInterface, Stringable +class LessThanOrEqualFilter implements FilterInterface, FilterAttributeInterface, Stringable { use AttributeValueAssertionTrait; diff --git a/src/FreeDSx/Ldap/Search/Filter/MatchingRuleFilter.php b/src/FreeDSx/Ldap/Search/Filter/MatchingRuleFilter.php index 604e4a2e..b5d3e4df 100644 --- a/src/FreeDSx/Ldap/Search/Filter/MatchingRuleFilter.php +++ b/src/FreeDSx/Ldap/Search/Filter/MatchingRuleFilter.php @@ -38,7 +38,7 @@ * * @author Chad Sikorra */ -class MatchingRuleFilter implements FilterInterface, Stringable +class MatchingRuleFilter implements FilterInterface, FilterAttributeInterface, Stringable { protected const CHOICE_TAG = 9; diff --git a/src/FreeDSx/Ldap/Search/Filter/PresentFilter.php b/src/FreeDSx/Ldap/Search/Filter/PresentFilter.php index 75b778c3..cd503fc7 100644 --- a/src/FreeDSx/Ldap/Search/Filter/PresentFilter.php +++ b/src/FreeDSx/Ldap/Search/Filter/PresentFilter.php @@ -29,7 +29,7 @@ * * @author Chad Sikorra */ -class PresentFilter implements FilterInterface, Stringable +class PresentFilter implements FilterInterface, FilterAttributeInterface, Stringable { use FilterAttributeTrait; diff --git a/src/FreeDSx/Ldap/Search/Filter/SubstringFilter.php b/src/FreeDSx/Ldap/Search/Filter/SubstringFilter.php index 658dbe1b..ad265b83 100644 --- a/src/FreeDSx/Ldap/Search/Filter/SubstringFilter.php +++ b/src/FreeDSx/Ldap/Search/Filter/SubstringFilter.php @@ -40,7 +40,7 @@ * * @author Chad Sikorra */ -class SubstringFilter implements FilterInterface, Stringable +class SubstringFilter implements FilterInterface, FilterAttributeInterface, Stringable { use FilterAttributeTrait; diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/PdoStorage.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/PdoStorage.php index 67c9ad12..179fc099 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/PdoStorage.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/PdoStorage.php @@ -182,6 +182,7 @@ public function list(StorageListOptions $options): EntryStream $composed->params, ), $deadline, + $options->attributes, ), false, ); @@ -208,6 +209,7 @@ public function list(StorageListOptions $options): EntryStream $this->generateRows( $this->prepareAndExecute($query->sql, $query->params), $deadline, + $options->attributes, ), $isPreFiltered, ); @@ -397,16 +399,27 @@ private function probeLeafSelectivity(SidecarLeaf $leaf): int /** * @return Generator */ + /** + * @param list|null $attributes + */ private function generateRows( PooledStatement $stmt, ?float $deadline, + ?array $attributes = null, ): Generator { + $allowed = $attributes === null + ? null + : array_fill_keys($attributes, true); + while (($row = $stmt->fetch()) !== false) { if ($deadline !== null && microtime(true) >= $deadline) { throw new TimeLimitExceededException(); } - $entry = $this->rowToEntry($row); + $entry = $this->rowToEntry( + $row, + $allowed, + ); if ($entry !== null) { yield $entry; } @@ -515,8 +528,13 @@ private function encodeAttributes(Entry $entry): string return serialize($attributes); } - private function rowToEntry(mixed $row): ?Entry - { + /** + * @param array|null $allowed Base names to materialize, or null for all. + */ + private function rowToEntry( + mixed $row, + ?array $allowed = null, + ): ?Entry { if (!is_array($row)) { return null; } @@ -541,6 +559,10 @@ private function rowToEntry(mixed $row): ?Entry $attributes = []; foreach ($raw as $name => $values) { + if ($allowed !== null && !isset($allowed[strtolower((string) strtok($name, ';'))])) { + continue; + } + $attributes[] = Attribute::fromArray( $name, $values, diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/StorageListOptions.php b/src/FreeDSx/Ldap/Server/Backend/Storage/StorageListOptions.php index 496a137d..1627b70a 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/StorageListOptions.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/StorageListOptions.php @@ -28,6 +28,7 @@ final class StorageListOptions { /** * @param SortKey[] $sortKeys + * @param list|null $attributes Lowercase base attribute names to materialize, or null for all. * @param (\Closure(string): (bool|null))|null $isIntegerOrderedResolver Resolves whether an attribute orders numerically. */ public function __construct( @@ -38,6 +39,7 @@ public function __construct( public readonly int $sizeLimit = 0, public readonly array $sortKeys = [], public readonly int $lookthroughLimit = 0, + public readonly ?array $attributes = null, public readonly ?Closure $isIntegerOrderedResolver = null, ) {} diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/WritableStorageBackend.php b/src/FreeDSx/Ldap/Server/Backend/Storage/WritableStorageBackend.php index e712adf8..b5c9869b 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/WritableStorageBackend.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/WritableStorageBackend.php @@ -16,6 +16,7 @@ use FreeDSx\Ldap\Control\Control; use FreeDSx\Ldap\Control\ControlBag; use FreeDSx\Ldap\Control\Sorting\SortingControl; +use FreeDSx\Ldap\Entry\Attribute; use FreeDSx\Ldap\Entry\Change; use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Entry\Entry; @@ -34,6 +35,7 @@ use FreeDSx\Ldap\Server\Backend\Write\Command\MoveCommand; use FreeDSx\Ldap\Server\Backend\Write\Command\UpdateCommand; use FreeDSx\Ldap\Search\Filter\EqualityFilter; +use FreeDSx\Ldap\Search\Filter\FilterAttributes; use FreeDSx\Ldap\Server\Backend\Storage\Exception\DnTooLongException; use FreeDSx\Ldap\Server\Backend\Storage\Exception\InvalidAttributeException; use FreeDSx\Ldap\Server\Backend\Storage\Exception\StorageIoException; @@ -207,6 +209,7 @@ public function search( ? $sortingControl->getSortKeys() : [], lookthroughLimit: $limits->maxSearchLookthrough, + attributes: $this->materializedAttributes($request), isIntegerOrderedResolver: fn(string $attribute): ?bool => $this->schema?->isIntegerOrdered($attribute), ); @@ -498,6 +501,43 @@ public function move( }); } + /** + * Base attribute names storage must materialize (requested plus filter-referenced), or null to materialize all. + * + * @return list|null + */ + private function materializedAttributes(SearchRequest $request): ?array + { + $names = array_map( + static fn(Attribute $attribute): string => strtolower($attribute->getName()), + $request->getAttributes(), + ); + + if ($names === [] || in_array('*', $names, true) || in_array('+', $names, true)) { + return null; + } + + $filterAttributes = FilterAttributes::referenced($request->getFilter()); + + if ($filterAttributes === null) { + return null; + } + + $materialized = []; + foreach ($names as $name) { + if ($name === '1.1') { + continue; + } + + $materialized[$name] = true; + } + foreach ($filterAttributes as $attribute) { + $materialized[$attribute] = true; + } + + return array_keys($materialized); + } + /** * Confirm the search base exists. * diff --git a/tests/integration/Storage/LdapBackendStorageTest.php b/tests/integration/Storage/LdapBackendStorageTest.php index 196b2ff0..b093322f 100644 --- a/tests/integration/Storage/LdapBackendStorageTest.php +++ b/tests/integration/Storage/LdapBackendStorageTest.php @@ -222,6 +222,74 @@ public function testAddPreservesAttributeOptionsOnRoundTrip(): void ); } + public function testInexactFilterOnUnrequestedAttributeStillMatchesAndProjects(): void + { + $this->authenticateUser(); + + $this->ldapClient()->create(Entry::fromArray( + 'cn=hazard,dc=foo,dc=bar', + [ + 'cn' => 'hazard', + 'sn' => 'Smithers', + 'mail' => 'hazard@foo.bar', + 'objectClass' => 'inetOrgPerson', + ], + )); + + // Substring is inexact: SQL yields candidates and PHP re-evaluates (sn) on the hydrated entry, so storage must + // materialize sn (filter-referenced) even though only cn was requested; projection then drops it. + $entries = $this->ldapClient()->search( + Operations::search(Filters::contains('sn', 'mither')) + ->base('dc=foo,dc=bar') + ->useSubtreeScope() + ->select('cn'), + ); + + $entry = $entries->first(); + self::assertNotNull($entry); + self::assertSame( + 'cn=hazard,dc=foo,dc=bar', + $entry->getDn()->toString(), + ); + self::assertSame( + ['hazard'], + $entry->get(new Attribute('cn'), true)?->getValues(), + ); + self::assertNull($entry->get(new Attribute('sn'), true)); + } + + public function testNoAttributesRequestStillMatchesAnInexactFilter(): void + { + $this->authenticateUser(); + + $this->ldapClient()->create(Entry::fromArray( + 'cn=noattr,dc=foo,dc=bar', + [ + 'cn' => 'noattr', + 'sn' => 'Jones', + 'objectClass' => 'inetOrgPerson', + ], + )); + + $entries = $this->ldapClient()->search( + Operations::search(Filters::contains('sn', 'one')) + ->base('dc=foo,dc=bar') + ->useSubtreeScope() + ->select('1.1'), + ); + + $entry = $entries->first(); + self::assertNotNull($entry); + self::assertSame( + 'cn=noattr,dc=foo,dc=bar', + $entry->getDn()->toString(), + ); + self::assertCount( + 0, + $entry->getAttributes(), + ); + } + public function testAddDuplicateDnFails(): void { $this->ldapClient()->bind('cn=user,dc=foo,dc=bar', '12345'); diff --git a/tests/unit/Search/Filter/FilterAttributesTest.php b/tests/unit/Search/Filter/FilterAttributesTest.php new file mode 100644 index 00000000..ddbe2e8f --- /dev/null +++ b/tests/unit/Search/Filter/FilterAttributesTest.php @@ -0,0 +1,79 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Tests\Unit\FreeDSx\Ldap\Search\Filter; + +use FreeDSx\Ldap\Search\Filter\FilterAttributes; +use FreeDSx\Ldap\Search\Filter\MatchingRuleFilter; +use FreeDSx\Ldap\Search\Filters; +use PHPUnit\Framework\TestCase; + +final class FilterAttributesTest extends TestCase +{ + public function test_it_collects_a_single_leaf_attribute_lowercased(): void + { + self::assertSame( + ['cn'], + FilterAttributes::referenced(Filters::equal('CN', 'x')), + ); + } + + public function test_it_strips_attribute_options_to_the_base_name(): void + { + self::assertSame( + ['cn'], + FilterAttributes::referenced(Filters::equal('cn;lang-en', 'x')), + ); + } + + public function test_it_walks_and_or_not_and_dedupes(): void + { + $filter = Filters::and( + Filters::equal('cn', 'x'), + Filters::or( + Filters::startsWith('mail', 'y'), + Filters::present('objectClass'), + ), + Filters::not(Filters::gte('uidNumber', '5')), + Filters::equal('cn', 'z'), + ); + + self::assertSame( + ['cn', 'mail', 'objectclass', 'uidnumber'], + FilterAttributes::referenced($filter), + ); + } + + public function test_it_collects_the_extensible_match_attribute_when_present(): void + { + self::assertSame( + ['cn'], + FilterAttributes::referenced(new MatchingRuleFilter('caseIgnoreMatch', 'cn', 'x')), + ); + } + + public function test_an_extensible_match_without_an_attribute_is_indeterminate(): void + { + self::assertNull(FilterAttributes::referenced(new MatchingRuleFilter('caseIgnoreMatch', null, 'x'))); + } + + public function test_any_indeterminate_leaf_makes_the_whole_filter_indeterminate(): void + { + $filter = Filters::and( + Filters::equal('cn', 'x'), + new MatchingRuleFilter('caseIgnoreMatch', null, 'x'), + ); + + self::assertNull(FilterAttributes::referenced($filter)); + } +} diff --git a/tests/unit/Server/Backend/Storage/Adapter/PdoStorageTest.php b/tests/unit/Server/Backend/Storage/Adapter/PdoStorageTest.php index 37b6f512..c5ed3044 100644 --- a/tests/unit/Server/Backend/Storage/Adapter/PdoStorageTest.php +++ b/tests/unit/Server/Backend/Storage/Adapter/PdoStorageTest.php @@ -407,6 +407,65 @@ public function test_list_from_root_returns_all_entries(): void self::assertCount(2, $results); } + public function test_list_materializes_only_the_allowed_base_attributes(): void + { + $this->storage->store(new Entry( + new Dn('cn=narrow,dc=example,dc=com'), + new Attribute('cn', 'narrow'), + new Attribute('cn;lang-en', 'Narrow EN'), + new Attribute('sn', 'Surname'), + new Attribute('mail', 'narrow@example.com'), + )); + + $narrow = $this->firstByDn( + $this->storage->list(new StorageListOptions( + baseDn: new Dn(''), + subtree: true, + filter: new AndFilter(), + attributes: ['cn'], + ))->entries, + 'cn=narrow,dc=example,dc=com', + ); + + self::assertNotNull($narrow); + // Only the allowed base name is built; its option subtype rides along, sn and mail are skipped. + self::assertSame( + ['cn', 'cn;lang-en'], + array_map( + static fn(Attribute $attribute): string => $attribute->getDescription(), + $narrow->getAttributes(), + ), + ); + } + + public function test_list_with_null_attributes_materializes_every_attribute(): void + { + $this->storage->store(new Entry( + new Dn('cn=full,dc=example,dc=com'), + new Attribute('cn', 'full'), + new Attribute('sn', 'Surname'), + new Attribute('mail', 'full@example.com'), + )); + + $full = $this->firstByDn( + $this->storage->list(new StorageListOptions( + baseDn: new Dn(''), + subtree: true, + filter: new AndFilter(), + ))->entries, + 'cn=full,dc=example,dc=com', + ); + + self::assertNotNull($full); + self::assertSame( + ['cn', 'sn', 'mail'], + array_map( + static fn(Attribute $attribute): string => $attribute->getDescription(), + $full->getAttributes(), + ), + ); + } + public function test_interleaved_lists_do_not_share_cursor_state(): void { $this->subject->add( @@ -1127,6 +1186,22 @@ protected function makeJournalingStorage(): ChangeJournalingInterface return PdoStorageFactory::forPcntl(PdoConfig::forSqlite(':memory:')); } + /** + * @param iterable $entries + */ + private function firstByDn( + iterable $entries, + string $dn, + ): ?Entry { + foreach ($entries as $entry) { + if ($entry->getDn()->toString() === $dn) { + return $entry; + } + } + + return null; + } + /** * @return list */