From 802317c705a2964b538e5b68a7c1149e9b1eebd3 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Mon, 6 Oct 2008 14:58:01 +0000 Subject: [PATCH] FEATURE Added HTTP method override support to HTTPRequest and Form (through $_POST['_method'] or $_SERVER['X-HTTP-Method-Override']), incl. unit tests ENHANCEMENT Added Form->FormHttpMethod() ENHANCEMENT Added HTTPRequest->httpMethod() ENHANCEMENT Added HTTPRequest::detect_method() git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@63679 467b73ca-7a2a-4603-9d3b-597d59a354a9 --- core/control/Director.php | 5 ++- core/control/HTTPRequest.php | 48 ++++++++++++++++++--- forms/Form.php | 36 ++++++++++++++-- tests/HTTPRequestTest.php | 84 ++++++++++++++++++++++++++++++++++++ tests/forms/FormTest.php | 35 ++++++++++++++- 5 files changed, 194 insertions(+), 14 deletions(-) diff --git a/core/control/Director.php b/core/control/Director.php index eedf1d2c8..3786caf1e 100644 --- a/core/control/Director.php +++ b/core/control/Director.php @@ -89,7 +89,7 @@ class Director { */ function direct($url) { $req = new HTTPRequest( - $_SERVER['REQUEST_METHOD'], + (isset($_SERVER['X-HTTP-Method-Override'])) ? $_SERVER['X-HTTP-Method-Override'] : $_SERVER['REQUEST_METHOD'], $url, $_GET, array_merge((array)$_POST, (array)$_FILES), @@ -137,7 +137,8 @@ class Director { * @param array $postVars The $_POST & $_FILES variables * @param Session $session The {@link Session} object representing the current session. By passing the same object to multiple * calls of Director::test(), you can simulate a peristed session. - * @param string $httpMethod The HTTP method, such as GET or POST. It will default to POST if postVars is set, GET otherwise + * @param string $httpMethod The HTTP method, such as GET or POST. It will default to POST if postVars is set, GET otherwise. + * Overwritten by $postVars['_method'] if present. * @param string $body The HTTP body * @param array $headers HTTP headers with key-value pairs * @return HTTPResponse diff --git a/core/control/HTTPRequest.php b/core/control/HTTPRequest.php index e91a27a6a..e351db6b7 100644 --- a/core/control/HTTPRequest.php +++ b/core/control/HTTPRequest.php @@ -25,12 +25,12 @@ class HTTPRequest extends Object implements ArrayAccess { protected $dirParts; /** - * The URL extension + * @var string $extension The URL extension (if present) */ protected $extension; /** - * The HTTP method: GET/PUT/POST/DELETE/HEAD + * @var string $httpMethod The HTTP method in all uppercase: GET/PUT/POST/DELETE/HEAD */ protected $httpMethod; @@ -63,14 +63,14 @@ class HTTPRequest extends Object implements ArrayAccess { * @var array $allParams Contains an assiciative array of all * arguments matched in all calls to {@link RequestHandlingData->handleRequest()}. * Its a "historical record" thats specific to the current call of - * {@link handleRequest()}, and only complete once the last call is made. + * {@link handleRequest()}, and is only complete once the "last call" to that method is made. */ protected $allParams = array(); /** * @var array $latestParams Contains an associative array of all * arguments matched in the current call from {@link RequestHandlingData->handleRequest()}, - * as denoted with a "$"-prefix in the $url_handler definitions. + * as denoted with a "$"-prefix in the $url_handlers definitions. * Contains different states throughout its lifespan, so just useful * while processed in {@link RequestHandlingData} and to get the last * processes arguments. @@ -223,7 +223,7 @@ class HTTPRequest extends Object implements ArrayAccess { * Construct a HTTPRequest from a URL relative to the site root. */ function __construct($httpMethod, $url, $getVars = array(), $postVars = array(), $body = null) { - $this->httpMethod = $httpMethod; + $this->httpMethod = strtoupper(self::detect_method($httpMethod, $postVars)); $url = preg_replace(array('/\/+/','/^\//', '/\/$/'),array('/','',''), $url); @@ -347,7 +347,7 @@ class HTTPRequest extends Object implements ArrayAccess { foreach($arguments as $k => $v) { if($v || !isset($this->allParams[$k])) $this->allParams[$k] = $v; } - + if($arguments === array()) $arguments['_matched'] = true; return $arguments; } @@ -410,7 +410,7 @@ class HTTPRequest extends Object implements ArrayAccess { if($count == 1) return array_shift($this->dirParts); else for($i=0;$i<$count;$i++) $return[] = array_shift($this->dirParts); } - + /** * Returns true if the URL has been completely parsed. * This will respect parsed but unshifted directory parts. @@ -452,4 +452,38 @@ class HTTPRequest extends Object implements ArrayAccess { } return $mimetypes; } + + /** + * @return string HTTP method (all uppercase) + */ + public function httpMethod() { + return $this->httpMethod; + } + + /** + * Gets the "real" HTTP method for a request. + * + * Used to work around browser limitations of form + * submissions to GET and POST, by overriding the HTTP method + * with a POST parameter called "_method" for PUT, DELETE, HEAD. + * Using GET for the "_method" override is not supported, + * as GET should never carry out state changes. + * Alternatively you can use a custom HTTP header 'X-HTTP-Method-Override' + * to override the original method in {@link Director::direct()}. + * The '_method' POST parameter overrules the custom HTTP header. + * + * @param string $origMethod Original HTTP method from the browser request + * @param array $postVars + * @return string HTTP method (all uppercase) + */ + public static function detect_method($origMethod, $postVars) { + if(isset($postVars['_method'])) { + if(!in_array(strtoupper($postVars['_method']), array('GET','POST','PUT','DELETE','HEAD'))) { + user_error('Director::direct(): Invalid "_method" parameter', E_USER_ERROR); + } + return strtoupper($postVars['_method']); + } else { + return $origMethod; + } + } } \ No newline at end of file diff --git a/forms/Form.php b/forms/Form.php index b8152d455..77440dc2d 100644 --- a/forms/Form.php +++ b/forms/Form.php @@ -319,6 +319,13 @@ class Form extends RequestHandlingData { $this->securityTokenAdded = true; } + // add the "real" HTTP method if necessary (for PUT, DELETE and HEAD) + if($this->FormMethod() != $this->FormHttpMethod()) { + $methodField = new HiddenField('_method', '', $this->FormHttpMethod()); + $methodField->setForm($this); + $extraFields->push($methodField); + } + return $extraFields; } @@ -465,16 +472,37 @@ class Form extends RequestHandlingData { } /** - * Returns the form method. + * Returns the real HTTP method for the form: + * GET, POST, PUT, DELETE or HEAD. + * As most browsers only support GET and POST in + * form submissions, all other HTTP methods are + * added as a hidden field "_method" that + * gets evaluated in {@link Director::direct()}. + * See {@link FormMethod()} to get a HTTP method + * for safe insertion into a
tag. * - * @return string 'get' or 'post' + * @return string HTTP method */ - function FormMethod() { + function FormHttpMethod() { return $this->formMethod; } /** - * Set the form method - get or post + * Returns the form method to be used in the tag. + * See {@link FormHttpMethod()} to get the "real" method. + * + * @return string Form tag compatbile HTTP method: 'get' or 'post' + */ + function FormMethod() { + if(in_array($this->formMethod,array('get','post'))) { + return $this->formMethod; + } else { + return 'post'; + } + } + + /** + * Set the form method: GET, POST, PUT, DELETE. * * @param $method string */ diff --git a/tests/HTTPRequestTest.php b/tests/HTTPRequestTest.php index dd8d6a436..22a85c5b1 100644 --- a/tests/HTTPRequestTest.php +++ b/tests/HTTPRequestTest.php @@ -15,4 +15,88 @@ class HTTPRequestTest extends SapphireTest { $this->assertEquals(array("_matched" => true), $request->match('add', true)); } + public function testHttpMethodOverrides() { + $request = new HTTPRequest( + 'GET', + 'admin/crm' + ); + $this->assertTrue( + $request->isGET(), + 'GET with no method override' + ); + + $request = new HTTPRequest( + 'POST', + 'admin/crm' + ); + $this->assertTrue( + $request->isPOST(), + 'POST with no method override' + ); + + $request = new HTTPRequest( + 'GET', + 'admin/crm', + array('_method' => 'DELETE') + ); + $this->assertTrue( + $request->isGET(), + 'GET with invalid POST method override' + ); + + $request = new HTTPRequest( + 'POST', + 'admin/crm', + array(), + array('_method' => 'DELETE') + ); + $this->assertTrue( + $request->isDELETE(), + 'POST with valid method override to DELETE' + ); + + $request = new HTTPRequest( + 'POST', + 'admin/crm', + array(), + array('_method' => 'put') + ); + $this->assertTrue( + $request->isPUT(), + 'POST with valid method override to PUT' + ); + + $request = new HTTPRequest( + 'POST', + 'admin/crm', + array(), + array('_method' => 'head') + ); + $this->assertTrue( + $request->isHEAD(), + 'POST with valid method override to HEAD ' + ); + + $request = new HTTPRequest( + 'POST', + 'admin/crm', + array(), + array('_method' => 'head') + ); + $this->assertTrue( + $request->isHEAD(), + 'POST with valid method override to HEAD' + ); + + $request = new HTTPRequest( + 'POST', + 'admin/crm', + array('_method' => 'head') + ); + $this->assertTrue( + $request->isPOST(), + 'POST with invalid method override by GET parameters to HEAD' + ); + } + } \ No newline at end of file diff --git a/tests/forms/FormTest.php b/tests/forms/FormTest.php index 732c072ed..26ef39ed6 100644 --- a/tests/forms/FormTest.php +++ b/tests/forms/FormTest.php @@ -3,7 +3,7 @@ * @package sapphire * @subpackage tests */ -class FormTest extends SapphireTest { +class FormTest extends FunctionalTest { public function testLoadDataFromRequest() { $form = new Form( @@ -45,5 +45,38 @@ class FormTest extends SapphireTest { $this->assertEquals($fields->fieldByName('othernamespace[key5][key6][key7]')->Value(), 'val7'); } + public function testFormMethodOverride() { + $form = $this->getStubForm(); + $form->setFormMethod('GET'); + $this->assertNull($form->dataFieldByName('_method')); + + $form = $this->getStubForm(); + $form->setFormMethod('PUT'); + $this->assertEquals($form->dataFieldByName('_method')->Value(), 'put', + 'PUT override in forms has PUT in hiddenfield' + ); + $this->assertEquals($form->FormMethod(), 'post', + 'PUT override in forms has POST in tag' + ); + + $form = $this->getStubForm(); + $form->setFormMethod('DELETE'); + $this->assertEquals($form->dataFieldByName('_method')->Value(), 'delete', + 'PUT override in forms has PUT in hiddenfield' + ); + $this->assertEquals($form->FormMethod(), 'post', + 'PUT override in forms has POST in tag' + ); + } + + protected function getStubForm() { + return new Form( + new Controller(), + 'Form', + new FieldSet(new TextField('key1')), + new FieldSet() + ); + } + } ?> \ No newline at end of file