Subject: Re: feedback on rfc2518bis-14

From: Julian Reschke <[email-masked]>
Date: Fri, 14 Apr 2006 18:49:09 +0200

Manfred Baedke wrote:
> 
> Section 1, para. 2:
>> ...  Also, the ability to link pages of any media type to related pages.
> Maybe it's only me, but i have no idea what exactly this sentence is 
> refering to.

Agreed. Needs clarification. Added reminder to 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.1.p.2>.

> Section 1, para. 5:
>> Namespace Operations: The ability to instruct the server to copy and
>> move Web resources, operations which change the URL.
> I think the wording could be more accurate here, maybe something like: 
> '... operations which change the mapping from URLs to resources.'

Agreed. Added to 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.1.p.5>.

> Section 3, para. 5:
>>    Internal Member (of a Collection) - Informally, a child resource of a
>>    collection.  Formally, a resource referenced by a path segment
>>    contained in the collection.
> As explained later in section 5, a collection contains mappings from 
> path segments to resources and not the path segments themselves.

Agreed. Fixed in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.3.p.5>.

> Section 5.1, para. 4:
> 
>>    Although implicit in [RFC2616] and [RFC3986], any resource, including
>>    collection resources, MAY be identified by more than one URI.  For
>>    example, a resource could be identified by multiple HTTP URLs.
>>   
> So any resource may be identified by multiple URIs, although it's 
> implicit in  [RFC2616] and [RFC3986]?
> The intended meaning of this sentence is probably that this is 
> _mentioned_ although it's implicit.

Agreed. Fixed in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.5.1.p.4>.

> Section 5:
> 
> Here a collection is defined to contain mappings from path segments to 
> resources, but later on, the containment relation is used with respect 
> to resources, too. Section 5 should explain what this means (something 
> like 'as a shortcut, a resource B will be said to be contained in the 
> collection resource A if there is a path segment mapping which maps to B 
> and wich is contained in A ). Without additional text, it would be 
> formally incorrect to say that a collection contains its internal members.

Agreed. Adding comment to 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.5.2.p.1>.

> Section 5.2, para. 3:
>> For all WebDAV compliant resources A and B, identified by URLs "U" and 
>> "V" respectively, such that "V" is equal to "U/SEGMENT", A MUST be a 
>> collection that contains a mapping from "SEGMENT" to B. So, if 
>> resource B with URL "http://example.com/bar/blah" is WebDAV compliant 
>> and if resource A with URL "http://example.com/bar/" is WebDAV 
>> compliant, then resource A must be a collection and must contain at 
>> least one mapping from "blah" to B.
> I think the collection must contain exactly one mapping from "blah" to 
> B, not at least one.

Agreed. Fixed in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.5.2.p.3>.

> Section 5.3, para. 5:
>>    If a WebDAV compliant resource has no WebDAV compliant children in
>>    the HTTP URL namespace hierarchy then the WebDAV compliant resource
>>    is not required to be a collection.
>>   
> After the technical term 'internal member' has been invented, I think it 
> should be used consistently instead of 'child'.

Agreed. Fixed in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.5.2.p.5>.

> Section 6:
> 
> There are two reasons why I am a little unhappy with this section: It 
> contains way to much redundance, and the abstract entity 'lock' together 
> with its applicable operations (for example: adding a resource to a 
> lock, adding an URI to a lock ...) is completely undefined. I will 
> address this in a separate mail.

Overall I agree with this one (I'll follow up on that one as well).

> Section 6.1.2:
>> A resource becomes directly locked when a LOCK request to the URL
>> of that resource creates a new lock.
> Should be: '... a LOCK request to an URL ...'

Agreed. Fixed in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.6.1.p.1>.

> Section 6.1.4:
>>        *  If an internal member resource is added to the collection, and
>>           if the new member resource does not already have a conflicting
>>           lock, then the resource MUST become indirectly locked by L.
>>
>>        *  If an internal member resource stops being a member of the
>>           collection, then the resource MUST no longer be indirectly
>>           locked by L.
>>   
> Should be: 'member resource' instead of 'internal member resource' in 
> both places.

Agreed. Added comments to 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.6.1.p.1>.

> Section 7, para. 2:
>> An exclusive write lock will prevent parallel changes to a resource
> The word 'parallel' should be dropped.

Agreed. Fixed in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.7.p.2>.

> Section 7.1:
>>    Only dead properties and live properties defined to respect locks are
>>    guaranteed not to change while write locked.
>>   
> So there are live properties which are lockable and may change their 
> values while they are locked, and there are live properties which 
> respect locks and must not change their values while they are locked? Is 
> this really intended or is this section historical and should be dropped?

I guess this requires further discussion. Adding comment to 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.7.p.3> 
for now.

> Section 7.3:
> 
> This section is generally quite verbose on lock-null resources. Since 
> these are deprecated, text addressing them should be moved to an appendix.

Agreed. Basically, this is Bugzilla issue 202 (see 
<http://ietf.cse.ucsc.edu:8080/bugzilla/show_bug.cgi?id=202> and 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.issue.bz202>).

> Section 7.3 para. 3:
>>    A successful lock request to an unmapped URL MUST result in the
>>    creation of an locked resource with empty content.
> Better: '... creation of  a locked non-collection resource with empty 
> content'.

Agreed. Fixed in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.7.3.p.3>.

> Section 7.3 para. 5:
>>       expire or are unlcoked, but are also removed if a resource is
>>       renamed or moved, or if any parent collection is renamed or moved.
>>   
> better: 'expire or are unlocked, but are also removed if a resource is 
> moved, or if any parent collection is moved'.

Agreed. WebDAV doesn't distinguish between renaming or moving. I fixed 
these in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.7.3>, 
but readability may suffer from the fact that the whole paragraph is 
marked up as being deleted from my version already :-).

> Section 7.3 para. 6:
>>    In the "locked empty resource" model, which is now the recommended
>>    implementation, a resource created with a LOCK is empty but otherwise
>>    behaves in every way as a normal resource.  It behaves the same way
>>    as a resource created by a PUT request with an empty body (and where
>>    a Content-Type and Content-Language was not specified), followed by a
>>    LOCK request to the same resource.  Following from this model, a
>>    locked empty resource:
>>
>>    o  Can be read, deleted, moved, copied, and in all ways behave as a
>>       regular resource, not a lock-null resource.
>>
>>    o  Appears as a member of its parent collection.
>>
>>    o  SHOULD NOT disappear when its lock goes away (clients must
>>       therefore be responsible for cleaning up their own mess, as with
>>       any other operation or any non-empty resource)
>>
>>    o  MAY NOT have values for properties like DAV:getcontentlanguage
>>       which haven't been specified yet by the client.
>>
>>    o  Can be updated (have content added) with a PUT request.
>>
>>    o  MUST NOT be converted into a collection.  The server MUST fail a
>>       MKCOL request (as it would with a MKCOL request to any existing
>>       non-collection resource).
>>
>>    o  MUST have defined values for DAV:lockdiscovery and DAV:
>>       supportedlock properties.
>>
>>    o  The response MUST indicate that a resource was created, by use of
>>       the "201 Created" response code (a LOCK request to an existing
>>       resource instead will result in 200 OK).  The body must still
>>       include the DAV:lockdiscovery property, as with a LOCK request to
>>       an existing resource.
> IMHO, this can be dropped completely, since it is not the case that 'a 
> resource is empty but otherwise behaves in every way as a normal 
> resource'. Rather an empty resource _is_ a completely normal resource. 
> No need to say anything about it.

(!)

> Section 7.3 para. 7:
>>    The client is expected to update the locked empty resource shortly
>>    after locking it, using PUT and possibly PROPPATCH.
> What am I missing here? Why is the client expected to do this?

Agreed.

> Section 7.4 para. 1:
>>    There are two kinds of collection write locks.  A "Depth 0" write
>>    lock on a collection protects the collection metadata plus the
>>    internal member URLs of that collection, while not protecting the
>>    content or metadata of child resources.  
> The term 'metadata' has not been defined anywhere in this specification. 
> Furthermore, I'd prefer using the term 'internal member' instead of 
> 'child resource'.

Added comment to 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.7.4.p.1>. 
I could live with "child resource" if we define it once upfront.

>>    A "Depth: infinity" write
>>    lock on a collection provides the same protection on that collection
>>    and also protects every descendent resource as if that resource were
>>    itself write locked.
>>   
> I'd prefer 'member' instead of descendent'. Furthermore, the wording 'as 
> if' seems unappropriate, since the members _are_ indeed write locked.

Agreed. Added comment to 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.7.4.p.1>.

> Section 7.4 para. 2:
>>    Expressed otherwise, a 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 binding name of a member URL.
>>   
> The term 'binding name' is undefined in this specification.

Agreed. Added comment to 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.7.4.p.2>. 
Should say "path segment mapping".

Dare I say that after all it seems to defining "path segment mapping" 
just avoiding to say "binding" may have been a silly decision. It's the 
same thing.

> Section 7.4 para. 3:
> 
> In the list of specific actions a locked collection is protected from, 
> replace 'member' with 'internal member' everywhere (or simply drop the 
> list, since it does not contain anything new).

Agreed. Added comment to 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.7.4.p.3>.

>>    In addition, a depth-infinity lock affects all write operations to
>>    all descendents of the locked collection.  With a depth-infinity
>>    lock, the root of the lock is directly locked, and all its
>>    descendants are indirectly locked.
>>   
> Replace 'descendents' with 'members' (I will not continue to repeat this 
> comment everytime, it turns out that it applies at a lot of places). 
> Furthermore, not the root of the lock is locked directly, but the 
> resource which the root is mapped to.

Agreed. Fixed in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.7.4.p.5>.

> Section 7.5 para.1:
>>    If a user agent is not required to have knowledge about a lock when
>>    requesting an operation on a locked resource, the following scenario
>>    might occur.
> For reasons of clarity, I prefer: 'A user agent is required to have 
> knowledge about a lock when requesting an operation on a locked resource 
> to prevent the following scenario:'

Good rewrite. Fixed in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.7.5.p.1>.

> Section 7.7
>> A client MUST NOT submit the same write lock request twice.
> Why not? The client will get an error response on the second request, 
> but that is perfectly ok.
>>    If an error is received in response to a refresh LOCK request the
>>    client MUST NOT assume that the lock was refreshed.
>>   
> Yes. I think this is quite clear. If the method failed, the client must 
> not assume that it succeeded. :)
> 
> IMHO, Sections 7.6 and 7.7 can be dropped without causing any harm.

Agreed. These two sections are completely useless and furthermore partly 
misleading.

> Section 9.2.1:
> 
> There are two paragraphs on the 403 return code.

This by itself is not really a problem. The document is just mentioning 
several reasons for 403s...

> Section 9.3:
>>    During MKCOL processing, a server MUST
>>    make the Request-URI a member of its parent collection
> Should be: 'internal member URI' instead of 'member'.

Doesn't hurt to be more precise here. Fixed in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.9.3.p.2>.

>>    The precise
>>    behavior of a MKCOL request when the body is present is undefined,
>>    but limited to creating collections, members of a collection, bodies
>>    of members and properties on the collections or members.
> Not sure if 'members' or 'internal members' are intended to be addressed 
> here.

I guess we can stay vague here.

> Section 9.6.
>> MUST destroy locks rooted on the deleted resource
> A look root is an URI, not a resource.

Good catch. Say "MUST destroy those locks where the lock root is the 
Request-URI." or something similar (fixed in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.9.6.p.2>).

> Section 9.6.1:
>> If an error occurs deleting an internal resource ...
> Use the defined term 'member resource' instead of 'internal resource'.

Agreed. There are more problems here:

>    If an error occurs deleting an internal resource (a resource other
>    than the resource identified in the Request-URI) then the response
>    can be a 207 (Multi-Status).  Multi-Status is used here to indicate
>    which internal resources could NOT be deleted, including an error

(see above)

>    code which should help the client understand which resources caused
>    the failure.  For example, the Multi-Status body could include a

...the error code is for finding out the reason, not the resource :-)

>    response with status 423 (Locked) if an internal resource was locked.

Fixes in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.9.6.1.p.6>.

> Section 9.7.1:
>> appropriately scoped parent
> What does 'appropiately scoped ' mean here. Since there is the defined 
> term 'namespace consistency', it should be used here.

I sort-of agree. Any proposal for spec text?

> section 9.9.3:
>>    If a resource exists at the destination and the Overwrite header is
>>    "T" then prior to performing the move the server MUST perform a
>>    DELETE with "Depth: infinity" on the destination resource.  If the
>>    Overwrite header is set to "F" then the operation will fail.
>>   
> Though it is defined later, mentioning the default here might be clearer.

Yes and no. Repeating it risks being inconsistent. Maybe we need a 
forward reference?

> Section 9.10:
>>    Any resource which supports the LOCK method MUST, at minimum, support
>>    the XML request and response formats defined herein.
>>   
> Seems pretty obvious. Better drop it.

Agreed.

> Section 9.10.1
>>    The resource identified in
>>    the Request-URI becomes the root of the lock.
> No, the Request-URI itself becomes the lock root.

Agreed. See comment in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.9.10.1.p.1>.

> Section 9.10.3:
>>    If the Depth header is set to infinity then the resource specified in
>>    the Request-URI along with all its internal members, all the way down
>>    the hierarchy, are to be locked.
> Better use 'members' instead of 'internal members' here.

Actually, that's a pretty severe bug. Fixed in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.9.10.3.p.3>.

> Section 9.10.4:
>>    Later on, the lock
>>    may go away but the empty resource remains.  Empty resources MUST
>>    then appear in PROPFIND responses including that URL in the response
>>    scope.  A server MUST respond successfully to a GET request to an
>>    empty resource, either by using a 204 No Content response, or by
>>    using 200 OK with a Content-Length header indicating zero length
> IMHO, this is at best misleading, since the reader might think that 
> there is something special about empty resources. Drop it.

+1.

> Section 10.1:
> 
> Maybe this is the right place to finally come up with a definition of 
> the widely used term 'DAV-compliant'.
>>    All DAV
>>    compliant resources MUST return the DAV header with compliance-class
>>    "1" on all OPTIONS responses.
> shouldn't it be: 'compliance-class "1" at least'?

That's actually correct. A level-2 or level-3 compliant resource will 
still return "1".

> Section 10.2:
>>    to the
>>    resource and its immediate children, ("Depth: 1"), or the resource
>>    and all its progeny ("Depth: infinity")
> should be: 'to the resource and its internal members ("Depth: 1"), or 
> the resource and all its members ("Depth: infinity")'.

Agreed. See fixes in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.10.2.p.2>.

>>    The Depth header only specifies the behavior of the method with
>>    regards to internal children.  If a resource does not have internal
>>    children then the Depth header MUST be ignored.
>>   
> replace 'internal children' with 'internal members'.

Agreed. See fixes in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.10.2.p.11>.

> Section 10.3
>>    If the source server cannot
>>    attempt a copy to the remote server, it MUST fail the request.
> Yes. What else should the server do? :)

Yep. I've argued a lot about this kind of language in the spec. Please 
get rid of it.

> Section 10.4.1:
>>    On the other hand, the request can succeed only
>>    if one of the described state lists succeeds.
> This should probably be: ' ...can succeed if only one...'

This has already been rewritten. Please review (from 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.10.4.1>):

    o  The first purpose is to make a request conditional by supplying a
       series of state lists.  If none of the state lists match the state
       of the resource it applies to, the request MUST fail with a 412
       (Precondition Failed) status.  Otherwise, the request may succeed.
       The matching functions for ETags and state tokens are defined in
       Section 10.4.4 below.

> Section 13.2:
> 
> The header should be ' Handling redirected member resources'.

Agreed.

> Section 15:
> 
> Live properties are lockable by default. The only live properties 
> defined in this spec that are not lockable are 'lockdiscovery' and 
> 'supportedlock'. Since even a locked live property may change its value, 
> locking a live property can only mean that it becomes protected against 
> PROPPATCH. Since 'lockdiscovery' and 'supportedlock' are protected 
> properties anyway, it does not make much sense to define them not lockable.

Agreed with the analysis. What should we do about this...?

> Section 16:
> 
> The term 'error code' should be replaced by 'status code' throughout 
> this section. The list of postconditions and preconditions is not very 
> readable due to missing document structure. Furthermore, i do not really 
> understand why a certain pre- or postcondition should be bound to a 
> certain status code.

Agreed.

> Section 20.7:
>>    This risk only applies to host address based UUID versions.  Section
>>    4 of [RFC4122] describes several other mechanisms for generating
>>    UUIDs that do involve the host address and therefore do not suffer
>>    from this risk.
>>   
> should be: '... UUIDs that do not involve the host address...'

Good catch. Fixed in 
<http://greenbytes.de/tech/webdav/draft-reschke-webdav-rfc2518bis-latest.html#rfc.section.20.7>.

> Section A.2:
>>    The philosophy of "Be flexible in
>>    what you accept and strict in what you send" still applies, but it
>>    must not be applied inappropriately.
> I am not sure whether it's wise to announce this philosophy and give 
> good examples for it being a bad philosophy in the same paragraph. IMHO, 
> it _is_ a bad philosophy.

The Robustness Principle is not about *accepting* bad data, but about 
*being prepared* for it (thus rejecting requests, not letting the server 
crash). I fear that most people believe it's about making the most of 
bad input, so I think I've given up on discussing this :-)

> Regards,
> Manfred
> 
> P.S: Since I never ever posted on the WebDav mailing list, I probably 
> should introduce myself a little bit: Being a coworker of Julian and 
> Stefan, I have been implementing backend components for SAP Netweaver 
> during the last years, in particular components related to content 
> management and versioning.

Manfred, thanks a lot for the valuable feedback (and for the long delay 
in replying).

Best regards, Julian