Openapi-specification: Allow requestBody for the DELETE method.

Created on 8 Jan 2019  路  21Comments  路  Source: OAI/OpenAPI-Specification

I would like the specification to allow a requestBody in the DELETE method and other methods without explicitly defined semantics.

One of the answers in this StackOverflow post states: "The spec does not explicitly forbid or discourage it, so I would tend to say it is allowed."
I would agree with that statement.

Currently the OpenAPI spec says "The requestBody is only supported in HTTP methods where the HTTP 1.1 specification RFC7231 has explicitly defined semantics for request bodies."

I think this should raise a notice, but not be unsupported and raise an error.

Addendum:

As I mentioned in a comment below, if you do choose to create a batch delete method on your API, whether or not anyone recommends it, make sure to check your cache settings and think about how these settings will interact with DELETE requests. Calling DELETE on /users with data [3,4] will not invalidate the cache for /users/3 or /users/4, so a GET request to either of them may return invalid data depending on cache settings. My comment below describes methods of mitigating this.

Most helpful comment

@darrelmiller

While I understand the desire to be able to describe existing APIs that have less than ideal behavior, we have to balance that with opening the door for more people to create new APIs that make this same mistake because we allow it.

I wouldn't expect people to creating APIs to constrain themselves by OpenAPI. when I create an API, first comes the needed functionality, and documenting it with OpenAPI or whatever tool I am using, that comes after. I wouldn't worry about "opening the door" for unencouraged behavior as that's not really OpenAPI's door to hold.

I'm using OpenAPI to document an API that does use delete with a request body, and is not going to change. seems to me I should be able to describe this API with a valid OpenAPI document.

All 21 comments

The way I read the latest specification for Operation requestBody I think it does not strictly prohibit DELETE body:

The request body applicable for this operation. The requestBody is only supported in HTTP methods where the HTTP 1.1 specification RFC7231 has explicitly defined semantics for request bodies. In other cases where the HTTP spec is vague, requestBody SHALL be ignored by consumers.

"... not supported ..." and "... SHALL ..." parts - no clear "cannot have" message.

It would be better to have more concrete language defining that it is allowed to put requestBody property under delete method in Open API v3 spec so existing UI tools stop showing errors when body encountered.

@keenle UI tools are correct to show an error as the spec says that any such request SHALL be ignored. If your UI tool is trying to show you where you are doing things that don't work, it should highlight this as an error.

@adjenks regarding

"The spec does not explicitly forbid or discourage it, so I would tend to say it is allowed."

"no defined semantics" is standards-ese for "seriously, don't do this, literally anything can happen, but for some reason (probably historical weirdness or difficulty of reliable enforcement) we can't outright forbid it"

Anytime you see "no defined semantics" or "the behavior is undefined" in an RFC, you should stay far away from it.

@handrews , agreed on UI behavior in response to the current definition of requestBody in Open API specification. My ask is rather to the specification to allow requestBody as RFC-7231 does not prohibit the payload with DELETE request:

A payload within a DELETE request message has no defined semantics; sending a payload body on a DELETE request might cause some existing implementations to reject the request.

I see the point in your suggestion to stay away from undefined stuff, though there are live services out there which accept payload with DELETE. The approach does make sense in certain scenarios which are out of the scope of this issue.

Users may be documenting an existing legacy API that cannot change, so there may be a DELETE method requiring a body. I understand it's not recommended, but because behavior is "undefined" in the RFC, I think it should be declared as "undefined" in OpenAPI, not "unsupported". Other methods explicitly prohibit the use of a body such as TRACE:

A client MUST NOT send a message body in a TRACE request.

So I believe the same distinction should be made in OpenAPI.

@adjenks While I understand the desire to be able to describe existing APIs that have less than ideal behavior, we have to balance that with opening the door for more people to create new APIs that make this same mistake because we allow it.

I don't know why the HTTP spec is explicit about the request body being not allowed on some methods and not on others.

I don't know why the HTTP spec is explicit about the request body being not allowed on some methods and not on others.

RFC 2616 didn't talk about request bodies for GET, DELETE, or TRACE at all, so I assume when they were working on RFC 7231 they locked things down as much as they could get away with.

RFC 2616 doesn't specifically allow them, it was just pretty light on details of method semantics in general. Part of the reason for the 723x RFCs was to improve that situation.

@darrelmiller

While I understand the desire to be able to describe existing APIs that have less than ideal behavior, we have to balance that with opening the door for more people to create new APIs that make this same mistake because we allow it.

I wouldn't expect people to creating APIs to constrain themselves by OpenAPI. when I create an API, first comes the needed functionality, and documenting it with OpenAPI or whatever tool I am using, that comes after. I wouldn't worry about "opening the door" for unencouraged behavior as that's not really OpenAPI's door to hold.

I'm using OpenAPI to document an API that does use delete with a request body, and is not going to change. seems to me I should be able to describe this API with a valid OpenAPI document.

{json:api} explicitly defines a DELETE request body for updating to-many relationships and it has quite well-defined semantics: Remove only the supplied set of resource identifier objects from the (potentially larger) full list of related resource identifier objects.

I am unable to fully represent a {json:api} schema with OAS if it explicitly prohibits what RFC 7231 allows (as undefined but not prohibited). Please (re)consider allowing a request body for DELETE.

If one does want to support a request body in their API, they must consider the caching consequences. As stated in RFC7234:

A cache MUST invalidate the effective Request URI (Section 5.5 of
[RFC7230]) as well as the URI(s) in the Location and Content-Location
response header fields (if present) when a non-error status code is
received in response to an unsafe request method.

So, using the example in {json:api}:

DELETE /articles/1/relationships/comments HTTP/1.1
Content-Type: application/vnd.api+json
Accept: application/vnd.api+json

{
  "data": [
    { "type": "comments", "id": "12" },
    { "type": "comments", "id": "13" }
  ]
}

The cache for the resource /articles/1/relationships/comments will be cleared. Imagine this sequence of events:

  1. User-agent requests GET /articles/1/relationships/comments
  2. Server returns 1000 comments and tells the user-agent (hypothetically) to cache it.
  3. User-agent requests GET /articles/1/relationships/comments/12
  4. Server returns 1 comment and tells the user-agent (hypothetically) to cache it.
  5. The DELETE request from the above example is made, it deletes objects 12 and 13 and returns Status 200.
  6. The browser interprets this as having deleted the resource at /articles/1/relationships/comments and it clears the cache for this path.
  7. User-agent requests GET /articles/1/relationships/comments again.
  8. Browser fetches a fresh 998 comments because cache was cleared. This is a good thing.
  9. User-agent requests GET /articles/1/relationships/comments/12 again.
  10. Browser pulls cached resource from steps 3&4. This is not so great.

It's a good thing that the cache was cleared for the main resource, since you don't want it to use the cached 1000 comments when there are only 998. So having it invalidate the base resource is good. However, user-agents will not invalidate /articles/1/relationships/comments/12 and /articles/1/relationships/comments/13 when requesting the batch delete resource. Therefore fetching a single comment (step 9) may return invalid results (10) if they were previously requested (3), cached, deleted via a batch request, and requested again, since the cache may be used.

Overall, if you do choose to create a batch delete method on your API, whether or not anyone recommends it, make sure to check your cache settings and think about how these settings will interact with DELETE requests.

To mitigate this you could disable caching of individual resources, but the client would then have to fetch every time. Using standard HTTP/1 you can't invalidate arbitrary caches, so deleting 12, and 13 in a batch wouldn't clear their cache, as mentioned before, unless you HTTP/2 server push which allows you to send arbitrary other resources along with the response for another request, so your response to a batch delete could pack with it the invalidation of those two resources.

There's a lot to consider here. I do think that it would be great to support a batch delete operation in multiple standards and protocols, but it needs to be thoroughly discussed and everyone would have to cooperate on the appropriate course of action.

It is also interesting to consider that using a custom method, which is the commonly recommended way to perform a batch delete request, such as POST /comments/deleteComments, or /comments:deleteComments, will also not invalidate any caches associated with the resources that your request intended to delete. So a subsequent request to GET /comments/5 after it has been deleted via a batch method may fetch stale and invalid results.

Tiny fact that is missing from this discussion and totally worth mentioning since it can be an obstacle on a path of upgrading from Swagger v2 to Open API v3 is that with Swagger v2 one can declare payload for DELETE method.

Swagger UI can render DELETE endpoint with payload.

This is a gentle bump to see if you are any closer to deciding on this issue?

We are in the process of migrating from Swagger v2 to Open API v3, we have some services that provides DELETE endpoints that currently rely on a request body. This is currently stopping us from fully adapting Open API v3 which has a lot of benefits over v2 for us.

Is there anything we can do from our end to help accelerate and resolve this issue?

@mikaeldanielsson I added it to the TSC agenda for Thursday.

@mikaeldanielsson According to my reading of https://www.openapis.org/participate/how-to-contribute this needs to become a "next proposal" which only the TSC members can label as such.

Am I correct @darrelmiller that this is the formal process for bringing issues to the TSC? (I "picked on" you since you appear to be the main committer of much of the TSC process documentation; Apologies if you are not the best contact for this and please do indicate the appropriate mechanism).

Does the TSC need a more formally-written request to consider this issue? At this point it appears to be:

  1. A requirement for backward-compatibility with Swagger 2 per @keenle
  2. A requirement for certain pre-existing API frameworks including https://jsonapi.org
  3. Not explicitly disallowed ("MUST NOT", "SHALL NOT" nor even "NOT RECOMMENDED" or "SHOULD NOT") by RFC 7231.
  4. Was not explicitly disallowed in RAML 1.0 which was also an input to OAS 3 (@usarid ?)

Language in the OAS could certainly be written to warn against use of the requestBody with DELETE.

I would be happy to collaborate on writing a more formal request or spec document PR.

As this is not a new feature, nor one that requires significant updates to the spec, I think a PR would be sufficient. We will discuss on Thursday's meeting and see if we think this is a fix that should be made. If we get consensus there then a PR would be gratefully accepted.

@darrelmiller per the TSC call, I'll begin working on a PR.

Is v3.0.3-dev the correct branch to base the PR on?

See #1929 (TSC Open Meeting of 2019-05-30) where this was discussed.

Is there any update for this? Thanks.

See #1937.

The issues/PRs mentioned here have been merged or are in progress and tracked in more current meeting agendas.

Oops too many tabs open, closed the wrong one, sorry!

Actually, looks like this issue's PR was also merged so I should have just left it closed 馃槅

Was this page helpful?
0 / 5 - 0 ratings