Re: [Fwd: Gen-art last call review of draft-ietf-webdav-rfc2518bis-17.txt]

Thanks for the review and the meta-review.  I'll be incorporating  
almost all of these, particularly the concrete fixes, in the post- 
last-call draft.

Lisa


On Feb 4, 2007, at 1:52 AM, Julian Reschke wrote:

>
>> From: Elwyn Davies <elwynd@dial.pipex.com>
>> To: General Area Review Team <gen-art@ietf.org>
>> Subject: Gen-art last call review of draft-ietf-webdav- 
>> rfc2518bis-17.txt
>> I have been selected as the General Area Review Team (Gen-ART)
>> reviewer for this draft (for background on Gen-ART, please see
>> _http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html_).
>> Please resolve these comments along with any other Last Call comments
>> you may receive.
>> Document: draft-ietf-webdav-rfc2518bis-17.txt
>> Reviewer: Elwyn Davies
>> Review Date: 30/01/2007
>> IETF LC End Date: 21/01/2007
>> IESG Telechat date: (if known) -
>> Summary: Apologies for the late review - I missed the aassignment  
>> somehow.
>
> Hi Elwyn. No need to apologize. Feedback is always good, even late.
>
>> This document is almost ready for the IESG.  There are a couple of
>> issues which need a little clarification IMO and the IANA  
>> considerations
>> are suffering from 'a standard problem' - RFC 2518 defined most of
>> things claimed to be needing registration as a result of this  
>> document
>> so they are not actually new, but RFC 2518 didn't actually have  
>> explicit
>> IANA considerations.  Consequently the IANA considerations need to be
>> rephrased to clarify that these are updated definitions rather than
>> new.  There are also some minor editorial nits that could get fixed
>> during the update. Caveat: I have not made a detailed check of the
>> syntax/semantics of the examples.
>> Comments:
>> Issues:
>> s4.3, para 2: 'Older clients will not break when they encounter
>> extensions because they will still have the data specified in the
>> original schema and MUST ignore elements they do not understand.'   
>> This
>> specification cannot enforce compliance retrospectively! s/MUST/were
>> required to/.
>
> Agreed. But I think it would be better to rephrase as:
>
> "Clients will not break when they encounter extensions because they  
> will
> still have the data specified in the original schema and MUST ignore
> elements they do not understand."
>
> (that is, just drop "Older").
>
> I'm +0.5 on getting rid of the MUST (extensibility is defined  
> somewhere
> else, and I'm strongly opposed to have normative requirements for the
> same thing in multiple places).
>
> Opened issue
> <http://ietf.osafoundation.org:8080/bugzilla/show_bug.cgi?id=262>.
>
>> s6.6, para 3: 'notifications should be sent': Is this supposed to  
>> be a
>> function of webdav?  This is the only mention of lock  
>> notifications in
>> the document.
>
> This text has always been in the spec, but you are the second one to
> complain.
>
> Opened issue
> <http://ietf.osafoundation.org:8080/bugzilla/show_bug.cgi?id=259>.
>
>> Potential security implications of lockdiscovery:  s6.8 requires a
>> compliant server to support lockdiscovery and expects the response to
>> this request to include the names of principals and potentially  
>> the lock
>> tokens for locks being held on a resource.  The privacy  
>> implications of
>> this are discussed in the Security Considerations but it does not  
>> appear
>> to be allowed to restrict or deny this request purely on security
>> grounds.  It is likely that some organizations might consider the
>> ability to determine who holds locks is a sensitive matter beyond  
>> just
>> issues of privacy, and the responses to lockdiscovery might be  
>> mediated
>> by access controls.
>
> I agree that this is a problem. The change in RFC2518bis has been done
> in an early draft; I don't recall any discussion related to this,  
> nor is
> this mentioned in the Changes section.
>
> Opened issue:
> <http://ietf.osafoundation.org:8080/bugzilla/show_bug.cgi?id=260>
>
>> s9.1: PROPFIND method: It is probably not a big deal, but the  
>> ability of
>> 'new' servers to deny depth-infinity PROPFIND requests interacts with
>> the suggestion that they should treat client requests without a Depth
>> header as depth-infinity requests.  This may result in a different
>> response from a fully compliant new server as compared with a  
>> legacy RFC
>> 2518 server.  Is this likely to cause severe disruption to legacy  
>> clients?
>
> I think this is a misunderstanding.
>
> - The default has not changed.
>
> - Existing servers - such as Apache mod_dav - already may reject the
> request, and have done so for a long time, with no interop problems
> being reported.
>
> IMHO the real issue here is the new client requirement (to always
> include a Depth header), which is completely pointless
> (<http://ietf.osafoundation.org:8080/bugzilla/show_bug.cgi?id=213>).
>
>> Treatment of 'allprop':  I believe that the various statements about
>> which live properties will be returned by 'allprop' are not fully
>> consistent.  In the third bullet of s9.1 it is stated that allprop  
>> gets
>> you 'property values for those properties defined in this  
>> specification'
>> and offers the 'include' element as a way to increase the set of  
>> values
>> returned. This interpretation is repeated in the examples (especially
>> s9.1.6) and s14.2. However, the paragraph next but one after the  
>> bullets
>> (split across the  page 40/41 page boundary) states 'For a live  
>> property
>> defined elsewhere, that definition can specify whether that live
>> property would be returned in 'allprop' requests or not.'  Finally
>> Appendix F.1 states that the allprop semantics 'have been relaxed so
>> that servers *may* [My emphasis] leave out live properties defined in
>> other specifications'.  So it appears that there are three different
>> possibilities here - some clean up is needed.
>
> Agreed.
>
> Opened issue:
> <http://ietf.osafoundation.org:8080/bugzilla/show_bug.cgi?id=261>.
>
>> s21 IANA Considerations:
>> The various items here do not require  new registrations as they were
>> all registered as a result of RFC 2518 (and RFC 4229). This document
>
> We've been told that we should update the registrations. See
> <http://ietf.osafoundation.org:8080/bugzilla/show_bug.cgi?id=86>).
>
>> updates the registrations (and in a sense formalizes them since  
>> RFC 2518
>> did not have an IANA Considerations section explicitly). s21.1 should
>> refer to RFC 4395 which controls the URI Scheme registry. s21.3  
>> should
>> refer to RFC 4229 which formalized the initial state of the message
>> header field registrations.  It occurs to me that I did not check if
>> there are any message headers which were in RFC 2518 but are now  
>> dropped
>> - if so this should probably be recorded here.
>
> Adding the two references is simple (opened: <http:// 
> ietf.osafoundation.org:8080/bugzilla/show_bug.cgi?id=264>).
>
> There indeed are headers that have been removed. However, they stay
> defined by RFC2518, so shouldn't they stay in the registry?
>
>
>> Editorial
>> =======
>
> Note: there are many more editorial issues, see
> <http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis- 
> latest.html>.
>
> Unless otherwise stated, I did the suggested changes in "my" proposed
> draft, and hope that the WG draft will follow.
>
>> general: Check the capitalization of headings (e.g., s7.5.2).
>
> Yes.
>
>> s1, paras 9/10: s/new/extra/ (2 places) - they certainly aren't  
>> new in
>> this specification.
>
> Yes.
>
>> s1, para 11: s/request and response/requests and responses/, s/all
>> all/all/, move cross-ref to Section 17 to end of sentence.
>
> Yes.
>
>> s4.3, para 3: s/undefined), not multiple values/undefined); it  
>> does not
>> have multiple values/
>
> Yes.
>
>> s6.1, item 4: This is the first appearance of the 'depth' concept  
>> and it
>> isn't defined previously.  I think something could be usefully  
>> added to
>> the terminology section to introduce depth, and specially infinite  
>> depth.
>
> You mean to Section 1? That may be non-trivial, because it requires  
> the collection definition.
>
>> general: various forms are used to refer to requests with 'Depth:
>> infinite' attributes.  'infinite depth', 'Depth: infinite' and
>> 'depth-infinite'.  It would be good to be consistent.
>
> Yes.
>
> Opened ticket: <http://ietf.osafoundation.org:8080/bugzilla/ 
> show_bug.cgi?id=263>
>
>> s6.1, item 5: It would be good to clarify that locks (or at least  
>> their
>> id's) are *globally* unique here.
>
> Yes.
>
>> s6.1, ietm 7: It would be helpful to quote If in this paragraph  
>> ("If")
>> as this is the first occurrence of the term and it is a little  
>> confusing
>> on first reading.
>
> Yes.
>
>> s6.2: s/Email/email/ (I think this is now the approved form).
>
> Yes.
>
>> s7.4, para 2: s/a write lock/any write lock/ (I was not sure  
>> initially
>> if we were still talking about infinite-depth locks).
>
> That would result in:
>
> "Expressed otherwise, any write lock protects any request that  
> would create a new resource in a write locked collection, any  
> request that would remove an internal member URL of a write locked  
> collection, and any request that would change the segment name of  
> any internal member."
>
> I'm not sure this is better (because of the repetition of "any").
>
>> s8.6.2, para 2: For the less well-informed, it would be useful to
>> explain the difference between weak and string Etags at the start of
>> this discussion(or at least give an immediate reference).  As is  
>> clear
>>>> from the later words, finding a definition elsewhere is not simple!
>
> I don't think we want to define it, so the proper fix IMHO is to  
> add a proper ref [RFC2616], Section 13.3.3).
>
> Also I note that the next paragraph claims:
>
> "Note that the meaning of an ETag in a PUT response is not clearly  
> defined either in this document or in RFC2616 (i.e., whether the  
> ETag means that the resource is octet-for-octet equivalent to the  
> body of the PUT request, or whether the server could have made  
> minor changes in the formatting or content of the document upon  
> storage). This is an HTTP issue, not purely a WebDAV issue, and is  
> being addressed in [I-D.draft-whitehead-http-etag]."
>
> ...which is incorrect in that there has been no activity on that  
> draft for almost one year (new issue: <http:// 
> ietf.osafoundation.org:8080/bugzilla/show_bug.cgi?id=265>).
>
>> s8.7:  The use of DeltaV here is obscure.  I understand it was a  
>> design
>> team name.
>
> Yes. Suggestion to replace by "the Versioning Extensions to WebDAV".
>
>> s9.2: I think I know what 'document order' means but it isn't  
>> actually
>> defined.
>
> That's a good catch. REC-XML doesn't define nor use it. REC-XPATH  
> uses it, but doesn't define it. Advice needed.
>
>> s9.6.1: I think it would be helpful to explicitly list which  
>> properties
>> are not returned because of the way allprop is defined.
>
> 9.1.6.
>
> That's hard to do, because all of the properties defined in RFC2518  
> are returned.
>
>> s10.7: The abbreviation LWS is not expanded until later.
>
> Yes.
>
>> s13, item 1: s/excecution/execution/
>
> Yes.
>
> Best regards, Julian
>

Received on Thursday, 8 February 2007 00:56:15 UTC