Re: [Gen-art] Gen-Art review of draft-ietf-sipcore-rfc3265bis-07.txt

[Adam Roach <[e-mail]>]

On 4/10/12 5:46 AM, Alexey Melnikov wrote:
>
> 3.2.1.  Identification of Reported Events, Event Classes, and Current
>         State
>
>    When present, the body of the NOTIFY request MUST be formatted into
>    one of the body formats specified in the "Accept" header field of the
>    corresponding SUBSCRIBE request.  This body will contain either the
>    state of the subscribed resource or a pointer to such state in the
>    form of a URI (see Section 5.4.13).
>
> Nit: or the default according to the event package definition, if no 
> Accept
> header field was specified.

I've added text to point this case out.

> Also, it might be good to reference RFC 3986 for URIs here.

Given that SIP uses URIs everywhere, I'm not sure what benefit is 
derived by referencing the URI syntax draft here.

> 4.1.1.  Detecting Support for SIP Events
>
>    The extension described in this document does not make use of the use
>    of "Require" or "Proxy-Require" header fields; similarly, there is no
>
> Nit: too many "use of".

Fixed.

> 4.1.3.  Receiving and Processing State Information
>
>    To prevent spoofing of events, NOTIFY requests SHOULD be
>    authenticated, using any defined SIP authentication mechanism.
>
> Minor: How can this SHOULD be satisfied? Any reference which might be 
> appropriate here?

Added: "...such as those described in sections 22.2 and 23 of [RFC3261]."

>
> 4.2.1.3.  Authentication/Authorization of SUBSCRIBE Requests
>
>    SIP authentication mechanisms are discussed in [RFC3261].  Note that,
>    even if the notifier node typically acts as a proxy, authentication
>    for SUBSCRIBE requests will always be performed via a "401" response,
>    not a "407;" notifiers always act as a user agents when accepting
>
> Nit: Is the ";" after "407" a typo?

Thanks -- that should have been outside the quotation marks (and 
probably should be a full stop, which I've changed it to).

>
> 4.4.4.  Allow-Events header field usage
>
>    The "Allow-Events" header field does not include a list of the etvent
>
>  typo: event

I don't find "etvent" in the document:

http://tools.ietf.org/html/draft-ietf-sipcore-rfc3265bis-07#section-4.4.4

Is it possible you accidentally edited a local copy of the file prior to 
your review?

I also don't find this typo in the source (which would surprise me in 
any case, as I made sure to run the -07 version through a spell checker).
>
>    template packages supported by an implementation.  If a subscriber
>    wishes to determine which event template packages are supported by a
>    notifier, it can probe for such support by attempting to subscribe to
>    the event template packages it wishes to use.
>
> Can you clarify how such request would look like? An example would be 
> nice.

I'm not sure what you're asking for here. It would be a SUBSCRIBE 
message, with an "Event" header field set to name the template you want 
to use. Basically it just says "we don't negotiate templates -- just try 
it and see if it fails."

> 5.4.3.  SUBSCRIBE Request Bodies
>
>    It is expected that most, but not all, event packages will define
>    syntax and semantics for SUBSCRIBE request bodies; these bodies will
>    typically modify, expand, filter, throttle, and/or set thresholds for
>    the class of events being requested.  Designers of event packages are
>    strongly encouraged to re-use existing MIME types for message bodies
>    where practical.
>
> Nit: MIME types are now called "media types" in more recent IETF RFCs.

Thanks. Fixed in three places.

> I would recommend pointing to the Media Type Registration Procedure 
> document [RFC 4288] here, which points to the IANA registry.

Done.

>
> 5.4.5.  NOTIFY Request Bodies
>
>    Event packages also MUST define which MIME type is to be assumed if
>    none are specified in the "Accept" header field of the SUBSCRIBE
>    request.
>
> The same nit as above.

Fixed.

>
> 7.2.  Reason Codes
>
>    This document further defines "reason" codes for use in the
>    "Subscription-State" header field (see Section 4.1.3).
>
>    Following the policies outlined in "Guidelines for Writing an IANA
>    Considerations Section in RFCs" [RFC5226], new reason codes require a
>    Standards Action.
>
> Minor: This would prevent registration of new Reason Codes in an 
> Experimental RFC (for example). I would like to double check that that 
> is intentional.

It never came up explicitly during working group discussions, to my memory.

>    Registrations with the IANA include the reason code being registered
>    and a reference to a published document which describes the event
>    package.  Insertion of such values takes place as part of the RFC
>    publication process or as the result of inter-SDO liaison activity.
>
> I don't think Standards Action allows for "inter-SDO liaison 
> activity", unless such documents from other SDOs are published as 
> Standard Track RFCs. So I find your text confusing: either your 
> registration procedure should also allow for direct IESG approvals (to 
> allow registrations from other SDOs with no RFCs), or you should 
> remove "as the result of inter-SDO liaison activity".

You're right. I suspect we really intended to make this registrable by 
external SDOs, meaning we probably really wanted Specification Required. 
I'm leaving this as-is for now, and will need to coordinate with our AD 
to iron out where to go with this.

> 8.4.  Augmented BNF Definitions
>
>    event-type        =  event-package *( "." event-template )
>
> Minor: Does this mean that multiple template packages can be applied?
> Is there any ordering for them?
Yes, and yes.
> How would "foo.A.B" differ from "foo.B.A"?

Right now, we have only one template defined -- but imagine that we did 
define a new "list" template event package (we almost did this some 
years ago) which is used to aggregate several resources into a single 
subscription.

"Event: presence.winfo.list" would subscribe to the aggregation of 
several watcher-info documents into a single list.

"Event: presence.list.winfo" would subscribe to the watcher-info state 
of the indicated list.

> Nit: id-nits complains:
>
>   -- Duplicate reference: RFC4660, mentioned in 'RFC4660', was also 
> mentioned
>      in 'RFC 4660'.
>
> "[RFC 4660]" reference is used in section 7.2.

id-nits is wrong. It incorrectly thinks that the paragraph starting with 
[RFC4660] on page 40 is trying to define a reference.

/a