Document: draft-ietf-xmpp-core-23
Reviewer: Michael A. Patton
Date: April 29, 2004

Summary: This draft is ready for publication as a PS RFC

[Note:  I'm not sufficiently up on XML details to be certain they got
them all correct.  They looked right to me, but I may have missed
something.  On the other hand, this is actually deployed, so I suspect
they did get it right.  The same applies to the security stuff, I
reviewed it enough to say that the description seems good enough for
an implementer to follow, but I won't say anything about how secure it
is.]


I did find some things that I thought worthy of comment.  All of these
are either not worth dealing with at this late stage, or trivial
enough to fix in the ID->RFC conversion.



The description of SRV is a bit spare...and really only as an aside in
security considerations...  However, I think it's fine for PS as-is,
and can either be expanded or deleted at PS->DS stage.


They describe a strict correlation between domains (and specifically
subdomains of a given domain) and the server to server activities.
I'm not sure I believe this, but as is it's fine for PS and if anyone
uses it as-is then it can stay for DS, if noone uses it, it would go
away in the normal process, and if something completely different gets
done, that can be handled by the impl. reports at that time.

I think the ref to [RANDOM] in 4.4 makes it normative.

Conversely, I don't see why [IMP-REQS] is normative rather than
informative.

In 4.7.2 they have an "example", and then describe the details saying
that a <text/> "SHOULD possess an 'xml:lang' attribute".  This
recommendation would be stronger if the example actually followed it,
ie. if the <text> in the example had a xml:lang= attribute.  9.3.2
does the same thing.

At the end of 6.2 when they speak about sending the features again,
they mention leaving out STARTTLS, but do they also want to leave out
the SASL related ones now that SASL has been concluded?  The example
later does do that.  A related question is whether the early feature
sets can (or SHOULD) leave out features not involved with security.
In section 7 they actually _imply_ this later is a MUST (the
parenthetical comment at end of second paragraph).  I suggest a short
additional sentence where the earlier feature transmissions are
described saying that if security is required before a feature, it
should not be mentioned.


The second paragraph of Section 3.1 is hard to follow, mostly because
it's essentially all one sentence.  This could be improved by
rewording the paragraph to break up the run on sentence.

Another problem with this paragraph is the phrase "the Nodeprep
(Appendix A) profile of [STRINGPREP]" and another similar one later,
read as if the appendix it references is in that other document, but
it actually means that Appendix A in this document, which happens to
refer to that other document.  It does actually say that, but not
until after the confusion has set in.  This same construct also
appears later, as well.  This could be fixed by just not mentioning
the [STRINGPREP] or rewording the reference to make it clearer.  The
12.1 text works a little better, but could still use some help.


Section 4.3 is the first mention of the fact that this is XMPP 1.0, I
think that fact should be mentioned earlier, say in 1.1.  Otherwise,
that reference needs clarification here...


Why does 12.1 say "a server SHOULD support...dialback" when elsewhere
it denegrates dialback and says it's only for compatability?  That
suggests a "MAY" to me, not a "SHOULD".


Harald, they ref your language tags doc.  If I remember correctly,
there is an updated one in prep.  Is that right?  Might want to note
that for RFC Editor, in case the new one can be refed instead.  But it
shouldn't slow this down.


Typos
-----

4.7.3 "the streams namespace name" => "the stream's namespace name"

14.9 One reference to Base 64 leaves out the 64, resulting in a
sentence starting "Base encoding".