[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: I-D ACTION:draft-ietf-ldapbis-filter-08.txt

Kurt D. Zeilenga wrote:

I have completed my review of the I-D.  While it's my personal
opinion that the revisions made are consistent with WG
consensus, I did find a few minor problems which likely should
be address prior to progression.  Most significantly, a few
aspects of the I-D need updated to be consistent with the
ASN.1 and ABNF definitions of [Protocol] and [Models] (as
possibly revised per recent discussions).  I suggest the
<extensible> rule be clarified and examples changed
slightly to demonstrate case-insensitive.  I also make a
few editorial suggestions to improve clarity and/or to
remove nits.  See attachment for details.

Thanks for your careful review. I added my responses inline below, prefaced with [mcs].



... 
11.2.     Editorial Changes...........................................10
12.    Appendix B: Changes Since Previous Document Revision...........11
12.1.     Editorial Changes...........................................11
13.    Intellectual Property Rights...................................11
14.    Full Copyright.................................................12


Last two sections should be unnumbered sections. 

Good catch.  Easily fixed.



1. Introduction 


I would like to see a reference to [Roadmap] on first use of LDAP
(as [Roadmap] is the (LDAP TS).  Maybe a slight rewording would
facilitate that.

[mcs] Good idea. Perhaps just replaced the [Protocol] reference below with [Roadmap]? Or do we need to keep this sentence too:

   This document is an integral part of the LDAP Technical Specification
   [Roadmap].

?


 The Lightweight Directory Access Protocol (LDAP) [Protocol] defines a
 network representation of a search filter transmitted to an LDAP
 server.  Some applications may find it useful to have a common way of
 representing these search filters in a human-readable form; LDAP URLs


cite [LDAPURL]. 

[mcs] OK; I will add it.


...

      LDAPString ::= OCTET STRING -- UTF-8 encoded,
                                  -- [Unicode] characters

 The AttributeDescription is a string representation of the attribute
 description and is defined in [Protocol].



This might be better worded:
	The AttributeDescription, as defined in [Protocol], is a string
	representation of the attribute description [Models].

to clarify that attribute descriptions are discussed in [Models].

[mcs] I like your suggestion. I'd also like to add "that is discussed in" or "that is described in" before "[Models]" in your suggested re-wording.


...
    lessorequal    = LANGLE EQUALS
    extensible     = attr [dnattrs]
                           [matchingrule] COLON EQUALS assertionvalue
                     / [dnattrs]
                            matchingrule COLON EQUALS assertionvalue
                     / COLON EQUALS assertionvalue



The grouping notation should be used here to improve clarity.
That is, ( ... ) / ( ... ) / ( ... ).  As presented, it
appears that ":=value" would be a valid extensible production.


[mcs] Agreed.  I will add the ()s.


...
 For AssertionValues that contain UTF-8 character data, each octet of
 the character to be escaped is replaced by a backslash and two hex
 digits, which form a single octet in the code of the character.


Delete this blank line (see below) 
>>>

[mcs] OK.


 For example, the filter checking whether the "cn" attribute contained
 a value with the character "*" anywhere in it would be represented as
 "(cn=*\2a*)".

As indicated by the valueencoding rule, implementations MUST escape 


wrap valueencoding in <>, as done for other rule appearance in
the prose.


[mcs] Good catch.

>>>> ... 
 The following examples illustrate the use of extensible matching.

      (cn:1.2.3.4.5:=Fred Flintstone)
      (cn:=Betty Rubble)
      (sn:dn:2.4.6.8.10:=Barney Rubble)
      (o:dn:=Ace Industry)
      (:1.2.3:=Wilma Flintstone)
      (:dn:2.4.6.8.10:=Dino)



Please change one of the :dn examples to uses :DN to remind folks
that its case insensitive, e.g.,
	(:DN:2.4.6.8.10:=Dino)


[mcs] Good idea.



Please change one of the :oid examples to use the descr form, e.g.
	(cn:caseExactMatch:=Fred Flintstone)


[mcs] Another good suggestion.


...
 The following examples illustrate the use of the escaping mechanism.

      (o=Parens R Us \28for all your parenthetical needs\29)
      (cn=*\2A*)
      (filename=C:\5cMyFile)
      (bin=\00\00\00\04)
      (sn=Lu\c4\8di\c4\87)
      (1.3.6.1.4.1.1466.0=\04\02\48\69)

 The first example shows the use of the escaping mechanism to
 represent parenthesis characters. The second shows how to represent a
 "*" in an assertion value, preventing it from being interpreted as a
 substring indicator. The third illustrates the escaping of the
 backslash character.

 The fourth example shows a filter searching for the four-byte value
 0x00000004, illustrating the use of the escaping mechanism to
 represent arbitrary data, including NUL characters.



Without knowing the byte order of 0x00000004, one cannot say
that the \00\00\00\04 is the proper value encoding.  Suggest
you instead use:
	the four octet value 00 00 00 04 (hex).

or:
	0x00000004 (big-endian, hex).

[mcs] I prefer your first suggestion (four octet value). I believe that is what was intended by the example originally.


>>>> ... 
 The fifth example illustrates the use of the escaping mechanism to
 represent various non-ASCII UTF-8 characters.


Would be good to say what those characters are. 

[mcs] OK.  I suggest we add the following text:

   Specifically, there are 5 characters in the <assertionvalue>
   portion of this example: LATIN CAPITAL LETTER L (U+004C),
   LATIN SMALL LETTER U (U+0075), LATIN SMALL LETTER C WITH CARON
   (U+010D), LATIN SMALL LETTER I (U+0069), and LATIN SMALL
   LETTER C WITH ACUTE (U+0107).


...
[X.690]     Specification of ASN.1 encoding rules: Basic, Canonical, and
          Distinguished Encoding Rules, ITU-T Recommendation X.690,
          1994.



As understanding of BER is not required to properly implement
Filter string representation specification, this reference can
be viewed as Informative.


[mcs] I agree.




8.  Informative References

None.


Add [LDAPURL]. 

[mcs] Yes.




9.  Acknowledgments

This document replaces RFC 2254 by Tim Howes. 


Please add:
	RFC 2254 was a product of the IETF ASID Working Group.

and insert paragraph break here. 

[mcs] OK.

-Mark