diff --git a/server/src/main/java/com/mirth/connect/server/controllers/DefaultConfigurationController.java b/server/src/main/java/com/mirth/connect/server/controllers/DefaultConfigurationController.java index ce330f784..9fb18ff44 100644 --- a/server/src/main/java/com/mirth/connect/server/controllers/DefaultConfigurationController.java +++ b/server/src/main/java/com/mirth/connect/server/controllers/DefaultConfigurationController.java @@ -921,6 +921,24 @@ public synchronized Map getConfigurationPropertie @Override public synchronized void setConfigurationProperties(Map map, boolean persist) throws ControllerException { + if (map == null) { + throw new ControllerException("The configuration map cannot be null."); + } + + /* + * The map may have been deserialized from a request body, which doesn't enforce its generic + * types. Check the raw entries so a bad map is rejected before it is persisted. + */ + for (Entry entry : ((Map) map).entrySet()) { + if (!(entry.getKey() instanceof String)) { + throw new ControllerException("Configuration map keys must be strings, but found " + typeName(entry.getKey()) + "."); + } + + if (!(entry.getValue() instanceof ConfigurationProperty)) { + throw new ControllerException("The configuration map entry \"" + entry.getKey() + "\" must be a ConfigurationProperty, but found " + typeName(entry.getValue()) + "."); + } + } + if (persist) { saveConfigurationProperties(map); } @@ -937,6 +955,10 @@ public synchronized void setConfigurationProperties(Map properties = getConfigurationProperties(); diff --git a/server/src/test/java/com/mirth/connect/server/controllers/DefaultConfigurationControllerTest.java b/server/src/test/java/com/mirth/connect/server/controllers/DefaultConfigurationControllerTest.java index 6fad4a432..ab37ab752 100644 --- a/server/src/test/java/com/mirth/connect/server/controllers/DefaultConfigurationControllerTest.java +++ b/server/src/test/java/com/mirth/connect/server/controllers/DefaultConfigurationControllerTest.java @@ -310,6 +310,24 @@ public void setConfigurationProperty_KeepsOtherEntries() throws Exception { assertEquals("second", saved.get("B").getComment()); } + @Test + public void setConfigurationProperty_Exception_NullProperty() throws Exception { + AtomicReference database = new AtomicReference(INITIAL_CONFIGURATION_MAP_XML); + DefaultConfigurationController configurationController = databaseBackedConfigurationController(database); + Map before = configurationController.getConfigurationMap(); + String savedBefore = database.get(); + + try { + configurationController.setConfigurationProperty("B", null, null); + fail("Exception should have been thrown"); + } catch (ControllerException e) { + // Expected + } + + assertEquals(savedBefore, database.get()); + assertEquals(before, configurationController.getConfigurationMap()); + } + @Test public void setConfigurationProperty_ExpectedVersionMatches() throws Exception { AtomicReference database = new AtomicReference(INITIAL_CONFIGURATION_MAP_XML); @@ -358,6 +376,44 @@ public void setConfigurationProperty_ExpectedMissing_KeyMissing() throws Excepti assertEquals(expected, configurationController.getConfigurationMap()); } + @Test + public void setConfigurationProperties_Exception_NullEntry() throws Exception { + assertInvalidEntryRejected("B", null); + } + + @Test + public void setConfigurationProperties_Exception_NullKey() throws Exception { + assertInvalidEntryRejected(null, new ConfigurationProperty("2", "second")); + } + + @Test + public void setConfigurationProperties_Exception_NonStringKey() throws Exception { + assertInvalidEntryRejected(5, new ConfigurationProperty("2", "second")); + } + + @Test + public void setConfigurationProperties_Exception_NonConfigurationPropertyValue() throws Exception { + assertInvalidEntryRejected("B", "2"); + } + + @Test + public void setConfigurationProperties_Exception_NullMap() throws Exception { + AtomicReference database = new AtomicReference(INITIAL_CONFIGURATION_MAP_XML); + DefaultConfigurationController configurationController = databaseBackedConfigurationController(database); + Map before = configurationController.getConfigurationMap(); + String savedBefore = database.get(); + + try { + configurationController.setConfigurationProperties(null, true); + fail("Exception should have been thrown"); + } catch (ControllerException e) { + // Expected + } + + assertEquals(savedBefore, database.get()); + assertEquals(before, configurationController.getConfigurationMap()); + } + private void assertConflict(String key, String expectedVersion) throws Exception { AtomicReference database = new AtomicReference(INITIAL_CONFIGURATION_MAP_XML); DefaultConfigurationController configurationController = databaseBackedConfigurationController(database); @@ -370,6 +426,31 @@ private void assertConflict(String key, String expectedVersion) throws Exception assertEquals(before, configurationController.getConfigurationMap()); } + /* + * Request bodies are deserialized without their generic types, so a map can hold entries that + * its declared type doesn't allow. + */ + @SuppressWarnings({ "unchecked", "rawtypes" }) + private void assertInvalidEntryRejected(Object key, Object value) throws Exception { + AtomicReference database = new AtomicReference(INITIAL_CONFIGURATION_MAP_XML); + DefaultConfigurationController configurationController = databaseBackedConfigurationController(database); + Map before = configurationController.getConfigurationMap(); + String savedBefore = database.get(); + + Map map = configurationController.getConfigurationProperties(); + map.put(key, value); + + try { + configurationController.setConfigurationProperties(map, true); + fail("Exception should have been thrown"); + } catch (ControllerException e) { + // Expected + } + + assertEquals(savedBefore, database.get()); + assertEquals(before, configurationController.getConfigurationMap()); + } + private DefaultConfigurationController databaseBackedConfigurationController(AtomicReference database) { DefaultConfigurationController.mirthConfig.setProperty(DefaultConfigurationController.CONFIGURATION_MAP_LOCATION, "database");