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

From: Mark Smith <[email-masked]>
Date: Tue, 02 Nov 2004 09:48:35 -0500

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

---

From: "Kurt D. Zeilenga" <[email-masked]>
Date: Tue, 02 Nov 2004 07:38:22 -0800

At 06:48 AM 11/2/2004, Mark Smith wrote:

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

Citing [Roadmap] on first use of LDAP is appropriate as
[Roadmap] defines what is LDAP.  This sentences makes it
clear that this I-D is integral part of LDAP, and was also
intended to be part of the I-D's description of how it
relates to other I-Ds (and obsoleted RFCs).  I note that in
many of the LDAPBIS I-Ds (see [Protocol] & [Models]), we're
now using the longer form:
   This document is a integral part of the LDAP technical
   specification [Roadmap] which obsoletes the previously
   defined LDAP technical specification, RFC 3377, in its
   entirety.
and immediately following this with the particular obsolete
information for this I-D.  I suggest you do likewise here.

(I just noticed that [LDAPDN] should use the long form, will
update that.)

---

From: Mark Smith <[email-masked]>
Date: Fri, 12 Nov 2004 09:47:49 -0500

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

There is one more issue here which came to me as I was editing the 
document:  Filter-08 *does* specify that ":=value" is a valid extensible 
production.  I think that is simply an error and that the third option 
within the extensible production should be removed (it was introduced by 
me between the -01 and -02 revisions more than 2 years ago).  Section 
4.5.1 (Search Request) of [Protocol] disallows this:

    If the matchingRule field is absent, the type field MUST be
    present, and an equality match is performed for that type.

As does 2251.  Which makes perfect sense.

-Mark