Re: [XCON] apps-team review of draft-ietf-xcon-ccmp-13

[Robert Sparks <[e-mail]>]

Adding the XCON WG list

On May 16, 2011, at 4:04 AM, Henry S. Thompson wrote:

> I have been selected as the Applications Area Review Team reviewer for
> this draft (for background on apps-review, please see
> http://www.apps.ietf.org/content/applications-area-review-team).
> 
> Please resolve these comments along with any other Last Call comments
> you may receive. Please wait for direction from your document shepherd
> or AD before posting a new version of the draft.
> 
> Document: draft-ietf-xcon-ccmp-13
> 
> Title: Centralized Conferencing Manipulation Protocol
> 
> Reviewer: Henry S. Thompson
> 
> Review Date: 2011-05-13
> 
> Summary: This draft is almost ready for publication as a Proposed
> Standard but has a few issues that should be fixed before publication
> 
> Major Issues: 
> 
> 4.2 Data Management
> 
> 1) The approach to detecting competing updates and their consequences
>   specified here seems unnecessarily complex.  Was the alternative of
>   including version numbers in _update_ messages (so that the server
>   could reject any update whose target version had been superseded)
>   considered and rejected?  If so, perhaps a brief explanation of why
>   it was rejected might be helful at this point.
> 
> 2) In a related point, the statement at the end of this section that
>   "a client subscribed to . . . notifications . . . will always have
>   the most up-to-date version" is clearly false, in-so-far as it
>   implies that such a client is guaranteed success for any update, as
>   there is clearly a race condition here.
> 
> 4.3 Data Model Compliance
> 
> 1) Again this approach seems unnecessarily complex -- why does the
>   data model have to constrain the initiation of a conference in this
>   way.  why not simply have messages which request new conference or
>   new user IDs?
> 
> 2) I'm also confused by the fact that _elements_ described here as
>   "mandatory" are not required by the schema.  Specifically in 5.1 we
>   will see that the 'confUserID' and 'confObjID' elements, which
>   correspond precisely to XCON-USERID and XCON-URI which are
>   described here as mandatory, appear in message type definitions as
>   minOccurs="0", i.e. as optional.  If they are optional, why is the
>   above gensym complexity needed?  If they are not optional, why
>   doesn't the schema say so?
> 
> 3) It is unusual to refer to aspects of a data model with words such
>   as 'element' and 'attribute', which are better reserved for use
>   with respect to _XML serializations_ of data model instances.  Ah,
>   I see by looking at draft-ietf-xcon-common-data-model that the XCON
>   data model is defined as an XML document.  It's undoubtedly too
>   late to do anything about that, but confounding data models and XML
>   serializations is usually considered to be a mistake. . .
> 
> 11. XML Schema
> 
> An http URI should be provided where this schema document can be found
> on its own, and an update policy for it (or, preferably, _two_ URIs,
> one for exactly this schema document, and one which will be updated if
> this document is revised or superseded).  (Likewise for DataModel.xsd
> and rfc4575.xsd.)
> 
> 12.5 CCMP Protocol Registry
> 
> Why are these registries needed?  No role is specified for them
> anywhere in the body of the document. Registries are not free, and if
> all the information in the registry is also in the published schemas
> it's not at all clear what purpose they will serve.
> 
> Minor Issues: 
> 
> 6.2. Alice gets detailed information about a specific blueprint
> 
> The blueprintResponse message is not schema-valid per ccmp.xsd.  On
> lines 32 and 33 of the example read
>                    <xcon:floor-request-handling>confirm
>                      </xcon:floor-request-handling>
> The problem really lies in DataModel.xsd -- whereas (correctly)
> ccmp.xsd uses xs:token as the base type for enumerated types,
> DataModel.xsd (in draft-ietf-xcon-common-data-model) uses xs:string,
> and the string value of the above element is "confirm
>                      ", which is not one of the allowed values.  The
> example should be corrected, or, for preference, the schema in
> draft-ietf-xcon-common-data-model should be changed to use xs:token as
> the base type for join-handling-type and all other enumerated types.
> 
> A similar problem occurs in the response in 6.3
> (floor-request-handling)
> 
> 6.9 Alice exploits a CCMP server extension
> 
> For compatibility with the actual response given, the extension schema
> document should have a target namespace, as follows:
> 
>   <?xml version="1.0" encoding="UTF-8"?>
>   <xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema"
> 	      targetNamespace="http://example.com/ccmp-extension-schema.xsd"
> 	      xmlns="http://example.com/ccmp-extension-schema.xsd">
> 
>     <xs:element name="confSummary" type="conf-summary-type"/>
> 
>     <xs:complexType name="conf-summary-type">
>       <xs:sequence>
> 	 <xs:element name="title" type="xs:string"/>
> 	 <xs:element name="status" type="xs:string"/>
> 	 <xs:element name="public" type="xs:boolean"/>
> 	 <xs:element name="media" type="xs:string"/>
>       </xs:sequence>
>     </xs:complexType>
> 
>   </xs:schema>
> 
> Or, better, the example _and_ the schema should be changed to read as
> follows:
> 
>   <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
>    <ccmp:ccmpResponse xmlns:info="urn:ietf:params:xml:ns:conference-info"
> 	   xmlns:ccmp="urn:ietf:params:xml:ns:xcon:ccmp"
> 	   xmlns:xcon="urn:ietf:params:xml:ns:xcon-conference-info"
> 	   xmlns:example="http://example.com/ccmp-extension">
>      <ccmpResponse xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
> 	  xsi:type="ccmp:ccmp-extended-response-message-type">
> 	  <confUserID>xcon-userid:[e-mail]</confUserID>
> 	  <confObjID>xcon:[e-mail]</confObjID>
> 	  <operation>retrieve</operation>
> 
> 
> 	  <response-code>200</response-code>
> 	  <response-string>success</response-string>
> 	  <ccmp:extendedResponse>
> 	     <extensionName>confSummaryRequest</extensionName>
> 	     <example:confSummary>
> 		 <title> Alice's conference </title>
> 		 <status> active </status>
> 		 <public> true </public>
> 		 <media> audio </media>
> 	     </example:confSummary>
> 	  </ccmp:extendedResponse>
>      </ccmpResponse>
>    </ccmp:ccmpResponse>
> 
>    <?xml version="1.0" encoding="UTF-8"?>
>    <xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema"
> 	       targetNamespace="http://example.com/ccmp-extension"
> 	       xmlns="http://example.com/ccmp-extension">
> 
>      <xs:element name="confSummary" type="conf-summary-type"/>
> 
>      <xs:complexType name="conf-summary-type">
> 	<xs:sequence>
> 	  <xs:element name="title" type="xs:string"/>
> 	  <xs:element name="status" type="xs:string"/>
> 	  <xs:element name="public" type="xs:boolean"/>
> 	  <xs:element name="media" type="xs:string"/>
> 	</xs:sequence>
>      </xs:complexType>
> 
>    </xs:schema>
> 
> Otherwise I've checked all the schemas for conformance and the
> examples for schema-validity.
> 
> 12.2 XML Schema Registration
> 
> Should include pointers to the RFCs which include the text of the
> schema documents named as "DataModel.xsd" and "rfc4575.xsd" in the
> schema docuemnt given in section 11.
> 
> 12.3 Media Type Registration
> 
> It seems unlikely that the proposed extension of 'ccmpxml' will see
> much use---4 characters seems to be the practical limit for
> extensions.
> 
> Nits: One more proofreading pass over the first three sections would
> be a good idea. . .
> 
> ht
> -- 
>       Henry S. Thompson, School of Informatics, University of Edinburgh
>      10 Crichton Street, Edinburgh EH8 9AB, SCOTLAND -- (44) 131 650-4440
>                Fax: [phone], e-mail: [e-mail]
>                       URL: http://www.ltg.ed.ac.uk/~ht/
> [mail from me _always_ has a .sig like this -- mail without it is forged spam]