Subject: [Simple] Re: draft-xml-patch and the comments

From: Jari Urpalainen <[email-masked]>
Date: Wed, 08 Feb 2006 15:56:53 +0200

Thanks, for your comments, more inline, I'll respond to each of your 
comments here.

ext Stephane Bortzmeyer wrote:

>[I'm not a member of SIMPLE - I'm interested in XML but not in SIP -
>so please keep me informed if you choose to reply.]
>
>I've just studied draft-ietf-simple-xml-patch-ops-01 and I have a few
>remarks. First, the insertion of comments:
>
>   An example for an addition of a comment node:
>
>   <add sel="doc/foo[@id='ert4773']" pos="before"><!-- comment --></add>
>
>IMHO, this is not good XML. A XML parser does not have to pass the
>comments to the application (the specification says "They are not part
>of the document's character data; an XML processor MAY, but need not,
>make it possible for an application to retrieve the text of
>comments.").
>
>And writing it that way makes difficult to add comments in patches:
>
><!-- This is a "real" comment --> 
><add sel="doc/foo[@id='ert4773']" pos="before">
>  <!-- This is to be added in the resulting XML file -->
></add>
>
>I would suggest something which is more XML and consistent with the
>way XSLT does it: a <comment> element.
>
><!-- This is a "real" comment --> 
><add sel="doc/foo[@id='ert4773']" pos="before">
>  <comment>This is to be added in the resulting XML file</comment>
></add>
>  
>
So you say XPath, c14n etc. are evil, bad xml ;-). This is to be used 
with "real" libraries, on open source implementation 
<http://sourceforge.net/projects/xmlpatch> which uses libxml2 happily 
supports full XPath 1.0, although a small subset of XPath 1.0 is needed 
here. So if the XML parser doesn't support these it can't do that for 
XPath or c14n either. Also e.g. a proper CDATA/entity/character 
reference handling may be rather difficult/clumsy. Anyhow, it is 
probably worth mentioning in the i-d.

btw. the opensource implementation will be updated shortly, and 
hopefully i can include an auto diff tool as well.

>I have a few suggestions for the Security Considerations section:
>
>Since a patch can potentially delete valuable information,
>implementations should take care of authenticating the patches before
>applying them, for instance with XML signature [RFC 3275] or transport
>security, such as HTTP authentication [RFC 2617] if the patch is
>served with HTTP.
>
>In addition, an implementation MAY choose to limit what a patch can
>do, by forbidding some operations such as <replace> or <delete> and/or
>by limiting the amount of changes that can be done.
>
>
>  
>
As you have noticed this i-d is a framework which doesn't define a 
Mime-Type, so it needs additional specs that include these type 
definitions. The whole text in security part is thus "hand waving" as 
the including spec defines these finally. Also e.g. authorization policy 
is really beyond the scope.

>For consistency, I suggest adding a "append" option to the
><xsd:simpleType name="pos">. It is the default, but, IMHO, it should
>be made explicit, for instance to allow patch authors to clearly state
>thay they want to append.
>
>  
>
Well, I had this psvi like feature in the previous version, but this is 
indeed a bad habit, one of those things that come from the w3c schema 
language.

>AFAIK, it is not specified what to do if one patch raises an error
>while the previous patches in a <diff> were OK. I suggest to add to
>section 5 "Error Handling" something like:
>
>The patches in a <diff> element are all applied with success or none is
>applied. An implementation MUST NOT produce a XML document which
>results from a partial application of the patches.
>
>An implementation MAY continue the patching after an error (to produce
>diagnostics, for instance), but, since the selectors are applied to
>the XML document already patched, these diagnostics are of doubtful
>value.
>  
>
The sentence "It is an error condition if any of the given operations 
can not be unambiguously fulfilled."  is supposed to explicitly state 
this. So some elaboration is needed, I assume.

>And one remark that may be offtopic: why W3C Schemas and not RelaxNG,
>which I much prefer (see RFC 4287 for an example)?
>  
>
I'll absolutely agree (and undoubtedly I've made noise about RNG many 
times). However, there's quite a lot inertia in this case as many 
content models are described with the w3c schema language here. Also in 
this particular case even the w3c schema can do its task quite well 
(which is not generally true). I've been thinking about adding also rng 
schema to this, so that both were informative just like in atom format.

Thanks,
Jari

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

---

From: Stephane Bortzmeyer <[email-masked]>
Date: Mon, 6 Feb 2006 23:01:15 +0100

---

From: Jari Urpalainen <[email-masked]>
Date: Fri, 10 Feb 2006 10:04:09 +0200

On Thu, 2006-02-09 at 17:07 +0100, ext ext Stephane Bortzmeyer wrote:
> On Wed, Feb 08, 2006 at 03:56:53PM +0200,
>  Jari Urpalainen <[email-masked]> wrote 
>  a message of 112 lines which said:
> 
> > ><xsd:simpleType name="pos">. It is the default, but, IMHO, it
> > >should be made explicit, for instance to allow patch authors to
> > >clearly state thay they want to append.
> > >
> > Well, I had this psvi like feature in the previous version, but this
> > is indeed a bad habit, one of those things that come from the w3c
> > schema language.
> 
> Could you elaborate? I absolutely do not see the connection with the
> idiosyncrasies of the W3C Schema language.
> 
> I was not suggesting to use the PSVI. I did not suggest to mess with
> the infoset. I suggested to add the "append" option and to let the
> application do the same thing for pos="append" and for no pos.

Sorry, my comment was badly phrased. With a reference to psvi I meant
extra processing, using default values for attributes is discouraged in
rfc3470 (the previous one had a default value "to", which meant append
rule for text, element, pi and comment, so "to" was also used with
attributes & namespaces). Having two possibilities for the same thing is
imo a bad thing. In this particular case, it just increases the "body"
size, i.e. the diff should be as small as possible, and also it adds an
unnecessary condition check for implementations. So it is additional
"noise" that is discouraged simply by disallowing it. Implementations
have to be careful in many other things as well before having a working
implementation.
br, 
Jari

---

From: ext Stephane Bortzmeyer <[email-masked]>
Date: Thu, 9 Feb 2006 11:55:42 +0100

On Wed, Feb 08, 2006 at 03:56:53PM +0200,
 Jari Urpalainen <[email-masked]> wrote 
 a message of 112 lines which said:

> So you say XPath, c14n etc. are evil, bad xml ;-). 

I've reread the Xpath specifiation and, while there is a comment()
test, it does not clearly say that comments MUST be processed (the XML
specification clearly says that it is not mandatory).

And you say that XSLT is bad :-) because XSLT treat comments
as... comments, not as information items like you do in your draft.

<xsl:template match="foo/bar">
   <xsl:comment>This one will be added as a comment in the
   output.</xsl:comment>
   <!-- This one is a real comment, it will be ignored -->
</xsl:template>

---

From: ext Stephane Bortzmeyer <[email-masked]>
Date: Thu, 9 Feb 2006 17:07:19 +0100