Re: Fwd: I-D Action: draft-murchison-webdav-prefer-05.txt

Thanks for the feedback Julian. Responses below.



On 09/19/2013 08:16 AM, Julian Reschke wrote:
> On 2013-09-17 14:04, Julian Reschke wrote:
>> (FYI)
>>
>>
>> -------- Original Message --------
>> Subject: I-D Action: draft-murchison-webdav-prefer-05.txt
>> Date: Thu, 12 Sep 2013 10:27:27 -0700
>> From: internet-drafts@ietf.org
>> Reply-To: internet-drafts@ietf.org
>> To: i-d-announce@ietf.org
>>
>>
>> A New Internet-Draft is available from the on-line Internet-Drafts
>> directories.
>>
>>
>>      Title           : Use of the Prefer Header Field in Web Distributed
>> Authoring and Versioning (WebDAV)
>>      Author(s)       : Kenneth Murchison
>>      Filename        : draft-murchison-webdav-prefer-05.txt
>>      Pages           : 30
>>      Date            : 2013-09-12
>> ...
>
> Here's my feedback.
>
> Summary: I like it. Thanks for doing the work.
>
> Detailed feedback below:
>
> 2.1
>
> "If the omission of such a DAV:propstat element would result in a 
> DAV:response XML element containing zero DAV:propstat elements, then 
> the server MUST substitute a DAV:propstat element consisting of an 
> empty DAV:prop element and a DAV:status element of value 200 (OK) 
> [I-D.ietf-httpbis-p2-semantics] in its place."
>
> This seems to be here to keep the response DTD-valid. Did you consider 
> just relaxing the DTD, and to allow a response without <DAV:prop>?


That is what the CalConnect members originally implemented as can be 
seen in Appendix B.6, but I believe that one of the implementers was 
concerned with this form of multi-status not being supported by 
libraries.  If you think this is a show-stopper, I can take it back to 
the CalDAV technical committee.



> "If the server honors and applies the return=minimal preference to the 
> processing of a PROPFIND request as described above, the server SHOULD 
> include a Preference-Applied [I-D.snell-http-prefer] header field 
> containing the "return=minimal" token in the response."
>
> Why?
>
> The Prefer spec says: "Use of the Preference-Applied header is only 
> necessary when it is not readily and obviously apparent that a server 
> applied a given preference and such ambiguity might have an impact on 
> the client's handling of the response."
>
> Isn't it obvious in this case? If it's not, maybe the SHOULD needs to 
> be a MUST?


Good question.  This is a recent addition at the request of a CalDAV 
client author who feels like it would be nice to know if the preference 
was applied prior to parsing the response body, presumably because it 
can/does have an effect on how his client processes the body.  I will 
take this back to the CalDAV technical committee and see if we want to 
change this to a MUST or just drop the requirement.



> 2.1.1
>
> "This example tries to fetch an unknown property from a 
> CARDDAV:addressbook [RFC6352] collection."
>
> Why do we need to mention CARDDAV here? This seems to be an unncessary 
> distraction...


Yeah.  I was attempting to use examples that show the usefulness in the 
various *DAV protocols, but maybe it is too much of a distraction.  What 
are your thoughts on using CALDAV:calendar-multiget and DAV:sync-token 
in the examples?  I think the CalDAV scheduling example in Section 3 
should stay, but maybe I should point out how/where the server changes 
the resource.



>
> Sections 2.3 and 2.4 suggest changing a 207 response to 200/201 and 
> require an empty message body. Why does it need to be empty?


The argument here is that we don't want the client to have to parse a 
body if the request is successful.  Do you recommend that we specify 204 
instead?



> A.
>
> "Authors are encouraged to implement the Brief header field 
> functionality in conjunction with this specification to further 
> promote interoperability with products that use the Brief header field 
> exclusively."
>
> Hmm. So we specifiy something new and tell people to implement the 
> "old" syntax as well?

Several of the CalConnect members originally implemented the Brief 
header because we liked the functionality and it already had deployment 
in Microsoft products.  I set out to formally document the Brief header, 
but the MS folks asked us not to, so we turned to Prefer.  I suppose the 
only reason to implement Brief at this point is to support older products.

If you feel that this is a show-stopper, I can certainly remove this text.



> Editorial:
>
> The draft is kind of chatty with respect to references. Example:
>
> "When a PROPFIND [RFC4918] method request contains a Prefer 
> [I-D.snell-http-prefer] header field with a preference of 
> "return=minimal", the server SHOULD omit all DAV:propstat XML elements 
> containing a DAV:status XML element of value 404 (Not Found) 
> [I-D.ietf-httpbis-p2-semantics] from the 207 (Multi-Status) [RFC4918] 
> response."
>
> My recommendation is to either strip some -- in particular the whole 
> draft is about WebDAV and Prefer, so keeping to point at 4918 all the 
> time is a bit tiring, or to make them more useful by making them more 
> specific:
>
> "When a PROPFIND ([RFC4918], Section 9.1) request contains a Prefer 
> header field with a preference of "return=minimal" 
> ([I-D.snell-http-prefer], Section 4.2) the server SHOULD omit all 
> DAV:propstat XML elements containing a DAV:status XML element of value 
> 404 (Not Found) ([I-D.ietf-httpbis-p2-semantics], Section 6.5.4) from 
> the 207 (Multi-Status) response ([RFC4918], Section 11.1)."

I agree.  I will remove the redundant references and/or be more specific 
where needed.



> Also/method request/request/

Fixed.


> 3.
>
> s/subsquent/subsequent/

Fixed.


> 9.2
>
> Please format the MSDN references so that the URIs turn up in the 
> generated TXT version.
>
> For example:
>
> <reference anchor="MSDN.aa563501" 
> target="http://msdn.microsoft.com/en-us/library/aa563501.aspx">
> ...

Done.

-- 
Kenneth Murchison
Principal Systems Software Engineer
Carnegie Mellon University

Received on Thursday, 19 September 2013 13:25:51 UTC