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

From: Elwyn Davies <[email-masked]>
Date: Sun, 04 Feb 2007 11:32:29 +0000

Hi.

No problems with most of this.  Deleted the stuff with agreed actions.

A couple of responses in line to clarify my points...

/Elwyn

Julian Reschke wrote:
>
>> 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>).
Right.  Just make it clear they are updates rather than original 
definitions - I think I said this rather better in the summary.
>
>> 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?
Yes. They will stay in the registry but given that 2518 doesn't 
explicitly define the registry entries, it would probably be worth 
noting the ones that are not updated (and saying this is the case) as 
well as thoses that are.
>
>> 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.
No. I meant in Section 3.  It fits conveniently just after the 
Collection definition :-)
>
>> 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").
True.  How about 'Expressed otherwise, a write lock of either kind 
protects any request...'?
>
>> 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).
Fine by me.
>
>
> 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>).
Agreed.
>
>> 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.
In that case I misunderstood the purpose of the example.  I thought it 
was intended to show that some properties didn't come back with allprop.
Maybe the example could be extended to demonstrate this in some way (and 
make it clear what didn't come back).

Regards,
Elwyn
> Best regards, Julian

_______________________________________________
Gen-art mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/gen-art

---

From: Julian Reschke <[email-masked]>
Date: Sun, 04 Feb 2007 10:52:44 +0100

> From: Elwyn Davies <[email-masked]>
> To: General Area Review Team <[email-masked]>
> 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>.

> 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.

> 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.

That would result in:

"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]."

> 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.

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

---

From: Julian Reschke <[email-masked]>
Date: Mon, 05 Feb 2007 13:16:33 +0100

> definitions - I think I said this rather better in the summary.

Ok, will do (meaning: I agree with this point of view and will make 
changes accordingly in "my" draft, which is *not* the WG draft).

> Collection definition :-)

OK, Section 3. Now the issue still is that to define the depth header, 
you need to understand collections, which only have a forward reference 
in Section 3.

Also, the real meaning of the "Depth" request header varies with 
requests. In particular, its usage in the context of LOCK is really 
different from what we do with, for instance, PROPFIND or COPY.

Thus, this probably should be fixed inside the Locking section (for 
which there have been proposals for a complete rewrite, for that matter).

> protects any request...'?

+1.

> make it clear what didn't come back).

We have an example for allprop vs live properties not being returned - 
in this case live properties defined in RFC3253 - in Section 9.1.4. That 
being said, we probably should minmally re-arrange the examples so that 
9.1.4 comes after 9.1.6 (9.1.4 being a special case of allprop).

Best regards, Julian

---

From: Lisa Dusseault <[email-masked]>
Date: Wed, 7 Feb 2007 16:56:03 -0800

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:

> Adding the two references is simple (opened: <http:// 
> ietf.osafoundation.org:8080/bugzilla/show_bug.cgi?id=264>).
>
> Opened ticket: <http://ietf.osafoundation.org:8080/bugzilla/ 
> show_bug.cgi?id=263>
>
> "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 don't think we want to define it, so the proper fix IMHO is to  
> add a proper ref [RFC2616], Section 13.3.3).
>
> 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>).
>
> That's a good catch. REC-XML doesn't define nor use it. REC-XPATH  
> uses it, but doesn't define it. Advice needed.
>

---

From: Julian Reschke <[email-masked]>
Date: Sat, 17 Feb 2007 20:04:49 +0100

Julian Reschke schrieb:
> ...

RFC2518 says in 
<http://greenbytes.de/tech/webdav/rfc2518.html#rfc.section.13.8>:

"The lockdiscovery property returns a listing of who has a lock, what 
type of lock he has, the timeout type and the time remaining on the 
timeout, and the associated lock token. The server is free to withhold 
any or all of this information if the requesting principal does not have 
sufficient access rights to see the requested data."

This was changed in RFC2518bis between drafts 1 and 2, and I'm not aware 
of any discussion about this, nor is this change recorded in the Changes 
section. Thus, it probably was unintentional, and I would recommend to 
restore the old text.

Best regards, Julian

---

From: Julian Reschke <[email-masked]>
Date: Sun, 25 Feb 2007 19:52:46 +0100

> <http://ietf.osafoundation.org:8080/bugzilla/show_bug.cgi?id=259>.

Proposed text change:

Proposed Change:

Section 6.6., para. 3:
OLD:

    If the timeout expires then the lock SHOULD be removed.  In this case
    the server SHOULD act as if an UNLOCK method was executed by the
    server on the resource using the lock token of the timed-out lock,
    performed with its override authority.  Thus logs should be updated
    with the disposition of the lock, notifications should be sent, etc.,
    just as they would be for an UNLOCK request.

NEW:

    If the timeout expires then the lock SHOULD be removed.  In this case
    the server SHOULD act as if an UNLOCK method was executed by the
    server on the resource using the lock token of the timed-out lock,
    performed with its override authority.  For instance, if the server
    implements some sort of logging and notification system for locking-
    related events, a lock timeout should be treated similar to an UNLOCK
    request.

(See also 
<http://ietf.osafoundation.org:8080/bugzilla/show_bug.cgi?id=259> and 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.issue.bz259>).

Best regards, Julian