Subject: [sipcore] info-events WGLC comments

From: Robert Sparks <[email-masked]>
Date: Fri, 31 Jul 2009 00:50:06 +0200

(as an individual)

I found having this WGLC right before and over IETF hindered my  
ability for a thorough review from scratch style review.
I suspect others are in the same boat.

To show that at least a few people have read this, here are the  
comments I have so far. This is more hastily written
than I'd like, so apologies for using document rather than priority  
order and apologies  if any of this isn't clear.

In case it's not clear from reading the below, I do not believe this  
document is quite ready to leave the working group.

1) Paragraph 1 of Section 1: "Note the INFO method does not change the  
SIP session state" - I think you mean
"INVITE usage state" or "session usage state". Otherwise, this  
continues to introduce confusion about
whether it changes dialog state (which it does).

2) The document needs a thorough scrubbing for "header field" vs  
"header". Paragraph 6 of section 1 is
a first example, speaking of the Recv-Info header instead of the Recv- 
Info header field. Similar care
needs to be taken when writing about header field _values_.

3) Nit - last paragraph before section 3 - spurious comma in last  
sentence

4) I find the structure of section 3 confusing in the extreme. The  
section might be better titled
"Event package negotiation" and the attempt to use UAC and UAS vs  
Initiating and target to
distinguish the actors in using the package and negotiating the  
package is not working well.
I strongly suggest rewriting this section from scratch and finding a  
way to identify the actors
that doesn't conflate so many things.  As written, I find it very  
likely that implementers will give
up and write code that guesses at the attempt.

To hopefully inform that rewrite (rather than informing just minor  
edits), here are some smaller
things I noted in the section

4-a) 2nd sentence, 3rd paragaph of 3.1: "This UA" is ambiguous. If the  
sentence as a whole is
really important, the point needs to be expanded.

4-b) 5th sentence, 4th paragraph of 3.1 (starting "That is,") : This  
is an incomplete sentence.

4-c) 2nd sentence, 5th paragraph of 31 (starting "This enables") : I  
could not make sense
out of this sentence during my first read. Only on writing these  
comments did I untangle it.
Suggest adding "INFO requests using this package mechanism" after the  
words "receive
any". (The confusion comes from grouping the words "any from a legacy  
UAS"). It would be
even better to reword the sentence to avoid this issue.

4-d) Paragraph one of 3.2 : This paragraph is particularly hard to  
follow. Suggest just
writing the extra words to call out sending an INVITE-usage creating/ 
updating response
or receiving one. (Taking this response focus can remove some of the  
confusion that
"request or response" is creating now).

4-e) 3rd sentence, 7th paragraph of 3.2: "either of these headers"  
doesn't make sense
since you've talked about 3 of these header fields explicitly and  
readers will probably
think of the implicit 4th.

4-f) Last sentence of 3.2 : (This one is fairly major) - This sentence  
can be read to say
a UAC following this spec MAY send a legacy INFO method outside of a  
usage created
by an INVITE request. This document OBSOLETES 2976. The behavior of an  
element
implementing _this_ specification really can't refer to that document  
for behavior and
it _especially_ can't ask the reader to infer behavior from that  
document. More words
are needed here (and probably elsewhere in the document) to avoid the  
very strange
situation of really needing a normative reference to the draft this  
one is obsoleting.

4-g) Last sentence of 3.3: This paragraph does not say quite what you  
mean. You are trying
to say an element can't assume it knows what "foo2" means only because  
it knows what
"foo1" means, or vice-versa. You are also trying to say package  
designers must not attempt
to encode this kind of extensibility into their package names I think.

4-h) Section 3.4 (and a few other places): What does it mean to  
"receive from" an
Info Package? Instances of this language, like "receive from Info  
Packages P and R"
should be changed to something like "receive Info requests indicating  
packages P and R"

5) Section 4.1, 3rd paragraph, 2nd sentence: This sentence starts "In  
general". When is
the rest of the sentence not the case (what is the exception to that  
"general")?

6) Section 4.1, 5th paragraph, last sentence: Nit (but an important  
one) This should say
once the UAC has received the field value, not once the UAS has sent it.

7) Section 4.1, 6th paragraph: This paragraph should note that an INFO  
might beat
a dialog-establishing provisional (or even final) response back  
(because of reordering,
or more likely because of Route/Record-Route mechanics that gave it a  
shorter path
than the response to the INVITE has to follow).

8) Section 4.2, 1st paragraph: It's really not clear this paragraph  
still reflects the
architectural consensus of the community given some other recent  
conversations.
I think we should explicitly reconfirm it's what we mean to say.

9) Section 4.2, 2nd paragraph: As written, this  paragraph can be read  
to say
that if a package allows requests to have a payload then all requests  
MUST
have a payload. Did you intend to preclude packages where the payload is
optional?

10) Section 4.2, 7th paragraph (MAY omit Content-Disposition): Would  
adding
"for this part" to the end of this sentence be wrong? I think it would  
avoid a problem
otherwise.

11) Section 4.2 - the two NOTEs. These are written to argue a point in  
the discussion
of the draft. They need to be rewritten as conclusions

12) 4.3 paragraph 1. where you say "on an existing session." I suggest  
using
"on an existing session dialog usage" or "on an existing session usage".

13) 4.3 paragraph 2: This paragraph precludes package specific error  
codes
to the INFO request itself, which I think was the architectural  
intent, but it
should be explicitly called out here.

14) 4.3 paragraph 5, last sentence. This sentence implies that strict  
compliance
with this framework does not allow the use of legacy INFO at all. Is  
that what
the group meant? Should the document say instead "If the UAS wishes to  
disallow
use of legacy INFO requests, ..."

15) 4.3 paragraph 6: s/session/session usage/

16) 4.4 Paragraph one: Please point out an otherwise or remove the
"Unless stated otherwise" at the start of this paragraph. If the intent
was to leave room for describing how you add bodies, then call
that out separately from the other basic dialog/routing mechanics.

17) Why is section 4.5 here. The document doesn't discuss the semantics
of an endpoint putting this header in a REGISTER request and this is a
gross under-specification of what a proxy/registrar might do with the  
data.
I will accep leaving it open to the future for this header to appear  
in a REGISTER
request, but we either need to say what it means or say explicitly  
that it
has no defined semantic.

18) Section 4.7 Paragraph 1: This paragraph is confused. It _is_  
possible to
determine order of the INFOs from the CSeq sequence (in case packets got
reordered). What is not possible is objecting to a gap. If something  
is willing
to wait for the NIT timeout, its possible to know that a gap won't be  
filled, so
it is possible (with that timing constraint) to _detect_ gaps. You  
just can't build
something that would consider that an error because some non-dialog- 
destroying
error might be generated at an intervening proxy.

19) Section 4.7 last paragraph, last sentence : What does it mean to  
violate a package?

20) I did not review the tables. Who has?

21) I did review one bit of text below the tables. The * explanation  
is wrong I think.
       Info-Package is not OPTIONAL for legacy messages, it is  
forbidden there. To
       prevent someone from thinking this OPTIONAL means it's OK to  
put Info-Package
       in a legacy request, I suggest changing this o* to an m* and  
noting that the header
       field is mandatory in package using requests and forbidden in  
legacy requests.

22) Section 6 Paragraph 2: Why is this MAY? What else is a server  
going to interpret
        such an INFO as? If this really is a MAY, please justify in  
the text why it is not MUST.

23) Section 7.7: Why is this MAY not MUST? If this really is a MAY,  
the document needs
to explain why it isn't MUST. Any package will at a minimum say what  
parts of the INFO
to pass to an application for further processing or something of that  
nature.

23) Section 7.7: "Since INFO does not change SIP state" is incorrect.  
It changes dialog state.
You meant the state of the invite session usage I think.

24) Section 7.7 Paragraph 2 points out that INFO can be used as the  
transport for other
transaction-based protocols. This needs a reference to the section on  
when to use something
other than INFO and it should probably repeat the statements on the  
MTU induced limitations.

25) Section 7.11 Paragraph 1: "Complete messages" are overkill and  
introduce an opportunity
for error in the specification that is much larger than the usual gain  
of explanatory power. I suggest
changing the sentence to focus on "all relative parts" rather than  
"complete".

26) Section 9.5: "Please add the following registration" doesn't seem  
to have a following registration.

27) I strongly suggest moving Appendix A into the main body. If we  
don't have it already, I suggest
requiring new packages to discuss their consideration of this advice.

28) A.4 "your only option" feels too strong here. Better scoping of  
the sentence, or softening the only
would help.

29) A.5.4 Second paragraph : Um, isn't this document creating an INFO  
framework? Maybe its
just too late as I'm writing this, but I think this is legacy text and  
it doesn't match the rest of the document.
Similarly (and backing up a little), the first paragraph reads very  
strangely for the current document.

30) Nit: Acknowlegments:  "My, we have been working on this for a long  
time" will not age well after this is published.

RjS