From 341f0e4c92b28b29c55e7b20e1876d3935bb5664 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Thu, 23 Jul 2015 09:51:00 +0100 Subject: [PATCH] Issue #2535302 by mbaynton, Berdir, kgoel, claudiu.cristea, tim.plunkett, RavindraSingh, cilefen, dawehner: Selecting too many files with JS off causes WSOD with data loss --- .../Plugin/Field/FieldWidget/FileWidget.php | 2 +- .../file/src/Tests/FileFieldTestBase.php | 59 +++++++++++++++++-- .../file/src/Tests/FileFieldWidgetTest.php | 48 ++++++++++++++- core/modules/simpletest/src/WebTestBase.php | 22 ++++--- 4 files changed, 114 insertions(+), 17 deletions(-) diff --git a/core/modules/file/src/Plugin/Field/FieldWidget/FileWidget.php b/core/modules/file/src/Plugin/Field/FieldWidget/FileWidget.php index 137d841217d..a2d1fefc68e 100644 --- a/core/modules/file/src/Plugin/Field/FieldWidget/FileWidget.php +++ b/core/modules/file/src/Plugin/Field/FieldWidget/FileWidget.php @@ -350,7 +350,7 @@ class FileWidget extends WidgetBase implements ContainerFactoryPluginInterface { $file = File::load($fid); $removed_names[] = $file->getFilename(); } - $args = array('%field' => $field_storage->getFieldName(), '@max' => $field_storage->getCardinality(), '@count' => $keep, '%list' => implode(', ', $removed_names)); + $args = array('%field' => $field_storage->getName(), '@max' => $field_storage->getCardinality(), '@count' => $uploaded, '%list' => implode(', ', $removed_names)); $message = t('Field %field can only hold @max values but there were @count uploaded. The following files have been omitted as a result: %list.', $args); drupal_set_message($message, 'warning'); $values['fids'] = array_slice($values['fids'], 0, $keep); diff --git a/core/modules/file/src/Tests/FileFieldTestBase.php b/core/modules/file/src/Tests/FileFieldTestBase.php index c03da58d18c..5874cf5dba7 100644 --- a/core/modules/file/src/Tests/FileFieldTestBase.php +++ b/core/modules/file/src/Tests/FileFieldTestBase.php @@ -147,18 +147,57 @@ abstract class FileFieldTestBase extends WebTestBase { /** * Uploads a file to a node. + * + * @param \Drupal\file\FileInterface $file + * The File to be uploaded. + * @param string $field_name + * The name of the field on which the files should be saved. + * @param $nid_or_type + * A numeric node id to upload files to an existing node, or a string + * indicating the desired bundle for a new node. + * @param bool $new_revision + * The revision number. + * @param array $extras + * Additional values when a new node is created. + * + * @return int + * The node id. */ - function uploadNodeFile($file, $field_name, $nid_or_type, $new_revision = TRUE, $extras = array()) { + function uploadNodeFile(FileInterface $file, $field_name, $nid_or_type, $new_revision = TRUE, array $extras = array()) { + return $this->uploadNodeFiles([$file], $field_name, $nid_or_type, $new_revision, $extras); + } + + /** + * Uploads multiple files to a node. + * + * @param \Drupal\file\FileInterface[] $files + * The files to be uploaded. + * @param string $field_name + * The name of the field on which the files should be saved. + * @param $nid_or_type + * A numeric node id to upload files to an existing node, or a string + * indicating the desired bundle for a new node. + * @param bool $new_revision + * The revision number. + * @param array $extras + * Additional values when a new node is created. + * + * @return int + * The node id. + */ + function uploadNodeFiles(array $files, $field_name, $nid_or_type, $new_revision = TRUE, array $extras = array()) { $edit = array( 'title[0][value]' => $this->randomMachineName(), 'revision' => (string) (int) $new_revision, ); + $node_storage = $this->container->get('entity.manager')->getStorage('node'); if (is_numeric($nid_or_type)) { $nid = $nid_or_type; + $node_storage->resetCache(array($nid)); + $node = $node_storage->load($nid); } else { - $node_storage = $this->container->get('entity.manager')->getStorage('node'); // Add a new node. $extras['type'] = $nid_or_type; $node = $this->drupalCreateNode($extras); @@ -171,13 +210,23 @@ abstract class FileFieldTestBase extends WebTestBase { $this->assertNotEqual($nid, $node->getRevisionId(), 'Node revision exists.'); } - // Attach a file to the node. + // Attach files to the node. $field_storage = FieldStorageConfig::loadByName('node', $field_name); - $name = 'files[' . $field_name . '_0]'; + // File input name depends on number of files already uploaded. + $field_num = count($node->{$field_name}); + $name = 'files[' . $field_name . "_$field_num]"; if ($field_storage->getCardinality() != 1) { $name .= '[]'; } - $edit[$name] = drupal_realpath($file->getFileUri()); + foreach ($files as $file) { + $file_path = $this->container->get('file_system')->realpath($file->getFileUri()); + if (count($files) == 1) { + $edit[$name] = $file_path; + } + else { + $edit[$name][] = $file_path; + } + } $this->drupalPostForm("node/$nid/edit", $edit, t('Save and keep published')); return $nid; diff --git a/core/modules/file/src/Tests/FileFieldWidgetTest.php b/core/modules/file/src/Tests/FileFieldWidgetTest.php index b696596ef39..7c24aabff62 100644 --- a/core/modules/file/src/Tests/FileFieldWidgetTest.php +++ b/core/modules/file/src/Tests/FileFieldWidgetTest.php @@ -112,8 +112,9 @@ class FileFieldWidgetTest extends FileFieldTestBase { // names). $field_name = 'test_file_field_1'; $field_name2 = 'test_file_field_2'; - $this->createFileField($field_name, 'node', $type_name, array('cardinality' => 3)); - $this->createFileField($field_name2, 'node', $type_name, array('cardinality' => 3)); + $cardinality = 3; + $this->createFileField($field_name, 'node', $type_name, array('cardinality' => $cardinality)); + $this->createFileField($field_name2, 'node', $type_name, array('cardinality' => $cardinality)); $test_file = $this->getTestFile('text'); @@ -215,6 +216,49 @@ class FileFieldWidgetTest extends FileFieldTestBase { $node = $node_storage->load($nid); $this->assertTrue(empty($node->{$field_name}->target_id), 'Node was successfully saved without any files.'); } + + $upload_files = array($test_file, $test_file); + // Try to upload multiple files, but fewer than the maximum. + $nid = $this->uploadNodeFiles($upload_files, $field_name, $type_name); + $node_storage->resetCache(array($nid)); + $node = $node_storage->load($nid); + $this->assertEqual(count($node->{$field_name}), count($upload_files), 'Node was successfully saved with mulitple files.'); + + // Try to upload more files than allowed on revision. + $this->uploadNodeFiles($upload_files, $field_name, $nid, 1); + $args = array( + '%field' => $field_name, + '@count' => $cardinality + ); + $this->assertRaw(t('%field: this field cannot hold more than @count values.', $args)); + $node_storage->resetCache(array($nid)); + $node = $node_storage->load($nid); + $this->assertEqual(count($node->{$field_name}), count($upload_files), 'More files than allowed could not be saved to node.'); + + // Try to upload exactly the allowed number of files on revision. + $this->uploadNodeFile($test_file, $field_name, $nid, 1); + $node_storage->resetCache(array($nid)); + $node = $node_storage->load($nid); + $this->assertEqual(count($node->{$field_name}), $cardinality, 'Node was successfully revised to maximum number of files.'); + + // Try to upload exactly the allowed number of files, new node. + $upload_files[] = $test_file; + $nid = $this->uploadNodeFiles($upload_files, $field_name, $type_name); + $node_storage->resetCache(array($nid)); + $node = $node_storage->load($nid); + $this->assertEqual(count($node->{$field_name}), $cardinality, 'Node was successfully saved with maximum number of files.'); + + // Try to upload more files than allowed, new node. + $upload_files[] = $test_file; + $this->uploadNodeFiles($upload_files, $field_name, $type_name); + + $args = [ + '%field' => $field_name, + '@max' => $cardinality, + '@count' => count($upload_files), + '%list' => $test_file->getFileName(), + ]; + $this->assertRaw(t('Field %field can only hold @max values but there were @count uploaded. The following files have been omitted as a result: %list.', $args)); } /** diff --git a/core/modules/simpletest/src/WebTestBase.php b/core/modules/simpletest/src/WebTestBase.php index 02cb027df8b..2ac0d4b2daa 100644 --- a/core/modules/simpletest/src/WebTestBase.php +++ b/core/modules/simpletest/src/WebTestBase.php @@ -1683,16 +1683,20 @@ abstract class WebTestBase extends TestBase { $post_array = $post; if ($upload) { foreach ($upload as $key => $file) { - $file = drupal_realpath($file); - if ($file && is_file($file)) { - // Use the new CurlFile class for file uploads when using PHP - // 5.5. - if (class_exists('CurlFile')) { - $post[$key] = curl_file_create($file); + if (is_array($file) && count($file)) { + // There seems to be no way via php's API to cURL to upload + // several files with the same post field name. However, Drupal + // still sees array-index syntax in a similar way. + for ($i = 0; $i < count($file); $i++) { + $postfield = str_replace('[]', '', $key) . '[' . $i . ']'; + $file_path = $this->container->get('file_system')->realpath($file[$i]); + $post[$postfield] = curl_file_create($file_path); } - else { - // @todo: Drop support for this when PHP 5.5 is required. - $post[$key] = '@' . $file; + } + else { + $file = $this->container->get('file_system')->realpath($file); + if ($file && is_file($file)) { + $post[$key] = curl_file_create($file); } } }