Re: comments on draft-ietf-ldapext-ldap-java-api-11.txt

[Rob Weltman <robw@worldspot.com>]

  I think a few things overlap with the discussion earlier in September
on this list, but I've provided responses to all on an item-by-item
basis.

  Thanks for your detailed review!

Rob

hahnt@us.ibm.com wrote:
> 
> Greetings,
> 
> I finally got around to reading this draft.  It looks great so far.
>  I've summarized my comments here:
> 
> Section 4.3.5:
> --------------
> "cn;lang-ja-JP-kanji" may be a subtype of "cn;lang-ja" - based on
> recent discussions on this list, I don't think that this is true.  It
> seems to me that "cn;lang-ja-JP-kanji" is really treated as a subtype
> of "cn".
> 
> similarly, I think the example in this section need fixing as well:
> 
> getAttribute( "cn", "lang-en-us" ); does NOT return "cn;lang-en"
> 
> getAttribute( "sn", "lang-en" ); does NOT return "sn"
> 
> formating for description of "attrName" should be fixed.

  I discussed the motivation for the behavior defined in the draft
earlier in September.


 
> Section 4.3.8:
> --------------
> Are attribute descriptions and attribute subtypes accounted for in
> this?  For example, will a:
> 
> attrSet.remove("name");
> 
> call result in all "cn", "sn", "cn;lang-us" attributes being removed
> from the LDAPAttributeSet?

  Yes

 
> Section 4.5.1:
> --------------
> Are alternative attribute type names, attribute subtypes, and
> attribute descriptions accounted for in this comparison
> implemenatation?

  Sorting is alphabetical, using the supplied attribute names. No
attempt is made to canonicalize alternative names to "primary" names.
Attribute subtypes are correctly sorted as a side-effect of their syntax
(which lends itself well to alphabetical sorting).

  Attribute descriptions are not referenced or accounted for.


> Section 4.5.4:
> --------------
> If an entry does not contain an attribute, is it considered greater
> than or less than an entry that does contain the attribute?

  Less than

 
> Section 4.6.4:
> --------------
> If the authentication method does not have an associated distinguished
> name, does this method return null?  An exception?

  It returns null if there is no DN associated with the binding (or lack
of binding) of the connection.

 
> Section 4.6.17, 4.6.18:
> -----------------------
> Is a bind() done as well?  If so, what method is used?  Is this
> controlled by the LDAPSearchConstraints object that is passed in?

  As currently defined, binds are always anonymous (since LDAP URLs do
not allow specifying a password, and since the draft does not state
if/how the constraints object is used to provide credentials for
binding).

  That is of limited practical use, and I think it is a good suggestion
to specify in the draft that the LDAPRebind or LDAPBind object is used
to obtain credentials for the bind. If neither object is present, the
search operation takes place with an anonymous connection.

 
> Section 4.7.1:
> --------------
> Maybe this was covered in previous discussions on this list.  It is
> not clear from the draft, however, what the subtle differences are
> between a LDAPBind and LDAPRebind class and where these differences
> are used.

  In practice, LDAPRebind is a simple interface for simple binds. For
anything more complex, including SASL, LDAPBind should be used.


> Section 4.7.8:
> --------------
> A clarification: If the bind proc is null (the default), then no
> authentication is done on the initial connection.

  Yes, except it doesn't apply to the initial connection anyway, but to
the new connection which is produced to follow a referral.

 
> Section 4.11.1:
> ---------------
> "after appropriate normalization".  Doesn't this imply that the CLIENT
> have full knowledge of the "active" schema?  More clarification as to
> the extent of normalization that will be done is needed here.  Full DN
> handling is actually quite complex.  If that is what is intended here,
> then it needs to be made clear.  Further, this could require some form
> of communication with an LDAP server to get a set of "schema" with
> which to work with.

  The normalization that is done is:

escape processing
white space between the RDN separator (",") and the following RDN
case-insensitivity for RDNs with case-insensitive syntax

  Yes, determining which RDNs have case-insensitive syntax does require
access to a subset of the schema of a particular server. However,
LDAPDN.equals is a static method which does not take an LDAP connection
or a schema as argument. In the Mozilla implementation, there is a
built-in table of standard attributes which do not have case-insensitive
syntax (most standard attributes do have case-insensitive syntax),
culled from the inetOrgPerson and other documents; then there is a
static method to specify additional attributes as "not
case-insensitive".


> Section 4.11.4, 4.11.5:
> -----------------------
> Will the values returned be appropriately escaped or un-escaped based
> on the escaping rules in RFC 2253?

  Yes

 
> Section 4.12.4 (and many others, including all of 4.29, 4.38, 4.39,
> 4.40):
> --------------------------------------------------------------------------
> 
> Why is a "String dn" returned/passed in and not a LDAPDN when a
> distinguished name is to be used?

  Passing an LDAPDN instead of a String was considered at the time of
the first draft (late Spring 1997), but rejected on the basis of one of
the design goals: use standard Java classes and types wherever possible,
facilitate adoption by not requiring class- and type-mapping. It is
assumed that most applications do not know about nor want to know about
LDAP DNs.


> Section 4.16.5:
> ---------------
> Are the constants referred to here for result codes defined as static
> constants anywhere?  I did not see them defined as such anywhere in
> this draft.  This could possibly be done in LDAPException as:
> 
> class LDAPException {
> ...
> public static final int NO_SUCH_OBJECT = 1;
> ...
> }

  The values are not defined in the draft, so as to avoid redundancy.
They are defined in RFC 2252.

 
> Section 4.21:
> -------------
> Since this set of modifications is ordered, should it be called
> LDAPModificationList instead?

  It could be, but it's not worth changing at this point.

 
> Section 4.21.2:
> ---------------
> A clarification.  Since the list is ordered, add() should clarify that
> it adds at the END of the list.

  OK


> Section 4.21.4:
> ---------------
> This is similar to the comment above on Section 4.3.8, are subtypes,
> attribute descriptions, and alternate attribute type names accounted
> for here?

  Same answer as for 4.3.8 :-)

 
> Section 4.24 and 4.25:
> ----------------------
> The setup of LDAPRebindAuth and LDAPRebind seem only to support the
> notion of "simple" authentication.  How are other authentication
> methods to be setup and used when following referrals?

  That is correct. LDAPBind must be used for SASL or potentially other
authentication methods.

 
> Section 4.26.1:
> ---------------
> Clarification: the full response is received prior to throwing the
> referral exception, correct?

  I'm not sure why you're referring to 4.26.1 with the question; is the
question answered in 4.35.4?

 
> Section 4.29.11:
> ----------------
> "of attribute definitions" should be "of LDAPAttributeSchema
> definitions".

  or "of LDAPAttributeSchema objects"

 
> Section 4.29.12:
> ----------------
> "of attribute definitions" should be "of LDAPObjectClassSchema
> definitions".

  or "of LDAPObjectClassSchema objects"

 
> Section 4.29.13:
> ----------------
> "of attribute definitions" should be "of LDAPMatchingRuleSchema
> definitions".
 
  or "of LDAPMatchingRuleSchema objects"


> Section 4.29.20-25:
> -------------------
> clarification: the returned Enumeration is an Enumeration of Strings,
> correct?

  Yes

 
> Section 4.30.1-3:
> -----------------
> I would prefer that these methods NOT be defined in the abstract base
> class.  They don't make sense for some derived classes.  This was
> alluded to later in the document (Section 4.37).

  It would mean that they would need to be declared in each of the seven
other classes which extend the base class, or that another intermediate
base class would need to be inserted - both unattractive options. I
think it is OK that the methods return null where the derived element
does not have the corresponding property.

 
> Section 4.30.6:
> ---------------
> clarification: the returned Enumeration is an Enumeration of Strings,
> correct?

  Yes

 
> Section 4.30.10,11,12:
> ----------------------
> These should clarify what LDAP operations are being performed.  I
> think that 4.30.10 is an LDAP MODIFY where a new attribute value is
> added to the existing attributetypes, objectclasses, ldapsyntaxes, or
> matchingrules attribute in the subschemasubentry, 4.30.11 is a LDAP
> MODIFY operation to delete a specific attribute value, and 4.30.12 is
> an LDAP MODIFY with a "delete value" + "add value" in the
> ModificationSet/List.  This should be described in these sections.

  OK


> Section 4.30.12:
> ----------------
> Also in this section, the value from "this.getValue()" is used as the
> "attribute value to delete", correct?

  Yes

 
> Section 4.31.1:
> ---------------
> Why is doReferrals set to "false" by default?

  Safety.

 
> non-SIMPLE re-bind authentication seems to need more thought, in
> general.  Is anybody working on this?

  Yes:

http://www.ietf.org/internet-drafts/draft-weltman-java-sasl-03.txt

  and also:

http://java.sun.com/aboutJava/communityprocess/jsr/jsr_028_sasl.html

  LDAPBind and SASL or startTLS is the way to go.

 
> Is hop_limit set to zero by default?  Does zero imply no limit?

  The default value and the semantics of zero should be defined in the
draft. Zero should imply no limit, but the default should be a
reasonable value (to allow reasonable nesting levels of reference while
preventing infinite loops); I propose 5.

 
> Section 4.31.3:
> ---------------
> LDAP_DEREF_NEVER  should be LDAPConnection.DEREF_NEVER
> LDAP_DEREF_FINDING should be LDAPConnection.DEREF_FINDING
> LDAP_DEREF_SEARCHING should be LDAPConnection.DEREF_SEARCHING
> LDAP_DEREF_ALWAYS should be LDAPConnection.DEREF_ALWAYS

  OK
 

> Section 4.32.1:
> ---------------
> What is meant by "outstanding requests"?  Is this the set of responses
> that have not yet been completely received from the remote server?  Or
> the set of responses that have not been "received" by the application?
>  Or both?

  The latter: requests that are yet to be processed by the application
(regardless of whether they have completely arrived from the server at
the time or not).

 
> Section 4.32.3:
> ---------------
> Would it be useful to have a "isReponseReceived( int msgid )" method
> as well?

  That might be useful.


> How about an "isComplete( int msgid )"?

  I'm not sure that makes sense: if all results have been retrieved for
a particular message ID, there is no longer a queue maintained for it.
getMessageIDs() will not return the ID. It would be difficult for the
application to distinguish between the request being complete (all
results retrieved for that ID) on the one hand, and an invalid ID on the
other.

 
> Section 4.35.5:
> ---------------
> "This the" should be "This is the"

  OK

 
> "or an LDAPReferralException." should be "or an LDAPReferralException
> is thrown."

  No. nextElement() doesn't throw exceptions. the LDAPReferralException
is returned as an object.

 
> Should there be a way to get the referrals FIRST (i.e. not at the end
> as an exception), so that other searches can be started in the
> background?  This would allow multiple outstanding searches to
> multiple servers to run in parallel instead of serializing referrals
> chasing.

  The text about referrals being returned after all other results stems
back to the original draft, I think. Back then there were no LDAPv3
servers, and the only referrals being returned by LDAP servers were of
the UMich type: as result code 14 in the response of a message. They, of
course, came after all search results.

  When support for LDAPv3 referrals became available in servers, it was
also supported in revisions of the draft. However the text implies that
also references returned by a server as search results will be saved up
and returned after all other search results (i.e. so they would be
delivered to the application in the same way as UMich referrals).

  I think it's time to drop that text. Results are returned to the
application in the order they are received from the server, period.

  Consider the case where a search returned a reference followed by
10,000 regular search results. Should an implementation of the API
buffer up all 10,000 before returning the reference to the application?
That seems inefficient.

 
> Section 4.38.1:
> ---------------
> attrNames description, "null for all attributes".  Should this be
> "null or a single value of "*" for all attributes".

  Yes. The latter is redundant to specify in this draft, since it is an
LDAP spec. Allowing "null" is an extension provided by the API.

 
> Section 4.39.13:
> ----------------
> Description of LDAPConnection.REFERRALS_HOP_LIMIT.  Change "no more
> than 5 referrals in a row" to "no more than 5 nested referrals."

  OK

 
> Section 4.40.1:
> ---------------
> Will the client send off abandon requests for all outstanding (but
> incomplete) operations if a bind() is invoked?

  No. That doesn't seem justified, IMHO.


> 
> Thanks,
> Tim Hahn
> 
> Internet: hahnt@us.ibm.com
> Internal: Timothy Hahn/Endicott/IBM@IBMUS or IBMUSM00(HAHNT)
> phone: 607.752.6388     tie-line: 8/852.6388
> fax: 607.752.3681