Subject: Comments on draft-dusseault-http-patch-03

From: Julian Reschke <[email-masked]>
Date: Sun, 25 Jul 2004 21:12:29 +0200

C-03-01, Section 1

"For example, the HTTP specification has no way for the server to send 
errors if the byte
range in a PUT is invalid."

I find this statement confusing. A server can signal an invalid byte range
through a 4xx status, just like any other client error condition. This
statement should either be removed or clarified.

C-03-02, Section 2

"This specification only defines usage of the platform-portable gdiff
[9] format identified as 'application/gdiff'."

As far as I can tell, there is no registration for "application/gdiff". 
Options:

1) do the registration in this RFC (appendix)
2) have a separate RFC for the gdiff format, including a registration
3) workaround the issue by saying that in the absence of a MIME type, 
the gdiff
format is assumed
4) ...?

(Personal preference: #1)

C-03-03, Section 3.1

"The PATCH method requests that the request body (a patch document) be
applied to the resource named in the Request-URI."

s/named in/identified by/

C-03-04, Section 3.1

"In the model defined in RFC3230 [5], the patch document is modelled
as an instance being sent to the server...."

It's unclear whether we are reusing RFC3250 or not. As far as I can tell,
this spec is independant of RCF3250, so I'm not sure what this paragraph
is trying to define.

C-03-05, Section 3.1

"The first way is to find out the resource ETag at the time the body is
downloaded, and use that Etag in the PATCH request to make sure
the resource is still unchanged."

Say how to do that (for instance, use "If-Match").

"Therefore, if neither strong ETags nor LOCKS are available from
the server, the client MUST use If-Last-Modified as a
less-reliable safeguard."

Or not attempt PATCH at all. Anyway:

s/If-last-Modified/If-Unmodified-Since ([RFC2616], section 14.28)/

"If the Request-URI identifies an unmapped URL,..."

URIs do not identify URLs. How about (stolen from RFC2616):

"If the Request-URI does not point to an existing resource, ..."

C-03-06, Section 3.1

The PATCH example uses the message body below:

        0xd1, 0xff, 0xd1, 0xff
        4
        249,0,0,2
        2,'X','Y
        249,0,2,2
        249,0,1,4
        0

As far as I understand gdiff, the body is actually binary, so it would 
contain
the bytes sequence 0xd1 0xff 0xd1 0xff 0x04 ..., and not some lines of ASCII
characters as implied above.

C-03-07, Sectionb 3.2.1

"A successful response SHOULD have the 204 No Content status code or
the 201 Created status code if a new resource was created.  Either
response indicates that the server successfully applied the delta and
that the response contains no body."

True, but in general *any* 2xx message with the notable exception of 207 
would
indicate that.

"The server SHOULD provide a MD5 hash of the resource entity after the
delta was applied.  This allows the client to verify the success of
the operation.  The PATCH method MUST cause the ETag to change if the
resulting entity is not identical to the original.  If the server
supports strong ETags, the server MUST return a strong ETag for use
in future client operations.  The server SHOULD return the
Last-Modified header in any case, but the server MUST return the
Last-Modified header if ETags aren't supported."

I don't like the interdependencies here. Why not simply say, in addition
to the response headers that would be generated for a PUT request on that
resource, the server SHOULD also provide Content-MD5.

Speaking of which; it would be nice if this spec could define a standard
WebDAV property holding the content MD5, sich as DAV:getcontentmd5.

C-03-08, Section 3.2.2

1) Point to the actual section of RFC3253 defining this 
(<http://greenbytes.de/tech/webdav/rfc3253.html#method.preconditions.and.postconditions>)

2) Make this optional for servers that do not care about WebDAV.

3) Put the condition names into a different namespace, unless the WebDAV WG
agrees to adopt these names (for instance use, urn:ietf:rfc:xxxx).

3) Stick to RFC3253's terminology (the names identify conditions, not 
errors, thus

s/DAV:delta-format-unsupported/p:delta-format-supported/
s/DAV:delta-format-forbidden-on-resource/p:delta-format-allowed-on-resource/
s/DAV:delta-format-badly-formatted/p:delta-documented-well-formatted/
s/DAV:delta-empty-resource/p:delta-format-applies-to-empty/
S/DAV:patch-result-invalid/p:patch-result-valid/

C-03-09, Section 3.3

"The server advertises its support for the features described here
with OPTIONS response headers.  The "Allow" OPTIONS header is already
defined in HTTP 1.1  to contain all the allowable methods on the
addressed resource, so it's natural to add PATCH."

It's not only natural; it's also a requirement.

"OPTIONS * is not used to advertise support for PATCH because the
delta formats supported are likely to change from one resource to
another.  A server MAY include the Accept-Patch header in response to
OPTIONS *, and its value MAY be the union of known supported delta
formats."

I think that if this paragraph is removed, the spec still says the same
thing. It doesn't add anything that doesn't already follow from RFC2616.

E-03-01

"Status of this Memo"

This section uses the outdated IETF IPR statement. Please see the updated
guidelines (<http://www.ietf.org/ietf/1id-guidelines.txt>) and/or the
updated xml2rfc documentation (<http://xml.resource.org/>).

E-03-02

"This standard defines the new method PATCH to alter a single existing
resource, in place, by applying a delta or diff file."

1) It's not a standard. It may become one later.

2) The text uses the terms "delta", "diff file" and later "patch document".
I think it would be wise to formally define *one* term for it once, and
use that one consistently throughout.

E-03-03, References

URL for [9] is missing.