Subject: [Simple] Comments and NITS for draft-ietf-simple-message-sessions-10.txt

From: Hisham Khartabil <[email-masked]>
Date: Wed, 16 Mar 2005 14:34:18 +0100

Below you will find comments followed by nits. If there is a comment  
that you would like to discuss further, please reply on a separate  
thread.

Comments
==========

Section 4
---------

- " ...while REPORT requests report on the status of an earlier
    SEND request."

Is it just a SEND request (ie chunk) or could it be also for an entire  
message?

Section 5.3
-----------

- "The second type of status is
    referred to as "request status" and may be returned in a REPORT
    transaction."

The same comment as above. Is it request status, message status or both  
does the REPORT request report? This also applies to the paragraph that  
follows. This needs to be more explicit.

- "partial": please provide a concrete example of how that is used.

Section 5.4
-----------

- "   Likewise, the active endpoint MUST immediately issue a SEND  
request.
    This initial SEND request MAY have a empty body, or MAY carry
    content."

   please explain why this is needed (to aid the remote end in tying an  
msrp session to a connection?)

- "When the first request arrives, its To-Path header field should
    contain a URL that the listening element handed out in the SDP for a
    session."

   why only the first request? should the check be for the first and  
subsequent requests? I think so since responses use those headers, or??

- "If it were legal to have multiple connections associated with the
       same session, a security problem would exist.  If the initial SEND
       request is not protected, an eavesdropper might learn the URL, and
       use it to insert messages into the session via a different
       connection."

   You can mention immediately after the above that more than 1 msrp  
session can use the same connection and that endpoints need to keep  
track of that.

- "for example, in a SIP re-INVITE or UPDATE [11] request." I prefer to  
remove the reference to UPDATE since UPDATE is not recommended to be  
used once a dialog has been confirmed in SIP.

- "If the receiving endpoint receives more than one message
    with the same Message-ID.  It SHOULD assume that the messages are
    duplicates."

   should it still generate a 200 response if required (no  
failure-report header with value "no")?

Section 7
---------

- I am confused as to what the closing sequence is what is the boundary  
marker? Does it include the CRLF before it? Is there a CRLF after it? I  
suggest a small section defining the "closing sequence" then fixing  
section 7.1 to follow that definition. Also please also explain the  
difference between a closing sequence and a boundary marker (one is  
part of the other).

Section 7.1
-----------

- "The
    body MUST be separated from the headers with an extra CRLF.  Note
    that, if no body is present, no blank line will be present between
    the headers and the boundary marker below."

   So does a SEND message with no body still contain the CRLF after the  
headers? Also, does the boundary marker still appear even if there is  
no body in the request? What do you mean by blank line? is it the CRLF  
sequence? if so, please change the term to say CRLF.

- The following paragraph contradicts the one quoted above:

"Finally, requests which have no body MUST NOT contain a Content-Type
    header or any other MIME specific header.  Requests without bodies
    MUST contain a closing sequence after the final header."

or should there still be a blank like (or CRLF) between headers and  
closing sequence?

This whole section is unclear. As suggestion, please define what  
closing sequence is in a separate section. Refer to it in section 7.1  
and clarify the exact appearance of the closing sequence (including the  
CRLF if it is part of it or not) when there is a body and when there is  
no body. (I would like to see the text before incorporating in the  
draft).

- "abandoning" appears in this section. Do you mean aborting? if so,  
please change the word to aborting and clarify how that differs from  
interrupting (interrupting means that a message will continue to be  
sent in future chunks and a + is used in the closing sequence. Aborting  
is when the message will not complete and a # is used in the closing  
sequence).

- Just another example of the confusing closing sequence: ", and add  
the closing sequence, including the "$", "#", or "+" character, and a  
final CRLF." This tells me there is a CRLF after a $, for example.

Section 7.1.1
--------------

- "When an endpoint has a message to deliver, it first generates a new
    unique Message-ID.  This ID MUST be unique within the scope of the
    session.  If necessary, it breaks the message into chunks.  It then
    generates a SEND request for each chunk, following the procedures for
    constructing requests (Section 7.1)."

Needs to be mentioned that if the session was one that was constructed  
as a recovery session for a failed one (connection lost, for example),  
then the message-id SHOULD be unique across those related sessions.

- should state that a status header SHOULD NOT appear in a SEND  
request. In the appropriate section it should say that the receiver of  
a SEND request MUST ignore a status header if it appears in a SEND  
request it received.

- Boundary string text appears in this section also. Please unify  
terminology as discussed above.

- "SEND requests larger than 2048 octets MUST be interrupted" or should  
it say "interruptible"? There is a difference. The former is saying  
that you must interrupt a SEND request in order to send a REPORT while  
the latter is saying that the SEND request must be interruptible in  
anticipation.

Section 7.3.1
-------------

- "generate a failure report with a 415 error code. Note that this  
failure report will not be sent if..." Do you mean transaction error  
response instead of failure report here? The text sometimes uses  
transaction response and failure reports interchangeably, I think we  
need to distinguish between the 2. Please correct the above and then  
add a paragraph defining transaction responses and failure reports.

- Last paragraph brings the same issue about REPORT that I mentioned  
above. Can't one REPORT be sent for the whole message instead of per  
request (chunk) even if the message was sent over multiple chunks? If  
so, please add text.

Section 8.2
-----------

- Explain the need for early media (usually a user types in a message  
and the session initiation happens in the background and a message is  
displayed to a user before they answer a session).

Section 9
----------

- MSRP-URL syntax has a TODO. Is that still needed?
- transport of only tcp is defined? What about TLS? I know that the s  
in msrps stands for secure, but it does not necessarily mean TLS. Or  
does it?
- namespace is defined and hard-coded as "000" only. The syntax does  
not allow for future namespaces.

Section 10
-----------

- Need to define default reason phrases for all responses
- Need a sentence or 2 under each response section indicating how a  
receiver of the response should react.

Section 10.5
------------

- " If the sender
    receives the 413 while in the process of sending a chunk, and the
    chunk is interruptible, the sender MUST abort sending the chunk."

I dont think the chunk should be interruptable in order to abort. If  
that is so, then we need to define that in an earlier section "if a  
chunk needs to be interruptable or abortable, then the byte-range  
range-end MUST be a * ".

Section 10.8
-------------

- "A 426 response indicates that the request is only allowed over TLS
    protected connections." should it say "over a protected connection,  
like TLS"? or is TLS the only security we will ever use with MSRP?

Section 11.5
------------

- I would like to make this example also demonstrate the use of  
Failure-Report: no, so please add that to the SEND request and remove  
the 200 ok response from the example.

======================================================================== 
=================================

NITS
=====

Section 1
----------

- Traditionally, Conventions section appears as section 2 while the  
introduction appears as section 1. Please swap.

Section 2
----------

- "The SIMPLE working group" and "enabled in SIMPLE" appears in the  
first 2 paragraphs. I suggest removing any reference to the name of the  
WG.
- Second paragraph references reference 19. You should remove this  
reference.
- 3rd para: "whose sessions can be included in an offer or answer" ->  
"whose sessions can be negotiated in an offer/answer"
- 4th para: add a reference to RFCxxxx that defines the DNS look up for  
im URIs

Section 4
----------

- The example: request-uri in INVITE should carry the URI of bob, not  
alice. The same goes for the ACK
- A slash appears in the paragraph after the MSRP example (3rd last  
line). This whole sentence around the / is confusing. Please rephrase.
- 3rd last paragraph introducing relays should be moved to the  
introduction section.

Section 5
---------

- "The start line of an MSRP request contains a unique boundary string  
that is used to indicate" -> "The start line of an MSRP request  
contains a transaction id that is also used as a unique boundary string  
to indicate"

Section 5.3
-----------

- "A receiver
    only sends a success REPORT if it knows that the request succeeded,
    and the sender requested a success report.  A receiver only sends a
    failure REPORT if the request failed and the sender requested failure
    reports." ->

   "A receiver
    only sends a success REPORT if it knows that the request was  
successfully delivered,
    and the sender requested a success report.  A receiver only sends a
    failure REPORT if the request failed to be delivered and the sender  
requested failure
    reports."

Section 5.4
-----------

- "When an endpoint notices such a failure" -> "When either endpoint  
notices such a failure"
- "A connection SHOULD not..." -> "A connection SHOULD NOT..."

Section  6
-----------

- "The constructions for "userinfo",  and "unreserved" are detailed in
    RFC2396bis [9]." ->

   "The constructions for "userinfo",  and "unreserved" are detailed in  
[9]." Remove "RFC23956bis".

  - The same is for indented comment towards the end of the section.  
Remove "RFC23956bis".

Section 6.2
------------

- Section 6.1 talks about hostport. This section talks about server  
part. They are the same thing so please unify the terminology between  
those sections. hostport is preferable.

Section 7.1
-----------

- 1st para: "Next, the sender places the target path in a To-Path  
header," -> "Next, the sender places the target URI in a To-Path  
header,"

Section 7.1.1
--------------

- I suggest changing the section name to "Sending SEND requests" for  
consistency reasons.
- "MUST inform the user if receives a non-recoverable error" -> "MUST  
inform the user if it receives a non-recoverable error"
- "REPORT requests MUST have
    the same Message-ID header value as the request they are reporting
    on.  They MAY also have the Byte-Range of the chunk they are
    reporting on.  If an MSRP endpoint receives a REPORT for a Message-ID
    it does not recognize, it SHOULD silently ignore the REPORT."

This is duplicate text that appears in section 7.1.2. So please remove.

Section 7.1.2
-------------

- " The endpoint then inserts
    a Status header field with a namespace of "000", a short-status of
    "200" and a relevant Reason phrase, and a Message-ID header field
    containing the value from the original request." -> change to

" The endpoint then inserts
    a Status header field with a namespace of "000", a short-status of
    "200" and a relevant Reason phrase. A Message-ID header field
    containing the value from the original request is also inserted into  
the REPORT request."

Section 7.3
-----------

- "Note
    that if the Failure-Report header had a value of "no", then no error
    report would be sent." change to

"Note
    that if the Failure-Report header had a value of "no", then no  
transaction response nor error
    report would be sent."

Section 7.3.1
--------------

- " It is technically illegal for the sender to prematurely interrupt
       a request that had anything other "*"" ->

" It is technically illegal for the sender to prematurely interrupt
       a request that had anything other than "*""

- "if bytes 1 to 100 was received" -> "if bytes 1 to 100 were received"
- "All MSRP
    endpoints MUST be able to receive the multipart/mixed and
    multipart/alternative MIME types." Please add references here.

- "If the Success-Report header is not set to "no",..." -> "If the  
Success-Report header is set to "yes",..."

Section 7.3.2
--------------

- "the Message-ID and the Byte-Range, if present." -> "the Message-ID,  
and the Byte-Range if present." (moved the comma to clarify that if the  
byte-range is present. someone could be deceived into thinking that if  
message-id is present).

- "When a device that..." -> "When an endpoint that..."

Section 8.1
-----------

- "wrapped inside containers." -> "wrapped inside container types  
listed in accept-types attribute."

Section 8.1.3
-------------

- "domain name from MSRP URL identifying..." -> "domain name from the  
MSRP URL identifying..."

Section 8.1.6
-------------

- Remove

Section 11.1
-------------

- Step 5: To-Path and From-Path in the 200 ok need to be swapped.
- Step 7: same as above

Section 16
----------

- Remove.

Regards,
Hisham

_______________________________________________
Simple mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/simple

---

From: Paul Kyzivat <[email-masked]>
Date: Wed, 23 Mar 2005 10:27:46 -0500

Hisham Khartabil wrote:

> Section 5.4
> -----------
> 
> - "   Likewise, the active endpoint MUST immediately issue a SEND  request.

It does a bunch of things:

- forces (and hence tests) establishment of connections all along
   the path.

- Confirms to the recipient which incoming connection will be used
   for this session.

- By confirming the connection used for the session, may enable the
   recipient to cease listening for new connections.

- Allows bogus connections to be discarded. (If a message on an expected
   session isn't received.

	Paul