Dne 21.11.2014 v 11:28 Tomas Babej napsal(a):

On 11/20/2014 04:01 PM, Jan Cholasta wrote:
Dne 19.11.2014 v 15:12 Tomas Babej napsal(a):

On 11/19/2014 02:03 PM, Jan Cholasta wrote:
Dne 19.11.2014 v 13:44 Tomas Babej napsal(a):

On 11/19/2014 12:51 PM, Martin Kosek wrote:
On 11/19/2014 12:41 PM, Tomas Babej wrote:
On 11/19/2014 12:24 PM, Martin Kosek wrote:
On 11/19/2014 12:03 PM, Tomas Babej wrote:

When constructing a parent DN in LDAPSearch, we should always
check that the parent object exists (hence use get_dn_if_exists),
rather than search on unexistant containers (which can happen
with get_dn).

Replaces get_dn calls with get_dn_if_exists in *-find commands
and makes sure proper error message is raised.

Doesn't it produce extra LDAP search thus making all our search
slower? Is that what we want?
No it does not make all of our LDAP search slower. It only
happens for
the objects that have parent objects, such as idoverrides or
... and makes them slower.

What I was pointing out here is that this is not a issue for ALL
commands (e.g user-find, group-find will not suffer from it), as you
incorrectly stated.

Wouldn't it be better to distinguish between LDAP
search with no results and LDAP search with missing parent DN? The
reply looks
different, at least in CLI:
Up to discussion. We would probably need to introduce a new
like ParentObjectNotFound.

# search result
search: 4
result: 0 Success

# search result
search: 4
result: 32 No such object
matchedDN: cn=accounts,dc=mkosek-f20,dc=test

Also, I do not think you can just stop using get_dn(), some
commands override
this call to get more complex searches (like host-find searching
for shortname).
Look into the get_dn_if_exists, it just wraps around get_dn, so no
here. Any custom behaviour is preserved.
Ah, ok, thanks for info.

To sum up, I think this is worth changing this behaviour by default,
ignoring a non-matching value of the parent object is not a correct
general approach in my opinion.
Well, that's the question. Whether we would leave DS to validate the
itself or do all the pre-check ourselves. To me, doing just one LDAP
search and
processing the error correctly looks better. But I can live even
with your
version then, I will leave the framework guardians like Honza or
Petr3 to decide.

+1 on single LDAP search and proper error processing.

I see now what you're trying to suggest. However, the reason boils
down to ipaldap.find_entries method not differentiating between a
LDAP search that returns error code 32 (No such object) and LDAP
search returning error code 0 (Success), but returning no results.

In both cases errors.NotFound is raised.

The reason I did not go this way is that changing the find_entries
is quite more invasive as this is the method subsenqently called by
any command.

You can always derive the new error (ParentNotFound or whatever) on
NotFound, so old code won't break.

Thanks for the suggestsions.

Attached is a new patch which hooks into find_entries method and
differentiates between the cases.

Why are you special casing base scope search?

Since base search is performed only on the entry specified by the DN,
there is no need to differentiate between ContainerNotFound and NotFound.

So the logic is as follows:

subtree search - ContainerNotFound is raised when container does not
exist, NotFound if search provided no results
onelevel search - the same
base search - NotFound is raised if the specified DN does not exist

There is a difference between a search on a non-existent entry and a search on an existent entry with a non-matching filter. This difference exists on LDAP level and applies to all search scopes, not just the base scope.

I don't think hiding this difference is useful at all. Remember that this bug exists because we were hiding the difference in the first place.

Also, after giving this some thought, I think it would be better to create a new error for the case where there is no match instead of the case where the entry does not exist. NotFound pretty much corresponds to LDAP's NO_SUCH_OBJECT, something like NoMatchingEntry or EmptyResult would fit the no-match result better.

Jan Cholasta

Freeipa-devel mailing list

Reply via email to