Commit c36743b9 authored by catch's avatar catch
Browse files

Issue #2482783 by dmsmidt, alexpott, mgifford, 20th, Sutharsan, Jo Fitzgerald,...

Issue #2482783 by dmsmidt, alexpott, mgifford, 20th, Sutharsan, Jo Fitzgerald, tim.plunkett, dandaman, xjm: File upload errors not set or shown correctly
parent af68144f
......@@ -673,6 +673,91 @@ function file_cron() {
}
}
/**
* Saves form file uploads.
*
* The files will be added to the {file_managed} table as temporary files.
* Temporary files are periodically cleaned. Use the 'file.usage' service to
* register the usage of the file which will automatically mark it as permanent.
*
* @param array $element
* The FAPI element whose values are being saved.
* @param \Drupal\Core\Form\FormStateInterface $form_state
* The current state of the form.
* @param null|int $delta
* (optional) The delta of the file to return the file entity.
* Defaults to NULL.
* @param int $replace
* (optional) The replace behavior when the destination file already exists.
* Possible values include:
* - FILE_EXISTS_REPLACE: Replace the existing file.
* - FILE_EXISTS_RENAME: (default) Append _{incrementing number} until the
* filename is unique.
* - FILE_EXISTS_ERROR: Do nothing and return FALSE.
*
* @return array|\Drupal\file\FileInterface|null|false
* An array of file entities or a single file entity if $delta != NULL. Each
* array element contains the file entity if the upload succeeded or FALSE if
* there was an error. Function returns NULL if no file was uploaded.
*
* @deprecated in Drupal 8.4.x, will be removed before Drupal 9.0.0.
* For backwards compatibility use core file upload widgets in forms.
*
* @internal
* This function wraps file_save_upload() to allow correct error handling in
* forms.
*
* @todo Revisit after https://www.drupal.org/node/2244513.
*/
function _file_save_upload_from_form(array $element, FormStateInterface $form_state, $delta = NULL, $replace = FILE_EXISTS_RENAME) {
// Get all errors set before calling this method. This will also clear them
// from $_SESSION.
$errors_before = drupal_get_messages('error');
$upload_location = isset($element['#upload_location']) ? $element['#upload_location'] : FALSE;
$upload_name = implode('_', $element['#parents']);
$upload_validators = isset($element['#upload_validators']) ? $element['#upload_validators'] : [];
$result = file_save_upload($upload_name, $upload_validators, $upload_location, $delta, $replace);
// Get new errors that are generated while trying to save the upload. This
// will also clear them from $_SESSION.
$errors_new = drupal_get_messages('error');
if (!empty($errors_new['error'])) {
$errors_new = $errors_new['error'];
if (count($errors_new) > 1) {
// Render multiple errors into a single message.
// This is needed because only one error per element is supported.
$render_array = [
'error' => [
'#markup' => t('One or more files could not be uploaded.'),
],
'item_list' => [
'#theme' => 'item_list',
'#items' => $errors_new,
],
];
$error_message = \Drupal::service('renderer')->renderPlain($render_array);
}
else {
$error_message = reset($errors_new);
}
$form_state->setError($element, $error_message);
}
// Ensure that errors set prior to calling this method are still shown to the
// user.
if (!empty($errors_before['error'])) {
foreach ($errors_before['error'] as $error) {
drupal_set_message($error, 'error');
}
}
return $result;
}
/**
* Saves file uploads to a new location.
*
......@@ -680,6 +765,10 @@ function file_cron() {
* Temporary files are periodically cleaned. Use the 'file.usage' service to
* register the usage of the file which will automatically mark it as permanent.
*
* Note that this function does not support correct form error handling. The
* file upload widgets in core do support this. It is advised to use these in
* any custom form, instead of calling this function.
*
* @param string $form_field_name
* A string that is the associative array key of the upload form element in
* the form array.
......@@ -710,6 +799,10 @@ function file_cron() {
* An array of file entities or a single file entity if $delta != NULL. Each
* array element contains the file entity if the upload succeeded or FALSE if
* there was an error. Function returns NULL if no file was uploaded.
*
* @see _file_save_upload_from_form()
*
* @todo: move this logic to a service in https://www.drupal.org/node/2244513.
*/
function file_save_upload($form_field_name, $validators = [], $destination = FALSE, $delta = NULL, $replace = FILE_EXISTS_RENAME) {
$user = \Drupal::currentUser();
......@@ -1200,9 +1293,8 @@ function file_managed_file_save_upload($element, FormStateInterface $form_state)
$files_uploaded = $element['#multiple'] && count(array_filter($file_upload)) > 0;
$files_uploaded |= !$element['#multiple'] && !empty($file_upload);
if ($files_uploaded) {
if (!$files = file_save_upload($upload_name, $element['#upload_validators'], $destination)) {
if (!$files = _file_save_upload_from_form($element, $form_state)) {
\Drupal::logger('file')->notice('The file upload failed. %upload', ['%upload' => $upload_name]);
$form_state->setError($element, t('Files in the @name field were unable to be uploaded.', ['@name' => $element['#title']]));
return [];
}
......
<?php
namespace Drupal\file\Tests;
use Drupal\file\Entity\File;
/**
* Tests the _file_save_upload_from_form() function.
*
* @group file
*
* @see _file_save_upload_from_form()
*/
class SaveUploadFormTest extends FileManagedTestBase {
/**
* Modules to enable.
*
* @var array
*/
public static $modules = ['dblog'];
/**
* An image file path for uploading.
*
* @var \Drupal\file\FileInterface
*/
protected $image;
/**
* A PHP file path for upload security testing.
*/
protected $phpfile;
/**
* The largest file id when the test starts.
*/
protected $maxFidBefore;
/**
* Extension of the image filename.
*
* @var string
*/
protected $imageExtension;
/**
* {@inheritdoc}
*/
protected function setUp() {
parent::setUp();
$account = $this->drupalCreateUser(['access site reports']);
$this->drupalLogin($account);
$image_files = $this->drupalGetTestFiles('image');
$this->image = File::create((array) current($image_files));
list(, $this->imageExtension) = explode('.', $this->image->getFilename());
$this->assertTrue(is_file($this->image->getFileUri()), "The image file we're going to upload exists.");
$this->phpfile = current($this->drupalGetTestFiles('php'));
$this->assertTrue(is_file($this->phpfile->uri), 'The PHP file we are going to upload exists.');
$this->maxFidBefore = db_query('SELECT MAX(fid) AS fid FROM {file_managed}')->fetchField();
// Upload with replace to guarantee there's something there.
$edit = [
'file_test_replace' => FILE_EXISTS_REPLACE,
'files[file_test_upload][]' => drupal_realpath($this->image->getFileUri()),
];
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$this->assertRaw(t('You WIN!'), 'Found the success message.');
// Check that the correct hooks were called then clean out the hook
// counters.
$this->assertFileHooksCalled(['validate', 'insert']);
file_test_reset();
}
/**
* Tests the _file_save_upload_from_form() function.
*/
public function testNormal() {
$max_fid_after = db_query('SELECT MAX(fid) AS fid FROM {file_managed}')->fetchField();
$this->assertTrue($max_fid_after > $this->maxFidBefore, 'A new file was created.');
$file1 = File::load($max_fid_after);
$this->assertTrue($file1, 'Loaded the file.');
// MIME type of the uploaded image may be either image/jpeg or image/png.
$this->assertEqual(substr($file1->getMimeType(), 0, 5), 'image', 'A MIME type was set.');
// Reset the hook counters to get rid of the 'load' we just called.
file_test_reset();
// Upload a second file.
$image2 = current($this->drupalGetTestFiles('image'));
$edit = ['files[file_test_upload][]' => drupal_realpath($image2->uri)];
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$this->assertRaw(t('You WIN!'));
$max_fid_after = db_query('SELECT MAX(fid) AS fid FROM {file_managed}')->fetchField();
// Check that the correct hooks were called.
$this->assertFileHooksCalled(['validate', 'insert']);
$file2 = File::load($max_fid_after);
$this->assertTrue($file2, 'Loaded the file');
// MIME type of the uploaded image may be either image/jpeg or image/png.
$this->assertEqual(substr($file2->getMimeType(), 0, 5), 'image', 'A MIME type was set.');
// Load both files using File::loadMultiple().
$files = File::loadMultiple([$file1->id(), $file2->id()]);
$this->assertTrue(isset($files[$file1->id()]), 'File was loaded successfully');
$this->assertTrue(isset($files[$file2->id()]), 'File was loaded successfully');
// Upload a third file to a subdirectory.
$image3 = current($this->drupalGetTestFiles('image'));
$image3_realpath = drupal_realpath($image3->uri);
$dir = $this->randomMachineName();
$edit = [
'files[file_test_upload][]' => $image3_realpath,
'file_subdir' => $dir,
];
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$this->assertRaw(t('You WIN!'));
$this->assertTrue(is_file('temporary://' . $dir . '/' . trim(drupal_basename($image3_realpath))));
}
/**
* Tests extension handling.
*/
public function testHandleExtension() {
// The file being tested is a .gif which is in the default safe list
// of extensions to allow when the extension validator isn't used. This is
// implicitly tested at the testNormal() test. Here we tell
// _file_save_upload_from_form() to only allow ".foo".
$extensions = 'foo';
$edit = [
'file_test_replace' => FILE_EXISTS_REPLACE,
'files[file_test_upload][]' => drupal_realpath($this->image->getFileUri()),
'extensions' => $extensions,
];
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$message = t('Only files with the following extensions are allowed:') . ' <em class="placeholder">' . $extensions . '</em>';
$this->assertRaw($message, 'Cannot upload a disallowed extension');
$this->assertRaw(t('Epic upload FAIL!'), 'Found the failure message.');
// Check that the correct hooks were called.
$this->assertFileHooksCalled(['validate']);
// Reset the hook counters.
file_test_reset();
$extensions = 'foo ' . $this->imageExtension;
// Now tell _file_save_upload_from_form() to allow the extension of our test image.
$edit = [
'file_test_replace' => FILE_EXISTS_REPLACE,
'files[file_test_upload][]' => drupal_realpath($this->image->getFileUri()),
'extensions' => $extensions,
];
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$this->assertNoRaw(t('Only files with the following extensions are allowed:'), 'Can upload an allowed extension.');
$this->assertRaw(t('You WIN!'), 'Found the success message.');
// Check that the correct hooks were called.
$this->assertFileHooksCalled(['validate', 'load', 'update']);
// Reset the hook counters.
file_test_reset();
// Now tell _file_save_upload_from_form() to allow any extension.
$edit = [
'file_test_replace' => FILE_EXISTS_REPLACE,
'files[file_test_upload][]' => drupal_realpath($this->image->getFileUri()),
'allow_all_extensions' => TRUE,
];
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$this->assertNoRaw(t('Only files with the following extensions are allowed:'), 'Can upload any extension.');
$this->assertRaw(t('You WIN!'), 'Found the success message.');
// Check that the correct hooks were called.
$this->assertFileHooksCalled(['validate', 'load', 'update']);
}
/**
* Tests dangerous file handling.
*/
public function testHandleDangerousFile() {
$config = $this->config('system.file');
// Allow the .php extension and make sure it gets renamed to .txt for
// safety. Also check to make sure its MIME type was changed.
$edit = [
'file_test_replace' => FILE_EXISTS_REPLACE,
'files[file_test_upload][]' => drupal_realpath($this->phpfile->uri),
'is_image_file' => FALSE,
'extensions' => 'php',
];
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$message = t('For security reasons, your upload has been renamed to') . ' <em class="placeholder">' . $this->phpfile->filename . '.txt' . '</em>';
$this->assertRaw($message, 'Dangerous file was renamed.');
$this->assertRaw(t('File MIME type is text/plain.'), "Dangerous file's MIME type was changed.");
$this->assertRaw(t('You WIN!'), 'Found the success message.');
// Check that the correct hooks were called.
$this->assertFileHooksCalled(['validate', 'insert']);
// Ensure dangerous files are not renamed when insecure uploads is TRUE.
// Turn on insecure uploads.
$config->set('allow_insecure_uploads', 1)->save();
// Reset the hook counters.
file_test_reset();
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$this->assertNoRaw(t('For security reasons, your upload has been renamed'), 'Found no security message.');
$this->assertRaw(t('File name is @filename', ['@filename' => $this->phpfile->filename]), 'Dangerous file was not renamed when insecure uploads is TRUE.');
$this->assertRaw(t('You WIN!'), 'Found the success message.');
// Check that the correct hooks were called.
$this->assertFileHooksCalled(['validate', 'insert']);
// Turn off insecure uploads.
$config->set('allow_insecure_uploads', 0)->save();
}
/**
* Tests file munge handling.
*/
public function testHandleFileMunge() {
// Ensure insecure uploads are disabled for this test.
$this->config('system.file')->set('allow_insecure_uploads', 0)->save();
$this->image = file_move($this->image, $this->image->getFileUri() . '.foo.' . $this->imageExtension);
// Reset the hook counters to get rid of the 'move' we just called.
file_test_reset();
$extensions = $this->imageExtension;
$edit = [
'files[file_test_upload][]' => drupal_realpath($this->image->getFileUri()),
'extensions' => $extensions,
];
$munged_filename = $this->image->getFilename();
$munged_filename = substr($munged_filename, 0, strrpos($munged_filename, '.'));
$munged_filename .= '_.' . $this->imageExtension;
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$this->assertRaw(t('For security reasons, your upload has been renamed'), 'Found security message.');
$this->assertRaw(t('File name is @filename', ['@filename' => $munged_filename]), 'File was successfully munged.');
$this->assertRaw(t('You WIN!'), 'Found the success message.');
// Check that the correct hooks were called.
$this->assertFileHooksCalled(['validate', 'insert']);
// Ensure we don't munge files if we're allowing any extension.
// Reset the hook counters.
file_test_reset();
$edit = [
'files[file_test_upload][]' => drupal_realpath($this->image->getFileUri()),
'allow_all_extensions' => TRUE,
];
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$this->assertNoRaw(t('For security reasons, your upload has been renamed'), 'Found no security message.');
$this->assertRaw(t('File name is @filename', ['@filename' => $this->image->getFilename()]), 'File was not munged when allowing any extension.');
$this->assertRaw(t('You WIN!'), 'Found the success message.');
// Check that the correct hooks were called.
$this->assertFileHooksCalled(['validate', 'insert']);
}
/**
* Tests renaming when uploading over a file that already exists.
*/
public function testExistingRename() {
$edit = [
'file_test_replace' => FILE_EXISTS_RENAME,
'files[file_test_upload][]' => drupal_realpath($this->image->getFileUri())
];
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$this->assertRaw(t('You WIN!'), 'Found the success message.');
// Check that the correct hooks were called.
$this->assertFileHooksCalled(['validate', 'insert']);
}
/**
* Tests replacement when uploading over a file that already exists.
*/
public function testExistingReplace() {
$edit = [
'file_test_replace' => FILE_EXISTS_REPLACE,
'files[file_test_upload][]' => drupal_realpath($this->image->getFileUri())
];
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$this->assertRaw(t('You WIN!'), 'Found the success message.');
// Check that the correct hooks were called.
$this->assertFileHooksCalled(['validate', 'load', 'update']);
}
/**
* Tests for failure when uploading over a file that already exists.
*/
public function testExistingError() {
$edit = [
'file_test_replace' => FILE_EXISTS_ERROR,
'files[file_test_upload][]' => drupal_realpath($this->image->getFileUri())
];
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$this->assertRaw(t('Epic upload FAIL!'), 'Found the failure message.');
// Check that the no hooks were called while failing.
$this->assertFileHooksCalled([]);
}
/**
* Tests for no failures when not uploading a file.
*/
public function testNoUpload() {
$this->drupalPostForm('file-test/save_upload_from_form_test', [], t('Submit'));
$this->assertNoRaw(t('Epic upload FAIL!'), 'Failure message not found.');
}
/**
* Tests for log entry on failing destination.
*/
public function testDrupalMovingUploadedFileError() {
// Create a directory and make it not writable.
$test_directory = 'test_drupal_move_uploaded_file_fail';
drupal_mkdir('temporary://' . $test_directory, 0000);
$this->assertTrue(is_dir('temporary://' . $test_directory));
$edit = [
'file_subdir' => $test_directory,
'files[file_test_upload][]' => drupal_realpath($this->image->getFileUri())
];
\Drupal::state()->set('file_test.disable_error_collection', TRUE);
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$this->assertRaw(t('File upload error. Could not move uploaded file.'), 'Found the failure message.');
$this->assertRaw(t('Epic upload FAIL!'), 'Found the failure message.');
// Uploading failed. Now check the log.
$this->drupalGet('admin/reports/dblog');
$this->assertResponse(200);
$this->assertRaw(t('Upload error. Could not move uploaded file @file to destination @destination.', [
'@file' => $this->image->getFilename(),
'@destination' => 'temporary://' . $test_directory . '/' . $this->image->getFilename()
]), 'Found upload error log entry.');
}
/**
* Tests that form validation does not change error messages.
*/
public function testErrorMessagesAreNotChanged() {
$error = 'An error message set before _file_save_upload_from_form()';
$edit = [
'files[file_test_upload][]' => drupal_realpath($this->image->getFileUri()),
'error_message' => $error,
];
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$this->assertRaw(t('You WIN!'), 'Found the success message.');
// Ensure the expected error message is present and the counts before and
// after calling _file_save_upload_from_form() are correct.
$this->assertText($error);
$this->assertRaw('Number of error messages before _file_save_upload_from_form(): 1');
$this->assertRaw('Number of error messages after _file_save_upload_from_form(): 1');
// Test that error messages are preserved when an error occurs.
$edit = [
'files[file_test_upload][]' => drupal_realpath($this->image->getFileUri()),
'error_message' => $error,
'extensions' => 'foo'
];
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$this->assertRaw(t('Epic upload FAIL!'), 'Found the failure message.');
// Ensure the expected error message is present and the counts before and
// after calling _file_save_upload_from_form() are correct.
$this->assertText($error);
$this->assertRaw('Number of error messages before _file_save_upload_from_form(): 1');
$this->assertRaw('Number of error messages after _file_save_upload_from_form(): 1');
// Test a successful upload with no messages.
$edit = [
'files[file_test_upload][]' => drupal_realpath($this->image->getFileUri()),
];
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$this->assertRaw(t('You WIN!'), 'Found the success message.');
// Ensure the error message is not present and the counts before and after
// calling _file_save_upload_from_form() are correct.
$this->assertNoText($error);
$this->assertRaw('Number of error messages before _file_save_upload_from_form(): 0');
$this->assertRaw('Number of error messages after _file_save_upload_from_form(): 0');
}
/**
* Tests that multiple validation errors are combined in one message.
*/
public function testCombinedErrorMessages() {
$textfile = current($this->drupalGetTestFiles('text'));
$this->assertTrue(is_file($textfile->uri), 'The text file we are going to upload exists.');
$edit = [
'files[file_test_upload][]' => [
drupal_realpath($this->phpfile->uri),
drupal_realpath($textfile->uri),
],
'allow_all_extensions' => FALSE,
'is_image_file' => TRUE,
'extensions' => 'jpeg',
];
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$this->assertRaw(t('Epic upload FAIL!'), 'Found the failure message.');
// Search for combined error message followed by a formatted list of messages.
$this->assertRaw(t('One or more files could not be uploaded.') . '<div class="item-list">', 'Error message contains combined list of validation errors.');
}
/**
* Tests highlighting of file upload field when it has an error.
*/
public function testUploadFieldIsHighlighted() {
$this->assertEqual(0, count($this->cssSelect('input[name="files[file_test_upload][]"].error')), 'Successful file upload has no error.');
$edit = [
'files[file_test_upload][]' => drupal_realpath($this->image->getFileUri()),
'extensions' => 'foo'
];
$this->drupalPostForm('file-test/save_upload_from_form_test', $edit, t('Submit'));
$this->assertResponse(200, 'Received a 200 response for posted test file.');
$this->assertRaw(t('Epic upload FAIL!'), 'Found the failure message.');
$this->assertEqual(1, count($this->cssSelect('input[name="files[file_test_upload][]"].error')), 'File upload field has error.');
}
}
......@@ -4,3 +4,9 @@ file.test:
_form: 'Drupal\file_test\Form\FileTestForm'
requirements:
_access: 'TRUE'
file.save_upload_from_form_test:
path: '/file-test/save_upload_from_form_test'
defaults:
_form: 'Drupal\file_test\Form\FileTestSaveUploadFromForm'
requirements:
_access: 'TRUE'