Subject: [sipcore] Genart last call review of draft-ietf-sipcore-content-id-06

From: Elwyn Davies <[email-masked]>
Date: Thu, 22 Jun 2017 16:02:39 -0700

Reviewer: Elwyn Davies
Review result: Ready with Nits

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-sipcore-content-id-06
Reviewer: Elwyn Davies
Review Date: 2017-06-22
IETF LC End Date: 2017-06-27
IESG Telechat date: Not scheduled for a telechat

Summary:
Ready with nits.  There are a couple of minor issues related to the procedures
after inappropriate usage of the new header.

Major issues:
None

Minor issues:
s3.4.1, last para: In line with the last sentence of Section 3.2, the
Content-ID URL only needs to be unique within the SIP message where it occurs.
 I assume it does not make sense to have a Content-ID header combined with a
mutipart message-body (this isn't stated - maybe it should be); I am not
sufficiently knowledgeable in this area of SIP to know if there could be
content-id URLs in other headers if there is a Content-ID header in the
message.  Thus either the statement about global uniqueness is irrelevant (as
there is only one) and can be removed or it should be made consistent with s3.2
so that the Content URLs are unique within the message. Global uniqueness is
neither possible or required.

s3.4.1, para 1: Following on from the previous comment: Is a UA allowed to add
a Content-ID header to a message with a multipart message-body?

s3.4.1: What should a UA do if it receives a message with a Content-ID header
when the message is not allowed to contain one?  Is there some generic error
procedure that should be referenced?

s6.1: I started looking for specification that told me that items added to the
SIP header fields registry effectively extend the definition of the
message-header production in Section 25.1of RFC 3261.  Bit obvious perhaps, but
it would be nice if it had been said somewhere!  Time for an Erratum perhaps?

Nits/editorial comments:
Abstract:  I would suggest adding a sentence that clarifies what the update of
RFC 5621 is modifying: OLD: The document also updates RFC 5621, to enable a
Content-ID URL to reference a complete message-body and metadata provided by
some additional SIP header fields. NEW: The document also updates RFC 5621,
which only allows a Content-ID URL to reference a body-part that is part of a
multipart message-body.  This update enables a Content-ID URL to reference a
complete message-body and metadata provided by some additional SIP header
fields. END

s1.2, first bullet: s/for providing location/for providing location information/

s1.4.1: Need to expand UAC (User Agent Client) on first usage.

s1.4.1, para 1: s/can be e.g. of/can be, for example, of/

s3.4.1: Need to expand UA (User Agent) on first usage (in section title).

s4, NEW text: s/allow creating/allow the creation of/

---

From: Christer Holmberg <[email-masked]>
Date: Fri, 23 Jun 2017 11:45:10 +0000

Hi Elwyn,

Thank You for the review! Please see inline.

>Summary:
>Ready with nits.  There are a couple of minor issues related to the procedures after inappropriate usage of the new header.
>
>Major issues:
>None
>
>Minor issues:
>
> s3.4.1, last para: In line with the last sentence of Section 3.2, the Content-ID URL only needs to be unique within the SIP message where it occurs.
> I assume it does not make sense to have a Content-ID header combined with a mutipart message-body (this isn't stated - maybe it should be);

I am not aware of any current use-case, but I see no reason to forbid it. Perhaps someone will come up with a use-case in future.

> I am not sufficiently knowledgeable in this area of SIP to know if there could be content-id URLs in other headers if there is a Content-ID 
> header in the message.  Thus either the statement about global uniqueness is irrelevant (as there is only one) and can be removed or it 
> should be made consistent with s3.2 so that the Content URLs are unique within the message. Global uniqueness is neither possible or required.

The statement about global uniqueness is a bug, and will be fixed. The value only has to be unique within the message.

> s3.4.1, para 1: Following on from the previous comment: Is a UA allowed to add a Content-ID header to a message with a multipart message-body?

I see no reason to forbid it.

> s3.4.1: What should a UA do if it receives a message with a Content-ID header when the message is not allowed to 
> contain one?  Is there some generic error procedure that should be referenced?

Normal RFC 3261 procedures apply. I don't think we want to copy/paste the generic header processing rules,

> s6.1: I started looking for specification that told me that items added to the SIP header fields registry effectively
> extend the definition of the message-header production in Section 25.1of RFC 3261.  Bit obvious perhaps, but it 
> would be nice if it had been said somewhere!  Time for an Erratum perhaps?

In the ABNF of RFC 3261, "message-header" contains the header fields defined in the RFC, plus "extension-header", which is used for new headers. I assume people think that is clear enough :)

Having said that, what you suggest is not necessarily a bad idea, but it is outside the scope of this draft.

>Nits/editorial comments:
>
>Abstract:  I would suggest adding a sentence that clarifies what the update of RFC 5621 is modifying: 
>
>OLD: The document also updates RFC 5621, to enable a Content-ID URL to reference a complete 
>message-body and metadata provided by some additional SIP header fields. 
>
>NEW: The document  also updates RFC 5621, which only allows a Content-ID URL to reference a 
>body-part that is part of a multipart message-body.  This update enables a Content-ID URL to 
>reference a complete message-body and metadata provided by some additional SIP header fields. END

Looks ok. I will modify as suggested.

>s1.2, first bullet: s/for providing location/for providing location information/

Will be fixed as suggested.

>s1.4.1: Need to expand UAC (User Agent Client) on first usage.

I realized the first usage is in section 1.3, so I will expend it there.

>s1.4.1, para 1: s/can be e.g. of/can be, for example, of/

Will be fixed as suggested.

>s3.4.1: Need to expand UA (User Agent) on first usage (in section title).

Will be fixed as suggested.

>s4, NEW text: s/allow creating/allow the creation of/

Will be fixed as suggested.

Regards,

Christer