+global\Form::httpSubmission()

Complexity

Conditions	21
Paths	1125

Size

Total Lines	131
Code Lines	63

Duplication

Lines	0
Ratio	0 %

Importance

Changes

0

<?php
	public function httpSubmission($request) {
		// Strict method check
		if($this->strictFormMethodCheck) {

			// Throws an error if the method is bad...
			if($this->formMethod != $request->httpMethod()) {
				$response = Controller::curr()->getResponse();
				$response->addHeader('Allow', $this->formMethod);
				$this->httpError(405, _t("Form.BAD_METHOD", "This form requires a ".$this->formMethod." submission"));
			}

			// ...and only uses the variables corresponding to that method type
			$vars = $this->formMethod == 'GET' ? $request->getVars() : $request->postVars();
		} else {
			$vars = $request->requestVars();
		}

		// Populate the form
		$this->loadDataFrom($vars, true);

true is of type boolean, but the function expects a integer.

It seems like $vars defined by $request->requestVars() on line 355 can also be of type null; however, Form::loadDataFrom() does only seem to accept array|object<DataObject>, maybe add an additional type check?

		// Protection against CSRF attacks
		$token = $this->getSecurityToken();
		if( ! $token->checkRequest($request)) {
			$securityID = $token->getName();
			if (empty($vars[$securityID])) {
				$this->httpError(400, _t("Form.CSRF_FAILED_MESSAGE",
					"There seems to have been a technical problem. Please click the back button, ".
					"refresh your browser, and try again."
				));
			} else {
				// Clear invalid token on refresh
				$data = $this->getData();
				unset($data[$securityID]);
				Session::set("FormInfo.{$this->FormName()}.data", $data);

$data is of type array, but the function expects a string.

				Session::set("FormInfo.{$this->FormName()}.errors", array());

array() is of type array, but the function expects a string.

				$this->sessionMessage(
					_t("Form.CSRF_EXPIRED_MESSAGE", "Your session has expired. Please re-submit the form."),
					"warning"
				);
				return $this->controller->redirectBack();
			}
		}

		// Determine the action button clicked
		$funcName = null;
		foreach($vars as $paramName => $paramVal) {

The expression $vars of type array|null is not guaranteed to be traversable. How about adding an additional type check?

			if(substr($paramName,0,7) == 'action_') {
				// Break off querystring arguments included in the action
				if(strpos($paramName,'?') !== false) {
					list($paramName, $paramVars) = explode('?', $paramName, 2);
					$newRequestParams = array();
					parse_str($paramVars, $newRequestParams);
					$vars = array_merge((array)$vars, (array)$newRequestParams);
				}

				// Cleanup action_, _x and _y from image fields
				$funcName = preg_replace(array('/^action_/','/_x$|_y$/'),'',$paramName);
				break;
			}
		}

		// If the action wasn't set, choose the default on the form.
		if(!isset($funcName) && $defaultAction = $this->defaultAction()){
			$funcName = $defaultAction->actionName();
		}

		if(isset($funcName)) {
			Form::set_current_action($funcName);
			$this->setButtonClicked($funcName);
		}

		// Permission checks (first on controller, then falling back to form)
		if(
			// Ensure that the action is actually a button or method on the form,
			// and not just a method on the controller.
			$this->controller->hasMethod($funcName)
			&& !$this->controller->checkAccessAction($funcName)
			// If a button exists, allow it on the controller
			&& !$this->actions->dataFieldByName('action_' . $funcName)
		) {
			return $this->httpError(
				403,
				sprintf('Action "%s" not allowed on controller (Class: %s)', $funcName, get_class($this->controller))
			);
		} elseif(
			$this->hasMethod($funcName)
			&& !$this->checkAccessAction($funcName)
			// No checks for button existence or $allowed_actions is performed -
			// all form methods are callable (e.g. the legacy "callfieldmethod()")
		) {
			return $this->httpError(
				403,
				sprintf('Action "%s" not allowed on form (Name: "%s")', $funcName, $this->name)
			);
		}
		// TODO : Once we switch to a stricter policy regarding allowed_actions (meaning actions must be set
		// explicitly in allowed_actions in order to run)
		// Uncomment the following for checking security against running actions on form fields
		/* else {
			// Try to find a field that has the action, and allows it
			$fieldsHaveMethod = false;
			foreach ($this->Fields() as $field){
				if ($field->hasMethod($funcName) && $field->checkAccessAction($funcName)) {
					$fieldsHaveMethod = true;
				}
			}
			if (!$fieldsHaveMethod) {
				return $this->httpError(
					403,
					sprintf('Action "%s" not allowed on any fields of form (Name: "%s")', $funcName, $this->Name())
				);
			}
		}*/

		// Validate the form
		if(!$this->validate()) {
			return $this->getValidationErrorResponse();
		}

		// First, try a handler method on the controller (has been checked for allowed_actions above already)
		if($this->controller->hasMethod($funcName)) {
			return $this->controller->$funcName($vars, $this, $request);
		// Otherwise, try a handler method on the form object.
		} elseif($this->hasMethod($funcName)) {
			return $this->$funcName($vars, $this, $request);
		} elseif($field = $this->checkFieldsForAction($this->Fields(), $funcName)) {

It seems like $this->Fields() can be null; however, checkFieldsForAction() does not accept null, maybe add an additional type check?

			return $field->$funcName($vars, $this, $request);
		}

		return $this->httpError(404);
	}