Subject: Re: [sipcore] Comments on draft-ietf-sipcore-rfc4244bis-02
From: Mary Barnes <[email-masked]>
To: "Elwell, John" <[email-masked]>
Cc: "[email-masked]" <[email-masked]>
Date: Tue, 2 Nov 2010 14:25:05 -0500
List-Id: SIP Core Working Group <sipcore.ietf.org>

Hi John,

Responses inline below [MB].  I will skip the ones that I replied to
in the other response [MB].

Thanks,
Mary.

On Mon, Nov 1, 2010 at 5:13 PM, Elwell, John
<[email-masked]> wrote:
> Despite the effort that has gone into addressing comments raised by people on -01, I still find -02 hard to understand in places. I have not had time to review the whole document, but here are some comments up to section 7. Just as I finished writing this I saw Mary's response to my comments on -01, which I will respond to tomorrow. Apologies for any overlap between the two.
>
>
> 1. As a general comment that I have made before, the document still talks about 'header' in many places where it is not referring to the entire header for a SIP message, but only to a particular header field such as History-Info. History-Info is only one field within the entire SIP header. See RFC 3261 section 6 - definitions of header and header field. Although RFC 3261 is not 100% consistent, in general things like Contact, To, From etc. are referred to as header fields, and hence History-Info should be too. Section 4.3 of RFC 4485 also calls for attention to be given to the correct use of terms header, header field and header field parameter, although it fails to say what the correct use is and one has to rely on RFC 3261.
[MB] I replied to this in the other email. But, yes, this doc does use
Header to mean Header field, but that's done in many other documents.
However, I can change all of those to say "header field" if it makes a
difference in understanding the document. [This might be something to
put on the RFC editor list if folks are insisting that we always use
"Header field"]  The term header field parameter AFAIK is only used to
refer to the two new header field parameters defined in this document.
[/MB].
>
> Also there are some instances of "header parameter field", which is not a normal term - it should be "header field parameter".
[MB] That's a typo. [/MB]
>
> 2. There is some confusion concerning Reason - sometimes it is referred to as a parameter (e.g., 7.1 3rd bullet, 7.1 last paragraph), sometimes as reason header, sometimes as reason, sometimes as Reason header, sometimes as Reason...
[MB] Logically, Reason is a "parameter" for the hi-entries. However,
rather than redefine the "parameter", we reuse the Reason header by
escaping it in the URI - the term Reason header was used for brevity.
I did add text in the -02 to clarify that in cases where it is
confusing. I can change all instances to refer to "escape a Reason
header in the hi-targeted-uri" rather than just "add a Reason header".
[/MB]
>
> 3. The word "entry" is used inconsistently. Mostly it occurs in "hi-entry", which is fine. But we also have "targeted-to-URI entry", "entry" (alone), History-Info entry (which is probably fine, as long as we say what it is), "header entry". Also "hi-entries".
[MB] Which term do you prefer?  I will change all the entry(s) to
hi-entry. The targeted-to-URI entry possibly should be
hi-targeted-to-uri. [/MB]

>
> 4. As another general comment, there are too many normative statements using the passive voice, and therefore hard to understand. To quote one example of the sort of ambiguity that can arise from using passive, in 7.3.2:
> "For retargets that are the result of an explicit SIP response, a
>   Reason MUST be associated with the hi-targeted-to-uri."
> Is this saying that an entity that inserts History-Info MUST include in hi-targeted-uri an escaped Reason header field? Or is this saying that a recipient of Reason MUST associate it with an hi-targeted-to-uri. I guess the first interpretation is more plausible, but why not say what is meant, rather than fudging it?
[MB] The Reason header is added to the hi-entry whose
hi-targeted-to-URI is being retargeted due to the response.  It is
added by the entity receiving the response.  Note that it is added to
the entry prior to the entry that is being added as a result of the
retargeting due to the response - i.e., it's not added to the
"current" hi-entry.  It's added to the previous.  The sentences
following the one that you highlight are intended to say just that.
That's why the term "associated" is loosely used because the next
sentences are the normative part. So, really, that first sentence
shouldn't be "MUST be" and would be more accurate to say "is". [/MB]
>
> 5. Section 1:
> "there is a need for a standard mechanism within
>   SIP for communicating the retargeting history of such a request"
> What is meant by "such a request"? We have not previously mentioned "request". The preceding text has talked about calls, not requests, but perhaps it should talk about requests.
[MB] I will change "such a request" to "call requests". [/MB]
>
> 6. Section 3:
> "Current network applications provide the ability for elements
>   involved with the call to exchange  additional information relating to
>   how and why the call was routed to a particular destination"
> Change "exchange" to "obtain". I think that would fit better with the examples that follow.
[MB] Okay. [/MB]
>
> 7. Section 3:
> "Capturing aliases and Globally Routable User Agent URIs (GRUUs)
>      [RFC5627], which can be overwritten by a home proxy  upon receipt"
> The term "home proxy" is not used in RFC 3261.
[MB] I will change to "proxy in the home domain" which is a term used
in RFC 5627 (earlier versions of GRUU did use the term "home
proxy")[/MB]
>
> 8. Section 4:
> "The  following information is carried in the History-Info header as
>   detailed in Section 7.1:"
> I think it would be better to say "For each target used for forwarding a request, the  following information is carried in the History-Info header as detailed in Section 7.1:"
> This would make it clear that there can be more than one instance of this information.
[MB] Okay. [/MB]
>
> 9. Section 4;
> "Target : A parameter indicating ..."
> This overloads the word target. Perhaps a better generic term for these parameters would be "Derivation".
[MB] I will just change to "Indicates.." which matches the form of the
other bullets in that list. [/MB]
>
> 10. Section 4:
> "A SIP entity changing the target of a
>   request in response to a redirect or REFER"
> A REFER does not cause an entity to change the target of a request - it causes an entity to generate a new request. H-I will indeed be added, but that is covered by the statement above about creating a new request.
[MB] Yep. [/MB]
>
> 11. "propagates any
>   History-Info header from the initial Request in the new request":
> Contrary to what it says in the first half of the sentence, this does indeed acknowledge that we have a new request as a result of REFER. However, the statement is incorrect because there is no normative statement (e.g., in section 5) concerning this special behaviour for REFER. There is merely a requirement in Appendix A.
[MB] I'll add text. [/MB]
>
> 12. Section 4:
> "Note that an hi-entry is not included for the fork to
>   sip:[email-masked] "
> This is misleading, because there is indeed H-I sent to 192.0.2.7. I think what it should say is that the H-I sent to 192.0.2.3 and subsequently sent back to Alice does not contain an entry for target 192.0.2.7.
[MB] yep. [/MB]
>
> 12bis. Section 5.1:
> "In addition, the UAC
>   SHOULD add  a History-Info header"
> Why is this only a SHOULD, not a MUST? What would be the exceptions?
[MB] This was issue  #28  where Dale suggested it be change from a
"MAY" to a "SHOULD" (with which you agreed in the email thread).  But,
I don't have a problem to change this to a MUST. [/MB]
>
> 13. Section 5.1:
> "As a result, intermediaries and
>   the UAS at least know the original Request-URI, and if the Request-
>   URI was modified by a previous hop. "
> This is inaccurate, since intermediaries may have removed H-I for privacy or other reasons. Best to delete the sentence.
[MB] Okay. HOwever, entries should not be removed for privacy - that
was a change from RFC 4244 - they MUST be anonymized.  [/MB]
>
> 14. Section 5.1:
> "A UAC that wants to ensure  that privacy not
>   be applied to its identity MUST include a Privacy header with a priv-
>   value of "none"."
> I am not sure that 'none' ensures anything - it is only a request according to RFC 3323.
[MB] Then I'll change it from "ensure" to "request". [/MB]
>
> 15. Section 5.1:
> "A new hi-entry  MUST then be added for the URI from
>   the Contact header (which becomes the new Request-URI) "
> The Contact header field could contain >1 URIs. I think what we want to say is that the UAC MUST populate the new-hi-entry with the URI chosen as the Request-URI for the new request.
[MB] Yes. [/MB]
>
> 16. Section 5.1:
> "If the Contact header contained any of
>   the header parameter fields defined in this specification"
> Don't understand. The parameters defined in this specification are parameters of the History-Info header field, so would not appear in a Contact header field. Admittedly there is some mention in 7.3.4 of using the parameters in a Contact header field, but there is nothing that specifies the extended syntax of the Contact header field.
[MB] The header field parameters are defined for the Contact header
(and History-Info) in the IANA section.  It is introduced in 7.1 and
the usage thereof described in 7.3.4.  [/MB]
>
> 17. Section 5.1:
> "MUST include the header parameter field  as a header parameter field
>   associated with the current hi-entry as described in Section 7.3.4."
> Is "current hi-entry" the "new hi-entry" from a couple of sentences earlier?
[MB] Yes. Do you have a terminology preference? [/MB]
>
> 18. Section 5.2.1:
> "The last hi-entry reflects the most recent
>   target and MUST contain the Request-URI for the received request,"
> If I interpret this correctly, this is talking about what the UAS receives. What the UAS receives is outside the control of the UAS, so we can't use RFC 2119 language. Should be "will".
[MB] Yep. [/MB]
>
> 19. Section 5.2.1:
> "e.g., the last proxy does
>   not support History-Info"
> In the previous sentence we used the term "previous entity", so it would be better to stick to that term rather than "last proxy".
[MB] Okay. [/MB]
>
> 20. Section 5.2.1:
> "hi-index attribute"
> and
> "hi-target attribute"
> Up till now we have called these parameters rather than attributes
[MB] I'll change to parameter [/MB].
>
> 21. Section 5.2.1:
> "The
>   addition of the missing hi-entry ensures that the most complete
>   information can be provided in the response ..."
> Not sure that it is "most complete" - "more complete" would be better, since there could still be some missing entries from upstream.
[MB] Most complete as "possible" - but I can change to "more". [/MB]
>
> 22. Section 5.2.1:
> "Prior to any application usage of the information, the validity MUST
>   be ascertained"
> What MUST ascertain the validity - the UAS or the application? Use active voice. How does it do this? Is this testable? I don't believe we can use RFC2119 language here.
[MB] That is saying that the UAS MUST check that the entries that it
is aware of contain the information that might be needed by an
application (i.e., the last entry, target header field parameters,
etc.). It's testable from a software implementation perspective - not
a protocol perspective.  I can change the MUST to a lower case
recommended .. ie., it is recommended that the UAS...Note, that this
was one of the SHOULDS (from RFC 4244) that folks wanted to change to
MUST.  [/MB]
>
> 23. Section 5.2.2:
> "If the "histinfo" option tag is received in a request, the UAS MUST
>   include any History-Info received in the request in the subsequent
>   response"
> This is incorrect for a B2BUA - a B2BUA should be allowed to use information received from its other interface. Also it is not necessarily correct for a regular UAS, since I would have thought a UAS should be required to include the hi-entry inserted in accordance with 5.2.1, rather than using just the H-I received.
[MB] We can reword something like the following to capture your concern:

   If the "histinfo" option tag is received in a request, the UAS MUST
   include any History-Info received in the request and any hi-entries
   added by the UAS in the subsequent response.  In addition, in
   the case of a B2BUA, the UAS MAY include any hi-entries
   received by the UAC in a response in the subsequent response
   sent by the UAS.

[/MB]

>
> 24. Section 5.2.2:
> "The processing of History-Info in responses follows the methodology
>   described in Section 16.7 of [RFC3261], "
> We can't refer to 16.7 of RFC 3261, because that is for proxies, not UASs.
[MB] See reply in other email response [/MB]
>
> 25. Section 5.3:
> "the redirect server MUST add the appropriate target header
>   field parameter to each Contact header as described in Section 7.3.4."
> I don't think 7.3.4 tells me how to do this. Section 7.3.4 does mention the Contact header field, but only in the context of adding these parameters to the History-Info header field (if I understand correctly).
[MB] 7.3.4 tells you which one to add based upon the mechanism by
which the targets are determined.  The header field parameter is added
based upon the same criteria. [/MB]
>
> 26. Section 6:
> "A Reason MAY be associated with the hi-targeted-to-uri that has been
>   retargeted as shown in the example in Appendix B.1."
> It would be good to be more precise, since B.1 is long - refer to A6, perhaps.
[MB] Okay. [/MB]
>
> 27. Section 6.1.1:
> "The proxy MUST include an hi-index attribute
>      with a value of "1""
> It says we are inserting an entry, not replacing any existing entries. Value "1" will not apply if there is an existing entry.
[MB] Correct. If the prior hop did not include an hi-entry, then one
is added. Even if there are already other hi-entries, there is no way
to know how many hops might have been missed and thus, the indexing
needs to be reset to 1 to reflect such. [/MB]
>
> 28. Section 6.1.1:
> "The proxy MUST add a separate hi-entry in each
>      separate outgoing request for each of the current (outgoing)
>      targets in the target set."
> Ambiguous - there should be only one entry per request - not one per target in each request.
> Perhaps it should say "For each outgoing request relating to a target in the target set, the proxy MUST add a single hi-entry identifying that target. For this new hi-entry, the proxy MUST..."
[MB] Yes - your suggestion is more clear. [/MB]
>
> 29. Section 6.1.2:
> "When re-sending a request as "
> I don't think this is really resending - we are retargeting, and therefore changing some aspects of the original request.
[MB] I'll change to "sending the retargeted request" [/MB]
>
> 30. Section 6.1.2:
> "in the order indicated by the indexing (see
>      Section 7.3.3)"
> We really need a more precise reference - where in 7.3.3 does it tell me how to use the aggregated information?
[MB] 7.3.3. refers to RFC 3261 Step 7
       "Aggregate Authentication Header Field Values", which states:
         "the proxy MUST collect any WWW-
         Authenticate and Proxy-Authenticate header field values from
         all other 401 (Unauthorized) and 407 (Proxy Authentication
         Required) responses received so far in this response context
         and add them to this response without modification before
         forwarding.  "

It's actually easier to just reword that to match what is done for
History-Info header field something like the following:
         The SIP entity MUST collect any History-Info header
         field values from
         all other responses received so far in this response context
         and add them to this response without modification before
         forwarding.
[/MB]

>
> 31. Section 6.1.2:
> "If the received error response did not include
>      any History-Info header fields, the proxy MUST use the same
>      History-Info header fields that were sent in the outgoing request
>      that failed to build the outgoing request."
> First, the two instances of "outgoing request" are confusing - I think the second should be "new outgoing request" or "retargeted request".
[MB] I'll change to "new outgoing request" since the other request
could also have been retargeted.
> Second, what if some responses include aggregated information and others don't?
[MB]  That shouldn't matter.  The receiving entity will use what it
can. If the normative procedures are followed, it's fairly
deterministic as to when the aggregated information should be used.
[/MB]
>
> 32. Section 6.1.2:
> "Same as per Step 2 above for the normal forwarding case
>      Section 6.1.1."
> This is confusing, because 6.1.1 is not on "normal forwarding case" but on "initial requests".
[MB]  Section 6.1.2. use of "initial" is referring to both the case of
the "initial" hi-entry to be added (bullet 1) AND the case whereby a
Proxy received a request and it is adding the "initial" hi-entry for
the first ("initial") retargeting.  The "normal" forwarding is
specifically bullet #3.  I think it would be clearer if we just
removed the word "initial" from the title and first paragraph and add
an explicit reference to bullet 3. [/MB]
>
> 33. Section 6.1.3:
> "When re-sending a request as "
> Again I don't think "re-sending" is the correct term.
[MB] I'll change to the "generating new requests"  [/MB]
>
> 34. Section 6.1.3:
> "   Step 1:  Including Previous Entries
>      The proxy MUST include the History-Info header fields that were
>      sent in the outgoing request that is being redirected."
> This seems to be a significant difference from the 4xx/5xx/6xx case. In that case we include any hi-entries from downstream entities that have sent back the 4xx/5xx/6xx - why not in the 3xx case too?
[MB] See response to issue 25 in your previous set of comments. [/MB]
>
> 35. Section 6.1.3:
> "add a Reason header this
>      entry"
> Change "this" to "to this".
>
> 36. "MUST forward captured History-Info in subsequent,
>   provisional, and final responses to the request sent by the ultimate
>   UAS (see Section 5.2)"
> If a response does not contain any captured H-I (because the UAS doesn't support it), is there any requirement on the proxy to generate it from its own information?
[MB] A proxy would have added an hi-entry for the UAS if it follows
the normative procedures in this document. But, there is no way for
the proxy to generate any additional hi-entries (in cases where the
UAS internally retargets or B2BUA case).   [/MB]
>
> 37. Section 7.1:
> "Reason: An optional parameter for History-Info,"
> It is not in the ABNF as a parameter of History-Info - in fact it is an escaped header field.
[MB] Correct - this is a summary of information associated with each
hi-entry and it is described in   as being escaped. Logically, the
information is there. Certainly, it's clear that it's not defined
specifically in the ABNF for the header. [/MB]
>
> 38. Section 7.1:
> "he hi-target is added for a hi-entry when it is
>      first added in a History-Info header field, and only one value is
>      permitted"
> I believe it should say "only one parameter is permitted". True that parameter ('rc' or 'mp') can have only one value, but I don't think that is the point we are trying to make.
[MB] Correct. [/MB]
>
> 39. Section 7.3.2:
> "If the response contains a Reason header for a protocol
>   that is not SIP (e.g., Q.850), it MUST be captured as an additional
>   Reason associated with the hi-targeted-to-uri that has been
>   retargeted, along with the SIP Response Code"
> I think this is saying that the entity MUST include two Reason parameters in the hi-entry. Perhaps we should state that more explicitly. Also, does order matter?
[MB] I can add (i.e., there will be two reason headers), but I figured
when you add "an additional" to one, that it's obvious that there are
two headers. Per RFC  , order does not matter. [/MB]
>
> 40. Section 7.3.3 item 6:
> "When a response is built and it represents the aggregate of
>       responses to multiple forks "
> Suddenly we are talking about forks when previously we were talking about branches.
[MB] A branch reflects a fork.  I'll clarify the text  in item 6, such
that it reads:
"the index MUST be captured
       for each forked request, creating a new branch,  per the rules above..."
and change the use of forks to branch in the body of item 6.
[/MB]
>
> 41. Section 7.1.3, item 6:
> "For example, if
>       a procesing entity received failure responses for forks 1.1.1 and
>       1.1.2, it would return both the 1.1.1 and 1.1.2 entries to the
>       entity that generated the hi-entry with index of 1."
> I can't figure this out - shouldn't "index of 1" be "index of 1.1"?
[MB] No.  The processing entity in that sentence has the index of 1.1.
The processing entity is the subject of that sentence and "it" returns
the response to the entity that would have added the hi-entry with
index of 1.[/MB]
>
> 42. Section 7.1.3, item 6:
> "See
>       Appendix B.1 for an example"
> It is unclear where exactly in B.1 I am supposed to look - searching for '1.1.1' or '1.1.2' produces no results. Please cite the specific messages.
[MB] Okay. [/MB]
>
> Although I didn't review beyond section 7, I happened to notice the following in Appendix B:
> "B.  An entity (UA or proxy) retargeting in response to a redirect
>           or REFER MUST include any Request History information from
>           the redirect/REFER in the new request."
> It is unclear how you can retarget in response to a REFER. A REFER requests leads to the generation of a new request - there is no retargeting.
[MB] I'll reword that to reflect that the REFER includes the
History-Information from previous retargets. [/MB]
>
> John
> _______________________________________________
> sipcore mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/sipcore
>