allow POST to create filenames if resource exists

[jeff-zucker]

POST on an existing resource or container should create an alternate filename (e.g. prepend a number) and save another version rather than overwriting.

[bourgeoa]

see PR #37

[bourgeoa]

replaced by PR #38

[bourgeoa]

@jeff-zucker
You proposed to replace my funny headers.slug by headers.location but it does not work because on POST location includes the pathname and not pathname + slug.
Have you an other idea to find the resource name created by the server.

Knowing that it works may be it is sufficient to test that :

• folder containing : slug and uuid.v1()-slug
• delete resource slug
• cannot delete folder

Then I can remove the funny headers.slug I added

[jeff-zucker]

I am fixing the location header and adding tests for it. That should eliminate the need for headers.slug.

We currently return 403 if user attempts to POST an aux resource. I believe that it should be 409 - method not allowed, rather than 403 authorization denied. How does NSS do it?

[Otto-AA]

We currently return 403 if user attempts to POST an aux resource. I believe that it should be 409 - method not allowed, rather than 403 authorization denied. How does NSS do it?

409 is Conflict, 405 would be "Method not allowed". But afaik 405 means that the http method (GET/POST/...) is not allowed which would not be true in our case because POST in general is allowed. I'd just go with 400 Bad Request.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Status

[bourgeoa]

I am fixing the location header and adding tests for it. That should eliminate the need for headers.slug.

We currently return 403 if user attempts to POST an aux resource. I believe that it should be 409 - method not allowed, rather than 403 authorization denied. How does NSS do it?

• actually in NSS returns 403, but it is a recent patch in aclChecker.js :

    // If the slug is an acl, reject
    if (this.isAcl(this.slug)) {
      this.aclCached[cacheKey] = Promise.resolve(false)
      return this.aclCached[cacheKey]
    }

• This is what I found in the new spec :

Reading and Writing Resources ## {#read-write}

Servers MUST respond with the 405 status code to requests using HTTP methods
that are not supported by the target resource.
[Source]

Resource type heuristics ### {#resource-type-heuristics}

When creating new resources, servers can determine an effective request URI's
type by examining the URI path ending ([[#uri-slash-semantics]]).

Clients who want to assign a URI to a resource, MUST use PUT and PATCH
requests. Servers MAY allow clients to suggest the URI of a resource created
through POST, using the HTTP Slug header as defined in [[!RFC5023]].

Clients who want the server to assign a URI of a resource, MUST use the POST
request.

[Source]

[jeff-zucker]

I agree with Otto that 400 bad request is most descriptive. 405 method not allowed could also work although I agree with Alain that in this case it's more "method not allowed for doing this" than a total "method not allowed". OTOH, that is what we do with an attempt to POST a container - it emits a 405. I'd say that POST of a container and of an ACL should both get either 400 or 405. Since the spec mentions 405, I'd say go with that.

I have location working for POST of a resource - it sends the correct URI for either a direct POST or a POST with a newly created slug and can be read with response.headers.get(). I am having a harder time with POST of container, I think because of the weirdness around trailing slashes in localStorage.

[Otto-AA]

Yes, I think that 405 is the better option. I didn't see the "[...] by the target resource" part, and I think that's exactly our use case (the POST method is not allowed for the target acl resource). This is the normal HTTP spec btw (https://tools.ietf.org/html/rfc7231#section-6.5.5)

When doing that, we shouldn't include POST in the Allow header, because it is not allowed for this resource (also part of the HTTP spec).

[jeff-zucker]

Good points. So I will change the code to 405, finalize location header for containers, and change the Allow header to remove POST for acl resources and containers. Are there any other situations in which the Allow header should be changed?

[bourgeoa]

• remove POST in allow header is for all auxiliary resources.
• slug for Container do not end with / it needs to be added to header location

[bourgeoa]

@jeff-zucker

change the Allow header to remove POST for acl resources and containers

I suppose you meant PUT for containers

[jeff-zucker]

Yes, thanks, I meant PUT container. And noted, POST should not be allowed on any aux resource.

I had already figured out about the trailing slash on container slugs. I now have a really odd situation in which the slug returned as location is a different slug than is used to create the container (but not the file).

[jeff-zucker]

In commit 6db5df4, I fixed the location header for containers when new slugs are generated (turned out to be a problem with the test) and changed the error code for attempt to POST to an aux to 405. I have some other fires to put out, so leaving the changes to the Allow header for another day - opening a separate issue on them.

[jeff-zucker]

I am looking into the allow header and have doubts about whether it is server-specific or resource-specific. In either case, it is really only a useful thing to have if we also support OPTIONS since the OPTIONS method is the only one that can be queried with the results of the ALLOW header, So, I'm lumping this in with the existing issue of creating an OPTIONS method.

@bourgeoa it there are other items on your checklist or that you are otherwise aware of where solid-rest doesn't match the spec, please raise issues.

[Otto-AA]

I am looking into the allow header and have doubts about whether it is server-specific or resource-specific. In either case, it is really only a useful thing to have if we also support OPTIONS since the OPTIONS method is the only one that can be queried with the results of the ALLOW header, So, I'm lumping this in with the existing issue of creating an OPTIONS method.

It's resource specific and should always be returned if 405 is returned, not only for OPTIONS (e.g. if someone tries to POST an acl resource, the server must return the ALLOW header for this specific resource). Here's the complete 405 spec:

6.5.5. 405 Method Not Allowed

The 405 (Method Not Allowed) status code indicates that the method
received in the request-line is known by the origin server but not
supported by the target resource. The origin server MUST generate an
Allow header field in a 405 response containing a list of the target
resource's currently supported methods.

A 405 response is cacheable by default; i.e., unless otherwise
indicated by the method definition or explicit cache controls (see
Section 4.2.2 of [RFC7234]).

[jeff-zucker]

@Otto-AA - good points, copying your comment to #20 and responding there