Subject: Re: [xmpp] COMMENT: draft-ietf-xmpp-3920bis

From: Peter Saint-Andre <[email-masked]>
Date: Thu, 02 Dec 2010 10:48:20 -0700

[ + [email-masked] ]

On 12/2/10 8:44 AM, Jari Arkko wrote:
> Comment:
> Section 3.2.1 says:
> 
>    6.  If the initiating entity fails to connect using that IP address
>        but the "A" or "AAAA" lookup returned more than one IP address,
>        then the initiating entity uses the next resolved IP address for
>        that hostname as the connection address.
> 
> I think this is slightly confusing. It sounds like the entity decides
> to use A (or AAAA) and sticks with that choice. I think what you want
> is ... the "A" or "AAAA" lookups returned more than one IP address...

Yes, that's correct. Fixed in several places.

> My colleague Ari Ker�nen reviewed this specification and had a few
> comments:
> 
> The abstract should state that this document obsoletes RFC 3920.

Agreed. Fixed.

> 1.4.  Terminology
> 
>     The term "whitespace" is used to refer to any character that matches
>     production [3] content of [XML]
> 
> It's not immediately clear what "production [3]" means here (same issue
> in sections 4.1 and 11.5) since it looks like a numeric reference.
> Perhaps something like "content of production [3] defined in [XML]"
> would be clearer.

Several folks have been confused by that format in the past. I suggest
changing all of these citations as follows:

1.4 ... matching the "S" production from [XML]

4.1 ... matching the "document" production from [XML]

11.5 ... matching the "XMLDecl" production from [XML]

11.5 ... matching the "SDDecl" production from [XML]

> 4.6.3.  id
> 
> Aren't there any restrictions on the content of the ID attribute, e.g.,
> max/min length (in addition to what is defined in [RANDOM]) and allowed
> characters?

The WG considered that topic during the revision process and decided not
to add restrictions.

> 4.6.5.  version
> 
>     defined value of the 'type' attribute for message, presence, or IQ
>     stanzas).  The minor version number MUST be ignored by an entity with
> 
> The "IQ" stanza is mentioned multiple times before it's definition. A
> brief explanation, e.g., in the section 2 and a reference to section
> 8.2.3 would help uninitiated readers. It might make sense to move the
> whole section 8.2 much earlier to the draft (say, to the beginning of
> section 4), so that all the examples would be more clear.

Based on the Gen-Art review, in my working copy I've added the following
paragraphs:

To Section 1.4...

   The term "XML stanza" (also "stanza") is defined under Section 4.1.
   There are three kinds of stanzas: message, presence, and IQ (short
   for "Info/Query").  These communication primitives are defined under
   Section 8.2.1, Section 8.2.2, and Section 8.2.3, respectively.

To Section 4.1...

   There are three kinds of stanzas: message, presence, and IQ (short
   for "Info/Query").  These stanza types provide three different
   communication primitives: a "push" mechanism for generalized
   messaging, a specialized "publish-subscribe" mechanism for
   broadcasting information about network availability, and a "request-
   response" mechanism for more structured exchanges of data (similar to
   [HTTP]).  Further explanation is provided under Section 8.2.1,
   Section 8.2.2, and Section 8.2.3, respectively.

> 4.8.3.12.  not-authorized
> 
> In the example, the stream element sent by the server is missing ">".
> Same problem also at least in sections 4.8.3.25, 6.4.1, 6.4.6, 9.1.1.,
> and 9.1.2.

Yes, someone in the WG noticed those the other day, and I've fixed them.

> 4.8.3.19.  see-other-host
> 
> It would be good to be more explicit about the format of the IP address
> and port (especially) with IPv6. The example shows how IPv6 address
> should be sent (with the brackets) but the text doesn't say anything
> about that and implementers may easily get it wrong. Also, it could make
> sense to recommend to follow RFC 5952 convention with the IPv6 addresses.

Those are good recommendations. I suggest adding the following sentence
to the first paragraph of 4.8.3.19:

   (The format of
   an IPv6 address MUST follow [IPv6-ADDR], which includes the enclosing
   the IPv6 address in square brackets '[' and ']' as originally defined
   by [URI].)

> The "required" element could be more explicitly defined (now it's
> mentioned in the context of TLS, but I'd assume it can be used with
> other features too).

###

Section 4.3.2 states:

   2.  Specify a way for the receiving entity to flag the feature as
       mandatory-to-negotiate for this interaction (e.g., this is done
       for TLS by including an empty <required/> element in the
       advertisement for that stream feature); it is RECOMMENDED that
       stream feature definitions for mandatory-to-negotiate features do
       so by including an empty <required/> element as is done for TLS.

      Informational Note: Because there is no generic format for
      indicating that a feature is mandatory-to-negotiate, it is
      possible that a feature which is not understood by the initiating
      entity might be considered mandatory-to-negotiate by the receiving
      entity, resulting in failure of the stream negotiation process.
      Although such an outcome would be undesirable, the working group
      deemed it rare enough that a generic format was not needed.

###

In fact the <required/> flag is not a generic approach for all stream
features (it is not used by SASL or resource binding). The WG had some
discussions about generalizing the approach and defining a <required/>
element that would be qualified by a different namespace and re-usable
by all stream features, but decided against it. However, I think the
first paragraph could be clarified a bit:

   2.  Specify a way for the receiving entity to flag the feature as
       mandatory-to-negotiate for this interaction (e.g., for TLS, this
       is done by including an empty <required/> element in the
       advertisement for that stream feature, but that is not a generic
       approach for all stream features); it is RECOMMENDED that stream
       feature definitions for new mandatory-to-negotiate features do so
       by including an empty <required/> element as is done for TLS.

> 8.3.3.13.  policy-violation
> 
>     (In the following example, the client sends an XMPP message that is
>     too large according to the server's local service policy.)
> 
> The example does not seem to match with the text above. Also, the type
> in the example is "cancel" while it "SHOULD be modify or wait" according
> to the spec.

Fixed:

###

8.3.3.13.  policy-violation

   The entity has violated some local service policy (e.g., a message
   contains words that are prohibited by the service); the server MAY
   choose to specify the policy in the <text/> element or in an
   application-specific condition element; the associated error type
   SHOULD be "modify" or "wait" depending on the policy being violated.

   (In the following example, the client sends an XMPP message
   containing words that are forbidden according to the server's local
   service policy.)

   C: <message from=[email-masked]/foo'
               to=[email-masked]'
               id='vq71f4nb'>
        <body>%#&@^!!!</body>
      </message>

   S: <message from=[email-masked]'
               id='vq71f4nb'
               to=[email-masked]/foo'>
        <error by='example.net' type='modify'>
          <policy-violation
              xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/>
        </error>
      </message>

###

Peter

-- 
Peter Saint-Andre
https://stpeter.im/