Subject: [xmpp] More 3920bis feedback

From: Matthew Wild <[email-masked]>
Date: Tue, 8 Jun 2010 17:45:37 +0100

Ok, I've reviewed from start to finish. As organised as the
multiple-thread approach Kev is taking is, I'm dumping the feedback in
a single post. If you want to comment on an issue it's probably best
to quote that section and start a new thread for it.

Section 1.3
	XEP-0001 seems to be lacking a link/reference.

Section 2.1 Global Addresses
	The domain "im.example.com" is perhaps a bad choice
	for example, since it may suggest to some that it is
	required (or expected) for XMPP to be on a sub-domain
	of the main domain (even though the text is fine in
	this regard).

Section 4.2.2 Stream Features Format
	Says that stream restarts happen for "TLS in all cases
	and SASL in the case when a security layer might be
	established". How do we know when the latter is true?
	
Secion 4.2.3 Restarts
	Restarts MUST be accepted by the receiving entity at
	any time. Does this conflict with the MUST NOT restart
	after resource binding?

	I also question the need for this "backwards compatibility".
	I've never seen stream restarts after features that didn't
	require them in the wild, and a "MUST" seems a bit
	too strong. Perhaps an implementation note would be
	more appropriate?

Section 4.6.3.4 connection-timeout
	Perhaps sending a stream error on a c2s stream /would/
	be helpful? (I can see that it is not such a big deal
	for an idle s2s stream to be closed, since it can be
	easily re-established again in either direction).

Section 4.6.3.14  policy-violation
	The example uses the error 'stanza-too-big' which is
	not defined, in a namespace that is not standard. To
	avoid confusion with a real error it may be worth
	changing it to a non-urn:xmpp:* namespace.
	urn:xmpp:example is used later on.

Section 4.6.3.23  unsupported-feature
	Same issue as policy-violation, but this more clearly
	uses "urn:xmpp:example".

Section 4.6.3.25  unsupported-version
	By defining contents and format, this condition may
	break the rule from 4.6.2 that <text> "MUST NOT be
	interpreted programmatically by an application."

Section 5.2.5 Renegotiation
	I think this has been brought up before, but is it
	really desired to put a complete ban on any renegotiation?
	(apologies if this is dealt with already)

Section 5.2.6 TLS Extensions
	Says that servers MUST support the server_name extension,
	however it doesn't give much information on how it would
	be used. It seems only useful in the context of 5223, which
	has never been standardized. Perhaps drop this requirement?
	Or at least reduce it to MAY.

Section 6.2.5 Data Formatting
	Tpyo: 'formattting'

Section 6.2.10 Round Trips
	There have been real-life cases of problems between clients
	and servers that do and don't support character data in
	<auth/> and <success/>. It seems to me that we should make
	it a requirement to support these (in addition to the inefficient
	forms), to avoid incompatibilities like this.

Section 6.3.5 Failure
	Says that when the client has no more mechanisms left to try, it
	SHOULD send an <abort/> element. As I understand it, <abort/> is
	used to end an in-progress negotiation. However at this point
	there would be no ongoing negotiation, and the server would just
	needlessly reply with another <failure><aborted/></failure> without
	changing any state.

Section 6.3.6 Success
	"The authorization identity communicated during SASL negotiation
	is used to determine the canonical address for the initiating
	client or server according to the receiving server" seems like it
	might contradict 4.2.6 and 6.2.8 regarding the use of
	authorization identities, especially in the context of s2s.

Section 8.1.1.2 Server-to-Server Streams
	Typo: 'atttribute'.

8.3.3.13  policy-violation
	Typo: 'servive'.

Section 8.4 Extended Content
	"If an entity receives a message stanza whose only child element
	 is qualified by a namespace it does not understand, it MUST
	 silently ignore the entire stanza." In the case of an unhandled
	 message stanza sent to the bare domain of the server we bounce an
	 error (service-unavailable) instead of silently dropping, as seems
	 required by section 10.2.1. This seems preferable to silently
	 dropping it to me.

Section 8.5 Stanza Size
	Is the limit inclusive or exclusive of the '<' and '>'? Common sense
	suggests inclusive, but the wording might be better.
	
	"If an entity receives a stanza that exceeds its maximum stanza size,
	 it MUST return a <not-acceptable/> stanza error." If the purpose of
	 the stanza size limit is to prevent server resource consumption,
	 replying with a stanza error is very tricky. Because the server would
	 have to continue reading and parsing the stanza to know when it has
	 finished, so it can send the reply. This can't be done without
	 significant resource consumption in any of the off-the-shelf XML
	 parsers I know. Although it sounds brutal, I suggest alternatively
	 allowing the server to give a policy-violation error. At some point
	 a XEP to help clients discover maximum stanza size would help greatly.
	
	 Actually after reading further it seems this is already suggested in
	 section 13.12, point 5. This section should probably be amended.

Section 9.1.3 Resource Binding
	The example bind request does not conatin the resource in a <resource>
	tag.

Section 13.9.2 Use of DNS
	"If the target domain name (e.g., "connect.example.net") is not in the
	queried domain (e.g., "example.com"), the connecting application MUST
	warn a human user (if any) about the mismatch and require the user to
	approve the connection before the client proceeds;". It isn't entirely
	clear to me how this text relates to servers. Are they exempt because
	they don't necessarily have a human user?

Section 13.11 Directory Harvesting
	Does it make sense to always return <service-unavailable/> for <message>
	stanzas to a non-existent user account? Since they wouldn't be returned
	if the user account existed, unless the server was configured to bounce
	messages from unsubscribed users.

Section 13.12 Denial of Service
	Point 6: Wouldn't policy-violation or even resource-constraint be better
	 here than unexpected-request?
	
Other issues:

The specification no longer explicitly allows or forbids CDATA, should it?

The specification is also silent on preservation of whitespace. XML is rather
vague on the matter but, as I read, does leave the door open for an application
to swallow whitespace on elements that don't have the magic
xml:space="preserve".