Updating RequestHandlerComponent to accept body of HTTP Delete requests #4108
Conversation
|
Seems reasonable to me. Can you please add tests for this. Though unsure if this should goto into 2.5 or 2.6. |
|
Seems reasonable to include in 2.5 as I would expect the HTTP verb to not impact how JSON is deserialized. |
|
Okay, I've added an assert in what I believe to be the correct existing unit test function. I would like to mention that one for PUT happens to be missing, but I did not add that. |
| @@ -601,6 +601,9 @@ public function testRequestContentTypes() { | |||
| $result = $this->RequestHandler->requestedWith(array('rss', 'atom')); | |||
| $this->assertFalse($result); | |||
|
|
|||
| $_SERVER['REQUEST_METHOD'] = 'DELETE'; | |||
| $this->assertEquals('json', $this->RequestHandler->requestedWith()); | |||
There was a problem hiding this comment.
Indentation is off, but I can fix that post merge.
There was a problem hiding this comment.
My apologies; I think I have spaces on instead of tabs :/
There was a problem hiding this comment.
Yeah it should be tab indentation.
|
Thanks 👍 |
|
Sorry for being late to the disusscion, but I agree with @ADmad, this should have gone into 2.6 as it adds functionality. |
|
@ravage84 Don't augment my response, I never said that it should have gone to 2.6 :) My comment said "unsure if this should go into 2.5 or 2.6". Mark decided it can go to 2.5 and I am fine with that. |
|
@ADmad sorry, wasn't my intend. |
|
I see this as a mistake not a new feature, which is why it went into 2.5 |
Fix fields cache to depend on the schemaName as well. Fixes cakephp#4108
Fixes bad merge of cakephp#1683. Refs cakephp#4108.
I was trying to use HTTP DELETE along with a JSON encoded body. CakePHP was ignoring the body content while it works for POST and PUT. I traced it down to this line of code.
During startup, the
RequestHandlerComponenttries to set$controller->request->dataif therequestedTypeis valid. This was not occurring for DELETE requests because of the checks inrequestedWithhttps://github.com/cakephp/cakephp/blob/master/lib/Cake/Controller/Component/RequestHandlerComponent.php#L216
I've patched
requestedWithto allow DELETE. There are considerable arguments why the body of a DELETE request should be allowed, the strongest being that it the RFC allows it. Here is a stack overflow discussion of that issue at hand:http://stackoverflow.com/questions/14323716/restful-alternatives-to-delete-request-body