Subject: feedback on rfc2518bis-14

From: Manfred Baedke <[email-masked]>
Date: Tue, 14 Mar 2006 18:22:45 +0100

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.

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

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.

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.

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.

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.

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

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.

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

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.

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

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?

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.

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

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

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?

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

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.

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

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:'

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.

Section 9.2.1:

There are two paragraphs on the 403 return code.

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

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

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

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.

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.

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.

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.

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.

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.

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'?

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")'.
>    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'.

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? :)

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

Section 13.2:

The header should be ' Handling redirected member resources'.

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.

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.

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

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.

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.