Re: [sipcore] WGLC: draft-ietf-sipcore-locparam - Christer's review

[James Winterbottom <[e-mail]>]

Thanks Christer, these all look pretty reasonable to me.

Thanks for the review.

James


> On 19 Aug 2019, at 5:23 am, Christer Holmberg <[e-mail]> wrote:
> 
> Hi,
> 
> Below is my WGLC review. 
> 
> NUTSHELL:
> 
>> From a technical point of view, the document is ready to move forward. However, there are some editorial issues that I'd like the authors to address.
> 
> ---
> 
> GENERAL:
> 
> Q_GEN_1:
> 
> - Please always use capital 'G' for Geolocation header field.
> 
> Q_GEN_2:
> 
> - Instead of talking about "The parameter defined in this specification", use the name of the parameter.
> 
> Q_GEN_3:
> 
> - In some places the text talks about the loc-src parameter. I think it shall talk about the location-source parameter instead, as that is the name in the ABNF. loc-src is just the encoding.
> 
> Q_GEN_4:
> 
> - While RFC 6442 talks generally about "SIP intermediaries", this draft talks about "proxies". Is there a reason for that?
> 
> ---
> 
> ABSTRACT:
> 
> Q_ABS_1:
> 
> The text gives some background about a problem, but there is no words about what the draft does to address that problem. I think there should be a sentence saying: "This document blah blah blah...".
> 
> ---
> 
> SECTION 1:
> 
> Q_1_1:
> 
> The first sentence says:
> 
>   "The SIP geolocation specification [RFC6442] describes the
>   "Geolocation" SIP header field which is used to indicate that the SIP
>   message is conveying location information.  The specification
>   suggests that only one location value should be conveyed."
> 
> - The Geolocation SIP header field does not only indicate that the SIP message is used to convey location information, it also contains a reference to the location information.
> 
> - Does RFC 6442 really suggest that only one location value should be conveyed? The way I read it is that an intermediary should not add location information to a request that already contains it. Would it be more appropriate to say:
> 
> "The specification specifies that SIP intermediaries should not add location values to a SIP request that already contains location value."
> 
> Q_1_2:
> 
> The text says:
> 
>   "This document adds a location-source (loc-src) parameter to the
>   location values in [RFC6442] so that the entity adding the location
>   value to geolocation header field can identify itself using its
>   hostname."
> 
> - I think you shall be more specific, and say something like:
> 
> "This document extends the Geolocation header field, by allowing an entity adding the location value to identity itself using a hostname. This is done by defining a new geoloc-param header field parameter, location-source."
> 
> ---
> 
> SECTION 3:
> 
> Q_3_1:
> 
> - s/specific/specification
> 
> ---
> 
> SECTION 4:
> 
> Q_4_1:
> 
> The syntax says:
> 
> location-source = "loc-src=" (hostname )
> hostname = <defined in RFC3261>
> 
> - I suggest "loc-src" EQUAL (hostname).
> 
> - s/(hostname)/hostname
> 
> 
> A_4_2:
> 
> The text says:
> 
>   "Only a fully qualified host name is valid, an IP address MUST NOT be
>   added by an entity conforming with this specification.  If a node
>   conforming to this specification receives a geolocation header field
>   with a loc-src parameter containing an IP address then the parameter
>   MUST be removed."
> 
> - It is not only about not supporting IP addresses - the syntax does not even allow an IP address. Perhaps something like:
> 
> "Only a fully qualified host name is valid. The syntax does not support IP addresses, and if an entity conforming to this..."
> 
> 
> Q_4_3:
> 
> The text says:
> 
>   "Any proxy adding a location value to a geolocation header field
>   SHOULD also add its host name using the loc-src parameter so that it
>   is clearly identified as the node adding the location."
> 
> - I suggest to say "A proxy conformant to this specification adding a location value..."
> 
> - s/SHOULD also add its host name using the loc-src parameter/SHOULD also add a location-source header field parameter".
> 
> 
> Q_4_4:
> 
> The text says:
> 
> "A UE MUST NOT provide a loc-src parameter value."
> 
> I suggest "A UA MUST NOT insert a location-source header field parameter"
> 
> ---
> 
> SECTION 7:
> 
> Q_7_1:
> 
> The text says: "the that they". I assume that is a mistake.
> 
> Q_7_2:
> 
> The text says:
> 
> "To avoid problems of wrong interpretation of loc-src the value may be discarded when passed to an other domain."
> 
> Is "removed" more appropriate than "discarded"?
> 
> 
> Regards,
> 
> Christer
> 
> 
> 
> 
> 
> -----Alkuperäinen viesti-----
> Lähettäjä: sipcore <[e-mail]> Puolesta A. Jean Mahoney
> Lähetetty: sunnuntai 18. elokuuta 2019 20.57
> Vastaanottaja: [e-mail]
> Aihe: Re: [sipcore] WGLC: draft-ietf-sipcore-locparam
> 
> Hi all,
> 
> The WGLC is being extended to Friday, Sept 13, since many people go on vacation in August and early September.
> 
> Thanks!
> 
> Jean
> 
> 
> 
> On 8/15/19 2:43 PM, A. Jean Mahoney wrote:
>> Hi all,
>> 
>> This begins the Working Group Last Call of draft-ietf-sipcore-locparam 
>> (Location Source Parameter for the SIP Geolocation Header Field).
>> 
>> Please post any feedback to the sipcore mailing list by Friday, August 30.
>> 
>> https://datatracker.ietf.org/doc/draft-ietf-sipcore-locparam/
>> 
>> Thanks!
>> 
>> Jean
>> 
>> _______________________________________________
>> sipcore mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/sipcore
> 
> _______________________________________________
> sipcore mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/sipcore
> 
> _______________________________________________
> sipcore mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/sipcore