Subject: [xmpp] Another 3920bis review

From: Waqas Hussain <[email-masked]>
Date: Tue, 8 Jun 2010 23:50:40 +0500

A list of issues and concerns follows:

1.4. Terminology

	> the term "silently ignore" means (1) for a routing entity (typically
	a server) to route the data unmodified to another server or to deliver
	the data unmodified to a client

	That’s a rather unintuitive definition of “silently ignored”. The spec
	uses this definition in some places, whereas in others the same phrase
	is used as meaning “discard the data” (e.g., section 7.1).

3.1. Scope

	Perhaps s/connection at the/connection to the/

3.3. Directionality

	s/inital/initial/

	Of the two ways for server-server verification mentioned, the legacy
	one should be last.

4.1. Overview

	s/negotation/negotiation/

4.2.2. Stream Features Format

	Can we please have a namespace for the <required/> element?

	It would be useful to make <required/> a MUST for MTN features.

4.2.3. Restarts

	> For the sake of backward compatibility, the receiving entity MUST
	accept stream restarts at any stage in the stream negotiation process
	even if the receiving entity has not indicated that a stream restart
	is mandatory at that stage.

	Uh? Entities do not indicate the necessity of stream features. This is
	defined by the features themselves. And expecting stream restarts at
	all stages isn't feasible without some serious hacks. I think this is
	a leftover from previous edits to the spec which included attributes
	to indicate stream restarts.

4.2.4. Resending Features

	Just to clarify, the server MUST send stream features after resource
	bind?

4.2.5. Completion of Stream Negotiation

	> (if the receiving entity has new features to offer, it can simply
	close the stream using a <reset/> stream error and then advertise the
	new features when the initiating entity reconnects, preferably closing
	existing streams in a staggered way so that not all of the initiating
	entities reconnect at once)

	This is fairly costly (all session state is lost, which as a user
	would annoy me quite a bit, and state wouldn’t necessarily be fully
	recoverable). This should be limited to new features which are MTN.
	And even for MTN features, possibly only if the admin requests or
	policy requires that existing connections be kicked.

4.2.6. Determination of Addresses

	s/negotation/negotiation/

4.3.3. Handling of Idle Streams

	> To close an idle stream with a local client or remote server, a
	server MUST close the stream without error as explained under Section
	4.3.2.

	It’s a SHOULD NOT in 4.3.2, and a MUST here.

4.4. Stream Attributes

	> Security Note: Until and unless the confidentiality and integrity of
	a stream header is ensured via Transport Layer Security as described
	under Section 5, an entity SHOULD NOT trust the attributes provided in
	a stream header.

	Assurance of confidentiality and integrity does not mean the data can
	be trusted. Trust must require authentication.

4.4.1. from

	> it MAY include the 'from' attribute and set its value to that
	identity

	This could be fairly useful, but isn’t very useful as a MAY (very few
	clients bother to support this).

4.4.3. id

	s/nonrepeating/non-repeating/

4.5.1. Declaration of Streams Namespace

	> A streams namespace declaration is REQUIRED in all XML stream
	headers, and the name of the streams namespace MUST be
	'http://etherx.jabber.org/streams'. If this rule is violated, the
	entity that receives the offending stream header MUST return a stream
	error to the sending entity, which SHOULD be <invalid-namespace/> but
	MAY be <bad-format/>.

	Is this requiring that we detect if a *prefix* is defined? If yes,
	then this effectively forbids using Expat (and many other parsers) in
	namespace aware mode (which is a fairly serious issue with the spec)..

4.5.2. Declaration of Default Namespace

	> If an entity receives a stream header with a default namespace it
	does not support, it MUST return an <invalid-namespace/> stream error.

	When using parsers in namespace aware mode, entities may not know
	anything about what the default namespace is. Parsers in namespace
	aware mode are only required to inform the application about
	namespaces associated with specific elements and attributes. Please
	don’t forbid our use of Expat in namespace aware mode.

4.5.3. Declaration of Other Namespaces

	> a stream header MUST NOT include namespace declarations for
	namespaces that are different from the streams namespace and the
	default namespace

	Dialback support can be advertised by this (as described in XEP-0220),
	and some servers require this for dialback (this was previously the
	only way to advertise dialback).

4.6.3.23. unsupported-feature

	I’d interpret the provided example as having one optional to negotiate
	feature, and no MTN features.

5.3.3.2. TLS Failure

	s/negotation/negotiation/

6.2.5. Data Formatting

	s/formattting/formatting/

6.2.6. Security Layers

	s/i.e,/i.e.,/

6.3.1. Exchange of Stream Headers and Stream Features

	s/negotation/negotiation/

7.5. Server-Generated Resource Identifier

	> A server that supports resource binding MUST be able to generate an
	XMPP resourcepart on behalf of a client.

	Replace “A server that supports” with “Servers”, as resource binding
	support is mandatory.

8.1.1.2. Server-to-Server streams

	s/atttribute/attribute/

8.1.2.1. Client-to-Server Streams

	> 2. [...] (a) validate that the value of the 'from' attribute
	provided by the client is that of a connected resource for the
	associated entity

	Does this mean a resource is allowed to send stanzas on behalf of
	another resource? This is rather surprising.

	> 5. [...] it MUST assume that the stanza is from the server

	It MUST assume that the stanza is from the user’s account on the
	server. A stanza from the server comes from the server’s hostname, as
	described in point 4.

8.2.3. IQ Semantics

	The proper response for an IQ with a missing ‘id’ attribute isn’t
	defined. In Prosody we assume it’s the empty string for compatibility
	with some clients.

8.3.3.8. jid-malformed

	Is it really okay to send a stanza to an invalid JID (even if it’s the
	jid-malformed error)?

8.3.3.13. policy-violation

	s/servive/service/

8.4. Extended Content

	s/qualifed/qualified/

13.7. Certificates

	s/geneneration/generation/

13.7.2.1.1. Case #1

	s/resposibility/responsibility/

--
Waqas Hussain

---

From: Dave Cridland <[email-masked]>
Date: Wed, 09 Jun 2010 12:43:15 +0100

> 	defined. In Prosody we assume it’s the empty string for  
M-Link bends the spec by returning the result (or error) with no id,  
for the same reason.

> 	s/geneneration/generation/
I kind of prefer the original, in a strange way...

Dave.
-- 
Dave Cridland - mailto:[email-masked] - xmpp:[email-masked]
  - acap://acap.dave.cridland.net/byowner/user/dwd/bookmarks/
  - http://dave.cridland.net/
Infotrope Polymer - ACAP, IMAP, ESMTP, and Lemonade