Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

$resource DELETE should set the request body if present. #3207

Closed
adamdecaf opened this issue Jul 11, 2013 · 20 comments
Closed

$resource DELETE should set the request body if present. #3207

adamdecaf opened this issue Jul 11, 2013 · 20 comments

Comments

@adamdecaf
Copy link
Contributor

It seems to be that $resource.remove doesn't allow for a request body to be set (but rather sets it as query parameters.) It also looks like it could be changed in one line of resource.js

I haven't found a clear consenseness of if it should be allowed or not. (As per this SO answer). Thoughts?

@squid314
Copy link

squid314 commented Aug 5, 2013

It is my understanding from reading various discussions and documentation across the internet regarding this issue that HTTP's DELETE method must provide all information on what is to be deleted in the URL, even though it is allowed to have body content. In addition, in my testing while building various web apps, several browsers don't send any body content with DELETE requests even if it is provided to the XMLHttpRequest. As such, I don't think that this should be added.

@adamdecaf
Copy link
Contributor Author

I have been reading a few articles / rfc discussions too, and it does seem like DELETE's shouldn't be allowed a body. (Since, as you said all of the information must be in the URL). Thanks for double checking.

@npetrovski
Copy link

I do not see a reason of not having body in the DELETE method. The body could be use for getting entity meta-data, version sync, etc, If I need to be honest I couldn't find the rfc where it is written that DELETE method should not present body; moreover I have seen a lot of software that are in fact count on the DELETE request body (for instance ElasticSearch).

Now, in present day angular $resource is a separate module, and when it comes to practically using it I found some very major problems:

  1. DELETE does not send request body
  2. DELETE does not send proper Content-Type
  3. $request does not allow fully replacing the initial URL template with a dynamic address (this is required when using HAL REST pagination for instance)

@squid314
Copy link

As discussed above @npetrovski, while the RFC does not state that a DELETE cannot have a body, it does state that the definition of what is to be deleted must be fully specified in the URL. Additionally, also specified above, some browsers don't send the body content for DELETE calls even if it is validly specified on the XMLHttpRequest object. Therefore, at least until Angular 2.0, I don't think this is worth considering. If you need to add extra metadata which does not change what is being deleted, you can always specify it as part of the headers of the request (which is probably where it should be).

Regarding ElasticSearch, the documentation for delete which I found states that "The query can either be provided using a simple query string as a parameter, or using the Query DSL defined within the request body." This states that either URL query parameters or the body content may be provided. Is there a different API call you were referencing?

@npetrovski
Copy link

From what you are saying I still cannot see a reason angular not keeping the option of sending body over the DELETE requests. Of course it might be stripped by some browsers, but until it is not a part of any RFC stating that entity must be only specified by the URL and the headers I see this problem as a lack of functionality.

@squid314
Copy link

I'm not sure you understand what the RFC states, though I have attempted to explain it. If you look at the spec for DELETE and compare it to the spec for something like POST or PUT, you should be able to see the difference. The POST and PUT specs state that the "enclosed entity" should be associated with the "Request-URI". However, the DELETE spec says nothing about an "enclosed entity" and only mentions the Request-URI stating "The DELETE method requests that the origin server delete the resource identified by the Request-URI." Therefore, even if all browsers actually included the body content when they sent the request, a fully compliant server could ignore the "enclosed entity" entirely. (I'm confident that a compliant browser and server could handle requests while observing or ignoring the "enclosed entity" randomly, though that would be very unlikely to be implemented.) Since both the browser and the server can ignore the "enclosed entity", the $resource service would have to either detect what the browser and server are actually doing and handle that situation specially or send the identifying information both in the Request-URI and the "enclosed entity". Personally, I don't think that Angular should be supporting a non-standard feature such as this in the $resource service.

@npetrovski
Copy link

@squid314 I am fascinated from how you guys can read between the lines and assume something that has never been written. The fact that (as you said) "enclosed entity" is not mentioned in the RFC2616 section 9.7 (DELETE) does not necessary mean that it should not be presented in the request. Moveover, if you read the rfc a bit more carefully you would notice that message body is ONLY and explicitly forbidden in TRACE requests (section 9.8). The other statements related to the message body say:
- an entity-body is only present when a message-body is present (section 7.2)
- the presence of a message-body is signaled by the inclusion of a Content-Length or Transfer-Encoding header (section 4.3)
- a message-body must not be included when the specification of the request method does not allow sending an entity-body (section 4.3)

Now, when it comes to the so-called fully compliant servers and browsers that are stripping the message body on DELETE requests - I have never seen a statement in agular.js documentation where it is written that it should be only used with one or another set of clients/servers. Thinking outside of the box the shiny js library you are providing can be also used by custom clents and server configurations - are you not agree? I don't think a prerogative of any js library should be to force one or another protocol constraint - the way it will be used must be in the developer's hands. Talking about developers - I can't see nothing more natural when implementing a standard CRUD to use respectively POST, GET, PUT and DELETE and having those implemented with asame manner.

I still believe having this regex (in the $resource lib) that defines $hasBody only for certain HTTP methods is bad written and should be considered for refactoring.

@squid314
Copy link

I am not assuming anything that "has never been written." I am claiming that the outcome of including a message-body or an entity-body is not defined in the HTTP spec for a DELETE request. Therefore a fully compliant server can do anything they want regarding the body content (including ignore it or core dump or start a large number of small fires in Yugoslavia). I am also not claiming that AngularJS is written to target any specific clients/servers/proxies/grapefruits/etc. However, the $resource service is written on top of the $http service and the $http service "is a core Angular service that facilitates communication with the remote HTTP servers via the browser's XMLHttpRequest object or via JSONP." (Emphasis mine; see here for original.) Therefore, Angular's $http service must be written to work within the bounds of a fully compliant implementation of HTTP (on both the client and server side). If you are asking the AngularJS developers to start working outside the official specification of HTTP, that is effectively asking them to begin supporting the custom unpublished very-close-to-but-not-quite-HTTP that you use. I am not claiming that it wouldn't be nice/interesting/fun/etc. to have the $resource be able to support this slightly different protocol, but I am claiming that I don't believe that it is reasonable to ask the AngularJS developers to start supporting this slight modification of the HTTP spec. I hope this makes my position more clear. (Don't forget that you can create your own fork of the repository, make the modification that you believe should be applied, and use that version of the library, updating your fork when Angular makes updates.)

@npetrovski
Copy link

@squid314

Again,

  1. Not defined does not necessary mean denied
  2. It is written in the official and accepted RFC (the same one you quoted) that message body should not be included when the method does not allow it (and it is explicitly forbidden in TRACE requests)
  3. I am not asking you guys start writing something that is outside of the official protocol specification, but just leave the protocol constraints to the web clients/servers to manage this. This wont make your lib inaccurate
  4. Correct me if am I wrong, but angular's $http does not implement the same restrictions (stripping the message body on DELETE) and it is still fully compliant
  5. and yes, thank you - I know I can fork and work my own version of $resource, but I guess this aint something I am targeting at the moment

Thank you for being responsive @squid314 . I believe I stated my point clear enough and if you guys don't want to accept it or have different understanding of how the RFC defines (or not defines) the problem it is totally up to you.

Good luck.

(there is no Yugoslavia anymore, the communism is over :P )

@squid314
Copy link

1., 2. I agree completely.
3. I'm not sure that I completely agree on this one, but that's okay.
4. So, I just looked through the code for the $http service (at least the version I have cloned). While it appears that the core $http service doesn't remove the post data from the request object before forwarding the request information to the underlying XMLHttpRequest, the shortcut methods like $http.delete don't include the post data while the shortcut methods like $http.post do include it. This means that you can attempt to submit a body as part of HEAD, DELETE, etc. requests if you set it up like $http(url, { method: 'delete', data: {blah: 'blah} }) rather than $http.delete(url, {blah: 'blah'}) (since the object is treated as the $http config hash). (I think that $http.delete(url, { data: {blah: 'blah'}}) will work but I'm not sure; probably easier to test it than walk through the code.) This information is mirrored in the docs for $http where put and post allow a data param but the rest of the shortcut methods don't.

(What? You don't think that I have equipped my web servers with time-travelling flamethrowers? Fine, you're right; I haven't. I just love the name, not sure why, and I keep forgetting that it is one of those no-longer-existing countries.)

verath added a commit to we4sz/DAT076 that referenced this issue Oct 25, 2014
* Adding a route now returns the added route object. Changed some method signatures to get the saved instance.
* Remove now uses path params and id, as angular does not allow for sending data in DELETE requests. angular/angular.js#3207
@anton000
Copy link

anton000 commented Nov 7, 2014

+1

@pmariduena
Copy link

I know this is closed, but I think it should be added (I agree with @npetrovski). Maybe a custom property could be added to $resource (under "options", say "enableDeleteRequestBody"), with a warning that this is not the officially Spec-compliant use of a DELETE (use at your own risk). But I would really like to use it. It would allow for batch deletes of multiple, non-consecutive objects that represent, say, rows in a database. specifying those rowIds as, say, query string params in the URL, or request headers, is messy. Apache Jetty server is definitely accepting my DELETE body request that I send from POSTman.

@adamdecaf
Copy link
Contributor Author

FWIW I converted the endpoints to something like this:

PUT /users

{
  "userIds": ["userId1"]
  "status": "Inactive"
}

@chrisvdb
Copy link

So, I have a 'DELETE' endpoint that requires a CSRF token in the body. Is it correct that I cannot access this endpoint with Angular's $http service?

@gkalpak
Copy link
Member

gkalpak commented Oct 16, 2015

As discussed here (and in other threads), neither $http's delete() shortcut (see #3207 (comment)), nor $resource's remove()/delete() methods support a body.
If you must, you can use a "generic" $http call like this: $http({method: 'delete', url: '...', data: ..., ...})

@chrisvdb
Copy link

I could not get it to work with the generic $http call either... I managed to change the endpoint to accept a cookie-based token instead.

@gkalpak
Copy link
Member

gkalpak commented Oct 20, 2015

Actually, I was wrong: $http.delete() seems to send the payload (demo) - it's just $resource's $delete()/$remove() methods that don't (for reasons already discussed).

So, there shouldn't be anything (Angular-specific) stopping you from having a request body with DELETE (using $http).

@dongseok0
Copy link

When I use generic $http with DELETE and payload, the payload is not appear from my node.js endpoint but it appears if I use PostMan. That was because content-type is not set. It seems only post and put set default as json.
https://docs.angularjs.org/api/ng/service/$http

@gkalpak
Copy link
Member

gkalpak commented May 6, 2016

@dongseok0, this happens because only PUT, POST and PATCH requests are expected to have a payload. If you want to send data with a DELETE requests, you have to specify the headers yourself.
BTW, this can also be configured globally (for all DELETE requests using $httpProvider.defaults.headers).

@ifbbprochris
Copy link

@gkalpak hey boddy
If I want to send data with a DELETE requests, how could I configure the $httpProvider.defaults.headers.

It should be like $httpProvider.defaults.headers.delete = { "Content-Type": "application/json;charset=utf-8" }? or like what?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants