Re: [VCARDDAV] AD review of draft-ietf-vcarddav-carddav-07.txt

[Cyrus Daboo <[e-mail]>]

Hi Alexey,

--On August 17, 2009 12:02:08 AM +0100 Alexey Melnikov 
<[e-mail]> wrote:

> Here is my detailed review:
>
> 1.  Introduction and Overview
>
>  [...]
>
>    3.  Principal namespace can be used to enumerate and find other users
>        on the system.
>
> A reference to the document defining "principal namespace" should be added
> here.

Actually "Principal namespace" is not quite right. Instead I have changed 
it to "Principal collections ...". I have also added references in all 
appropriate places in that list in Section 1.

>  [...]
>
>    5.  Well-defined internationalization support through standard HTTP.
>
> This is a bit misleading, because this is or isn't true depending
> on which HTTP feature you mean here.

As per thread with Julian, I have changed the text to:

    Well-defined internationalization support through WebDAV's use of XML.

>
> 3.  Requirements Overview
>
>    o  MUST support secure transport as defined in [RFC2818] using TLS
>       [RFC5246];
>
> This recently came up in review of
> draft-ietf-geopriv-http-location-delivery-15.txt:
>
> RFC 2818, Section 3.1 says:
>
>   Matching is performed using the matching rules specified by
>   [RFC2459].  If more than one identity of a given type is present in
>   the certificate (e.g., more than one dNSName name, a match in any one
>   of the set is considered acceptable.) Names may contain the wildcard
>   character * which is considered to match any single domain name
>   component or component fragment. E.g., *.a.com matches foo.a.com but
>   not bar.foo.a.com. f*.com matches foo.com but not bar.com.
>
> Based on the discussion during an IESG telechat several ADs agreed that
> f* wildcards shouldn't be allowed anymore. So, the document should say
> that it complies with RFC 2818, except for f* type wildcards are not
> allowed. (wildcards in the leftmost label are still allowed). This is
> consistent with the advice from RFC 5280.
>
>
> I also think this document should reference RFC 5280.

I have changed the text to:

   o  MUST support secure transport as defined in [RFC2818] using TLS
      [RFC5246] and using the certificate validation procedures
      described in [RFC5280];

Is that sufficient?

>
> 6.2.1.  CARDDAV:addressbook-description Property
>
>    Name:  addressbook-description
>
>    Namespace:  urn:ietf:params:xml:ns:carddav
>
>    Purpose:  Provides a human-readable description of the address book
>       collection.
>
> BCP 18 (RFC 2277), section 4.2 applies to elements carrying human
> readable text.
> Any human readable text needs to be [optionally] accompanied by RFC 4646
> language tags.
>
> You even use of xml:lang in your example later in this section. However
> the definition doesn't allow for this attribute:
>
>    Definition:
>
>        <!ELEMENT addressbook-description (#PCDATA)>
>        <!-- PCDATA value: string -->

As per thread with Julian, I have added some addition text to the 
Description of that property to indicate that xml:lang is used to indicate 
a language tag for the value.

>
> 6.2.3.  CARDDAV:max-resource-size Property
>
>    Name:  max-resource-size
>
>    Namespace:  urn:ietf:params:xml:ns:carddav
>
>    Purpose:  Provides a numeric value indicating the maximum size in
>       octets of a resource that the server is willing to accept when an
>       address object resource is stored in an address book collection.
>
>    Value:  Any text representing a numeric value.
>
> Ok, this is probably pedantic. But "Any text representing a numeric
> value" might mean that hexadecimal formats are allowed, but I am sure
> this is not what you meant here.

Comment in XML fragment now says "positive decimal integer".

>
> 8.3.1.  CARDDAV:supported-collation-set Property
>
>
>    Description:  The CARDDAV:supported-collation-set property contains
>       zero or more CARDDAV:supported-collation elements which specify
>       the collection identifiers of the collations supported by the
>       server.
>
> According to section 8.3 the "i;unicode-casemap" collation is "MUST
> implement"
> (and the default), so I think this should say "one or more".
> Also the definition needs to be updated accordingly:
>
>    Definition:
>
>          <!ELEMENT supported-collation-set (supported-collation*)>
>
>          <!ELEMENT supported-collation (#PCDATA)>

Actually section 8.3 requires two collations to be supported. I have 
updated text and XMl; fragment accordingly.

>
> 9.  Guidelines
>
> Suggest renaming the section to say that this section defines client
> guidelines.

Title changed to "Client Guidelines".

>
> 9.4.  Finding Other Users' Address Books
>
>    To find other users' address books, the DAV:principal-property-search
>
> I think this needs a reference to RFC 3744.
>
>    REPORT can be used to filter on some properties and return others.
>    To search for an address book owned by a user named "Laurie", the
>    REPORT request body would look like this:

Fixed with some re-wording of the text and addition of the reference.

>
> 10.5.1.  CARDDAV:prop-filter XML Element
>
>    Name:  prop-filter
>
>    Namespace:  urn:ietf:params:xml:ns:carddav
>
>    Purpose:  Limits the search to specific vCard properties.
>
>    Description:  The CARDDAV:prop-filter XML element specifies a search
>       criteria on a specific vCard property (e.g., NICKNAME).  An
>       address object is said to match a CARDDAV:prop-filter if:
>
>       *  A vCard property of the type specified by the "name" attribute
>          exists, and the CARDDAV:prop-filter is empty, or it matches any
>          CARDDAV:text-match conditions if specified, and that CARDDAV:
>          param-filter child elements also match.
>
> Does this mean that "param-filter" matches in either case, or only
> if CARDDAV:text-match is specified?

param-filter can appear without a text-match. I have reworded that 
paragraph to make this clearer.

> 13.  Security Considerations
>
>    HTTP protocol transactions are sent in the clear over the network
>    unless protection from snooping is negotiated.  This can be
>    accomplished by use of TLS as defined in [RFC2818].  In particular,
>    if HTTP Basic authentication is available,
>
> This needs a reference.
>
>    the server MUST allow TLS
>    to be used at the same time, and SHOULD prevent use of Basic
>    authentication when TLS is not in use.
>
>  [...]
>
>    This specification currently relies on standard HTTP authentication
>    mechanisms for identifying users.  These comprise Basic and Digest
>    authentication as well as SSL using client-side certificates.
>
> All three mechanisms need Informative References.

References added.

>
> 16.2.  Informative References
>
>  [...]
>
>    [RFC4511]  Sermersheim, J., "Lightweight Directory Access Protocol
>               (LDAP): The Protocol", RFC 4511, June 2006.
>
> I know that Kurt strongly prefers when RFC 4510 is used for referencing
> LDAP.

Fixed.

I have just posted draft -08 that contains all the changes listed here. 
Hopefully this is now ready for IESG last call?


-- 
Cyrus Daboo