[Date Prev][Date Next]
[Chronological]
[Thread]
[Top]
Comments on draft-ietf-ldapext-ldap-java-api-15.txt
Please cc me in the discussion. I'm not on this mailing list.
_____________________
Rosanna Lee
Java Software, Sun Microsystems, Inc
rosanna.lee@sun.com
-----------------------------------------------------------------------
1. Section 1.2 The LDAP classes
The LDAPConnection class also provides fields for storing settings
that are specific to the LDAP session (such as limits on the number
of results returned or timeout limits).
In fact, the LDAPConnection class does not provide fields because none
has been declared. What the class does provide are get/set methods
for updating the state of the connection.
2. LDAPSearchResult and LDAPSearchResults
A natural conclusion of examining these names is that they should be
related: LDAPSearchResults is probably a collection of
"LDAPSearchResult"s. But these classes are not related at all. An
LDAPSearchResults is an enumeration of LDAPEntry while
LDAPSearchResult is a result from an async search. One of these
classes should be renamed to avoid confusion.
3. Section 1.4 Interfaces
LDAPEntryComparator An interface to support arbitrary sorting
algorithms for entries returned by a
search operation. The basic Java LDAP
classes include one implementation:
LDAPCompareAttrNames, to sort in
ascending order based on one or more
attribute names.
Not just ascending, but ascending or descending order.
4. Section 1.4 Interfaces
LDAPRebind A programmer desiring to supply
credentials to the default
reauthentication behavior when
automatically following referrals MUST
...
Supplying "credentials" to a "behavior" is very strange wording.
5. Section 1.4 Interfaces
LDAPBind This interface allows a programmer to
override the default authentication and
reauthentication behavior when
automatically following referrals and
search references. It is used to specify
the authentication mechanism used on
automatic referral following.
This description seems too restrictive. LDAPBind is used to specify
arbitrary behavior--behavior that might be unrelated to
authentication--when following referrals.
6. Section 1.4 Interfaces
LDAPUnsolicitedNotificationListener Allows a client to be notified
when unsolicited messages arrive from a
server. Unsolicited messages have a
message ID of 0. An implementation of the
Java LDAP API SHOULD NOT generate
messages with an ID of 0.
Is the Java LDAP API even allowed to generate messages? Isn't message
generation the responsibility of the server?
7. Section 1.5 Classes
LDAPConstraints Defines options controlling all
operations.
Do you mean "Defines options controlling all operations on the
Directory"? Or perhaps "... on LDAPConnection"?
8. Section 1.5 Classes
LDAPExtendedResponse The response returned by an LDAP server
on an asynchronous extended operation
request. It extends LDAPResponse.
Not async.
9. Section 1.5 Classes
LDAPDITContentRuleSchema Represents a specific DIT content rule in
the directory schema.
LDAPDITStructureRuleSchema Represents a specific DIT structure rule
in the directory schema.
LDAPMatchingRuleUseSchema Represents a specific matching rule use
in the directory schema.
LDAPNameFormSchema Represents a specific name form in the
directory schema.
LDAPSyntaxSchema Represents a specific syntax definition
in the directory schema.
Delete 'specific'.
10. Section 1.5 Classes
LDAPAttributeSchema Represents a schematic definition of a
particular attribute in a particular
Directory Server.
LDAPMatchingRuleSchema Represents the schematic definition of a
particular matching rule in a particular
Directory Server.
LDAPObjectClassSchema Represents the schematic definition of a
particular object class in a particular
Directory Server.
LDAPSchema Represents the schema controlling one or
more entries held by a particular
Directory Server.
Delete 'particular'. Why 'in a particular Directory Server'? In fact,
you can create one of these and it need not be associated with any
Directory Server until you decide to add it to one.
11. Section 1.5 Classes
LDAPMessage Base class for LDAP request and response
messages. Subclassed by response messages
used in asynchronous operations.
LDAPResponse Represents a message received from an
LDAP server in response to an
asynchronous request. It extends
LDAPMessage.
async is incorrect. Also used for non-async extendedOperation.
12. Section 1.5 Classes
LDAPResponseListener Low-level mechanism for processing
asynchronous messages received from a
server.
LDAPSearchListener Low-level mechanism for queuing
asynchronous search results received from
a server.
'Low-level'? Compared to what? Is there a high-level mechanism for
doing the same thing? If not, delete 'low-level'.
13. Section 1.5 Classes
LDAPSchemaElement Base class for representing LDAP schema
elements.
Base class for representing an LDAP schema element. (singular).
14. Section 1.5 Classes
LDAPUrl Encapsulates parameters of an LDAP Url
query, as defined in [LDAPURL].
Strange wording. Why isn't this simply "Represents an LDAP URL
[LDAPURL]"?
15. Section 1.7 LDAP API use
For the asynchronous methods, exceptions are raised only for
connection errors.
Are local errors in the "connection errors" category?
16. Section 1.7 LDAP API use
To facilitate user feedback during synchronous searches, intermediate
search results can be obtained before the entire search operation is
completed by specifying the number of entries to return at a time.
Standard Java Enumerations are used to parse synchronous search
results.
parse -> obtain
Users of the enumeration don't parse anything, although the underlying
Enumeration implementation might.
17. Section 3.6 Array Return Values
Section 3.6 belongs here in section 1.7, where similar things are
talked about, instead of being in a section named 'Implementation
Considerations'.
18. Section 2.2.1 Constructors
public LDAPAttributeSchema(String name,
String oid,
String description,
String syntaxString,
boolean single
String superior,
String[] aliases,
boolean obsolete,
String equality,
String ordering,
String substring,
boolean collective,
boolean userMod,
int usage)
Which ones of these arguments are optional? The descriptions seem to
say that the following are optional:
description
superior
aliases
equality
ordering
substring
RFC 2252 says that the following are optional:
name/aliases
description
obsolete
superior
equality
ordering
substring
usage
syntaxString
userMod
collective
single
Need to clarify in the descriptions which are optional. If conforming
to RFC 2252, then need to specify whether the corresponding methods
(such as getSyntaxString()) can return null.
19 Section 2.3 public class LDAPAttributeSet
implements Cloneable
If LDAPAttributeSet is really a set, then it should implement
java.util.Set so that it benefits from the utilties offered by the
Java Collections Framework.
20.. Section 2.3.5 getAttribute
Returns a single best-match attribute, or none if no match is
available in the entry.
entry -> set
21. Section 2.3.2 add
public void add(LDAPAttribute attr)
Adds the specified attribute to this attribute set.
What happens if attr with same name already exists? Overwritten?
Ignored? Runtime exception? Need to specify.
22. Section 2.3.4 elementAt
public LDAPAttribute elementAt(int index)
throws ArrayIndexOutOfBoundsException
Returns the attribute at the position specified by the index. The
index is 0-based.
and Section 2.3.9 removeElementAt
public void removeElementAt(int index)
throws ArrayIndexOutOfBoundsException
Removes the attribute at the position specified by the index.
Are the contents of LDAPAttributeSet ordered? A set, by definition, is
unordered. How is the order affected by addition of a new attribute?
Added to the beginning? end? random? If LDAPAttributeSet is ordered,
then the class should be renamed and the behavior of these methods
specified.
23. Section 2.3.5 getAttribute (subtyping)
getAttribute( "cn", "lang-en-us" ) returns the "cn;lang-en"
attribute.
getAttribute( "sn", "lang-en" ) returns the "sn" attribute.
getAttribute( "cn", "lang-ja" ) returns null.
These seem to violate subtyping rules and RFC 2596. A lang-en is not a
lang-en-us. A lang-ja-JP-kanji is a lang-ja. An 'sn' is not an
'sn;lang-en'.
24. Section 2.3.5 getAttribute
... If there are
no matching attributes, null is returned. If the results are
ambiguous, the exception UnsupportedOperationException is thrown.
UnsupportedOperationException seems to be the wrong exception to throw
here. RuntimeExceptions are not intended to be used in this way. The
nature of the failure seem to indicate the need for a checked
exception and that caller of getAttribute() should expect the
exception if the parameter specification is ambiguous
25. Section 2.4.1 bind
... An implementation may
access the host, port, credentials, and other information in the
original LDAPConnection object ...
Contrary to this statement, there does not appear to be a way for the
implementation to access the credentials of the original
LDAPConnection.
26. Section 2.5 public class LDAPCompareAttrNames
Behavior unspecified as to how entries without the specified attribute
name/names are treated for ascending and descending orders.
27. LDAPCompareAttrNames is a LDAPEntryComparator. Yet the name sounds
like a method instead of a comparator. Better choices might be
LDAPAttrNamesEntryComparator or LDAPAttrNamesComparator
28. Section 2.5.1 Constructors
ascendingFlags Array of flags, one for each attrName, where each
one is true to sort in ascending order, false for
descending order. An LDAPException is thrown if
the length of ascendingFlags is not greater than
or equal to the length of attrNames.
Why is greater than acceptable? Why not just require equality?
Otherwise, you need to add text to say that the extras are ignored.
29. Section 2.6.2 abandon
The application is responsible for not calling this method more than
once with a particular id or LDAPSearchResults.
What is the behavior otherwise? Undefined? What is the behavior if
abandon is called on a nonexistent message id? LDAPException?
30. Section 2.6.3 add
The application is responsible for not specifying attribute values
which are not valid according to the syntax defined for the
attributes. It is also responsible for including all attributes which
are required for the entry.
What is the behavior otherwise? LDAPException? Which error codes? Why
single out attribute values in particular for conformance? Why not
list requirements on dn, etc. Need to clarify or delete.
31. Section 2.6.5 bind (passwd security)
passwd is explicitly typed to be String. Using String for the password
contradicts Section 4, which says:
Implementations of this API SHOULD be cautious when handling
authentication credentials. In particular, keeping long-lived copies
of credentials without the application's knowledge is discouraged.
The common practice in the JDK is to use char[] for passwords.
32. Section 2.6.5 bind (version)
public void bind(int version,
String dn,
String passwd)
throws LDAPException
and related overloads with 'version' parameter.
version LDAP protocol version requested: currently 2 or
3.
What if the user specifies 2 and the server accepts? Then the session
proceeds in v2? Then how does that reconcile with the earlier
statement in Section 1.1 The LDAP model.
Before the client encodes and sends a string value to a server, the
string values are converted from the Java 16-bit Unicode format to
UTF-8 format, which many LDAPv3 protocol elements and value
encodings use. The integrity of double-byte and other non-ASCII
v2 doesn't use UTF-8.
33. Section 2.6.5 bind (more comments on version)
... This API is specifically designed
for use with LDAPv3. Unless the API provides specific support (as
defined in other documents) for other versions of LDAP, version 3
should be used.
How does this reconcile with the version argument description, which
seems to imply that both 2 & 3 are supported.
version LDAP protocol version requested: currently 2 or
3.
34. Section 2.6.5 bind (dn/passwd parameters)
dn If non-null and non-empty, specifies that the
connection and all operations through it SHOULD
be authenticated with dn as the distinguished
name.
passwd If non-null and non-empty, specifies that the
connection and all operations through it SHOULD
be authenticated with dn as the distinguished
name and passwd as password.
Doesn't say what happens if dn is null or empty, or if passwd is null
or empty, or if dn is non-empty and passwd is empty. Need to specify.
Also, why is it only SHOULD? Is the implementation free to ignore dn
and passwd and authenticate using any dn/passwd it chooses?
Doesn't say how to do anonymous bind.
Should explicitly say that non-empty dn and non-empty passwd will be
used to perform simple bind. "simple bind" is mentioned elsewhere in
the class (e.g., isBound()) but not in this section at all.
35. Section 2.6.5 bind (authzId variants)
public void bind(String dn,
String authzId,
Hashtable props,
javax.security.auth.callback.CallbackHandler cbh)
throws LDAPException
public void bind(String dn,
String authzId,
Hashtable props,
javax.security.auth.callback.CallbackHandler cbh,
LDAPConstraints cons)
throws LDAPException
These appear to be missing the String[] mechanisms parameter, and if
added, aren't these equivalent to the two other definitions that
follow them?
36. Section 2.6.5 bind (authzId variants)
Should use java.util.Map instead of java.util.Hashtable because
it allows the client to use more efficient implementations.
Similar change to 2.6.24 getSaslBindProperties
public Hashtable getSaslBindProperties()
37. Section 2.6.5 bind (SASL bind wording)
A SASL bind call() may involved
multiple protocol requests. An attempt to invoke an operation other
than bind between bind requests in a multi-stage bind results in an
LDAPException with the result code SASL_BIND_IN_PROGRESS.
How can someone do this? Isn't the SASL bind method synchronous? It
returns only when SASL authentication terminates either successfully
or unsuccessfully.
call() -> call
38. Section 2.6.5 bind (authzId variants - parameters)
authzId If not null and not empty, an LDAP authzID [AUTH]
to be passed to the SASL layer.
What if it is null or empty? Should say that if null or empty, that
the authzId will be treated as an empty string and processed as per
RFC 2222. (Or, instead of referencing 2222, say what RFC 2222 does
with it).
Also, is the dn allowed to be null and/or empty for SASL bind? Need
to specify.
cbh A class which may be called by the Mechanism
Driver to obtain additional information required,
such as additional credentials.
"Mechanism Driver" not defined in this document.
39. Section 2.6.8 connect (default port)
port Contains the TCP port number to connect to or
contact. The default LDAP port is 389. "port" is
ignored for any host identifier which includes a
colon and port number.
How do you specify the default port? Wording from 2.41.1 seems more
appropriate.
port Port number for LDAP server (use
LDAPConnection.DEFAULT_PORT for default port).
40. Section 2.6.8 connect (host/port specification)
host Each host
identifier may include a trailing colon and port
number.
and
port ... "port" is
ignored for any host identifier which includes a
colon and port number.
This is an odd feature. It is also inconsistent with the Java
platform's networking library. Since both host and port are already
separate arguments, it seems cleaner to leave them separate.
Same comment applies to Section 2.41.1.
41. Section 2.6.8 connect (IPv6)
host Contains a host identifier consisting of a
hostname, an IPv4 dotted string, or an IPv6
reference [IPv6] representing the IP address of
host running an LDAP server to connect to.
"[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]:4389"
The format for literal IPv6 addresses is described in RFC 2373. RFC
2732 describes the format for literal IPv6 addresses in URLs. 'host'
here is being used in the context of a host address, not in the
context of a URL. Therefore, the reference should be to RFC 2373 and
the example shown should be without the '[' and ']' (and port, See 40).
42. Section 2.6.9 delete
dn Distinguished name of the entry to modify.
Should be 'the entry to delete'.
43. Section 2.6.10 disconnect
Disconnects from the LDAP server. The API implementation sends an
Unbind request to the server with any controls specified by the
LDAPConstraints object before closing the connection. Before the
object can perform LDAP operations again, it MUST reconnect to the
server by calling either connect or bind (bind will attempt to
reconnect).
This seems to contradict Section 2.6.6 clone :
The network connection remains
open until all clones have disconnected or gone out of scope. Any
connection opened after cloning is private to the object making the
connection.
Need to clarify one or the other.
44. Section 2.6.12 finalize
First, defining finalizers in general degrades performance of the JVM
severely. Second, the finalize() method is reserved for finalization
and should not be used as a common method name. Finally, finalize()
should never be declared public. To do this in a public API forces
poor implementations. If a method is needed for cleaning up the
connection that is different from disconnect, call it something else.
45. Section 2.6.14 getAuthenticationMethod
"none" The current authentication state has not been
established by use of the bind operation. This
is the initial state upon connect().
Perhaps useful to add that the method can be in "none" if the last
bind failed or if last successful bind was anonymous.
46. Section 2.6.16 getHost
Returns the host name of the LDAP server to which the object is or
was last connected, in the format originally specified.
and Section 2.6.19 getPort
public int getPort()
Returns the port number of the LDAP server to which the object is or
was last connected.
What if last connection attempt was unsuccessful? What if no
connection attempt was ever made? For host, do you get null or empty
string? For port, what do you get?
47. Section 2.6.17 getInputStream and Section 2.6.18 getOutputStream
public InputStream getInputStream()
Returns the stream used by the connection object for receiving data
from the LDAP server.
What if connect() was not called or the connection has been
disconnected? Do you get null? Exception?
48. Section 2.6.20 getProperty
LDAP_PROPERTY_SDK ("version.sdk") The version of
this SDK, as a
Float data type.
A Float seems like an odd type to use for versioning.
LDAP_PROPERTY_PROTOCOL ("version.protocol") The
highest supported
version of the LDAP
protocol, as a
Float data type.
Same comment as above.
LDAP_PROPERTY_SECURITY ("version.security") A
comma-separated
list of the types
of authentication
supported, as a
String.
What is meant by authentication types? SASL mechanism names? Or things
like "none", "simple" or "sasl"? Need to clarify.
Are the properties available related at all to those added via
setProperty()?
Why do these property names start with "version."?
What is the naming convention for the properties and who controls this
namespace?
49. Section 2.6.20 getProperty
LDAP_PROPERTY_SDK ("version.sdk") The version of
this SDK, as a
Float data type.
Why not use the versioning facilities in java.lang.Package?
50. Section 2.6.21 getProtocolVersion
public int getProtocolVersion ()
Returns the protocol version that the connection is bound to (2 or
3).
How come this returns an int but LDAP_PROPERTY_PROTOCOL returns a
Float?
What is returned if connection is not bound?
51. Section 2.6.27 isBound
public boolean isBound()
Indicates whether the object has authenticated to the connected LDAP
server (other than anonymously with simple bind). It returns false
initially, false upon a bind request, and true after successful
completion of the last outstanding non-anonymous simple bind.
What about last successful SASL bind? A better name for this method
might be isAuthenticated(), as indicated by the method description.
52. Section 2.6.30 modify
The application is responsible for specifying attribute values which
are valid according to the syntax defined for the attributes.
Otherwise? Why single this out? What about validity of other
arguments?
53. setSearchConstraints still shows up in the following 2 sections.
Section 2.6.6 clone
Section 2.33 public class LDAPSearchConstraints
extends LDAPConstraints
A set of options to control a search operation. There is always an
LDAPSearchConstraints associated with an LDAPConnection object; its
values can be changed with LDAPConnection.setSearchConstraints, or
overridden by passing an LDAPConstraints object to the search
operation.
54. Section 2.6.15 getConstraints
public LDAPConstraints getConstraints()
Returns a copy of the set of constraints associated with this
connection. These constraints apply to all operations performed
through this connection (unless a different set of constraints is
specified when calling an operation method).
What if none was set? Do you get null or a copy of a default
LDAPConstraints?
Same comment for Section 2.6.25 getSearchConstraints.
55. Section 2.6.35 setConstraints
public void setConstraints(LDAPConstraints cons)
Sets the constraints that apply to all operations performed through
this connection (unless a different set of constraints is specified
when calling an operation method). An LDAPSearchConstraints object
which is passed to this method will override all constraints, while
an LDAPConstraints object will only affect the base constraints.
Can I pass in null to clear any previous set constraints? Or must cons
be nonnull?
56. Section 2.6.11 extendedOperation
op Object which contains an object identifier of the
extended operation, which SHOULD be one
recognized by the particular server this client
is connected to, and operation-specific sequence
of Octet String or BER-encoded value(s).
At the API level, there should be no requirement that the operation is
recognized by the server. Even RFC 2251 doesn't go that far.
57. Section 2.6.33 rename
newParentdn -> newParentDn for consistent use of capitalization.
Renames an existing entry or subtree in the directory, possibly
repositioning it in the directory tree.
Parameters are:
dn Current distinguished name of the entry.
Method description doesn't say what happens when newParentDn is
omitted. Need to specify.
58. Section 2.6.36 setInputStream
public void setInputStream(InputStream stream)
Sets the stream used by the connection object for receiving data from
the LDAP server.
and Section 2.6.37 setOutputStream
public void setOutputStream(OutputStream stream)
Sets the stream used by the connection object to send data to the
LDAP server.
First, these are low-level service-provider methods that should not be
mixed up with user-level methods in the same class. These are
dangerous methods that should not be used lightly. Second, you need
to describe the purpose of these methods other than what it does.
There is a brief mention of its use in the change log, but that's not
part of the main spec. Third, you need to describe the implications of
using these methods. What happens to the async listeners when you
invoke either of these?
Similarly, getOutputStream() and getInputStream() also seem like they
don't belong in a user-level API.
59. Section 2.6.38 setProperty
public void setProperty(String name, Object value)
throws LDAPException
Sets a property of a connection object.
No property names have been defined at this time, but the mechanism
is in place in order to support revisional as well as dynamic
extensions to operation modifiers.
It seems to be jumping the gun to define an API for which there is
currently no use. Also, need to explain how are properties are
related to "client controls"? When should you use which to control
client-side behavior?
60. Section 2.6.40 setSocketFactory
Can this be called more than once? Do you need appropriate permissions to
do this?
61. Section 2.6.41 startTLS
There is no support for the client to stop TLS and downgrade to a
plain unauthenticated LDAP connection. A major advantage of the Start
TLS extension (instead of just LDAP over SSL) is the client's ability
to multiplex encrypted and clear requests on the same
connection. Despite the lack of current server support, the API should
not be deficient.
62. Section 2.7.9 setReferralFollowing
... Referrals of any type
other than to an LDAP server (i.e. a referral URL other than
ldap://something) are ignored on automatic referral following. The
default is false.
Is this true also if you setReferralHandler(LDAPBind)? When you use
LDAPBind, aren't you given all of the URLs regardless of their scheme?
63. Section 2.7.2 getClientControls
public LDAPControl[] getClientControls()
Returns controls to be used by the interface.
and Section 2.7.7 setClientControls
Sets controls for use by the interface.
Not clear what 'used by the interface' means. An interface is a
declaration. Perhaps you mean "the implementation of the Java LDAP
API". The whole concept of "client controls" is confusing. See 64.
64. Section 3.1 Client and Server Controls
LDAPv3 operations can be extended through the use of controls.
Controls MAY be sent to a server or returned to the client with any
LDAP message. These controls are referred to as server controls. The
LDAP API also supports a client-side extension mechanism through the
use of client controls (these controls affect the behavior of the
LDAP API only and are never sent to a server). A common class is used
to represent both types of controls - LDAPControl.
The API is inventing new terms and concepts that don't mesh well with
those already defined in RFC 2251. "Control" has a very specific
meaning in RFC 2251. It maps to server controls as described in this
document. The API invents "client controls", using the same structure
to modify behavior of the client library. There are several problems
with this. First, it has a very clumsy structure (Section 2.8.1)
public LDAPControl(String id,
boolean critical,
byte[] value)
This structure is appropriate for server controls because it fits well
within the RFC 2251 framework. For client controls, why force control
values to fit within a byte[]? If byte[] won't' be used for client
controls, then why use LDAPControl? Is there a common encoding for
byte[] value that makes it useful for client controls?
Second, what is the relationship between a property and a client control?
Section 2.6.38 setProperty says:
No property names have been defined at this time, but the mechanism
is in place in order to support revisional as well as dynamic
extensions to operation modifiers.
It seems the properties could be used as operation modifiers. Is that
what client controls are for too? When should you use which?
Third, are there examples of client controls? Are they standard?
65. Section 2.7.7 setClientControls
public void setClientControls( LDAPControl control )
public void setClientControls( LDAPControl[] controls )
Sets controls for use by the interface.
What if the client control is unsupported? Does this method throw an
exception? Or does LDAPConnection throw an exception somewhere down
the line when it tries to use the client controls?
Why is the first method name plural when the argument is singular?
66. Section 2.7.11 setServerControls
public void setServerControls( LDAPControl control )
Why is the method name plural when the argument is singular?
67. LDAPConstraints.getReferralHandler() was removed.
How can the LDAPConnection implementation get the LDAPReferralHandler
from the LDAPConstraints/LDAPSearchContraints argument to handle
referrals?
68. Section 2.7.10 setReferralHandler
public void setReferralHandler (LDAPReferralHandler binder)
Specifies the object that will process authentication requests. The
default is null.
and Section 2.7.1 Constructors
public LDAPConstraints(int msLimit,
boolean doReferrals,
LDAPReferralHandler binder,
int hop_limit)
Poor choice of name for LDAPReferralHandler interface leads to
confusion about exactly what it is. 'binder' is a handler?
69. Poor descriptions of LDAPReferralHandler.
Section 1.4:
LDAPReferralHandler Interface that is a shared ancestor to
LDAPBind and LDAPRebind.
And in 2.7.10 setReferralHandler
public void setReferralHandler (LDAPReferralHandler binder)
Specifies the object that will process authentication requests. The
default is null.
Parameters are:
binder An object that implements LDAPBind or LDAPRebind.
These descriptions need to be made more useful instead of just
describing the class hierarchy. What is its exact purpose? To
customize referral handling? To customize authentication done during
referral handling? Describing its intended use would be more useful.
If its purpose is to process authentication requests, why is it called
LDAPReferralHandler? Probably both the interface name and the
descriptions need work.
70. LDAPBind and LDAPRebind, LDAPRebindAuth
Section 2.4 public interface LDAPBind
Section 2.25 public interface LDAPRebind
Section 2.26 public class LDAPRebindAuth
These interface and class names give a poor indication of their
function.
LDAPRebindAuth: Why not use java.net.PasswordAuthentication? The
PasswordAuthentication class is a {String, char[]} pair. Password is
best represented as a char[] so that you can zero it out, instead of
being stored in an immutable entity like a String. Using String for
the password contradicts Section 4, which says:
Implementations of this API SHOULD be cautious when handling
authentication credentials. In particular, keeping long-lived copies
of credentials without the application's knowledge is discouraged.
The name LDAPRebind is not at all indicative of the purpose of this
interface. How about:
LDAPRebind -> LdapAuthenticator
Similarly, LDAPBind is a poor choice of name. Seems like it is related
to LDAPRebind except performing the bind the first time. But that's
not what the interface is for. How about:
LDAPBind -> LdapConnectionReferralHandler
71. Section 2.8.1 Constructors
critical If true, the LDAP operation will fail with
UNAVAILABLE_CRITICAL_EXTENSION if the server does
not support this Control.
value Control-specific data.
The description of the 'critical' parameter cannot be used if you want
LDAPControl to represent both client and server controls.
What is the format of value? Same comment as 73.
72. Section 2.8.6 register
public static void register(String oid, Class controlClass)
Do you need appropriate permissions to do this? Explicit use of Class
means that you're forcing the caller to deal with class loading
issues.
controlClass A class which can instantiate an LDAPControl.
A factory can also instantiate an LDAPControl, but you had a stricter
requirement earlier:
The controlClass MUST be an extension of LDAPControl.
Need to clarify argument description.
73. Section 2.8.4 getValue
public byte[] getValue()
Returns the control-specific data of the object.
and Section 2.8.7 setValue
protected void setValue(byte[] value)
Sets the control-specific data of the object. This method is for use
by extensions of LDAPControl.
What is the format of value? Is it in ASN.1 BER encoding, ready to
be sent as is to the server? Or is it an octet string that the Java
LDAP API implementation must encode using ASN.1 BER, by adding in the
tag and length.
This needs to be specified because it is the contract between the
developer of the control and the developer of the LDAP client library,
which might be different entities.
74. Section 3.1 Client and Server Controls
Support for specific controls is defined in a package "controls"
subordinate to the main LDAP package.
Support for controls may be defined in any package (user-defined,
vendor-supplied, etc), not just those in the "controls"
subpackage. Perhaps standard controls might be defined in a "controls"
subpackage but why even mention it here, especially since the other
package hasn't been standardized?
75. Section 2.1.2 addValue (both forms): How are duplicates handled?
76. Section 2.1.4 getByteValues
public Enumeration getByteValues()
Returns an enumerator for the values of the attribute in byte[]
format.
and Section 2.1.5 getByteValueArray
public byte[][] getByteValueArray()
Returns the values of the attribute as an array of byte[].
What if the values are of type String? What encoding is used for the
conversion? UTF-8 won't work for v2.
77. Section 2.1.13 removeValue
public void removeValue(String attrString)
Removes a string value from the attribute.
What if attribute values are byte[]? How is equality of attribute
values determined? What if the value does not exist? Need to specify.
public void removeValue(byte[] attrBytes)
Removes a byte[]-formatted value from the attribute.
What if attribute value is a String? How is equality of attribute
values determined? What if the value does not exist? Need to specify.
78. Section 2.1.8 getStringValueArray
public String[] getStringValueArray()
Returns the values of the attribute as an array of Strings. This
method should only be called if the attribute values are known to be
strings with values restricted to UTF-8. The returned Strings have
undefined values if the attribute contains values not restricted to
UTF-8.
and Section 2.1.9 getStringValues
public Enumeration getStringValues()
Returns an enumerator for the string values of an attribute. This
method should only be called if the attribute values are known to be
strings with values restricted to UTF-8. The returned Strings have
undefined values if the attribute contains values not restricted to
UTF-8.
How do I know whether a string has an undefined value? Do you mean
the String is null or does it contain funny characters (by inspection)?
79. Section 2.1 public class LDAPAttribute
This class allows you to get the attr values back as either String or
byte[] but there appears to be no way of finding out what was the
intended presentation format (String or binary). Suppose I'm writing a
generic browser and reads some attributes from the directory. What is
the recommended approach to display as much of the attribute values as
strings as possible and indicate that others are binary?
80. None of the classes are Serializable. Some data types, such as
LdapAttribute, LdapAttributeSet, and the schema classes seem like they
would benefit from being Serializable.
81. Why isn't LDAPAttribute Cloneable? It has a constructor that
accepts an LDAPAttribute. Why do that instead of making it cloneable?
Why use a different pattern than LDAPAttributeSet?
82. Incorrect/unresolved citations
Section 2.6.41 startTLS
Begin using the Transport Layer Security (TLS) protocol for session
privacy [10].
Section 2.29.4 getResultCode
Returns the result code in a server response, as defined in [5].
Section 2.11.2 escapeRDN
Returns the RDN after escaping the characters requiring escaping [4].
83. 'raw' descriptions
Section 2.2.1 Constructors
public LDAPAttributeSchema(String raw)
Constructs an attribute definition from the raw String value returned
on a Directory query for "attributetypes".
You probably mean a single attribute value from the "attributetypes"
attribute. But the string value might not have been gotten from a
directory query. Better to just say that 'raw' is a string encoded
using the AttributeTypeDescription syntax [ATTR].
Similar comments apply to the following constructors:
Section 2.9.1 Constructors
public LDAPDITContentRuleSchema(String raw)
Constructs a DIT content rule from the raw String value returned on a
schema query for "dITContentRules".
Section 2.10.1 Constructors
public LDAPDITStructureRuleSchema(String raw)
Constructs a DIT structure rule from the raw String value returned on
a schema query for "dITStructureRules".
Section 2.18.1 Constructors
public LDAPMatchingRuleSchema(String rawMatchingRule,
String rawMatchingRuleUse)
Constructs a matching rule definition from the raw String values
returned on a Directory query for "matchingRule" and for
"matchingRuleUse" for the same rule.
Section 2.19.1 Constructors
public LDAPMatchingRuleUseSchema(String raw)
Constructs a matching rule use definition from the raw String value
returned on a schema query for "matchingRuleUse".
Section 2.23.1 Constructors
public LDAPNameFormSchema(String raw)
Constructs a DIT content rule from the raw String value returned on a
schema query for "nameForms".
Section 2.24.1 Constructors
public LDAPObjectClassSchema(String raw)
Constructs an object class definition from the raw String value
returned on a Directory query for "objectclasses".
Section 2.39.1 Constructors
public LDAPSyntaxSchema(String raw)
Constructs a syntax from the raw String value returned on a schema
query for "LDAPSyntaxes".
84. 2.9.1 Constructors
public LDAPDITContentRuleSchema(String name,
String oid,
String description,
boolean obsolete,
String[] auxiliary,
String[] required,
String[] optional,
String[] precluded,
String[] aliases)
Which ones of these arguments are optional? The descriptions seem to
say that the following are optional:
description
aliases
RFC 2252 says that the following are optional:
name/aliases
description,
obsolete,
auxiliary,
required,
optional,
precluded,
Need to clarify in the descriptions which are optional. If conforming
to RFC 2252, then need to specify whether the corresponding methods
(such as getName()) can return null.
85. Section 2.10.1 Constructors
public LDAPDITStructureRuleSchema(String name,
int ruleID,
String description,
boolean obsolete,
String nameForm,
String[] superiorIDs,
String[] aliases)
Which ones of these arguments are optional? The descriptions seem to
say that the following are optional:
description
superiorIDs
aliases
RFC 2252 says that the following are optional:
name/aliases
description
superiorIDs
obsolete
Need to clarify in the descriptions which are optional. If conforming
to RFC 2252, then need to specify whether the corresponding methods
(such as getName()) can return null.
86. Section 2.10.2 getNameForm
public String getNameForm()
Returns the NameForm that this structure rule controls. You can get
the actual object class that this structure rule controls by calling
getNameForm().getObjectClass().
The method returns a String, not the NameForm. The last sentence needs
correction, such as
ldapSchema.getNameForm(ditStructRule.getNameForm()).getObjectClass().
87. Section 2.11 public class LDAPDN
A utility class representing a distinguished name (DN).
First, poor choice of capitalization. Second, this class does not
represent a distinguished name. It is a utility class for manipulating
distinguished names.
88. Section 2.12.2 getAttribute
public LDAPAttribute getAttribute(String attrName)
Section 2.12.3 getAttributeSet
public LDAPAttributeSet getAttributeSet(String subtype)
These are redundant with 2.3.5 getAttribute and 2.3.7 getSubset.
LDAPAtrributeSet has a superset of similar methods for doing this. Why
duplicate a subset here?
89. Section 2.13 public interface LDAPEntryComparator
Why doesn't this method extend from java.util.Comparator? In fact,
why is this interface necessary at all? Why doesn't
LDAPCompareAttrNames just implement Comparator? Using Comparator would
make available all of the sorting and searching utilities available in
the Java Collections framework. Also, having
public int compare(Object o1, Object o2)
is a lot more flexible than
public boolean isGreater(LDAPEntry entry1, LDAPEntry entry2)
90. Section 2.14 public class LDAPException
extends Exception
Thrown to indicate that an error has occurred. An LDAPException can
result from physical problems (such as network errors) as well as
problems with LDAP operations (for example, if the LDAP add operation
fails because an entry already exists with the DN of the entry to be
added).
And also due to "local errors"?
91. Section 2.14.2 errorCodeToString
Section 2.14.5 getLDAPResultCode
One method says "errorCode", the other says "LDAPResultCode". Do you
mean the same thing? If so, use the same names.
92. Section 2.14.4 getLDAPErrorMessage
public String getLDAPErrorMessage()
Returns the error message, if this message is available (that is, if
this message was set). If the message was not set, this method
returns null.
How is this set? via the 'message' argument to the constructor? How
is this different from the result of Throwable.getMessage()? The
description doesn't mention anything LDAP-specific, so why is the
method named getLDAPErrorMessage()?
93. Section 2.14.7 toString
public String toString()
Overrides the default toString implementation. It expands all the
nested exceptions.
Don't quite know what "expanding the nested exceptions" mean. Does it
mean showing the class names of the nested exception, if any? In which
order?
94. Section 2.14.2 errorCodeToString
public static String errorCodeToString( int code )
Returns a String representing an arbitrary result code in the default
Locale, or null if there is no such code known to the API.
public static String errorCodeToString( int code, Locale locale )
Returns a String representing an arbitrary result code in the
specified Locale, or null if there is no such code or if a String
representation is not available for the requested Locale.
Not an arbitrary result code, but the specified result code.
95. Section 2.14.5 getLDAPResultCode
public int getLDAPResultCode()
Why 'LDAPResultCode' and not simply 'ResultCode'? Some error codes
(such as NO_MEMORY, USER_CANCELLED) might not even be LDAP related.
96. Section 2.15.1 Constructors
public LDAPExtendedOperation(String oid,
byte[] value)
value The operation-specific data of the operation.
Section 2.15.3 getValue
public byte[] getValue()
Returns the operation-specific data (not a copy, a reference).
and Section 2.15.4 setValue
protected void setValue(byte[] value)
Same comment as 73.
97. Section 2.16.2 getValue
public byte[] getValue()
Returns the raw bytes of the value part of the response.
Similar comment as 73.
98. Section 2.16 public class LDAPExtendedResponse
extends LDAPResponse
An LDAPExtendedResponse object encapsulates a server response to an
asynchronous extended operation request.
and Section 1.5 Classes
LDAPExtendedResponse The response returned by an LDAP server
on an asynchronous extended operation
request. It extends LDAPResponse.
The extended operation need not be asynchronous. For example, it is
returned by (Section 2.6.11)
public LDAPExtendedResponse extendedOperation(
LDAPExtendedOperation op )
throws LDAPException
public LDAPExtendedResponse extendedOperation(
LDAPExtendedOperation op,
LDAPConstraints cons )
throws LDAPException
which are synchronous forms of extendedOperation.
99. Section 2.17 public interface LDAPListener
Represents the message queue associated with a particular
asynchronous LDAP operation or operations.
If it is a message queue, why is it called a listener?
100. Inappropriate use of the "Listener" pattern for
LDAPListener
LDAPResponseListener
LDAPSearchListener
Throughout the Java platform, interfaces that are called "Listener"
are callback interfaces that contain methods that are invoked when an
asynchronous event occurs. The LDAP listeners, however, are not
callback interfaces but instead have an entirely different usage
pattern. Furthermore, there is an interface in this package,
LDAPUnsolicitedNotificationListener, that is a callback interface.
Having two different types of "listeners" in the same package is
confusing enough, not to mention the confusion created by one type
being different from those in the Java platform.
As 98 points out, the description of the LDAP listeners indicates the
poor name choice right away. In Section 1.3, the following sentence
appears:
The listener is a message queue
associated with the request, and it is the responsibility of the
client to read messages out of the queue and process them.
If the listener is a message queue, why call it a listener at all?
This incorrect use of "listener" makes the API confusing to use.
101. Section 2.17.3 isResponseReceived
Reports true if a response has been received from the server for a
particular message ID but not retrieved with getResponse. If there is
no outstanding operation for the message ID (or if it is zero or a
negative number), IllegalArgumentException is thrown.
and Section 2.17.2 getResponse
Blocks until a response is available for a particular message ID, or
until all operations associated with the message ID have completed or
been canceled, and returns the response. If there is no outstanding
operation for the message ID (or if it is zero or a negative number),
IllegalArgumentException is thrown.
Perhaps a better name for these methods might be hasResponse() and
nextResponse(), respectively, because they fit the descriptions and
usage pattern better.
102. Section 2.17.1 getMessageIDs
public int[] getMessageIDs()
Didn't explain what is a message ID. Perhaps can be resolved as simply
as making a reference to Section 4.1.1.1 of RFC 2251. Similar comment
for methods in LDAPListener, LDAPMessage and the
LDAPConnection.abandon() method.
103. Section 2.17.4 merge
public void merge(LDAPListener listener2)
Merges two listeners. Moves/appends the content from another
listener to this one.
and Section 2.30.4 merge
public void merge(LDAPListener listener2)
Can you merge different types of listeners (LDAPResponseListener with
LDAPSearchListener and vice versa)? What about a user-defined
LDAPListener implementation that is neither LDAPResponseListener nor
LDAPSearchListener?
Also, is this operation destructive in terms of listener2? After the
operation, would listener2.getMessageIDs() return an empty array and
its outstanding responses be removed? Need to be more explicit.
104. Section 2.2.6 getSyntaxString
public String getSyntaxString()
Returns the Object Identifier of the syntax of the attribute in
dotted numerical format.
Section 2.2.1 Constructors
public LDAPAttributeSchema(String name,
String oid,
String description,
String syntaxString, ...
syntaxString Object Identifier of the syntax of the attribute
- in dotted-decimal format.
Section 2.18.1 Constructors
syntaxString Object Identifier of the syntax that this
matching rule is valid for - in dotted-decimal
format.
Section 2.18.3 getSyntaxString
public String getSyntaxString()
Returns the OID of the syntax that this matching rule is valid for.
Why aren't these methods called getSyntaxOid() and the argument to the
constructors called syntaxOid instead of syntaxString?
105. Section 2.18.1 Constructors
public LDAPMatchingRuleSchema(String name,
String oid,
String description,
String[] attributes,
String syntaxString,
String[] aliases)
public LDAPMatchingRuleSchema(String name,
String oid,
String description,
boolean obsolete,
String syntaxString,
String[] aliases)
For all of the classes that are subclasses of LDAPSchemaElement, each
has a single constructor with some optional arguments and a
constructor that accepts a raw string. Why does this class need two
similar constructors, one with 'obsolete' and the other 'attributes'.
106. 2.18.1 Constructors
Which ones of these arguments are optional? The descriptions seem to say that
the following are optional:
description
attributes
aliases
RFC 2252 says that the following are optional:
name/aliases,
description,
obsolete
attributes
Need to clarify in the descriptions which are optional. If conforming
to RFC 2252, then need to specify whether the corresponding methods
(such as getName()) can return null.
107. 2.19.1 Constructors
public LDAPMatchingRuleUseSchema(String name,
String oid,
String description,
boolean obsolete,
String[] attributes,
String[] aliases)
Which ones of these arguments are optional? The descriptions seem to say that
the following are optional:
description
aliases
RFC 2252 says that the following are optional:
name/aliases
description
obsolete
Need to clarify in the descriptions which are optional. If conforming
to RFC 2252, then need to specify whether the corresponding methods
(such as getName()) can return null.
108. Section 2.20 public class LDAPMessage
Base class for asynchronous LDAP request and response messages.
Not always async. Also used for LDAPExtendedResponse, which may be
gotten as a synchronous response.
109. Section 2.21.1 Constructors
ADD The value should be added to
the attribute
DELETE The value should be removed
from the attribute
REPLACE The value should replace all
existing values of the
attribute
If you're going to explain these here, then make the descriptions
complete or refer to RFC 2251. For example, ADD doesn't just add a
value, it might affect the entire attribute.
110. Section 2.21.4 Constants of LDAPModification
Same comment as 109.
111. Section 2.22 public class LDAPModificationSet
A "set", by definition, is unordered. Why does a set have the
following methods?
2.22.3 elementAt
2.22.5 removeElementAt
Also, in Section 2.22.4 remove
public void remove(String name)
Removes the first attribute with the specified name in the set of
modifications.
'the first' is meaningless in a set. Replace with 'an'.
If it is really a set, then it should implement the java.util.Set
interface.
Furthermore, RFC 2251 requires the modifications to be a sequence:
ModifyRequest ::= [APPLICATION 6] SEQUENCE {
object LDAPDN,
modification SEQUENCE OF SEQUENCE {
operation ENUMERATED {
add (0),
delete (1),
replace (2) },
modification AttributeTypeAndValues } }
It might be more efficient and simpler to eliminate
LDAPModificationSet and just use LDAPModification[].
112. Section 2.23.1 Constructors
public LDAPNameFormSchema(String name,
String oid,
String description,
boolean obsolete,
String objectClass,
String[] required,
String[] optional,
String[] aliases)
Which ones of these arguments are optional? The descriptions seem to
say that the following are optional:
description
aliases
RFC 2252 says that the following are optional:
name/aliases
description
obsolete
optional
Need to clarify in the descriptions which are optional. If conforming
to RFC 2252, then need to specify whether the corresponding methods
(such as getName()) can return null.
113. Section 2.24.1 Constructors
public LDAPObjectClassSchema(String name,
String oid,
String[] superiors,
String description,
String[] required,
String[] optional,
int type,
String[] aliases,
boolean obsolete)
Which ones of these arguments are optional? The descriptions seem to
say that the following are optional:
description
aliases
RFC 2252 says that the following are optional:
name/aliases
desc
obsolete
superiors
required
optional
type
Need to clarify in the descriptions which are optional. If conforming
to RFC 2252, then need to specify whether the corresponding methods
(such as getName()) can return null.
114. Section 2.26.3 getPassword
public byte[] getPassword()
Returns the password to be used for reauthentication on automatic
referral following.
What if passwd was of type String when passed into the constructor?
Is UTF-8 encoding used for the conversion always? What if you're using
LDAP v2?
115. Section 2.27.3 getReferrals
public String[] getReferrals()
Gets the list of referral URLs (LDAP URLs to other servers) returned ...
The referral list MAY include URLs of a type other than ones for an
LDAP server (i.e. a referral URL other than ldap://something).
Delete "LDAP" from the first sentence because it contradicts RFC 2251
and the last sentence in the section.
116. Section 2.28 public interface LDAPReferralHandler
Shared ancestor to the two types of referal objects - LDAPBind and
LDAPRebind.
Typo. referal -> referral.
117. Section 2.29 public class LDAPResponse
extends LDAPMessage
Represents the response to a particular asynchronous LDAP operation.
Not always asynchronous.
118. Section 2.31 public class LDAPSchema (method naming)
The methods in this class has names that are confusing. For example,
in Section 2.31.3 getAttribute:
public LDAPAttributeSchema getAttribute( String name )
The result returned is actually an attribute schema, not an attribute.
Similarly for Section 2.31.24 getSyntax
public LDAPSyntaxSchema getSyntax( String oid )
and Section 2.31.25 getSyntaxes
public Enumeration getSyntaxes()
Returns an enumeration of LDAPSyntaxSchema objects.
getSyntaxSchema() and getSyntaxSchemas() seem more appropriate names.
All of the get* method would benefit by employing more appropriate
names.
119. Section 2.31 public class LDAPSchema (method parameters)
LDAPSchema defines several get methods based on the schema
definition's name.
public LDAPAttributeSchema getAttribute( String name )
public Enumeration getAttributeNames()
public LDAPDITContentRuleSchema getDITContentRule( String name )
public Enumeration getDITContentRuleNames ()
public Enumeration getDITStructureRuleNames ()
public LDAPMatchingRuleSchema getMatchingRule( String name )
public Enumeration getMatchingRuleNames()
public LDAPMatchingRuleUseSchema getMatchingRuleUse( String name )
public Enumeration getMatchingRuleUseNames()
public LDAPNameFormSchema getNameForm( String name )
public Enumeration getNameFormNames ()
public LDAPObjectClassSchema getObjectClass( String name )
public Enumeration getObjectClassNames()
Yet, according to RFC 2252, the corresponding schema definitions have
the oid as mandatory and name as optional.
AttributeTypeDescription = "(" whsp
numericoid whsp ; AttributeType identifier
[ "NAME" qdescrs ] ; name used in AttributeType
...
ObjectClassDescription = "(" whsp
numericoid whsp ; ObjectClass identifier
[ "NAME" qdescrs ]
...
MatchingRuleDescription = "(" whsp
numericoid whsp ; MatchingRule identifier
[ "NAME" qdescrs ]
...
MatchingRuleUseDescription = "(" whsp
numericoid whsp ; MatchingRule identifier
[ "NAME" qdescrs ]
...
DITContentRuleDescription = "("
numericoid ; Structural ObjectClass identifier
[ "NAME" qdescrs ]
...
NameFormDescription = "(" whsp
numericoid whsp ; NameForm identifier
[ "NAME" qdescrs ]
...
DITStructureRuleDescription = "(" whsp
ruleidentifier whsp ; DITStructureRule identifier
[ "NAME" qdescrs ]
Aren't the methods in LDAPSchema out of sync with RFC 2252? Is the
only way to find a definition by OID to do a linear search?
120. Section 2.31.2 fetchSchema
Seeing that there is a fetchSchema method in LDAPSchema, a user might
expect to find a storeSchema method in the same class but the store
methods are actually found in LDAPSchemaElement and has entirely
different names. The LDAPSchema description should describe this, or
better yet, be re-structured so that the store methods are in the same
class.
Here's one suggestion of how to make things symmetric and more
intuitive.
Redefine the constructors as follows.
public LDAPSchema(LDAPConnection ld) throws LDAPException
public LDAPSchema(LDAPConnection ld, String dn) throws LDAPException
Remove LDAPSchemaElement.add/LDAPSchemaElement.modify/
LDAPSchemaElement.remove/ and add the following to LDAPSchema.
public void add(LDAPSchemaElement) throws LDAPException
public void modify(LDAPSchemaElement) throws LDAPException
public void remove(LDAPSchemaElement) throws LDAPException
If you need a method to refresh the schema, then add
public void refresh() throws LDAPException;
The schema from ld need not be loaded immediately when the constructor
is called. In fact, the implementation might load it completely or
even partially on demand, as required by the get* methods. The get
methods would need a throws LDAPException clause.
The result is a much cleaner design in which all of the physical
schema access and update methods are localized in one class instead of
being spread over two.
121. Section 2.31.2 fetchSchema
The fetchSchema methods are the only methods that interact with a
Directory Server. The other methods access information acquired
through fetchSchema. An LDAPException is thrown as for
LDAPConnection.search() (2.6.34) if the schema cannot be retrieved
with the specified connection, or if the subschemaSubentry attribute
used to locate the schema contains multiple values.
The first two sentences are implementation details and should be
omitted. The 3rd sentence describes how the schema is fetched (i.e.,
via a search) -- again, implementation detail that should be omitted.
Delete "as for LDAPConnection.search() (2.6.34)".
122. Section 2.3.2 public abstract class LDAPSchemaElement
2.32.1 add
2.32.10 modify
2.32.11 remove
See 120. Should move functionality to LDAPSchema, which already deals
with the physical schema tree directly.
123. Why isn't LDAPSchemaElement Cloneable or Serializable? It would
be useful to be able to clone and then modify the copy. It would be
useful to be able to serialize schema elements to a file or other
storage medium.
124. Can these return null?
Section 2.32.2 getAliases
public String[] getAliases()
Section 2.32.3 getDescription
public String getDescription()
Section 2.32.4 getName
public String getName()
Section 2.32.5 getID
public String getID()
(LDAPDITStructureRuleSchema says getID() returns null).
125. Section 2.32.6 getQualifier
public String[] getQualifier(String name)
Returns an array of all values of a specified optional or
experimental qualifier of the element. This method may be used to
access the values of vendor-specific qualifiers (which begin with "X-
" [ATTR]).
and Section 2.32.7 getQualifierNames
public Enumeration getQualifierNames()
Returns an enumeration of all qualifiers of the element which are not
defined in [ATTR].
Does getQualifier and getQualifierNames see the same namespace? Their
method names imply so but their descriptions don't.
getQualifier is a very general name and one would expect to get any
qualifier, not just optional and experimental ones. Also, which are
optional? Perhaps need to say that the list of optional qualifiers
varies for each type of LDAPSchemaElement and is defined in RFC 2252.
Also, if the method description is correct, then should choose a
different name for the method.
Same comments apply to Section 2.32.12 setQualifier.
The method description for getQualifierNames suggests that the
namespace is more restrictive than getQualifier's. If so, then
the method name should reflect this.
126. Section 2.32.8 getValue (method name)
public String getValue()
Returns a String in a format suitable for directly adding to a
Directory, as a value of the particular schema element attribute.
Why not call this method toString()? The "value" of a schema element
is what? It seems that this method is returning the string
representation, and the typical way to do that is to use toString().
The name getValue() seems like it is equivalent to the other get
methods (such as getName(), getID()). If toString() seems too generic,
then how about toRfc2252Syntax()?
127. Section 2.32.8 getValue (method description)
public String getValue()
Returns a String in a format suitable for directly adding to a
Directory, as a value of the particular schema element attribute.
The description is too vague. In fact, this method should be abstract
and a getValue() method should be added to each subclass of
LDAPSchemaElement that clearly states the expected output of the
method. It should use the language suggested for the 'raw' parameter
descriptions. (See 83).
For example, for LDAPAttributeSchema, getValue() must return
a string encoded using the AttributeTypeDescription syntax [ATTR].
128. Section 2.7 public class LDAPConstraints
Why isn't LDAPConstraints cloneable? That would make it easier to make
a copy of an existing LDAPConstraints and modify one or two
parameters. This is especially useful for LDAPSearchConstraints.
129. Section 2.33.1 Constructors
It might be useful to have an LDAPSearchConstraints constructor that
accepts a LDAPConstraints argument.
130. Section 2.33.1 Constructors
dereference Specifies when aliases should be dereferenced.
MUST be either DEREF_NEVER, DEREF_FINDING,
DEREF_SEARCHING, or DEREF_ALWAYS (DEREF_NEVER by
default).
and Section 2.33.3 getDereference
public int getDereference()
Specifies when aliases should be dereferenced. Returns either
DEREF_NEVER, DEREF_FINDING, DEREF_SEARCHING, or
DEREF_ALWAYS.
either -> one of
131. Section 2.34.3 isComplete
public boolean isComplete(int msgid)
Reports true if all results have been received for a particular
message ID. For search, it is true if a searchResultDone response has
been received from the server for the id. ...
LDAPSearchListener is used only for searches. 'For search' is
redundant.
132. These concrete classes do not have any explicit constructors and
no set methods to update their state. A public no-argument constructor
will be made available by default. The user should not be creating
instances of these. These are objects returned by the Java LDAP API
implementation. They should be probably be interfaces instead of
concrete classes.
2.16 public class LDAPExtendedResponse extends LDAPResponse
2.20 public class LDAPMessage
2.29 public class LDAPResponse extends LDAPMessage
2.35 public class LDAPSearchResult extends LDAPMessage
2.36 public class LDAPSearchResultReference extends LDAPMessage
2.37 public class LDAPSearchResults implements Enumeration
133. Section 2.37.5 nextElement
public Object nextElement()
Returns the next result in the enumeration as an Object. This the
default implementation of Enumeration.nextElement(). The returned
value may be an LDAPEntry or an LDAPException.
Returning an exception is poor design and nonstandard. No other API
in the Java platform uses enumerations in this way. This design
forces the caller to check whether each element is a real answer or an
exception. Look at the Appendix A, 8.1 example.
LDAPSearchResults res = ld.search( MY_SEARCHBASE, ...);
/* Loop on results until finished */
while ( res.hasMoreElements() ) {
/* Next directory entry */
LDAPEntry findEntry = (LDAPEntry)res.nextElement();
With nextElement() defined the way it is, if you follow this example,
then you'll get a ClassCastException if nextElement() returns an
LDAPException. To avoid the exception, you'd need to write something
like
/* Next directory entry */
Object answer = res.nextElement();
if (answer instanceof LDAPEntry) {
findEntry = (LDAPEntry) answer;
} else if (answer instanceof LDAPException) {
throw (LDAPException) answer;
}
As illustrated by this example, this API definition forces the user to
write very awkward code.
134. Section 2.37.5 nextElement
public Object nextElement()
Returns the next result in the enumeration as an Object. This the
default implementation of Enumeration.nextElement(). The returned
value may be an LDAPEntry or an LDAPException.
Can an LDAPException be returned midstream or is it always the last
element in the enumeration? Are some exception fatal while others not?
135. Section 2.37.1 getCount
public int getCount()
Returns a count of the entries and exceptions in the search result.
If the search is asynchronous (batch size not 0), this reports the
number of results received so far.
LDAPSearchResults is returned only for synchronous searches. A batch
size of nonzero is not defined in this document as making a search
'asynchronous'. Probably want to use different wording to avoid
confusion with real async searches.
Exceptions are also included in this count so the caller has no real
way of knowing how many entries there are.
136. Section 2.37.6 sort
This implies that
LDAPSearchResults.nextElement() will always block until all results
have been retrieved, after a sort operation.
This also means batch size will be ignored (be effectively 0) if you
invoke sort. This method seems rather awkward. There is no
corresponding functionality in the LDAP C API. It would seem more
flexible and appropriate to just delete this method and use the
sort/search facilities available in the Java Collections
framework. That way, you have greater control over the data structures
and algorithm.
137. Section 2.37.2 getResponseControls
public LDAPControl[] getResponseControls()
Returns the latest Server Controls returned by a Directory Server
in the context of this search request.
Is this supposed to return the controls associated with each LDAPEntry
that got returned? Should it be invoked in tandem with each call to
nextElement()? Does it block in the same way nextElement() does? Or
does it return the controls returned by the server since the last time
getResponseControls() was invoked.
138. Section 2.37.3 hasMoreElements
public boolean hasMoreElements()
Specifies whether or not there are more search results in the
enumeration. If true, there are more search results.
What about exceptions? Does an exception in the enumeration count as a
"search result"?
139. Section 2.38 public interface LDAPSocketFactory.
There is already a standard abstract class for socket factory:
javax.net.SocketFactory. javax.net.SocketFactory is available as part
of the Java Secure Socket Extension (JSSE) and also as part of the
Java 2 Platform, Standard Edition, v 1.4. It is a general socket
factory mechanism and encompasses the functionality of
LDAPSocketFactory.
There is nothing LDAP-specific about LDAPSocketFactory.
public Socket makeSocket(String host, int port)
throws IOException, UnknownHostException
Even if you decide not to use javax.net.SocketFactory, at least change
the name of this method to "createSocket" so that it is consistent
with javax.net.SocketFactory.
140. Section 2.39.1 Constructors
... Note
that adding and removing syntaxes is not typically a supported
feature of LDAP servers.
Don't know why this is here in the constructor class. Calling the
constructor won't add or remove syntaxes from LDAP servers. It might
belong in the class description or even maybe the LDAPSchema
class. This note might be applicable for other types of schema
elements as well and to schema updates in general.
141. Section 2.41.3 encode
Encodes an arbitrary string. Any illegal characters are encoded as
%HH.
"illegal" is not the correct term to use here. This method could
benefit from a better description.
Also, not an arbitrary string, but the specified string.
142. Section 2.41.5 getAttributes
public Enumeration getAttributes()
Returns an Enumerator for the attribute names specified in the URL.
Null OK? Or will empty enumeration be returned if there are none.
143. Section 2.41.12 getUrl
public String getUrl()
Returns a valid string representation of this LDAP URL.
Why not call this method toString() instead of getUrl()? The method
description seems to indicate that toString() is appropriate.
144. Section 2.41.1 Constructors
public LDAPUrl(String host,
int port,
String dn,
String[] attrNames,
int scope, ...
attrNames Names or OIDs of attributes to retrieve. Passing
a null array signifies that all user attributes
are to be retrieved. Passing a value of "*"
allows you to specify that all user attributes as
well as any specified operational attributes are
to be retrieved.
Can't pass a value of "*" because constructor wants a String[].
The description of "*" contradicts ALL_USER_ATTRS (Section 2.6.41).
ALL_USER_ATTRS ("*") Used with search in an attribute list to
indicate that all attributes (other than
operational attributes) are to be returned.
145. Section 2.41.7 getExtensions
public String[] getExtensions ()
Returns any LDAP URL extensions specified, or null if none are
specified. Each extension is a type=value expression. The =value part
MAY be omitted. The expression MAY be prefixed with '!' if it is
mandatory for evaluation of the URL.
The only constructor that allows extensions to be specified is the one
that takes a raw URL String. Is that intended or is a String[]
extensions argument missing from the 3rd constructor.
146. Section 3.1 Client and Server Controls
Server controls returned to a client as part of the response to a
synchronous operation can be obtained with
LDAPConnection.getResponseControls()
And LDAPSearchResults.getResponseControls().
147. Section 3.2 Referral handling and exceptions,
The "Synchronous requests" section often talks about exceptions being
"thrown". However, those exceptions are also just returned via
LDAPSearchResults.nextElement(), not necessarily thrown.
148. Section 3.2 Referral handling and exceptions
When a referral is received, the application receives an LDAPResponse
object with a result code of REFERRAL. If a continuation reference is
received the application receives an LDAPSearchResultReference
object.
Not true for v2.
149. Section 3.2 Referral handling and exceptions
Throughout this section, the document uses "the API" when "the
implementation of the Java LDAP API" would be more appropriate.
Two occurrences of "an LDAPReferral exception" should be changed to
"an LDAPReferralException".
150. Section 3.5 Invalid responses
If a message is received by an API implementation from the server and
the message cannot be interpreted as an LDAP PDU, an LDAPException
MUST be thrown with a result code of INVALID_RESPONSE.
Same comment as 147.
151 Section 3.4 Level of compatibility (package name)
Implementations ... and MUST be
implemented with the package name org.ietf.ldap.
The package name should be specified in Section 1 or 2, not in Section
3, under Implementation Considerations. The package name is not an
implementation consideration, it is part of the API specification,
just like a class name or a method name.
152. Section 3.4 Level of compatibility
Implementations of the API are to be binary compatible, and ...
Binary compatibility of Java programs is defined by the Java
Programming Language and the Java Virtual Machine
Specifications. Don't know the value added by the first sentence.
Might be more useful to specify which version of the Java Platform and
which Optional Packages this API depends on. There is a dependency on
JAAS (javax.security.auth.callback.*).
153. Section 8 Appendix A - Sample java LDAP programs
All code samples should follow the Code Conventions for the Java
Programming Language, available at
http://java.sun.com/docs/codeconv/
154. Section 8.1
/* Fetch the schema (anonymous is okay for reading) */
This comment is not true for Active Directory: you need to be
authenticated to read the schema.
155. Section 8.1, ShowSchema example
);
/* What are the possible attributes for "person"? */
LDAPObjectClassSchema o =
schema.getObjectClass( "person" );
if ( o != null ) {
Enumeration required = o.getRequiredAttributes();
Enumeration optional = o.getOptionalAttributes();
System.out.println(
"Required attributes for person:" );
while( required.hasMoreElements() )
System.out.println( " " +
(String)required.nextElement() );
System.out.println(
"Optional attributes for person:" );
while(optional.hasMoreElements() )
System.out.println( " " +
(String) optional.nextElement() );
);
LDAPObjectClassSchema.getRequiredAttributes() and
LDAPObjectClassSchema.getOptionalAttributes() return String[], not
Enumeration, and the subsequent while loops need to be changed
accordingly.
The first and last line of code sample shown should be "}", not ");".
156. Section 8 Appendix
The ModifyEmail example on page 124 is supposed to be asynchronous
but it shows a synchronous usage of "modify" and "bind".
157. Section 2.11.5 isValid
Returns true if the string conforms to distinguished name syntax
(section 3 of [DN]. It MAY return true also if the string conforms to
section 4 of [DN].
Missing closing parens.
158. Section 2.14.8 Result codes
The meanings of the result codes not covered by RFC 2251 need to be
stated explicitly.
AMBIGUOUS_RESPONSE (0x65)
AUTH_UNKNOWN (0x56)
CLIENT_LOOP (0x60)
CONNECT_ERROR (0x5b)
CONTROL_NOT_FOUND (0x5d)
DECODING_ERROR (0x54)
ENCODING_ERROR (0x53)
FILTER_ERROR (0x57)
INVALID_RESPONSE (0x64)
LOCAL_ERROR (0x52)
LDAP_NOT_SUPPORTED (0x5c)
LDAP_TIMEOUT (0x55)
MORE_RESULTS_TO_RETURN (0x5f)
NO_MEMORY (0x5a)
NO_RESULTS_RETURNED (0x5e)
REFERRAL_LIMIT_EXCEEDED (0x61)
SERVER_DOWN (0x51)
USER_CANCELLED (0x58)
TLS_NOT_SUPPORTED (0x70)
Also, isn't NO_MEMORY already covered by the VM error
OutOfMemoryError?