Extensible Messaging and Presence Protocol (XMPP): Core
draft-ietf-xmpp-3920bis-22

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...

My colleague Ari Keränen reviewed this specification and had a few
comments:

The abstract should state that this document obsoletes RFC 3920.

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.


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?


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.


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.


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.


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).


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.

Please consider clarifying that you are not requiring elements to reject streams that use a prefix with
the literal value "stream", only that you are allowing them to do so to allow legacy implementations to be
compliant. It might help to be even more explicit that clients that chose not to use the literal "stream"
prefix may have interoperability problems with those legacy implementations. Also consider pointing to this
exception to the general principal of allowing the sender to set the string it uses as a prefix label in the
section on page 124 (where you note that "receiving entities MUST NOT depend on the prefix strings to have
any particular value.")

Some additional explanation of SRV's resolving to "." meaning "don't use SRV" would help.
Why is this mechanism there?

In 4.2.5 - Please consider a little more explanation on why an initiating element might send stanzas to itself?

Please consider adding forward pointers from the keep-alive sections (such as 4.5.1) to the requirement
to not send whitespace during TLS and SASL negotiation. (5.3.3, 6.3.5)

In 4.8.3.9 - "not a valid JID" may be ambiguous - is it intended to include "not well formed"?

In 5.3.5 - what does "blindly attempt" mean? why is the requirement around it SHOULD NOT? 
When is it OK to blindly attempt?

In 5.4.3.3 - the "Implementation Note:" actually contains a normative requirement defining protocol behavior.
Similarly, 8.3.3.14 has a Security Note with normative requirements. I strongly suggest moving actual
protocol requirements out of those notes.

In 6.3.8 - Can you provide a reference to where is the behavior for acting on the behalf of another party is specified?

In 6.4.6  - Can you add some text justifying why the entity SHOULD terminate the connection attempt 
if the from identity and the SASL authentication identity do not match, and why this is a SHOULD not a MUST?

In 8.3.3.15 : This section doesn't provide a way to protect against redirect loops (such as see-other-host did). Should it?

In 10.4.1 : should this "will" be a MUST?

In 13.1 : has the group considered a "fail this stanze if it would go over a hop that isn't TLS protected" request extension?