Subject: Re: [Speechsc] WGLC MRCPv2-12 - More comments

From: Magnus Westerlund <[email-masked]>
Date: Mon, 02 Apr 2007 17:39:09 +0200

Hi,

Here is the rest of the comments I have. I probably could find a few 
more if I read certain sections at all or in more detail. But there are 
a few issues here that you certainly should correct before going forward.

1. Section 4.3, Second paragraph: There is missing a empty line between 
the text and the ABNF.

2. Section 5.1, Third paragraph
"   If a message contains a message body, the message MUST contain
    content-headers indicating the MIME-type and encoding of the data in
    the message body."

Shouldn't also the message body length header be indicated here as 
required. I know there is the message length value which allows 
inferring the message body-length.

3. Section 5.2:

Is the request-id scoped by the channel ID or per TCP connection? I 
think the distinction is quite important if multiple MRCP resources 
share the TCP channel.

4. General comment: Usage of DIGIT fields without maximum specified 
sizes. I am a bit worried that you specify a lot of fields to use 
1*DIGIT. The reason is that this makes for parsers complicated and may 
lead to some interoperability issues. How many of the implementations 
would for example accept a 4000 digit number as a request-ID on the 
request line? It is valid by syntax, but isn't this complicate things a 
bit to much. Comparing or verifying arbitrarily long strings of digit 
makes life more complicated. I would recommend that you specify 
something that fits into 32 or 64 bits of unsigned safely in the places 
where it makes sense. Considerations for handling large message also 
plays into this as there is a potential for doing attacks on the server 
using these.

5. Section 5.3:

    The mrcp-version field MUST contain the version of the MRCPv2
    protocol running on the server.

I think this should be clarified to: The version of the request if 
supported otherwise the highest version supported.

6. Section 5.3:

Are you certain that no-ones needs to extend the possible request 
states? If not, I recommend that you add to the ABNF something that 
describes what syntax rules any future extension must follow. This can 
be seen as a general comment on the whole ABNF. All fields that are 
intended to be possible to extend should define extension syntax. 
Otherwise extending things without knowing what parsers handles without 
crashing/barfing makes life difficult.

7. Section 5.4, Code 503:
Are you allowing proxies in the client to server communication? It 
doesn't seem like that and I would recommend against going there. Life 
gets much more complicated. Thus I wonder if this error message is needed.

8. Section 6.2:

field-content should be defined in ABNF rather than using hand waving. 
And yes, I know it is an action of enumeration. However it is an very 
useful exercise.

9. Section 6.2:

   "Multiple headers with the same name MAY be present in a message if
    and only if the entire value for that header is defined as a comma-
    separated list [i.e., #(values)]."

Do you have a good reason to allowing this? Are it is actually used or 
is it only cluttering up your parsers? If the later I would propose that 
you remove it.

10. Section 6.2.1, Is it intended to not allow any LWS in the 
Channel-Identifier header? LWS = Linear White Space

11. Section 6.2.3, Is it intended to not allow any LWS in this header, 
especially around the ","?

12. Section 6.2.6: First, why are you saying a "restricted set" of media 
types? Is it really MRCPv2 that supports them or rather that some of the 
resources supports some media types. Secondly I think you should try to 
use "Media Types" rather than MIME here. Basically I think the paragraph 
needs rewording to be clear that the Content-Type header used Media 
Types to describe what the body of the request or response is.

13. section 6.2.9: Which of the RFC 2616 listed content-encodings are 
required to be supported if any? And how does a client learns which are 
supported by the other entity? You seem to need at least Accept-Encoding 
[H.14.3] to allow the requestor to specify supported encodings for the 
response. However that doesn't work for requests with body. Maybe using 
the SDP negotiation channel to declare capabilities?

14. Section 6.2.13: Extensibility of the cache-control directives.

15. Section 6.2.15: I find it somewhat confusing that you use different 
ABNF between this section and Section 15, for example with "CommentURL". 
Maybe a bit more alignment or at least a clearer warning in the intro 
about that they may differ.

16. Section 8.4.9: This section has ABNF for the Timestamp header. 
However the descriptive text discusses only at the Speech-Marker header. 
Should the Timestamp header has its own subsection? Even if not it needs 
some text and a change of the grammar in the section.

17. Section 8.5.1: Second paragraph page 52:
I am missing a normative reference to text/uri-list.

18. Section 8.5.1, Third paragraph page 52: Missing reference to the 
.wav format and "sun audio format". These are normative references and 
needs to be indicated which specification that applies.

19. Page 61: Pause Example: Both message length and Content-Length has 
the wrong value. According to my check the body is 609 bytes. This error 
seems to exist for several of the other examples in section 8.

20. Section 8.9 and Section 8.4.8. Why isn't speech marker used in PAUSE 
responses?

21. section 9.1: What effect does interpretation have on the state machine?

22. Section 9.4.5. At the end of the first paragraph there is an extra ".".

23. Section 9.4.8: Why is the Waveform URI quoted using < instead of "?

24. Section 9.5.1: Wrong Content length in example.

Skipped sections 9.6, 10 and 11.

25. Section 12.4: How does one handle authentication information related 
to access of content over HTTP and HTTPS? Isn't the client in many case 
the one having the rights to use things and needs to transfer them 
temporarily to a MRCPv2 server?

26. Section 13.2.1 and 13.5: Has these media types been reviewed by the 
IETF Types list which is a requirement for publication?

27. Section 13.6: Has the URI been on review on [email-masked]?

28. Section 13. All the templates are hard to read due to lack of ":" 
between field explanation and value. The templates are also crushed 
together and the lack of blank lines makes it hard to separate things.

29. Section 14.1: I think one should consider changing the s= string in 
the example.

30. Section 14.1: First text comment:
"  The client requests the server to create synthesizer resource control
    channel to do speech synthesis.  This also adds a media pipe to send
    the generated speech.  Note that in this example, the client request
    the reuse of an existing MRCPv2 TCP pipe between the client and the
    server."

What pre-existing TCP connection are we talking about here? If it exist 
prior to the invites then you better be more explicit about it?

31. Section 14.1: Content-Length header in examples does not match reality.

32. Section 15:
Using Bill Fenners ABNF parser I do get a few missing definitions in 
addition to the ones from RFC 4234:

; accept UNDEFINED
; accept-charset UNDEFINED
; content-id UNDEFINED
; x20 UNDEFINED
; boolean UNDEFINED
; interpret-text UNDEFINED
; FLOAT UNDEFINED

You should also consider if these are correct:
; cmid-attribute defined but not used
; generic-message defined but not used
; timestamp defined but not used

I guess "generic-message" is just fine. But timestamp seems like a 
possible error.

33. Section 17.1: ID-nits find a few reference problems:
   * Downref: Normative reference to an Informational RFC: RFC 4313 
(ref. '1')
   - Outdated reference: draft-ietf-mmusic-sdp-comedia has been published as
     RFC 4145
   * Obsolete normative reference: RFC 2109 (ref. '15')
   - Outdated reference: draft-ietf-mmusic-sdescriptions has been 
published as
     RFC 4568
   - Obsolete informational reference (is this intentional?): RFC 2833 (ref.
     '29')

Please check them. Any you will need to move the requirements doc to the 
informational part of the list.

Cheers

Magnus Westerlund

IETF Transport Area Director & TSVWG Chair
----------------------------------------------------------------------
Multimedia Technologies, Ericsson Research EAB/TVM/M
----------------------------------------------------------------------
Ericsson AB                | Phone +46 8 4048287
Torshamsgatan 23           | Fax   +46 8 7575550
S-164 80 Stockholm, Sweden | mailto: [email-masked]
----------------------------------------------------------------------

_______________________________________________
Speechsc mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/speechsc
Supplemental web site:
&lt;http://www.standardstrack.com/ietf/speechsc&gt;