Merge branch 'origin/3.0' into 3.1

core/startup/ErrorControlChain.php

@@ -0,0 +1,121 @@
+<?php
+
+/**
+ * Class ErrorControlChain
+ *
+ * Runs a set of steps, optionally suppressing (but recording) any errors (even fatal ones) that occur in each step.
+ * If an error does occur, subsequent steps are normally skipped, but can optionally be run anyway
+ *
+ * Normal errors are suppressed even past the end of the chain. Fatal errors are only suppressed until the end
+ * of the chain - the request will then die silently.
+ *
+ * The exception is if an error occurs and BASE_URL is not yet set - in that case the error is never suppressed.
+ *
+ * Usage:
+ *
+ * $chain = new ErrorControlChain();
+ * $chain->then($callback1)->then($callback2)->then(true, $callback3)->execute();
+ *
+ * WARNING: This class is experimental and designed specifically for use pre-startup in main.php
+ * It will likely be heavily refactored before the release of 3.2
+ */
+class ErrorControlChain {
+	protected $error = false;
+	protected $steps = array();
+
+	protected $suppression = true;
+
+	/** We can't unregister_shutdown_function, so this acts as a flag to enable handling */
+	protected $handleFatalErrors = false;
+
+	public function hasErrored() {
+		return $this->error;
+	}
+
+	public function setErrored($error) {
+		$this->error = (bool)$error;
+	}
+
+	public function setSuppression($suppression) {
+		$this->suppression = (bool)$suppression;
+	}
+
+	/**
+	 * Add this callback to the chain of callbacks to call along with the state
+	 * that $error must be in this point in the chain for the callback to be called
+	 *
+	 * @param $callback - The callback to call
+	 * @param $onErrorState - false if only call if no errors yet, true if only call if already errors, null for either
+	 * @return $this
+	 */
+	public function then($callback, $onErrorState = false) {
+		$this->steps[] = array(
+			'callback' => $callback,
+			'onErrorState' => $onErrorState
+		);
+		return $this;
+	}
+
+	public function thenWhileGood($callback) {
+		return $this->then($callback, false);
+	}
+
+	public function thenIfErrored($callback) {
+		return $this->then($callback, true);
+	}
+
+	public function thenAlways($callback) {
+		return $this->then($callback, null);
+	}
+
+	public function handleError() {
+		if ($this->suppression && defined('BASE_URL')) throw new Exception('Generic Error');
+		else return false;
+	}
+
+	protected function lastErrorWasFatal() {
+		$error = error_get_last();
+		return $error && $error['type'] == 1;
+	}
+
+	public function handleFatalError() {
+		if ($this->handleFatalErrors && $this->suppression && defined('BASE_URL')) {
+			if ($this->lastErrorWasFatal()) {
+				ob_clean();
+				$this->error = true;
+				$this->step();
+			}
+		}
+	}
+
+	public function execute() {
+		set_error_handler(array($this, 'handleError'), error_reporting());
+		register_shutdown_function(array($this, 'handleFatalError'));
+		$this->handleFatalErrors = true;
+
+		$this->step();
+	}
+
+	protected function step() {
+		if ($this->steps) {
+			$step = array_shift($this->steps);
+
+			if ($step['onErrorState'] === null || $step['onErrorState'] === $this->error) {
+				try {
+					call_user_func($step['callback'], $this);
+				}
+				catch (Exception $e) {
+					if ($this->suppression && defined('BASE_URL')) $this->error = true;
+					else throw $e;
+				}
+			}
+
+			$this->step();
+		}
+		else {
+			// Now clean up
+			$this->handleFatalErrors = false;
+			restore_error_handler();
+		}
+	}
+}

core/startup/ParameterConfirmationToken.php

@@ -0,0 +1,113 @@
+<?php
+
+/**
+ * Class ParameterConfirmationToken
+ *
+ * When you need to use a dangerous GET parameter that needs to be set before core/Core.php is
+ * established, this class takes care of allowing some other code of confirming the parameter,
+ * by generating a one-time-use token & redirecting with that token included in the redirected URL
+ *
+ * WARNING: This class is experimental and designed specifically for use pre-startup in main.php
+ * It will likely be heavily refactored before the release of 3.2
+ */
+class ParameterConfirmationToken {
+	protected $parameterName = null;
+	protected $parameter = null;
+	protected $token = null;
+
+	protected function pathForToken($token) {
+		if (defined('BASE_PATH')) {
+			$basepath = BASE_PATH;
+		}
+		else {
+			$basepath = rtrim(dirname(dirname(dirname(dirname(__FILE__)))), DIRECTORY_SEPARATOR);
+		}
+
+		require_once(dirname(dirname(__FILE__)).'/TempPath.php');
+		$tempfolder = getTempFolder($basepath ? $basepath : DIRECTORY_SEPARATOR);
+
+		return $tempfolder.'/token_'.preg_replace('/[^a-z0-9]+/', '', $token);
+	}
+
+	protected function genToken() {
+		// Generate a new random token (as random as possible)
+		require_once(dirname(dirname(dirname(__FILE__))).'/security/RandomGenerator.php');
+		$rg = new RandomGenerator();
+		$token = $rg->randomToken('md5');
+
+		// Store a file in the session save path (safer than /tmp, as open_basedir might limit that)
+		file_put_contents($this->pathForToken($token), $token);
+
+		return $token;
+	}
+
+	protected function checkToken($token) {
+		$file = $this->pathForToken($token);
+		$content = null;
+
+		if (file_exists($file)) {
+			$content = file_get_contents($file);
+			unlink($file);
+		}
+
+		return $content == $token;
+	}
+
+	public function __construct($parameterName) {
+		// Store the parameter name
+		$this->parameterName = $parameterName;
+		// Store the parameter value
+		$this->parameter = isset($_GET[$parameterName]) ? $_GET[$parameterName] : null;
+		// Store the token
+		$this->token = isset($_GET[$parameterName.'token']) ? $_GET[$parameterName.'token'] : null;
+
+		// If a token was provided, but isn't valid, just throw a 403
+		if ($this->token && (!$this->checkToken($this->token))) {
+			header("HTTP/1.0 403 Forbidden", true, 403);
+			die;
+		}
+	}
+
+	public function parameterProvided() {
+		return $this->parameter !== null;
+	}
+
+	public function tokenProvided() {
+		return $this->token !== null;
+	}
+
+	public function params() {
+		return array(
+			$this->parameterName => $this->parameter,
+			$this->parameterName.'token' => $this->genToken()
+		);
+	}
+
+	public function reloadWithToken() {
+		global $url;
+
+		// Are we http or https?
+		$proto = 'http';
+
+		if(isset($_SERVER['HTTP_X_FORWARDED_PROTOCOL'])) {
+			if(strtolower($_SERVER['HTTP_X_FORWARDED_PROTOCOL']) == 'https') $proto = 'https';
+		}
+
+		if((!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] != 'off')) $proto = 'https';
+		if(isset($_SERVER['SSL'])) $proto = 'https';
+
+		// What's our host
+		$host = $_SERVER['HTTP_HOST'];
+
+		// What's our GET params (ensuring they include the original parameter + a new token)
+		$params = array_merge($_GET, $this->params());
+		unset($params['url']);
+
+		// Join them all together into the original URL
+		$location = "$proto://" . $host . BASE_URL . $url . ($params ? '?'.http_build_query($params) : '');
+
+		// And redirect
+		header('location: '.$location, true, 302);
+		die;
+	}
+}

docs/en/changelogs/3.0.6.md

@@ -1,5 +1,32 @@
 # 3.0.6 (Not yet released)
 
+## Overview
+
+ * Security: Require ADMIN for `?flush=1` (stop denial of service attacks)
+ ([#1692](https://github.com/silverstripe/silverstripe-framework/issues/1692))
+
+## Details
+
+### Security: Require ADMIN for ?flush=1
+
+Flushing the various manifests (class, template, config) is performed through a GET
+parameter (`flush=1`). Since this action requires more server resources than normal requests,
+it can facilitate [denial-of-service attacks](https://en.wikipedia.org/wiki/Denial-of-service_attack).
+
+To prevent this, main.php now checks and only allows the flush parameter in the following cases:
+
+ * The [environment](/topics/environment-management) is in "dev mode"
+ * A user is logged in with ADMIN permissions
+ * An error occurs during startup
+
+This applies to both `flush=1` and `flush=all` (technically we only check for the existence of any parameter value)
+but only through web requests made through main.php - CLI requests, or any other request that goes through
+a custom start up script will still process all flush requests as normal.
+
 ## Upgrading
 
- * If you have created your own composite database fields, then you shoulcd amend the setValue() to allow the passing of an object (usually DataObject) as well as an array.
+ * If you have created your own composite database fields, then you should amend the setValue() to allow the passing of
+   an object (usually DataObject) as well as an array.
+
+ * If you have provided your own startup scripts (ones that include core/Core.php) that can be accessed via a web
+   request, you should ensure that you limit use of the flush parameter

docs/en/changelogs/3.1.0.md

@@ -18,6 +18,8 @@
 
 ### Framework
 
+ * Security: Require ADMIN for `?flush=1` (stop denial of service attacks)
+ ([#1692](https://github.com/silverstripe/silverstripe-framework/issues/1692))
  * Static properties are immutable and private, you must use Config API
  * Statics in custom Page classes need to be "private"
  * `$default_cast` is now `Text` instead of `HTMLText`, to secure templates from XSS by default
@@ -37,6 +39,24 @@
  * Support for [Composer](http://getcomposer.org) dependency manager (also works with 3.0)
  * Added support for filtering incoming HTML from TinyMCE (disabled by default, see [security](/topics/security))
 
+## Details
+
+### Security: Require ADMIN for ?flush=1
+
+Flushing the various manifests (class, template, config) is performed through a GET
+parameter (`flush=1`). Since this action requires more server resources than normal requests,
+it can facilitate [denial-of-service attacks](https://en.wikipedia.org/wiki/Denial-of-service_attack).
+
+To prevent this, main.php now checks and only allows the flush parameter in the following cases:
+
+ * The [environment](/topics/environment-management) is in "dev mode"
+ * A user is logged in with ADMIN permissions
+ * An error occurs during startup
+
+This applies to both `flush=1` and `flush=all` (technically we only check for the existence of any parameter value)
+but only through web requests made through main.php - CLI requests, or any other request that goes through
+a custom start up script will still process all flush requests as normal.
+
 ## Upgrading
 
 ### Statics in custom Page classes need to be "private"

docs/en/reference/urlvariabletools.md

@@ -17,10 +17,8 @@ Append the option and corresponding value to your URL in your browser's address
 
  | URL Variable | | Values | | Description                                                     | 
  | ------------ | | ------ | | -----------                                                     | 
- | flush        | | 1,all  | | This will clear out all cached information about the page.  This is used frequently during development - for example, when adding new PHP or SS files. See below for value descriptions. | 
+ | flush=1      | | 1      | | Clears out all caches. Used mainly during development, e.g. when adding new classes or templates. Requires "dev" mode or ADMIN login |
  | showtemplate | | 1      | | Show the compiled version of all the templates used, including line numbers.  Good when you have a syntax error in a template. Cannot be used on a Live site without **isDev**.  **flush** can be used with the following values: |
- | ?flush=1     | |        | | Flushes the current page and included templates |
- | ?flush=all   | |        | | Flushes the entire template cache |            
 
 ## General Testing
 

docs/en/topics/caching.md

@@ -0,0 +1,26 @@
+# Caching
+
+## Built-In Caches
+
+The framework uses caches to store infrequently changing values.
+By default, the storage mechanism is simply the filesystem, although
+other cache backends can be configured. All caches use the `[api:SS_Cache]` API.
+
+The most common caches are manifests of various resources: 
+
+ * PHP class locations (`[api:SS_ClassManifest]`)
+ * Template file locations and compiled templates (`[api:SS_TemplateManifest]`)
+ * Configuration settings from YAML files (`[api:SS_ConfigManifest]`)
+ * Language files (`[api:i18n]`)
+
+Flushing the various manifests is performed through a GET
+parameter (`flush=1`). Since this action requires more server resources than normal requests,
+executing the action is limited to the following cases when performed via a web request:
+
+ * The [environment](/topics/environment-management) is in "dev mode"
+ * A user is logged in with ADMIN permissions
+ * An error occurs during startup
+
+## Custom Caches
+
+See `[api:SS_Cache]`.

docs/en/topics/index.md

@@ -4,6 +4,7 @@ This section provides an overview on how things fit together, the "conceptual gl
 It is where most documentation should live, and is the natural "second step" after finishing the tutorials.
 
  * [Access Control and Page Security](access-control): Restricting access and setting up permissions on your website
+ * [Caching](caching): Explains built-in caches for classes, config and templates. How to use your own caches.
  * [Command line Usage](commandline): Calling controllers via the command line interface using `sake`
  * [Configuration](configuration): Influence behaviour through PHP and YAML configuration
  * [Controller](controller): The intermediate layer between your templates and the data model

forms/UploadField.php

@@ -1195,6 +1195,7 @@ class UploadField extends FileField {
 	 * 
 	 * @param SS_HTTPRequest $request
 	 * @return SS_HTTPResponse
+	 * @return SS_HTTPResponse
 	 */
 	public function upload(SS_HTTPRequest $request) {
 		if($this->isDisabled() || $this->isReadonly() || !$this->canUpload()) {
@@ -1222,6 +1223,7 @@ class UploadField extends FileField {
 		// Format response with json
 		$response = new SS_HTTPResponse(Convert::raw2json(array($return)));
 		$response->addHeader('Content-Type', 'text/plain');
+		if(!empty($return['error'])) $response->setStatusCode(403);
 		return $response;
 	}
 

main.php

@@ -56,52 +56,113 @@ if (version_compare(phpversion(), '5.3.2', '<')) {
 /**
  * Include SilverStripe's core code
  */
-require_once('core/Core.php');
+require_once('core/startup/ErrorControlChain.php');
+require_once('core/startup/ParameterConfirmationToken.php');
 
-// IIS will sometimes generate this.
-if(!empty($_SERVER['HTTP_X_ORIGINAL_URL'])) {
-	$_SERVER['REQUEST_URI'] = $_SERVER['HTTP_X_ORIGINAL_URL'];
-}
+$chain = new ErrorControlChain();
+$token = new ParameterConfirmationToken('flush');
 
-// PHP 5.4's built-in webserver uses this
-if (php_sapi_name() == 'cli-server') {
-	$url = $_SERVER['REQUEST_URI'];
-	
-	// Querystring args need to be explicitly parsed
-	if(strpos($url,'?') !== false) {
-		list($url, $query) = explode('?',$url,2);
-		parse_str($query, $_GET);
-		if ($_GET) $_REQUEST = array_merge((array)$_REQUEST, (array)$_GET);
-	}
-	
-	// Pass back to the webserver for files that exist
-	if(file_exists(BASE_PATH . $url)) return false;
+$chain
+	// First, if $_GET['flush'] was set, but no valid token, suppress the flush
+	->then(function($chain) use ($token){
+		if (isset($_GET['flush']) && !$token->tokenProvided()) {
+			unset($_GET['flush']);
+		}
+		else {
+			$chain->setSuppression(false);
+		}
+	})
+	// Then load in core
+	->then(function(){
+		require_once('core/Core.php');
+	})
+	// Then build the URL (even if Core didn't load beyond setting BASE_URL)
+	->thenAlways(function(){
+		global $url;
 
-// Apache rewrite rules use this
-} else if (isset($_GET['url'])) {
-	$url = $_GET['url'];
-	// IIS includes get variables in url
-	$i = strpos($url, '?');
-	if($i !== false) {
-		$url = substr($url, 0, $i);
-	}
-	
-// Lighttpd uses this
-} else {
-	if(strpos($_SERVER['REQUEST_URI'],'?') !== false) {
-		list($url, $query) = explode('?', $_SERVER['REQUEST_URI'], 2);
-		parse_str($query, $_GET);
-		if ($_GET) $_REQUEST = array_merge((array)$_REQUEST, (array)$_GET);
-	} else {
-		$url = $_SERVER["REQUEST_URI"];
-	}
-}
+		// IIS will sometimes generate this.
+		if(!empty($_SERVER['HTTP_X_ORIGINAL_URL'])) {
+			$_SERVER['REQUEST_URI'] = $_SERVER['HTTP_X_ORIGINAL_URL'];
+		}
 
-// Remove base folders from the URL if webroot is hosted in a subfolder
-if (substr(strtolower($url), 0, strlen(BASE_URL)) == strtolower(BASE_URL)) $url = substr($url, strlen(BASE_URL));
+		// PHP 5.4's built-in webserver uses this
+		if (php_sapi_name() == 'cli-server') {
+			$url = $_SERVER['REQUEST_URI'];
 
-// Connect to database
-require_once('model/DB.php');
+			// Querystring args need to be explicitly parsed
+			if(strpos($url,'?') !== false) {
+				list($url, $query) = explode('?',$url,2);
+				parse_str($query, $_GET);
+				if ($_GET) $_REQUEST = array_merge((array)$_REQUEST, (array)$_GET);
+			}
+
+		// Apache rewrite rules use this
+		} else if (isset($_GET['url'])) {
+			$url = $_GET['url'];
+			// IIS includes get variables in url
+			$i = strpos($url, '?');
+			if($i !== false) {
+				$url = substr($url, 0, $i);
+			}
+
+		// Lighttpd uses this
+		} else {
+			if(strpos($_SERVER['REQUEST_URI'],'?') !== false) {
+				list($url, $query) = explode('?', $_SERVER['REQUEST_URI'], 2);
+				parse_str($query, $_GET);
+				if ($_GET) $_REQUEST = array_merge((array)$_REQUEST, (array)$_GET);
+			} else {
+				$url = $_SERVER["REQUEST_URI"];
+			}
+		}
+
+		// Remove base folders from the URL if webroot is hosted in a subfolder
+		if (substr(strtolower($url), 0, strlen(BASE_URL)) == strtolower(BASE_URL)) $url = substr($url, strlen(BASE_URL));
+	})
+	// Then start up the database
+	->then(function(){
+		require_once('model/DB.php');
+		global $databaseConfig;
+		if ($databaseConfig) DB::connect($databaseConfig);
+	})
+	// Then if a flush was requested, redirect to it
+	->then(function($chain) use ($token){
+		if ($token->parameterProvided() && !$token->tokenProvided()) {
+			// First, check if we're in dev mode, or the database doesn't have any security data
+			$canFlush = Director::isDev() || !Security::database_is_ready();
+
+			// Otherwise, we start up the session if needed, then check for admin
+			if (!$canFlush) {
+				if(!isset($_SESSION) && (isset($_COOKIE[session_name()]) || isset($_REQUEST[session_name()]))) {
+					Session::start();
+				}
+
+				if (Permission::check('ADMIN')) {
+					$canFlush = true;
+				}
+				else {
+					$loginPage = Director::absoluteURL(Config::inst()->get('Security', 'login_url'));
+					$loginPage .= "?BackURL=" . urlencode($_SERVER['REQUEST_URI']);
+
+					header('location: '.$loginPage, true, 302);
+					die;
+				}
+			}
+
+			// And if we can flush, reload with an authority token
+			if ($canFlush) $token->reloadWithToken();
+		}
+	})
+	// Finally if a flush was requested but there was an error while figuring out if it's allowed, do it anyway
+	->thenIfErrored(function() use ($token){
+		if ($token->parameterProvided() && !$token->tokenProvided()) {
+			$token->reloadWithToken();
+		}
+	})
+	->execute();
+
+// If we're in PHP's built in webserver, pass back to the webserver for files that exist
+if (php_sapi_name() == 'cli-server' && file_exists(BASE_PATH . $url) && is_file(BASE_PATH . $url)) return false;
 
 global $databaseConfig;
 
@@ -112,17 +173,15 @@ if(!isset($databaseConfig) || !isset($databaseConfig['database']) || !$databaseC
 	}
 	$s = (isset($_SERVER['SSL']) || (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] != 'off')) ? 's' : '';
 	$installURL = "http$s://" . $_SERVER['HTTP_HOST'] . BASE_URL . '/install.php';
-	
+
 	// The above dirname() will equate to "\" on Windows when installing directly from http://localhost (not using
 	// a sub-directory), this really messes things up in some browsers. Let's get rid of the backslashes
 	$installURL = str_replace('\\', '', $installURL);
-	
+
 	header("Location: $installURL");
 	die();
 }
 
-DB::connect($databaseConfig);
-
 // Direct away - this is the "main" function, that hands control to the appropriate controller
 DataModel::set_inst(new DataModel());
 Director::direct($url, DataModel::inst());

tests/core/startup/ErrorControlChainTest.php

@@ -0,0 +1,90 @@
+<?php
+
+class ErrorControlChainTest extends SapphireTest {
+
+	function testErrorSuppression() {
+		$chain = new ErrorControlChain();
+
+		$chain
+			->then(function(){
+				user_error('This error should be suppressed', E_USER_ERROR);
+			})
+			->execute();
+
+		$this->assertTrue($chain->hasErrored());
+	}
+
+	function testMultipleErrorSuppression() {
+		$chain = new ErrorControlChain();
+
+		$chain
+			->then(function(){
+				user_error('This error should be suppressed', E_USER_ERROR);
+			})
+			->thenAlways(function(){
+				user_error('This error should also be suppressed', E_USER_ERROR);
+			})
+			->execute();
+
+		$this->assertTrue($chain->hasErrored());
+	}
+
+	function testExceptionSuppression() {
+		$chain = new ErrorControlChain();
+
+		$chain
+			->then(function(){
+				throw new Exception('This exception should be suppressed');
+			})
+			->execute();
+
+		$this->assertTrue($chain->hasErrored());
+	}
+
+	function testMultipleExceptionSuppression() {
+		$chain = new ErrorControlChain();
+
+		$chain
+			->then(function(){
+				throw new Exception('This exception should be suppressed');
+			})
+			->thenAlways(function(){
+				throw new Exception('This exception should also be suppressed');
+			})
+			->execute();
+
+		$this->assertTrue($chain->hasErrored());
+	}
+
+	function testErrorControl() {
+		$preError = $postError = array('then' => false, 'thenIfErrored' => false, 'thenAlways' => false);
+
+		$chain = new ErrorControlChain();
+
+		$chain
+			->then(function() use (&$preError) { $preError['then'] = true; })
+			->thenIfErrored(function() use (&$preError) { $preError['thenIfErrored'] = true; })
+			->thenAlways(function() use (&$preError) { $preError['thenAlways'] = true; })
+
+			->then(function(){ user_error('An error', E_USER_ERROR); })
+
+			->then(function() use (&$postError) { $postError['then'] = true; })
+			->thenIfErrored(function() use (&$postError) { $postError['thenIfErrored'] = true; })
+			->thenAlways(function() use (&$postError) { $postError['thenAlways'] = true; })
+
+			->execute();
+
+		$this->assertEquals(
+			array('then' => true, 'thenIfErrored' => false, 'thenAlways' => true),
+			$preError,
+			'Then and thenAlways callbacks called before error, thenIfErrored callback not called'
+		);
+
+		$this->assertEquals(
+			array('then' => false, 'thenIfErrored' => true, 'thenAlways' => true),
+			$postError,
+			'thenIfErrored and thenAlways callbacks called after error, then callback not called'
+		);
+	}
+
+}

tests/forms/uploadfield/UploadFieldTest.php

@@ -153,6 +153,33 @@ class UploadFieldTest extends FunctionalTest {
 		$this->assertEquals($relationCount + 1, $record->ManyManyFiles()->Count());
 	}
 
+	/**
+	 * Partially covered by {@link UploadTest->testUploadAcceptsAllowedExtension()},
+	 * but this test additionally verifies that those constraints are actually enforced
+	 * in this controller method.
+	 */
+	public function testAllowedExtensions() {
+		$this->loginWithPermission('ADMIN');
+
+		$invalidFile = 'invalid.php';
+		$_FILES = array('AllowedExtensionsField' => $this->getUploadFile($invalidFile));
+		$response = $this->post(
+			'UploadFieldTest_Controller/Form/field/AllowedExtensionsField/upload',
+			array('AllowedExtensionsField' => $this->getUploadFile($invalidFile))
+		);
+		$this->assertTrue($response->isError());
+		$this->assertContains('Extension is not allowed', $response->getBody());
+
+		$validFile = 'valid.txt';
+		$_FILES = array('AllowedExtensionsField' => $this->getUploadFile($validFile));
+		$response = $this->post(
+			'UploadFieldTest_Controller/Form/field/AllowedExtensionsField/upload',
+			array('AllowedExtensionsField' => $this->getUploadFile($validFile))
+		);
+		$this->assertFalse($response->isError());
+		$this->assertNotContains('Extension is not allowed', $response->getBody());
+	}
+
 	/**
 	 * Test that has_one relations do not support multiple files
 	 */
@@ -880,6 +907,9 @@ class UploadFieldTestForm extends Form implements TestOnly {
 		$fieldCanAttachExisting = UploadField::create('CanAttachExistingFalseField')
 			->setCanAttachExisting(false);
 
+		$fieldAllowedExtensions = new UploadField('AllowedExtensionsField');
+		$fieldAllowedExtensions->getValidator()->setAllowedExtensions(array('txt'));
+
 		$fields = new FieldList(
 			$fieldNoRelation,
 			$fieldHasOne,
@@ -894,7 +924,8 @@ class UploadFieldTestForm extends Form implements TestOnly {
 			$fieldDisabled,
 			$fieldSubfolder,
 			$fieldCanUploadFalse,
-			$fieldCanAttachExisting
+			$fieldCanAttachExisting,
+			$fieldAllowedExtensions
 		);
 		$actions = new FieldList(
 			new FormAction('submit')
@@ -924,4 +955,4 @@ class UploadFieldTest_Controller extends Controller implements TestOnly {
 	public function Form() {
 		return new UploadFieldTestForm($this, 'Form');
 	}
-}
+}