From e152fefcdff53e466946cd7ec14a2e03a31466ef Mon Sep 17 00:00:00 2001
From: Nathaniel Catchpole <catch@35733.no-reply.drupal.org>
Date: Wed, 4 Feb 2015 11:59:36 +0000
Subject: [PATCH] Issue #2414991 by alexpott: Prevent
hook_config_schema_info_alter from adding or removing definitions
---
.../Schema/ConfigSchemaAlterException.php | 14 +++++
.../Drupal/Core/Config/TypedConfigManager.php | 25 +++++++++
.../Core/Plugin/DefaultPluginManager.php | 16 ++++--
.../config/src/Tests/ConfigSchemaTest.php | 51 +++++++++++++++++++
.../schema/config_schema_test.schema.yml | 2 +
.../config_schema_test.module | 23 +++++++++
core/modules/system/system.api.php | 7 ++-
7 files changed, 133 insertions(+), 5 deletions(-)
create mode 100644 core/lib/Drupal/Core/Config/Schema/ConfigSchemaAlterException.php
create mode 100644 core/modules/config/tests/config_schema_test/config_schema_test.module
diff --git a/core/lib/Drupal/Core/Config/Schema/ConfigSchemaAlterException.php b/core/lib/Drupal/Core/Config/Schema/ConfigSchemaAlterException.php
new file mode 100644
index 000000000000..fa87e06d9643
--- /dev/null
+++ b/core/lib/Drupal/Core/Config/Schema/ConfigSchemaAlterException.php
@@ -0,0 +1,14 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Config\Schema\ConfigSchemaAlterException.
+ */
+
+namespace Drupal\Core\Config\Schema;
+
+/**
+ * Exception for when hook_config_schema_info_alter() adds or removes schema.
+ */
+class ConfigSchemaAlterException extends \RuntimeException {
+}
diff --git a/core/lib/Drupal/Core/Config/TypedConfigManager.php b/core/lib/Drupal/Core/Config/TypedConfigManager.php
index a78436404a06..22f43c1db873 100644
--- a/core/lib/Drupal/Core/Config/TypedConfigManager.php
+++ b/core/lib/Drupal/Core/Config/TypedConfigManager.php
@@ -8,7 +8,9 @@
namespace Drupal\Core\Config;
use Drupal\Component\Utility\NestedArray;
+use Drupal\Component\Utility\String;
use Drupal\Core\Cache\CacheBackendInterface;
+use Drupal\Core\Config\Schema\ConfigSchemaAlterException;
use Drupal\Core\Config\Schema\ConfigSchemaDiscovery;
use Drupal\Core\Config\Schema\Element;
use Drupal\Core\Extension\ModuleHandlerInterface;
@@ -296,6 +298,29 @@ public function hasConfigSchema($name) {
return is_array($definition) && ($definition['class'] != '\Drupal\Core\Config\Schema\Undefined');
}
+ /**
+ * {@inheritdoc}
+ */
+ protected function alterDefinitions(&$definitions) {
+ $discovered_schema = array_keys($definitions);
+ parent::alterDefinitions($definitions);
+ $altered_schema = array_keys($definitions);
+ if ($discovered_schema != $altered_schema) {
+ $added_keys = array_diff($altered_schema, $discovered_schema);
+ $removed_keys = array_diff($discovered_schema, $altered_schema);
+ if (!empty($added_keys) && !empty($removed_keys)) {
+ $message = 'Invoking hook_config_schema_info_alter() has added (@added) and removed (@removed) schema definitions';
+ }
+ elseif (!empty($added_keys)) {
+ $message = 'Invoking hook_config_schema_info_alter() has added (@added) schema definitions';
+ }
+ else {
+ $message = 'Invoking hook_config_schema_info_alter() has removed (@removed) schema definitions';
+ }
+ throw new ConfigSchemaAlterException(String::format($message, ['@added' => implode(',', $added_keys), '@removed' => implode(',', $removed_keys)]));
+ }
+ }
+
/**
* {@inheritdoc}
*/
diff --git a/core/lib/Drupal/Core/Plugin/DefaultPluginManager.php b/core/lib/Drupal/Core/Plugin/DefaultPluginManager.php
index 95928e4e24ed..5c37aa99d7bd 100644
--- a/core/lib/Drupal/Core/Plugin/DefaultPluginManager.php
+++ b/core/lib/Drupal/Core/Plugin/DefaultPluginManager.php
@@ -224,9 +224,7 @@ protected function findDefinitions() {
foreach ($definitions as $plugin_id => &$definition) {
$this->processDefinition($definition, $plugin_id);
}
- if ($this->alterHook) {
- $this->moduleHandler->alter($this->alterHook, $definitions);
- }
+ $this->alterDefinitions($definitions);
// If this plugin was provided by a module that does not exist, remove the
// plugin definition.
foreach ($definitions as $plugin_id => $plugin_definition) {
@@ -242,4 +240,16 @@ protected function findDefinitions() {
return $definitions;
}
+ /**
+ * Invokes the hook to alter the definitions if the alter hook is set.
+ *
+ * @param $definitions
+ * The discovered plugin defintions.
+ */
+ protected function alterDefinitions(&$definitions) {
+ if ($this->alterHook) {
+ $this->moduleHandler->alter($this->alterHook, $definitions);
+ }
+ }
+
}
diff --git a/core/modules/config/src/Tests/ConfigSchemaTest.php b/core/modules/config/src/Tests/ConfigSchemaTest.php
index c7a6f55513ae..a2a3510e771b 100644
--- a/core/modules/config/src/Tests/ConfigSchemaTest.php
+++ b/core/modules/config/src/Tests/ConfigSchemaTest.php
@@ -9,6 +9,7 @@
use Drupal\Core\Config\FileStorage;
use Drupal\Core\Config\InstallStorage;
+use Drupal\Core\Config\Schema\ConfigSchemaAlterException;
use Drupal\Core\TypedData\Type\IntegerInterface;
use Drupal\Core\TypedData\Type\StringInterface;
use Drupal\simpletest\KernelTestBase;
@@ -436,4 +437,54 @@ function testColonsInSchemaTypeDetermination() {
$this->assertEqual($definition['type'], 'test_with_parents.plugin_types.*');
}
+ /**
+ * Tests hook_config_schema_info_alter().
+ */
+ public function testConfigSchemaInfoAlter() {
+ /** @var \Drupal\Core\Config\TypedConfigManagerInterface $typed_config */
+ $typed_config = \Drupal::service('config.typed');
+ $typed_config->clearCachedDefinitions();
+
+ // Ensure that keys can not be added or removed by
+ // hook_config_schema_info_alter().
+ \Drupal::state()->set('config_schema_test_exception_remove', TRUE);
+ $message = 'Expected ConfigSchemaAlterException thrown.';
+ try {
+ $typed_config->getDefinitions();
+ $this->fail($message);
+ }
+ catch (ConfigSchemaAlterException $e) {
+ $this->pass($message);
+ $this->assertEqual($e->getMessage(), 'Invoking hook_config_schema_info_alter() has removed (config_schema_test.hook) schema definitions');
+ }
+
+ \Drupal::state()->set('config_schema_test_exception_add', TRUE);
+ $message = 'Expected ConfigSchemaAlterException thrown.';
+ try {
+ $typed_config->getDefinitions();
+ $this->fail($message);
+ }
+ catch (ConfigSchemaAlterException $e) {
+ $this->pass($message);
+ $this->assertEqual($e->getMessage(), 'Invoking hook_config_schema_info_alter() has added (config_schema_test.hook_added_defintion) and removed (config_schema_test.hook) schema definitions');
+ }
+
+ \Drupal::state()->set('config_schema_test_exception_remove', FALSE);
+ $message = 'Expected ConfigSchemaAlterException thrown.';
+ try {
+ $typed_config->getDefinitions();
+ $this->fail($message);
+ }
+ catch (ConfigSchemaAlterException $e) {
+ $this->pass($message);
+ $this->assertEqual($e->getMessage(), 'Invoking hook_config_schema_info_alter() has added (config_schema_test.hook_added_defintion) schema definitions');
+ }
+
+ // Tests that hook_config_schema_info_alter() can add additional metadata to
+ // existing configuration schema.
+ \Drupal::state()->set('config_schema_test_exception_add', FALSE);
+ $definitions = $typed_config->getDefinitions();
+ $this->assertEqual($definitions['config_schema_test.hook']['additional_metadata'], 'new schema info');
+ }
+
}
diff --git a/core/modules/config/tests/config_schema_test/config/schema/config_schema_test.schema.yml b/core/modules/config/tests/config_schema_test/config/schema/config_schema_test.schema.yml
index cbc3c6190023..719865a47d24 100644
--- a/core/modules/config/tests/config_schema_test/config/schema/config_schema_test.schema.yml
+++ b/core/modules/config/tests/config_schema_test/config/schema/config_schema_test.schema.yml
@@ -207,3 +207,5 @@ test_with_parents.plugin_types.*:
value:
type: string
+config_schema_test.hook:
+ type: string
diff --git a/core/modules/config/tests/config_schema_test/config_schema_test.module b/core/modules/config/tests/config_schema_test/config_schema_test.module
new file mode 100644
index 000000000000..3ce1170222e1
--- /dev/null
+++ b/core/modules/config/tests/config_schema_test/config_schema_test.module
@@ -0,0 +1,23 @@
+<?php
+
+/**
+ * @file
+ * Tests configuration schema functionality.
+ */
+
+/**
+ * Implements hook_config_schema_info_alter().
+ */
+function config_schema_test_config_schema_info_alter(&$definitions) {
+ if (\Drupal::state()->get('config_schema_test_exception_add')) {
+ $definitions['config_schema_test.hook_added_defintion'] = $definitions['config_schema_test.hook'];
+ }
+ if (\Drupal::state()->get('config_schema_test_exception_remove')) {
+ unset($definitions['config_schema_test.hook']);
+ }
+
+ // Since code can not be unloaded only alter the definition if it exists.
+ if (isset($definitions['config_schema_test.hook'])) {
+ $definitions['config_schema_test.hook']['additional_metadata'] = 'new schema info';
+ }
+}
diff --git a/core/modules/system/system.api.php b/core/modules/system/system.api.php
index cfdf5e2b2106..eb4eb9bebebf 100644
--- a/core/modules/system/system.api.php
+++ b/core/modules/system/system.api.php
@@ -580,8 +580,8 @@ function hook_config_import_steps_alter(&$sync_steps, \Drupal\Core\Config\Config
* configuration schema type to change default labels or form element renderers
* used for configuration translation.
*
- * It is strongly advised not to use this hook to add new data types or to
- * change the structure of existing ones. Keep in mind that there are tools
+ * If implementations of this hook add or remove configuration schema a
+ * ConfigSchemaAlterException will be thrown. Keep in mind that there are tools
* that may use the configuration schema for static analysis of configuration
* files, like the string extractor for the localization system. Such systems
* won't work with dynamically defined configuration schemas.
@@ -591,6 +591,9 @@ function hook_config_import_steps_alter(&$sync_steps, \Drupal\Core\Config\Config
* @param $definitions
* Associative array of configuration type definitions keyed by schema type
* names. The elements are themselves array with information about the type.
+ *
+ * @see \Drupal\Core\Config\TypedConfigManager
+ * @see \Drupal\Core\Config\Schema\ConfigSchemaAlterException
*/
function hook_config_schema_info_alter(&$definitions) {
// Enhance the text and date type definitions with classes to generate proper
--
GitLab