Subject: [apps-review] secdir/apps-area/last-call review of draft-ietf-vcarddav-vcardrev-19

From: Barry Leiba <[email-masked]>
Date: Mon, 18 Apr 2011 12:59:38 -0400

This is a "triple-play" review -- I've been asked to review
draft-ietf-vcarddav-vcardrev-19 by the security directorate and the
applications area review team, and I'd planned to do my own review as
well.

SECDIR boilerplate:
I have reviewed this document as part of the security directorate's
ongoing effort to review all IETF documents being processed by the
IESG.  These comments were written primarily for the benefit of the
security area directors.  Document editors and WG chairs should treat
these comments just like any other last call comments.

APPS-REVIEW boilerplate:
I have been selected as the Applications Area Review Team reviewer for
this draft (for background on apps-review, please see
http://www.apps.ietf.org/content/applications-area-review-team).
Please resolve these comments along with any other Last Call comments
you may receive. Please wait for direction from your document shepherd
or AD before posting a new version of the draft.

My plat du jour:
I should note that I've participated in the vcarddav working group,
and have made comments on and contributed text to this document.  That
said, I'm looking at this with a "from scratch" mind, to be sure I'm
considering things afresh.

General comments:

The document is pretty much ready to ship.  The comments I make below
are minor, non-controversial, and easily fixed.  This is a mature spec
that's had quite a lot of review already.  Also, I see no
security-related issues with the document.

Reviewers of the companion document, draft-ietf-vcarddav-vcardxml,
will have to make sure the properties and parameters match up
correctly.  There is a general issue that this "legacy" vCard format
does not have a hierarchy that allows nested elements, which limits
its extensibility somewhat.  The working group did consider whether to
abandon this format in favour of using XML *only* for v4.0, and that
was deemed infeasible: there remains extensive use of this format, and
it needs to be retained, at least for now.

Specific comments:

------------------------------
   3.1.  Character Set

   The character set for vCard is UTF-8 as defined in [RFC3629].  There
   is no way to override this.  It is invalid to specify a different
   character set in the "charset" MIME parameter (see Section 10.1).

This isn't consistent with section 10.1.  UTF-8 is an encoding, not a
character set.  Section 10.1 says this, in reference to parameters on
the media type:

      "charset": as defined for text/plain [RFC2046]; encodings other
      than UTF-8 [RFC3629] MUST NOT be used.

That is the correct characterization.  The problem is that this is
used inconsistently in general, and there's been an extended
controversy in EAI about this very thing.  RFC 3536 specifies
internationalization terminology, and that's currently being updated
by draft-hoffman-rfc3536bis.  From that document:

   charset

      A charset is a method of mapping a sequence of octets to a
      sequence of abstract characters.  A charset is, in effect, a
      combination of one or more CCSs with a CES.  Charset names are
      registered by the IANA according to procedures documented in
      [RFC2978]. <NONE>

      Many protocol definitions use the term "character set" in their
      descriptions.  The terms "charset" or "character encoding scheme"
      and "coded character set" are strongly preferred over the term
      "character set" because "character set" has other definitions in
      other contexts and this can be confusing.

I suggest re-wording 3.1 thus (and an informative reference to RFC
3536 would be reasonable):

NEW
   3.1.  Charset

   The charset [see RFC3536 for internationalization terminology] for vCard
   is UTF-8 as defined in [RFC3629].  There is no way to override this.  It is
   invalid to specify a value other than "UTF-8" in the "charset" MIME
   parameter (see Section 10.1).
------------------------------

   3.3.  ABNF Format Definition

OLD
     ; When parsing a content line, folded lines must first
     ; be unfolded according to the unfolding procedure
     ; described above.
     ; When generating a content line, lines longer than 75
     ; characters SHOULD be folded according to the folding
     ; procedure described above.

NEW
     ; When parsing a content line, folded lines must first
     ; be unfolded according to the unfolding procedure
     ; described in section 3.2.
     ; When generating a content line, lines longer than 75
     ; characters SHOULD be folded according to the folding
     ; procedure described in section 3.2.

OLD
   A line that begins with a white space character is a continuation of
   the previous line, as described above.

NEW
   A line that begins with a white space character is a continuation of
   the previous line, as described in section 3.2.
------------------------------

Section 3.3 says this:
   Parameter values
   MAY be case sensitive or case insensitive, depending on their
   definition.  Based on experience with vCard 3 interoperability, it is
   RECOMMENDED that property and parameter names be upper-case on
   output.

Some of the parameter definitions ("value" (5.2) and "type" (5.6), for
example) use text strings for their values, and give no guidance on
case.  I suggest changing the paragraph in section 3.3:

NEW
   Parameter values
   MAY be case-sensitive or case-insensitive, depending on their
   definitions.  Parameter values that are not explicitly defined
   as being case-sensitive are case-insensitive. Based on
   experience with vCard 3 interoperability, it is RECOMMENDED
   that property names and parameter names be upper-case on output.

------------------------------

3.4. Property Value Escaping

OLD
   Finally, BACKSLASH characters in values MUST be escaped with a
   BACKSLASH character.  NEWLINE (ASCII decimal 10) characters in values
   MUST be encoded by two characters: a BACKSLASH followed by an 'n'
   (ASCII decimal 110).

This is inconsistent with section 4.1.

NEW
   Finally, BACKSLASH characters in values MUST be escaped with a
   BACKSLASH character.  NEWLINE (ASCII decimal 10) characters in values
   MUST be encoded by two characters: a BACKSLASH followed by either an
   'n' (ASCII decimal 110) or an 'N' (ASCII decimal 78).
------------------------------

   4.5.  INTEGER

I note that the ABNF allows "-0" (and, in fact, "-00000000") as a
valid integer.  If that's intended, that's fine; I just wanted to
point it out.
------------------------------

5.9.  SORT-AS

Is this parameter value intended to be case-sensitive?  I think so,
and that should be specified.  Let's fix the "less/fewer" error, while
we're here.

OLD
   This parameter's value is a comma-separated list which MUST have as
   many or less elements as the corresponding property value has
   components.

NEW
   This parameter's value is a comma-separated list which MUST have as
   many or fewer elements as the corresponding property value has
   components.  This parameter's value is case-sensitive.
------------------------------

   6.3.1.  ADR

   Special notes:  The structured type value consists of a sequence of
      address components.  The component values MUST be specified in
      their corresponding position.  The structured type value
      corresponds, in sequence, to the post office box; the extended
      address (e.g. apartment or suite number); the street address; the
      locality (e.g., city); the region (e.g., state or province); the
      postal code; the country name (full name in the language specified
      in Section 5.1).  When a component value is missing, the
      associated component separator MUST still be specified.

SUGGESTION:
I think this would be easier to read and get right as a compact list,
rather than as a paragraph of text.  Maybe like this:

NEW
   Special notes:  The structured type value consists of a sequence of
      address components.  The component values MUST be specified in
      their corresponding position.  The structured type value
      corresponds, in sequence, to
         the post office box;
         the extended address (e.g. apartment or suite number);
         the street address;
         the locality (e.g., city);
         the region (e.g., state or province);
         the postal code;
         the country name (full name in the language specified in
            Section 5.1).
      When a component value is missing, the associated component
      separator MUST still be specified.
------------------------------

   8.  Example: Authors' vCards

There's nothing wrong with leaving Pete Resnick's vCard in there as an
example, but I'll point out that he hasn't been a document editor
since -16 was posted.  I'll also point out that these are not
"authors", but "editors".  Just sayin'....
------------------------------

   9.  Security Considerations

I believe the specified security considerations are adequate, and I
have no issues with them.
------------------------------

   10.1.  MIME Type Registration

   Person & email address to contact for further information:  Simon
      Perreault <[email-masked]>

Section 10.2.1 specifies the creation of a mailing list,
<[email-masked]>.  Perhaps that should be the contact email address
for the MIME type registration as well.
------------------------------

   10.2.1.  Registration Procedure

Nit: change "Standard Tracks RFC" to "Standards Track RFC", throughout.
------------------------------

That's all....

Barry