From 29d34d1d7c2a5959f124ef1fa238c7b6ca7d792d Mon Sep 17 00:00:00 2001 From: Tony Germano Date: Tue, 27 Oct 2026 23:49:19 -0400 Subject: [PATCH 1/2] Add configuration map validation tests Cover rejecting invalid input in database mode on both the single-entry and whole-map paths: a null map, and a null or wrongly typed key or entry. These fail against the current code. Request bodies are deserialized without their generic types, so a map can hold a null or a value of any type. A null or wrongly typed entry is written to the database before building the in-memory map throws, and on the next startup the configuration map fails to load and then reads as empty. A null or wrongly typed key is persisted the same way; a null key is accepted outright, and the Administrator's configuration map tab sorts keys with a case-insensitive comparator that cannot handle it. Issue: https://github.com/OpenIntegrationEngine/engine/issues/520 Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Tony Germano --- .../DefaultConfigurationControllerTest.java | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) 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"); From cbe54b60da1153738250926108cb3b979b8168be Mon Sep 17 00:00:00 2001 From: Tony Germano Date: Tue, 27 Oct 2026 02:53:07 -0400 Subject: [PATCH 2/2] Validate configuration map entries before saving Request bodies for either configuration map PUT endpoint are deserialized without their generic types, so setConfigurationProperties can receive a null map, or a map whose keys or entries are null or of the wrong type, for example from an XML body of or an entry with an key. These were persisted in database mode before the in-memory update failed, leaving a database row that breaks loading the map on the next startup. Check the raw entries first and throw a ControllerException instead. Fixes #520 Issue: https://github.com/OpenIntegrationEngine/engine/issues/520 Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Tony Germano --- .../DefaultConfigurationController.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) 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();