From 83874f2bcf32414048cc09df1571169ac645659b Mon Sep 17 00:00:00 2001 From: catch Date: Fri, 1 Mar 2024 14:02:18 +0000 Subject: [PATCH] Issue #3412361 by Wim Leers, phenaproxima, catch, effulgentsia: Mark Editor config schema as fully validatable --- .../Core/Config/Schema/SchemaCheckTrait.php | 7 + .../Controller/CKEditor5ImageController.php | 4 +- .../src/Plugin/CKEditor5Plugin/Image.php | 17 +- .../ckeditor5/src/Plugin/Editor/CKEditor5.php | 14 ++ ...extEditorObjectDependentValidatorTrait.php | 7 +- .../src/Functional/ImageUploadAccessTest.php | 19 +-- .../CKEditor5UpdateImageToolbarItemTest.php | 9 +- .../src/FunctionalJavascript/AdminUiTest.php | 49 ++++++ .../FunctionalJavascript/CKEditor5Test.php | 5 + .../src/Kernel/CKEditor5PluginManagerTest.php | 5 +- .../tests/src/Kernel/ValidatorsTest.php | 23 ++- .../editor/config/schema/editor.schema.yml | 96 ++++++++--- core/modules/editor/editor.module | 48 ++++++ core/modules/editor/editor.post_update.php | 23 +++ core/modules/editor/src/Entity/Editor.php | 15 ++ .../editor/src/Form/EditorImageDialog.php | 21 +-- .../tests/fixtures/update/editor-3412361.php | 41 +++++ .../update/editor.editor.umami_basic_html.yml | 64 +++++++ .../update/filter.format.umami_basic_html.yml | 55 ++++++ .../src/Functional/EditorDialogAccessTest.php | 19 ++- .../Rest/EditorResourceTestBase.php | 6 +- ...rSanitizeImageUploadSettingsUpdateTest.php | 104 ++++++++++++ .../src/Kernel/EditorImageDialogTest.php | 4 + .../tests/src/Kernel/EditorValidationTest.php | 157 +++++++++++++++++- .../tests/src/Functional/EditorTest.php | 8 +- .../install/editor.editor.basic_html.yml | 6 - .../install/editor.editor.full_html.yml | 2 +- .../install/editor.editor.basic_html.yml | 6 +- .../install/editor.editor.full_html.yml | 6 +- .../Tests/Component/Utility/BytesTest.php | 1 + 30 files changed, 751 insertions(+), 90 deletions(-) create mode 100644 core/modules/editor/tests/fixtures/update/editor-3412361.php create mode 100644 core/modules/editor/tests/fixtures/update/editor.editor.umami_basic_html.yml create mode 100644 core/modules/editor/tests/fixtures/update/filter.format.umami_basic_html.yml create mode 100644 core/modules/editor/tests/src/Functional/Update/EditorSanitizeImageUploadSettingsUpdateTest.php diff --git a/core/lib/Drupal/Core/Config/Schema/SchemaCheckTrait.php b/core/lib/Drupal/Core/Config/Schema/SchemaCheckTrait.php index 88682009ae2..fbc1edec5d8 100644 --- a/core/lib/Drupal/Core/Config/Schema/SchemaCheckTrait.php +++ b/core/lib/Drupal/Core/Config/Schema/SchemaCheckTrait.php @@ -52,6 +52,13 @@ trait SchemaCheckTrait { 'This value should not be blank.', ], ], + 'editor.editor.*' => [ + // @todo Fix stream wrappers not being available early enough in + // https://www.drupal.org/project/drupal/issues/3416735 + 'image_upload.scheme' => [ + '^The file storage you selected is not a visible, readable and writable stream wrapper\. Possible choices: <\/em>\.$', + ], + ], ]; /** diff --git a/core/modules/ckeditor5/src/Controller/CKEditor5ImageController.php b/core/modules/ckeditor5/src/Controller/CKEditor5ImageController.php index 1a9cc15d1fb..43f4d9b44cf 100644 --- a/core/modules/ckeditor5/src/Controller/CKEditor5ImageController.php +++ b/core/modules/ckeditor5/src/Controller/CKEditor5ImageController.php @@ -175,7 +175,9 @@ class CKEditor5ImageController extends ControllerBase { * Gets the image upload validators. */ protected function getImageUploadValidators(array $settings): array { - $max_filesize = min(Bytes::toNumber($settings['max_size']), Environment::getUploadMaxSize()); + $max_filesize = $settings['max_size'] + ? Bytes::toNumber($settings['max_size']) + : Environment::getUploadMaxSize(); $max_dimensions = 0; if (!empty($settings['max_dimensions']['width']) || !empty($settings['max_dimensions']['height'])) { $max_dimensions = $settings['max_dimensions']['width'] . 'x' . $settings['max_dimensions']['height']; diff --git a/core/modules/ckeditor5/src/Plugin/CKEditor5Plugin/Image.php b/core/modules/ckeditor5/src/Plugin/CKEditor5Plugin/Image.php index d897abd265f..c243eaec697 100644 --- a/core/modules/ckeditor5/src/Plugin/CKEditor5Plugin/Image.php +++ b/core/modules/ckeditor5/src/Plugin/CKEditor5Plugin/Image.php @@ -63,16 +63,27 @@ class Image extends CKEditor5PluginDefault implements CKEditor5PluginConfigurabl */ public function validateConfigurationForm(array &$form, FormStateInterface $form_state) { $form_state->setValue('status', (bool) $form_state->getValue('status')); - $form_state->setValue(['max_dimensions', 'width'], (int) $form_state->getValue(['max_dimensions', 'width'])); - $form_state->setValue(['max_dimensions', 'height'], (int) $form_state->getValue(['max_dimensions', 'height'])); + $directory = $form_state->getValue(['directory']); + $form_state->setValue(['directory'], trim($directory) === '' ? NULL : $directory); + $max_size = $form_state->getValue(['max_size']); + $form_state->setValue(['max_size'], trim($max_size) === '' ? NULL : $max_size); + $max_width = $form_state->getValue(['max_dimensions', 'width']); + $form_state->setValue(['max_dimensions', 'width'], trim($max_width) === '' ? NULL : (int) $max_width); + $max_height = $form_state->getValue(['max_dimensions', 'height']); + $form_state->setValue(['max_dimensions', 'height'], trim($max_height) === '' ? NULL : (int) $max_height); } /** * {@inheritdoc} */ public function submitConfigurationForm(array &$form, FormStateInterface $form_state) { + $settings = $form_state->getValues(); + if (!$settings['status']) { + // Remove all other settings to comply with config schema. + $settings = ['status' => FALSE]; + } // Store this configuration in its out-of-band location. - $form_state->get('editor')->setImageUploadSettings($form_state->getValues()); + $form_state->get('editor')->setImageUploadSettings($settings); } /** diff --git a/core/modules/ckeditor5/src/Plugin/Editor/CKEditor5.php b/core/modules/ckeditor5/src/Plugin/Editor/CKEditor5.php index b12841651f8..87d46bcc5e3 100644 --- a/core/modules/ckeditor5/src/Plugin/Editor/CKEditor5.php +++ b/core/modules/ckeditor5/src/Plugin/Editor/CKEditor5.php @@ -660,7 +660,13 @@ class CKEditor5 extends EditorBase implements ContainerFactoryPluginInterface { // All plugin settings have been collected, including defaults that depend // on visibility. Store the collected settings, throw away the interim state // that allowed determining which defaults to add. + // Create a new clone, because the plugins whose data is being stored + // out-of-band may have modified the Text Editor config entity in the form + // state. + // @see \Drupal\editor\EditorInterface::setImageUploadSettings() + // @see \Drupal\ckeditor5\Plugin\CKEditor5Plugin\Image::submitConfigurationForm() unset($eventual_editor_and_format_for_plugin_settings_visibility); + $submitted_editor = clone $form_state->get('editor'); $submitted_editor->setSettings($settings); // Validate the text editor + text format pair. @@ -903,6 +909,14 @@ class CKEditor5 extends EditorBase implements ContainerFactoryPluginInterface { return implode('][', array_merge(explode('.', $property_path), ['settings'])); } + // Image upload settings are stored out-of-band and may also trigger + // validation errors. + // @see \Drupal\ckeditor5\Plugin\CKEditor5Plugin\Image + if (str_starts_with($property_path, 'image_upload.')) { + $image_upload_setting_property_path = str_replace('image_upload.', '', $property_path); + return 'editor][settings][plugins][ckeditor5_image][' . implode('][', explode('.', $image_upload_setting_property_path)); + } + // Everything else is in the subform. return 'editor][' . static::mapViolationPropertyPathsToFormNames($property_path, $form); } diff --git a/core/modules/ckeditor5/src/Plugin/Validation/Constraint/TextEditorObjectDependentValidatorTrait.php b/core/modules/ckeditor5/src/Plugin/Validation/Constraint/TextEditorObjectDependentValidatorTrait.php index 63bf2f0dad6..9b334f742ba 100644 --- a/core/modules/ckeditor5/src/Plugin/Validation/Constraint/TextEditorObjectDependentValidatorTrait.php +++ b/core/modules/ckeditor5/src/Plugin/Validation/Constraint/TextEditorObjectDependentValidatorTrait.php @@ -29,8 +29,13 @@ trait TextEditorObjectDependentValidatorTrait { ]); } else { - assert($this->context->getRoot()->getDataDefinition()->getDataType() === 'editor.editor.*'); + assert(in_array($this->context->getRoot()->getDataDefinition()->getDataType(), ['editor.editor.*', 'entity:editor'], TRUE)); $text_format = FilterFormat::load($this->context->getRoot()->get('format')->getValue()); + // This validator must not complain about a missing text format. + // @see \Drupal\Tests\editor\Kernel\EditorValidationTest::testInvalidFormat() + if ($text_format === NULL) { + $text_format = FilterFormat::create([]); + } } assert($text_format instanceof FilterFormatInterface); diff --git a/core/modules/ckeditor5/tests/src/Functional/ImageUploadAccessTest.php b/core/modules/ckeditor5/tests/src/Functional/ImageUploadAccessTest.php index de8bb0e6578..5b224460e24 100644 --- a/core/modules/ckeditor5/tests/src/Functional/ImageUploadAccessTest.php +++ b/core/modules/ckeditor5/tests/src/Functional/ImageUploadAccessTest.php @@ -27,8 +27,14 @@ class ImageUploadAccessTest extends ImageUploadTest { $response = $this->uploadRequest($url, $test_image, 'test.jpg'); $this->assertSame(404, $response->getStatusCode()); - $editor = $this->createEditorWithUpload([ - 'status' => FALSE, + $editor = $this->createEditorWithUpload(['status' => FALSE]); + + // Ensure that images cannot be uploaded when image upload is disabled. + $response = $this->uploadRequest($url, $test_image, 'test.jpg'); + $this->assertSame(403, $response->getStatusCode()); + + $editor->setImageUploadSettings([ + 'status' => TRUE, 'scheme' => 'public', 'directory' => 'inline-images', 'max_size' => '', @@ -36,14 +42,7 @@ class ImageUploadAccessTest extends ImageUploadTest { 'width' => 0, 'height' => 0, ], - ]); - - // Ensure that images cannot be uploaded when image upload is disabled. - $response = $this->uploadRequest($url, $test_image, 'test.jpg'); - $this->assertSame(403, $response->getStatusCode()); - - $editor->setImageUploadSettings(['status' => TRUE] + $editor->getImageUploadSettings()) - ->save(); + ])->save(); $response = $this->uploadRequest($url, $test_image, 'test.jpg'); $this->assertSame(201, $response->getStatusCode()); diff --git a/core/modules/ckeditor5/tests/src/Functional/Update/CKEditor5UpdateImageToolbarItemTest.php b/core/modules/ckeditor5/tests/src/Functional/Update/CKEditor5UpdateImageToolbarItemTest.php index 483ebd6bf2b..82379a24d87 100644 --- a/core/modules/ckeditor5/tests/src/Functional/Update/CKEditor5UpdateImageToolbarItemTest.php +++ b/core/modules/ckeditor5/tests/src/Functional/Update/CKEditor5UpdateImageToolbarItemTest.php @@ -138,7 +138,14 @@ class CKEditor5UpdateImageToolbarItemTest extends UpdatePathTestBase { function (ConstraintViolation $v) { return (string) $v->getMessage(); }, - iterator_to_array(CKEditor5::validatePair($editor_after, $filter_format_after)) + // @todo Fix stream wrappers not being available early enough in + // https://www.drupal.org/project/drupal/issues/3416735. Then remove the + // array_filter(). + // @see \Drupal\Core\Config\Schema\SchemaCheckTrait::$ignoredPropertyPaths + array_filter( + iterator_to_array(CKEditor5::validatePair($editor_after, $filter_format_after)), + fn(ConstraintViolation $v) => $v->getMessage() != 'The file storage you selected is not a visible, readable and writable stream wrapper. Possible choices: .', + ) )); } diff --git a/core/modules/ckeditor5/tests/src/FunctionalJavascript/AdminUiTest.php b/core/modules/ckeditor5/tests/src/FunctionalJavascript/AdminUiTest.php index c0b7a793194..3bbf63a8309 100644 --- a/core/modules/ckeditor5/tests/src/FunctionalJavascript/AdminUiTest.php +++ b/core/modules/ckeditor5/tests/src/FunctionalJavascript/AdminUiTest.php @@ -177,6 +177,55 @@ JS; $this->assertFalse($media_tab->isVisible(), 'Media settings should be removed when media filter disabled'); } + /** + * Tests that image upload settings (stored out of band) are validated too. + */ + public function testImageUploadSettingsAreValidated(): void { + $page = $this->getSession()->getPage(); + $assert_session = $this->assertSession(); + + $this->addNewTextFormat($page, $assert_session); + $this->drupalGet('admin/config/content/formats/manage/ckeditor5'); + + // Add the image plugin to the CKEditor 5 toolbar. + $this->assertNotEmpty($assert_session->waitForElement('css', '.ckeditor5-toolbar-item-drupalInsertImage')); + $this->triggerKeyUp('.ckeditor5-toolbar-item-drupalInsertImage', 'ArrowDown'); + $assert_session->assertExpectedAjaxRequest(1); + + // Open the vertical tab with its settings. + $page->find('css', '[href^="#edit-editor-settings-plugins-ckeditor5-image"]')->click(); + $this->assertTrue($assert_session->waitForText('Enable image uploads')); + + // Check the "Enable image uploads" checkbox. + $assert_session->checkboxNotChecked('editor[settings][plugins][ckeditor5_image][status]'); + $page->checkField('editor[settings][plugins][ckeditor5_image][status]'); + $assert_session->assertExpectedAjaxRequest(2); + + // Enter a nonsensical maximum file size. + $page->fillField('editor[settings][plugins][ckeditor5_image][max_size]', 'foobar'); + $this->assertNoRealtimeValidationErrors(); + + // Enable another toolbar item to trigger validation. + $this->triggerKeyUp('.ckeditor5-toolbar-item-sourceEditing', 'ArrowDown'); + $assert_session->assertExpectedAjaxRequest(3); + + // The expected validation error must be present. + $assert_session->elementExists('css', '[role=alert]:contains("This value must be a number of bytes, optionally with a unit such as "MB" or "megabytes".")'); + + // Enter no maximum file size because it is optional, this should result in + // no validation error and it being set to `null`. + $page->findField('editor[settings][plugins][ckeditor5_image][max_size]')->setValue(''); + + // Remove a toolbar item to trigger validation. + $this->triggerKeyUp('.ckeditor5-toolbar-item-sourceEditing', 'ArrowUp'); + $assert_session->assertExpectedAjaxRequest(4); + + // No more validation errors, let's save. + $this->assertNoRealtimeValidationErrors(); + $page->pressButton('Save configuration'); + $assert_session->pageTextContains('The text format ckeditor5 has been updated'); + } + /** * Ensure CKEditor 5 admin UI's real-time validation errors do not accumulate. */ diff --git a/core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5Test.php b/core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5Test.php index edc3b94576d..3e17c31e00f 100644 --- a/core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5Test.php +++ b/core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5Test.php @@ -95,6 +95,10 @@ class CKEditor5Test extends CKEditor5TestBase { 'scheme' => 'public', 'directory' => 'inline-images', 'max_size' => '', + 'max_dimensions' => [ + 'width' => NULL, + 'height' => NULL, + ], ], ])->save(); $this->assertSame([], array_map( @@ -643,6 +647,7 @@ JS; 'reversed' => FALSE, 'startIndex' => FALSE, ], + 'multiBlock' => TRUE, ], 'ckeditor5_sourceEditing' => [ 'allowed_tags' => [], diff --git a/core/modules/ckeditor5/tests/src/Kernel/CKEditor5PluginManagerTest.php b/core/modules/ckeditor5/tests/src/Kernel/CKEditor5PluginManagerTest.php index 1ae537c185b..66a78c4f7cd 100644 --- a/core/modules/ckeditor5/tests/src/Kernel/CKEditor5PluginManagerTest.php +++ b/core/modules/ckeditor5/tests/src/Kernel/CKEditor5PluginManagerTest.php @@ -1030,7 +1030,6 @@ PHP, $sneaky_plugin_id => ['configured_subset' => $configured_subset], ], ], - 'image_upload' => [], ]); // Invalid subsets are allowed on unsaved Text Editor config entities, @@ -1257,7 +1256,9 @@ PHP, 'format' => 'dummy', 'editor' => 'ckeditor5', 'settings' => $text_editor_settings, - 'image_upload' => [], + 'image_upload' => [ + 'status' => FALSE, + ], ]); FilterFormat::create([ 'format' => 'dummy', diff --git a/core/modules/ckeditor5/tests/src/Kernel/ValidatorsTest.php b/core/modules/ckeditor5/tests/src/Kernel/ValidatorsTest.php index bb18c3cef65..dac6a409510 100644 --- a/core/modules/ckeditor5/tests/src/Kernel/ValidatorsTest.php +++ b/core/modules/ckeditor5/tests/src/Kernel/ValidatorsTest.php @@ -86,7 +86,9 @@ class ValidatorsTest extends KernelTestBase { 'format' => 'dummy', 'editor' => 'ckeditor5', 'settings' => $ckeditor5_settings, - 'image_upload' => [], + 'image_upload' => [ + 'status' => FALSE, + ], ]); $typed_config = $this->typedConfig->createFromNameAndData( @@ -182,7 +184,10 @@ class ValidatorsTest extends KernelTestBase { ], ], 'violations' => [ - 'settings.plugins.ckeditor5_language' => 'Configuration for the enabled plugin "Language" (ckeditor5_language) is missing.', + 'settings.plugins.ckeditor5_language' => [ + 'Configuration for the enabled plugin "Language" (ckeditor5_language) is missing.', + "'language_list' is a required key because settings.plugins.%key is ckeditor5_language (see config schema type ckeditor5.plugin.ckeditor5_language).", + ], ], ]; $data['valid language plugin configuration: un'] = [ @@ -1056,7 +1061,7 @@ class ValidatorsTest extends KernelTestBase { ], ], 'image_upload' => [ - 'status' => TRUE, + 'status' => FALSE, ], 'filters' => [], 'violations' => [ @@ -1102,7 +1107,7 @@ class ValidatorsTest extends KernelTestBase { ], ], 'image_upload' => [ - 'status' => TRUE, + 'status' => FALSE, ], 'filters' => [], 'violations' => [ @@ -1163,8 +1168,15 @@ class ValidatorsTest extends KernelTestBase { ], ], ], - 'image' => [ + 'image_upload' => [ 'status' => TRUE, + 'scheme' => 'public', + 'directory' => 'inline-images', + 'max_size' => NULL, + 'max_dimensions' => [ + 'width' => NULL, + 'height' => NULL, + ], ], 'filters' => [], 'violations' => [], @@ -1621,7 +1633,6 @@ class ValidatorsTest extends KernelTestBase { ], 'plugins' => [], ], - 'image_upload' => [], ]); $this->assertSame([], $this->validatePairToViolationsArray($text_editor, $text_format, TRUE)); diff --git a/core/modules/editor/config/schema/editor.schema.yml b/core/modules/editor/config/schema/editor.schema.yml index 0c0278b7155..ae035cc4cab 100644 --- a/core/modules/editor/config/schema/editor.schema.yml +++ b/core/modules/editor/config/schema/editor.schema.yml @@ -7,6 +7,11 @@ editor.editor.*: format: type: string label: 'Name' + constraints: + # @see \Drupal\editor\Entity\Editor::getFilterFormat() + # @see \Drupal\editor\Entity\Editor::calculateDependencies() + ConfigExists: + prefix: 'filter.format.' editor: type: string label: 'Text editor' @@ -17,30 +22,71 @@ editor.editor.*: settings: type: editor.settings.[%parent.editor] image_upload: + type: editor.image_upload_settings.[status] + constraints: + FullyValidatable: ~ + +editor.image_upload_settings.*: + type: mapping + label: 'Image uploads' + constraints: + FullyValidatable: ~ + mapping: + status: + type: boolean + label: 'Status' + +editor.image_upload_settings.1: + type: editor.image_upload_settings.* + label: 'Image upload settings' + constraints: + FullyValidatable: ~ + mapping: + scheme: + type: string + label: 'File storage' + constraints: + Choice: + callback: \Drupal\editor\Entity\Editor::getValidStreamWrappers + message: 'The file storage you selected is not a visible, readable and writable stream wrapper. Possible choices: %choices.' + directory: + type: string + label: 'Upload directory' + nullable: true + constraints: + # `""` is not allowed, but `null` is. + NotBlank: + allowNull: true + Regex: + # Forbid any kind of control character. + # @see https://stackoverflow.com/a/66587087 + pattern: '/([^\PC])/u' + match: false + message: 'The image upload directory is not allowed to span multiple lines or contain control characters.' + max_size: + # @see \Drupal\file\Plugin\Validation\Constraint\FileSizeLimitConstraintValidator + type: bytes + label: 'Maximum file size' + nullable: true + max_dimensions: type: mapping - label: 'Image upload settings' + label: 'Maximum dimensions' mapping: - status: - type: boolean - label: 'Status' - scheme: - type: string - label: 'File storage' - directory: - type: string - label: 'Upload directory' - max_size: - type: string - label: 'Maximum file size' - max_dimensions: - type: mapping - label: 'Maximum dimensions' - mapping: - width: - type: integer - nullable: true - label: 'Maximum width' - height: - type: integer - nullable: true - label: 'Maximum height' + width: + type: integer + nullable: true + label: 'Maximum width' + constraints: + Range: + # @see editor_image_upload_settings_form() + min: 1 + max: 99999 + height: + type: integer + nullable: true + label: 'Maximum height' + constraints: + Range: + # @see editor_image_upload_settings_form() + min: 1 + max: 99999 diff --git a/core/modules/editor/editor.module b/core/modules/editor/editor.module index d1f3c5c21a2..6925eea7b14 100644 --- a/core/modules/editor/editor.module +++ b/core/modules/editor/editor.module @@ -8,6 +8,7 @@ use Drupal\Core\Url; use Drupal\Component\Utility\Html; use Drupal\Core\Form\SubformState; +use Drupal\editor\EditorInterface; use Drupal\editor\Entity\Editor; use Drupal\Core\Entity\FieldableEntityInterface; use Drupal\Core\Field\FieldDefinitionInterface; @@ -252,6 +253,15 @@ function editor_form_filter_admin_format_submit($form, FormStateInterface $form_ if ($settings = $form_state->getValue(['editor', 'settings'])) { $editor->setSettings($settings); } + // When image uploads are disabled (status = FALSE), the schema for image + // upload settings does not allow other keys to be present. + // @see editor.image_upload_settings.* + // @see editor.image_upload_settings.1 + // @see editor.schema.yml + $image_upload_settings = $editor->getImageUploadSettings(); + if (!$image_upload_settings['status']) { + $editor->setImageUploadSettings(['status' => FALSE]); + } $editor->save(); } } @@ -641,3 +651,41 @@ function editor_filter_format_presave(FilterFormatInterface $format) { $editor->setStatus($status)->save(); } } + +/** + * Implements hook_ENTITY_TYPE_presave(). + */ +function editor_editor_presave(EditorInterface $editor) { + // @see editor_post_update_sanitize_image_upload_settings() + $image_upload_settings = $editor->getImageUploadSettings(); + // When image uploads are disabled, then none of the other key-value pairs + // make sense. + // TRICKY: the configuration system has historically stored `type: boolean` + // not as `true` and `false`, but as `1` and `0`, so use `==`, not `===`. + // @see editor_post_update_sanitize_image_upload_settings() + if (!array_key_exists('status', $image_upload_settings) || $image_upload_settings['status'] == FALSE) { + $editor->setImageUploadSettings(['status' => FALSE]); + } + else { + // When image uploads are enabled, then some of the key-value pairs need + // some conversions to comply with the config schema. Note that all these + // keys SHOULD exist, but because validation has historically been absent, + // err on the side of caution. + // @see editor_post_update_sanitize_image_upload_settings() + if (array_key_exists('directory', $image_upload_settings) && $image_upload_settings['directory'] === '') { + $image_upload_settings['directory'] = NULL; + } + if (array_key_exists('max_size', $image_upload_settings) && $image_upload_settings['max_size'] === '') { + $image_upload_settings['max_size'] = NULL; + } + if (array_key_exists('max_dimensions', $image_upload_settings)) { + if (!array_key_exists('width', $image_upload_settings['max_dimensions']) || $image_upload_settings['max_dimensions']['width'] === 0) { + $image_upload_settings['max_dimensions']['width'] = NULL; + } + if (!array_key_exists('height', $image_upload_settings['max_dimensions']) || $image_upload_settings['max_dimensions']['height'] === 0) { + $image_upload_settings['max_dimensions']['height'] = NULL; + } + } + $editor->setImageUploadSettings($image_upload_settings); + } +} diff --git a/core/modules/editor/editor.post_update.php b/core/modules/editor/editor.post_update.php index 112409ecb0e..c62567af7c8 100644 --- a/core/modules/editor/editor.post_update.php +++ b/core/modules/editor/editor.post_update.php @@ -5,6 +5,8 @@ * Post update functions for Editor. */ +use Drupal\Core\Config\Entity\ConfigEntityUpdater; +use Drupal\editor\EditorInterface; use Drupal\filter\Entity\FilterFormat; use Drupal\filter\FilterFormatInterface; use Drupal\filter\FilterPluginCollection; @@ -44,3 +46,24 @@ function editor_post_update_image_lazy_load(): void { } } } + +/** + * Clean up image upload settings. + */ +function editor_post_update_sanitize_image_upload_settings(&$sandbox = []) { + $config_entity_updater = \Drupal::classResolver(ConfigEntityUpdater::class); + + $callback = function (EditorInterface $editor) { + $image_upload_settings = $editor->getImageUploadSettings(); + // Only update if the editor has image uploads: + // - empty image upload settings + // - disabled and >=1 other keys in its image upload settings + // - enabled (to tighten the key-value pairs in its settings). + // @see editor_editor_presave() + return !array_key_exists('status', $image_upload_settings) + || ($image_upload_settings['status'] == FALSE && count($image_upload_settings) >= 2) + || $image_upload_settings['status'] == TRUE; + }; + + $config_entity_updater->update($sandbox, 'editor', $callback); +} diff --git a/core/modules/editor/src/Entity/Editor.php b/core/modules/editor/src/Entity/Editor.php index b8c041df85b..25ff5f8e992 100644 --- a/core/modules/editor/src/Entity/Editor.php +++ b/core/modules/editor/src/Entity/Editor.php @@ -4,6 +4,7 @@ namespace Drupal\editor\Entity; use Drupal\Component\Plugin\Exception\PluginNotFoundException; use Drupal\Core\Config\Entity\ConfigEntityBase; +use Drupal\Core\StreamWrapper\StreamWrapperInterface; use Drupal\editor\EditorInterface; /** @@ -207,4 +208,18 @@ class Editor extends ConfigEntityBase implements EditorInterface { return $this; } + /** + * Computes all valid choices for the "image_upload.scheme" setting. + * + * @see editor.schema.yml + * + * @return string[] + * All valid choices. + * + * @internal + */ + public static function getValidStreamWrappers(): array { + return array_keys(\Drupal::service('stream_wrapper_manager')->getNames(StreamWrapperInterface::WRITE_VISIBLE)); + } + } diff --git a/core/modules/editor/src/Form/EditorImageDialog.php b/core/modules/editor/src/Form/EditorImageDialog.php index 6e4cf9db270..323f24817ce 100644 --- a/core/modules/editor/src/Form/EditorImageDialog.php +++ b/core/modules/editor/src/Form/EditorImageDialog.php @@ -96,25 +96,15 @@ class EditorImageDialog extends FormBase { // Construct strings to use in the upload validators. $image_upload = $editor->getImageUploadSettings(); - if (!empty($image_upload['max_dimensions']['width']) || !empty($image_upload['max_dimensions']['height'])) { - $max_dimensions = $image_upload['max_dimensions']['width'] . 'x' . $image_upload['max_dimensions']['height']; - } - else { - $max_dimensions = 0; - } - $max_filesize = min(Bytes::toNumber($image_upload['max_size']), Environment::getUploadMaxSize()); $existing_file = isset($image_element['data-entity-uuid']) ? \Drupal::service('entity.repository')->loadEntityByUuid('file', $image_element['data-entity-uuid']) : NULL; $fid = $existing_file ? $existing_file->id() : NULL; $form['fid'] = [ '#title' => $this->t('Image'), '#type' => 'managed_file', - '#upload_location' => $image_upload['scheme'] . '://' . $image_upload['directory'], '#default_value' => $fid ? [$fid] : NULL, '#upload_validators' => [ 'FileExtension' => ['extensions' => 'gif png jpg jpeg'], - 'FileSizeLimit' => ['fileLimit' => $max_filesize], - 'FileImageDimensions' => ['maxDimensions' => $max_dimensions], ], '#required' => TRUE, ]; @@ -132,6 +122,17 @@ class EditorImageDialog extends FormBase { if ($image_upload['status']) { $form['attributes']['src']['#access'] = FALSE; $form['attributes']['src']['#required'] = FALSE; + + if (!empty($image_upload['max_dimensions']['width']) || !empty($image_upload['max_dimensions']['height'])) { + $max_dimensions = $image_upload['max_dimensions']['width'] . 'x' . $image_upload['max_dimensions']['height']; + } + else { + $max_dimensions = 0; + } + $max_filesize = min(Bytes::toNumber($image_upload['max_size'] ?? 0), Environment::getUploadMaxSize()); + $form['fid']['#upload_location'] = $image_upload['scheme'] . '://' . ($image_upload['directory'] ?? ''); + $form['fid']['#upload_validators']['FileSizeLimit'] = ['fileLimit' => $max_filesize]; + $form['fid']['#upload_validators']['FileImageDimensions'] = ['maxDimensions' => $max_dimensions]; } else { $form['fid']['#access'] = FALSE; diff --git a/core/modules/editor/tests/fixtures/update/editor-3412361.php b/core/modules/editor/tests/fixtures/update/editor-3412361.php new file mode 100644 index 00000000000..390f07699e1 --- /dev/null +++ b/core/modules/editor/tests/fixtures/update/editor-3412361.php @@ -0,0 +1,41 @@ +insert('config') + ->fields([ + 'collection', + 'name', + 'data', + ]) + ->values([ + 'collection' => '', + 'name' => 'filter.format.umami_basic_html', + 'data' => serialize($umami_basic_html_format), + ]) + ->execute(); + +$umami_basic_html_editor = Yaml::decode(file_get_contents(__DIR__ . '/editor.editor.umami_basic_html.yml')); +$umami_basic_html_editor['format'] = 'umami_basic_html'; +$connection->insert('config') + ->fields([ + 'collection', + 'name', + 'data', + ]) + ->values([ + 'collection' => '', + 'name' => 'editor.editor.umami_basic_html', + 'data' => serialize($umami_basic_html_editor), + ]) + ->execute(); diff --git a/core/modules/editor/tests/fixtures/update/editor.editor.umami_basic_html.yml b/core/modules/editor/tests/fixtures/update/editor.editor.umami_basic_html.yml new file mode 100644 index 00000000000..5a0f96f3a68 --- /dev/null +++ b/core/modules/editor/tests/fixtures/update/editor.editor.umami_basic_html.yml @@ -0,0 +1,64 @@ +uuid: c82794ef-c451-49c6-be67-39e2b0649a47 +langcode: en +status: true +dependencies: + config: + - filter.format.basic_html + module: + - ckeditor5 +format: basic_html +editor: ckeditor5 +settings: + toolbar: + items: + - bold + - italic + - '|' + - link + - '|' + - bulletedList + - numberedList + - '|' + - blockQuote + - '|' + - heading + - '|' + - sourceEditing + - '|' + plugins: + ckeditor5_heading: + enabled_headings: + - heading2 + - heading3 + - heading4 + - heading5 + - heading6 + ckeditor5_list: + properties: + reversed: false + startIndex: true + multiBlock: false + ckeditor5_sourceEditing: + allowed_tags: + - '' + - '
' + - '
' + - '
' + - '' + - '
' + - '