From fe47f02260e7768dccf0626ab14675eabd2734c3 Mon Sep 17 00:00:00 2001 From: Finnegan's Owner <44065187+pacmano1@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:47:22 -0600 Subject: [PATCH 1/3] Validate metadata column names in message search to prevent SQL injection The message search mappers interpolate client-supplied custom metadata column names into SQL as identifiers (MyBatis ${} substitution) in every dialect, via the metaDataSearch and textSearchMetaDataColumns filters, allowing SQL injection by an authenticated user. Validate each referenced column name against the channel's own defined columns before the filter reaches the data layer, rejecting unknown names. Enforced at the REST boundary (MessageServlet, 400) and backstopped in DonkeyMessageController for callers that reach it directly. The search value and operator were already bound/enum-constrained. Signed-off-by: Finnegan's Owner <44065187+pacmano1@users.noreply.github.com> --- .../server/api/servlets/MessageServlet.java | 39 +++++- .../controllers/DonkeyMessageController.java | 20 ++++ .../server/util/MetaDataColumnValidator.java | 82 +++++++++++++ .../api/servlets/MessageServletTest.java | 107 +++++++++++++++++ .../util/MetaDataColumnValidatorTest.java | 112 ++++++++++++++++++ 5 files changed, 359 insertions(+), 1 deletion(-) create mode 100644 server/src/main/java/com/mirth/connect/server/util/MetaDataColumnValidator.java create mode 100644 server/src/test/java/com/mirth/connect/server/util/MetaDataColumnValidatorTest.java diff --git a/server/src/main/java/com/mirth/connect/server/api/servlets/MessageServlet.java b/server/src/main/java/com/mirth/connect/server/api/servlets/MessageServlet.java index cc98bbe0a9..92316e1033 100644 --- a/server/src/main/java/com/mirth/connect/server/api/servlets/MessageServlet.java +++ b/server/src/main/java/com/mirth/connect/server/api/servlets/MessageServlet.java @@ -56,12 +56,14 @@ import com.mirth.connect.server.api.DontCheckAuthorized; import com.mirth.connect.server.api.MirthServlet; import com.mirth.connect.server.api.providers.ResponseCodeFilter; +import com.mirth.connect.server.controllers.ChannelController; import com.mirth.connect.server.controllers.ConfigurationController; import com.mirth.connect.server.controllers.ControllerFactory; import com.mirth.connect.server.controllers.EngineController; import com.mirth.connect.server.controllers.EventController; import com.mirth.connect.server.controllers.MessageController; import com.mirth.connect.server.util.DICOMMessageUtil; +import com.mirth.connect.server.util.MetaDataColumnValidator; import com.mirth.connect.util.MessageImporter.MessageImportException; import com.mirth.connect.util.messagewriter.EncryptionType; import com.mirth.connect.util.messagewriter.MessageWriterOptions; @@ -73,6 +75,7 @@ public class MessageServlet extends MirthServlet implements MessageServletInterf private static EngineController engineController; private static ConfigurationController configurationController; private static EventController eventController; + private static ChannelController channelController; public MessageServlet(@Context HttpServletRequest request, @Context ContainerRequestContext containerRequestContext, @Context SecurityContext sc) { super(request, containerRequestContext, sc); @@ -89,6 +92,7 @@ protected void initializeControllers() { engineController = controllerFactory.createEngineController(); configurationController = controllerFactory.createConfigurationController(); eventController = controllerFactory.createEventController(); + channelController = controllerFactory.createChannelController(); } @Override @@ -169,6 +173,7 @@ public Long getMaxMessageId(String channelId) { @Override @CheckAuthorizedChannelId public List getMessages(String channelId, MessageFilter filter, Boolean includeContent, Integer offset, Integer limit) { + validateMetaDataColumns(channelId, filter); return messageController.getMessages(filter, channelId, includeContent, offset, limit); } @@ -176,12 +181,14 @@ public List getMessages(String channelId, MessageFilter filter, Boolean @CheckAuthorizedChannelId public List getMessages(String channelId, Long minMessageId, Long maxMessageId, Long minOriginalId, Long maxOriginalId, Long minImportId, Long maxImportId, Calendar startDate, Calendar endDate, String textSearch, Boolean textSearchRegex, Set statuses, Set includedMetaDataIds, Set excludedMetaDataIds, String serverId, Set rawContentSearches, Set processedRawContentSearches, Set transformedContentSearches, Set encodedContentSearches, Set sentContentSearches, Set responseContentSearches, Set responseTransformedContentSearches, Set processedResponseContentSearches, Set connectorMapContentSearches, Set channelMapContentSearches, Set sourceMapContentSearches, Set responseMapContentSearches, Set processingErrorContentSearches, Set postprocessorErrorContentSearches, Set responseErrorContentSearches, Set metaDataSearches, Set metaDataCaseInsensitiveSearches, Set textSearchMetaDataColumns, Integer minSendAttempts, Integer maxSendAttempts, Boolean attachment, Boolean error, Boolean includeContent, Integer offset, Integer limit) { MessageFilter filter = getMessageFilter(minMessageId, maxMessageId, minOriginalId, maxOriginalId, minImportId, maxImportId, startDate, endDate, textSearch, textSearchRegex, statuses, includedMetaDataIds, excludedMetaDataIds, serverId, rawContentSearches, processedRawContentSearches, transformedContentSearches, encodedContentSearches, sentContentSearches, responseContentSearches, responseTransformedContentSearches, processedResponseContentSearches, connectorMapContentSearches, channelMapContentSearches, sourceMapContentSearches, responseMapContentSearches, processingErrorContentSearches, postprocessorErrorContentSearches, responseErrorContentSearches, metaDataSearches, metaDataCaseInsensitiveSearches, textSearchMetaDataColumns, minSendAttempts, maxSendAttempts, attachment, error); + validateMetaDataColumns(channelId, filter); return messageController.getMessages(filter, channelId, includeContent, offset, limit); } @Override @CheckAuthorizedChannelId public Long getMessageCount(String channelId, MessageFilter filter) { + validateMetaDataColumns(channelId, filter); return messageController.getMessageCount(filter, channelId); } @@ -189,12 +196,14 @@ public Long getMessageCount(String channelId, MessageFilter filter) { @CheckAuthorizedChannelId public Long getMessageCount(String channelId, Long minMessageId, Long maxMessageId, Long minOriginalId, Long maxOriginalId, Long minImportId, Long maxImportId, Calendar startDate, Calendar endDate, String textSearch, Boolean textSearchRegex, Set statuses, Set includedMetaDataIds, Set excludedMetaDataIds, String serverId, Set rawContentSearches, Set processedRawContentSearches, Set transformedContentSearches, Set encodedContentSearches, Set sentContentSearches, Set responseContentSearches, Set responseTransformedContentSearches, Set processedResponseContentSearches, Set connectorMapContentSearches, Set channelMapContentSearches, Set sourceMapContentSearches, Set responseMapContentSearches, Set processingErrorContentSearches, Set postprocessorErrorContentSearches, Set responseErrorContentSearches, Set metaDataSearches, Set metaDataCaseInsensitiveSearches, Set textSearchMetaDataColumns, Integer minSendAttempts, Integer maxSendAttempts, Boolean attachment, Boolean error) { MessageFilter filter = getMessageFilter(minMessageId, maxMessageId, minOriginalId, maxOriginalId, minImportId, maxImportId, startDate, endDate, textSearch, textSearchRegex, statuses, includedMetaDataIds, excludedMetaDataIds, serverId, rawContentSearches, processedRawContentSearches, transformedContentSearches, encodedContentSearches, sentContentSearches, responseContentSearches, responseTransformedContentSearches, processedResponseContentSearches, connectorMapContentSearches, channelMapContentSearches, sourceMapContentSearches, responseMapContentSearches, processingErrorContentSearches, postprocessorErrorContentSearches, responseErrorContentSearches, metaDataSearches, metaDataCaseInsensitiveSearches, textSearchMetaDataColumns, minSendAttempts, maxSendAttempts, attachment, error); + validateMetaDataColumns(channelId, filter); return messageController.getMessageCount(filter, channelId); } @Override @CheckAuthorizedChannelId public void reprocessMessages(String channelId, MessageFilter filter, boolean replace, boolean filterDestinations, Set reprocessMetaDataIds) { + validateMetaDataColumns(channelId, filter); doReprocessMessages(channelId, filter, replace, filterDestinations, reprocessMetaDataIds); } @@ -202,6 +211,7 @@ public void reprocessMessages(String channelId, MessageFilter filter, boolean re @CheckAuthorizedChannelId public void reprocessMessages(String channelId, Long minMessageId, Long maxMessageId, Long minOriginalId, Long maxOriginalId, Long minImportId, Long maxImportId, Calendar startDate, Calendar endDate, String textSearch, Boolean textSearchRegex, Set statuses, Set includedMetaDataIds, Set excludedMetaDataIds, String serverId, Set rawContentSearches, Set processedRawContentSearches, Set transformedContentSearches, Set encodedContentSearches, Set sentContentSearches, Set responseContentSearches, Set responseTransformedContentSearches, Set processedResponseContentSearches, Set connectorMapContentSearches, Set channelMapContentSearches, Set sourceMapContentSearches, Set responseMapContentSearches, Set processingErrorContentSearches, Set postprocessorErrorContentSearches, Set responseErrorContentSearches, Set metaDataSearches, Set metaDataCaseInsensitiveSearches, Set textSearchMetaDataColumns, Integer minSendAttempts, Integer maxSendAttempts, Boolean attachment, Boolean error, boolean replace, boolean filterDestinations, Set reprocessMetaDataIds) { final MessageFilter filter = getMessageFilter(minMessageId, maxMessageId, minOriginalId, maxOriginalId, minImportId, maxImportId, startDate, endDate, textSearch, textSearchRegex, statuses, includedMetaDataIds, excludedMetaDataIds, serverId, rawContentSearches, processedRawContentSearches, transformedContentSearches, encodedContentSearches, sentContentSearches, responseContentSearches, responseTransformedContentSearches, processedResponseContentSearches, connectorMapContentSearches, channelMapContentSearches, sourceMapContentSearches, responseMapContentSearches, processingErrorContentSearches, postprocessorErrorContentSearches, responseErrorContentSearches, metaDataSearches, metaDataCaseInsensitiveSearches, textSearchMetaDataColumns, minSendAttempts, maxSendAttempts, attachment, error); + validateMetaDataColumns(channelId, filter); doReprocessMessages(channelId, filter, replace, filterDestinations, reprocessMetaDataIds); } @@ -235,6 +245,7 @@ public void run() { @Override @CheckAuthorizedChannelId public void removeMessages(String channelId, MessageFilter filter) { + validateMetaDataColumns(channelId, filter); messageController.removeMessages(channelId, filter); } @@ -242,6 +253,7 @@ public void removeMessages(String channelId, MessageFilter filter) { @CheckAuthorizedChannelId public void removeMessages(String channelId, Long minMessageId, Long maxMessageId, Long minOriginalId, Long maxOriginalId, Long minImportId, Long maxImportId, Calendar startDate, Calendar endDate, String textSearch, Boolean textSearchRegex, Set statuses, Set includedMetaDataIds, Set excludedMetaDataIds, String serverId, Set rawContentSearches, Set processedRawContentSearches, Set transformedContentSearches, Set encodedContentSearches, Set sentContentSearches, Set responseContentSearches, Set responseTransformedContentSearches, Set processedResponseContentSearches, Set connectorMapContentSearches, Set channelMapContentSearches, Set sourceMapContentSearches, Set responseMapContentSearches, Set processingErrorContentSearches, Set postprocessorErrorContentSearches, Set responseErrorContentSearches, Set metaDataSearches, Set metaDataCaseInsensitiveSearches, Set textSearchMetaDataColumns, Integer minSendAttempts, Integer maxSendAttempts, Boolean attachment, Boolean error) { MessageFilter filter = getMessageFilter(minMessageId, maxMessageId, minOriginalId, maxOriginalId, minImportId, maxImportId, startDate, endDate, textSearch, textSearchRegex, statuses, includedMetaDataIds, excludedMetaDataIds, serverId, rawContentSearches, processedRawContentSearches, transformedContentSearches, encodedContentSearches, sentContentSearches, responseContentSearches, responseTransformedContentSearches, processedResponseContentSearches, connectorMapContentSearches, channelMapContentSearches, sourceMapContentSearches, responseMapContentSearches, processingErrorContentSearches, postprocessorErrorContentSearches, responseErrorContentSearches, metaDataSearches, metaDataCaseInsensitiveSearches, textSearchMetaDataColumns, minSendAttempts, maxSendAttempts, attachment, error); + validateMetaDataColumns(channelId, filter); messageController.removeMessages(channelId, filter); } @@ -316,6 +328,7 @@ public MessageImportResult importMessagesServer(String channelId, String path, b @Override @CheckAuthorizedChannelId public int exportMessagesServer(String channelId, MessageFilter filter, int pageSize, MessageWriterOptions writerOptions) { + validateMetaDataColumns(channelId, filter); try { return messageController.exportMessages(channelId, filter, pageSize, writerOptions); } catch (Exception e) { @@ -328,6 +341,7 @@ public int exportMessagesServer(String channelId, MessageFilter filter, int page public int exportMessagesServer(String channelId, Long minMessageId, Long maxMessageId, Long minOriginalId, Long maxOriginalId, Long minImportId, Long maxImportId, Calendar startDate, Calendar endDate, String textSearch, Boolean textSearchRegex, Set statuses, Set includedMetaDataIds, Set excludedMetaDataIds, String serverId, Set rawContentSearches, Set processedRawContentSearches, Set transformedContentSearches, Set encodedContentSearches, Set sentContentSearches, Set responseContentSearches, Set responseTransformedContentSearches, Set processedResponseContentSearches, Set connectorMapContentSearches, Set channelMapContentSearches, Set sourceMapContentSearches, Set responseMapContentSearches, Set processingErrorContentSearches, Set postprocessorErrorContentSearches, Set responseErrorContentSearches, Set metaDataSearches, Set metaDataCaseInsensitiveSearches, Set textSearchMetaDataColumns, Integer minSendAttempts, Integer maxSendAttempts, Boolean attachment, Boolean error, int pageSize, ContentType contentType, boolean destinationContent, boolean encrypt, boolean includeAttachments, String baseFolder, String rootFolder, String filePattern, String archiveFileName, String archiveFormat, String compressFormat, String password, EncryptionType encryptionType) { MessageFilter filter = getMessageFilter(minMessageId, maxMessageId, minOriginalId, maxOriginalId, minImportId, maxImportId, startDate, endDate, textSearch, textSearchRegex, statuses, includedMetaDataIds, excludedMetaDataIds, serverId, rawContentSearches, processedRawContentSearches, transformedContentSearches, encodedContentSearches, sentContentSearches, responseContentSearches, responseTransformedContentSearches, processedResponseContentSearches, connectorMapContentSearches, channelMapContentSearches, sourceMapContentSearches, responseMapContentSearches, processingErrorContentSearches, postprocessorErrorContentSearches, responseErrorContentSearches, metaDataSearches, metaDataCaseInsensitiveSearches, textSearchMetaDataColumns, minSendAttempts, maxSendAttempts, attachment, error); MessageWriterOptions writerOptions = getMessageWriterOptions(contentType, destinationContent, encrypt, includeAttachments, baseFolder, rootFolder, filePattern, archiveFileName, archiveFormat, compressFormat, password, encryptionType); + validateMetaDataColumns(channelId, filter); try { return messageController.exportMessages(channelId, filter, pageSize, writerOptions); } catch (Exception e) { @@ -387,7 +401,30 @@ private void sendServerEventWithAttributes(Map attributes) { eventController.dispatchEvent(event); } - + + /** + * Validates that every custom metadata column referenced by the filter matches a column actually + * defined on the channel. The message search mappers splice these column names directly into the + * SQL as identifiers (MyBatis ${} substitution), so an unvalidated name is a SQL injection vector. + * Rejecting anything not in the channel's own column set closes that hole; the search value and + * operator are already safe (bound parameter and enum-constrained, respectively). + * + * This runs at the servlet (API) boundary because both the parameter-based and MessageFilter-based + * endpoints for each operation accept these column names from the client. + */ + private void validateMetaDataColumns(String channelId, MessageFilter filter) { + // Primary check at the REST boundary: reject any custom metadata column the filter references + // that is not defined on the channel. Column names are matched exactly against the channel's + // (upper-cased) columns, which come from the in-memory channel cache (no database hit) and are + // only looked up when the filter actually references a custom column. Callers that reach + // DonkeyMessageController directly (e.g. a plugin) are covered by a last-resort backstop there. + String unknownColumn = MetaDataColumnValidator.findUnknownColumn(filter, () -> channelController.getMetaDataColumns(channelId)); + if (unknownColumn != null) { + logger.warn("Rejected message search for channel " + channelId + " referencing unknown metadata column: " + unknownColumn); + throw new MirthApiException(Response.Status.BAD_REQUEST); + } + } + private MessageFilter getMessageFilter(Long minMessageId, Long maxMessageId, Long minOriginalId, Long maxOriginalId, Long minImportId, Long maxImportId, Calendar startDate, Calendar endDate, String textSearch, Boolean textSearchRegex, Set statuses, Set includedMetaDataIds, Set excludedMetaDataIds, String serverId, Set rawContentSearches, Set processedRawContentSearches, Set transformedContentSearches, Set encodedContentSearches, Set sentContentSearches, Set responseContentSearches, Set responseTransformedContentSearches, Set processedResponseContentSearches, Set connectorMapContentSearches, Set channelMapContentSearches, Set sourceMapContentSearches, Set responseMapContentSearches, Set processingErrorContentSearches, Set postprocessorErrorContentSearches, Set responseErrorContentSearches, Set metaDataSearches, Set metaDataCaseInsensitiveSearches, Set textSearchMetaDataColumns, Integer minSendAttempts, Integer maxSendAttempts, Boolean attachment, Boolean error) { MessageFilter filter = new MessageFilter(); filter.setMinMessageId(minMessageId); diff --git a/server/src/main/java/com/mirth/connect/server/controllers/DonkeyMessageController.java b/server/src/main/java/com/mirth/connect/server/controllers/DonkeyMessageController.java index 9a4366c844..df657ba7f2 100644 --- a/server/src/main/java/com/mirth/connect/server/controllers/DonkeyMessageController.java +++ b/server/src/main/java/com/mirth/connect/server/controllers/DonkeyMessageController.java @@ -63,6 +63,7 @@ import com.mirth.connect.server.util.DICOMMessageUtil; import com.mirth.connect.server.util.ListRangeIterator; import com.mirth.connect.server.util.ListRangeIterator.ListRangeItem; +import com.mirth.connect.server.util.MetaDataColumnValidator; import com.mirth.connect.server.util.SqlConfig; import com.mirth.connect.util.AttachmentUtil; import com.mirth.connect.util.MessageEncryptionUtil; @@ -147,6 +148,7 @@ public long getMinMessageId(String channelId, boolean readOnly) { @Override public Long getMessageCount(MessageFilter filter, String channelId) { + validateMetaDataColumns(channelId, filter); if (filter.getIncludedMetaDataIds() != null && filter.getIncludedMetaDataIds().isEmpty() && filter.getExcludedMetaDataIds() == null) { return 0L; } @@ -190,6 +192,7 @@ public Long getMessageCount(MessageFilter filter, String channelId) { @Override public List getMessages(MessageFilter filter, String channelId, Boolean includeContent, Integer offset, Integer limit) { + validateMetaDataColumns(channelId, filter); // Provide a default value if any of the below 3 parameters are null. includeContent = includeContent == null ? false : includeContent; offset = offset == null ? 0 : offset; @@ -295,6 +298,7 @@ public List getMessageAttachment(String channelId, Long messageId, b @Override public void removeMessages(String channelId, MessageFilter filter) { + validateMetaDataColumns(channelId, filter); EngineController engineController = ControllerFactory.getFactory().createEngineController(); FilterOptions filterOptions = new FilterOptions(filter, channelId, false); @@ -341,6 +345,7 @@ public void removeMessages(String channelId, MessageFilter filter) { } public void reprocessMessages(String channelId, MessageFilter filter, boolean replace, Collection reprocessMetaDataIds) throws ControllerException { + validateMetaDataColumns(channelId, filter); EngineController engineController = ControllerFactory.getFactory().createEngineController(); Channel deployedChannel = engineController.getDeployedChannel(channelId); if (deployedChannel == null) { @@ -676,6 +681,21 @@ private List searchMessages(MessageFilter filter, String ch } } + /** + * Last-resort backstop against SQL injection through custom metadata column names. The primary + * check runs at the REST boundary (MessageServlet), but a caller that reaches this controller + * directly - notably an external plugin using MessageController.getInstance() - bypasses it. The + * search mappers interpolate these column names into SQL as identifiers, so an undefined column + * must never reach them. This throws (rather than returning a 400) because reaching it means a + * caller skipped the proper gate, which is a programming error. + */ + private void validateMetaDataColumns(String channelId, MessageFilter filter) { + String unknownColumn = MetaDataColumnValidator.findUnknownColumn(filter, () -> ControllerFactory.getFactory().createChannelController().getMetaDataColumns(channelId)); + if (unknownColumn != null) { + throw new IllegalArgumentException("Message search referenced a metadata column that is not defined on channel " + channelId + ": " + unknownColumn); + } + } + private Map searchAll(SqlSession session, Map params, MessageFilter filter, Long localChannelId, boolean includeMessageData, FilterOptions filterOptions) { Map foundMessages = new HashMap(); diff --git a/server/src/main/java/com/mirth/connect/server/util/MetaDataColumnValidator.java b/server/src/main/java/com/mirth/connect/server/util/MetaDataColumnValidator.java new file mode 100644 index 0000000000..7b3ac78645 --- /dev/null +++ b/server/src/main/java/com/mirth/connect/server/util/MetaDataColumnValidator.java @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: Open Integration Engine + +package com.mirth.connect.server.util; + +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.function.Supplier; + +import org.apache.commons.collections4.CollectionUtils; + +import com.mirth.connect.donkey.model.channel.MetaDataColumn; +import com.mirth.connect.model.filters.MessageFilter; +import com.mirth.connect.model.filters.elements.MetaDataSearchElement; + +/** + * Validates the custom metadata column names referenced by a message search filter against the + * columns actually defined on a channel. The message search mappers interpolate these names into SQL + * as identifiers (MyBatis ${} substitution), so an unvalidated name is a SQL injection vector. + * + *

+ * This is a pure check: it never throws and never looks anything up. Callers pass in the channel's + * defined columns and decide what to do with an unknown column - the REST layer returns 400, the + * controller layer throws as a last-resort backstop for callers that bypass the REST layer. + *

+ */ +public final class MetaDataColumnValidator { + + private MetaDataColumnValidator() {} + + /** + * Returns the first metadata column name referenced by the filter that is not defined on the + * channel, or {@code null} if every referenced column is valid. Column names are matched exactly + * against the channel's (upper-cased) column names; a {@code null} referenced name is treated as + * unknown and returned as the string {@code "null"} so the result stays unambiguous. + * + *

+ * The defined columns are supplied lazily and are only requested when the filter actually + * references a custom column, so a search that uses none costs no channel lookup. + *

+ */ + public static String findUnknownColumn(MessageFilter filter, Supplier> definedColumnsSupplier) { + if (filter == null) { + return null; + } + + boolean hasMetaDataSearch = CollectionUtils.isNotEmpty(filter.getMetaDataSearch()); + boolean hasTextSearchColumns = CollectionUtils.isNotEmpty(filter.getTextSearchMetaDataColumns()); + if (!hasMetaDataSearch && !hasTextSearchColumns) { + return null; + } + + List definedColumns = definedColumnsSupplier.get(); + Set allowedColumns = new HashSet(); + if (definedColumns != null) { + for (MetaDataColumn column : definedColumns) { + if (column.getName() != null) { + allowedColumns.add(column.getName()); + } + } + } + + if (hasMetaDataSearch) { + for (MetaDataSearchElement element : filter.getMetaDataSearch()) { + if (element.getColumnName() == null || !allowedColumns.contains(element.getColumnName())) { + return String.valueOf(element.getColumnName()); + } + } + } + + if (hasTextSearchColumns) { + for (String columnName : filter.getTextSearchMetaDataColumns()) { + if (columnName == null || !allowedColumns.contains(columnName)) { + return String.valueOf(columnName); + } + } + } + + return null; + } +} diff --git a/server/src/test/java/com/mirth/connect/server/api/servlets/MessageServletTest.java b/server/src/test/java/com/mirth/connect/server/api/servlets/MessageServletTest.java index 5dce80294a..5bec3bb471 100644 --- a/server/src/test/java/com/mirth/connect/server/api/servlets/MessageServletTest.java +++ b/server/src/test/java/com/mirth/connect/server/api/servlets/MessageServletTest.java @@ -11,6 +11,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; @@ -24,6 +25,8 @@ import java.io.InputStream; import java.net.URI; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Date; import java.util.HashMap; @@ -51,16 +54,22 @@ import com.mirth.connect.client.core.Operation; import com.mirth.connect.client.core.api.MirthApiException; +import com.mirth.connect.donkey.model.channel.MetaDataColumn; +import com.mirth.connect.donkey.model.channel.MetaDataColumnType; import com.mirth.connect.donkey.server.channel.ChannelException; import com.mirth.connect.donkey.server.channel.DispatchResult; import com.mirth.connect.donkey.server.message.batch.BatchMessageException; import com.mirth.connect.model.LoginStatus; import com.mirth.connect.model.LoginStatus.Status; import com.mirth.connect.model.User; +import com.mirth.connect.model.filters.MessageFilter; +import com.mirth.connect.model.filters.elements.MetaDataSearchElement; import com.mirth.connect.server.api.providers.ResponseCodeFilter; import com.mirth.connect.server.controllers.AuthorizationController; +import com.mirth.connect.server.controllers.ChannelController; import com.mirth.connect.server.controllers.ControllerFactory; import com.mirth.connect.server.controllers.EngineController; +import com.mirth.connect.server.controllers.MessageController; import com.mirth.connect.server.controllers.UserController; public class MessageServletTest { @@ -69,6 +78,8 @@ public class MessageServletTest { static ControllerFactory controllerFactory; static EngineController engineController; + static MessageController messageController; + static ChannelController channelController; static HttpSession session; static HttpServletRequest request; static ContainerRequestContext context; @@ -88,6 +99,12 @@ public void setup() throws Exception { when(engineController.dispatchRawMessage(eq("batchMessageException"), any(), anyBoolean(), anyBoolean())).thenThrow(new BatchMessageException()); when(controllerFactory.createEngineController()).thenReturn(engineController); + messageController = mock(MessageController.class); + when(controllerFactory.createMessageController()).thenReturn(messageController); + + channelController = mock(ChannelController.class); + when(controllerFactory.createChannelController()).thenReturn(channelController); + UserController userController = mock(UserController.class); when(userController.authorizeUser(anyString(), anyString(), anyString())).thenReturn(new LoginStatus(Status.SUCCESS, "")); when(userController.getUser(anyInt(), anyString())).thenAnswer((InvocationOnMock invocation) -> { @@ -203,6 +220,96 @@ public void testRestrictedUserCanRemoveMessagesWithoutClearingStats() { verify(engineController, times(2)).removeAllMessages(any(), anyBoolean(), anyBoolean(), any()); } + @Test + public void testMetaDataSearchRejectsUnknownColumn() { + when(channelController.getMetaDataColumns("channel1")).thenReturn(definedColumns("STATUS")); + MessageServlet servlet = new MessageServlet(request, context, sc, controllerFactory); + + MessageFilter filter = new MessageFilter(); + filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("STATUS\" = '' OR '1'='1' --", "EQUAL", "x", false))); + + try { + servlet.getMessageCount("channel1", filter); + fail("Expected MirthApiException for an unknown metadata column"); + } catch (MirthApiException e) { + assertEquals(400, e.getResponse().getStatus()); + } + verify(messageController, times(0)).getMessageCount(any(), anyString()); + } + + @Test + public void testTextSearchRejectsUnknownColumn() { + when(channelController.getMetaDataColumns("channel1")).thenReturn(definedColumns("STATUS")); + MessageServlet servlet = new MessageServlet(request, context, sc, controllerFactory); + + MessageFilter filter = new MessageFilter(); + filter.setTextSearchMetaDataColumns(new ArrayList(Arrays.asList("BOGUS"))); + + try { + servlet.getMessageCount("channel1", filter); + fail("Expected MirthApiException for an unknown text-search metadata column"); + } catch (MirthApiException e) { + assertEquals(400, e.getResponse().getStatus()); + } + verify(messageController, times(0)).getMessageCount(any(), anyString()); + } + + @Test + public void testMetaDataSearchAllowsDefinedColumn() { + when(channelController.getMetaDataColumns("channel1")).thenReturn(definedColumns("STATUS")); + MessageServlet servlet = new MessageServlet(request, context, sc, controllerFactory); + + MessageFilter filter = new MessageFilter(); + filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("STATUS", "EQUAL", "x", false))); + + servlet.getMessageCount("channel1", filter); + verify(messageController, times(1)).getMessageCount(filter, "channel1"); + } + + @Test + public void testMetaDataSearchRejectsNonUpperCaseColumn() { + when(channelController.getMetaDataColumns("channel1")).thenReturn(definedColumns("STATUS")); + MessageServlet servlet = new MessageServlet(request, context, sc, controllerFactory); + + MessageFilter filter = new MessageFilter(); + // Column names are matched exactly; a non-upper-case name would not match the actual DB + // column either, so it is rejected rather than silently returning nothing. + filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("status", "EQUAL", "x", false))); + + try { + servlet.getMessageCount("channel1", filter); + fail("Expected MirthApiException for a non-upper-case metadata column"); + } catch (MirthApiException e) { + assertEquals(400, e.getResponse().getStatus()); + } + verify(messageController, times(0)).getMessageCount(any(), anyString()); + } + + @Test + public void testRejectsSearchWhenChannelHasNoColumns() { + when(channelController.getMetaDataColumns("channel1")).thenReturn(null); + MessageServlet servlet = new MessageServlet(request, context, sc, controllerFactory); + + MessageFilter filter = new MessageFilter(); + filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("STATUS", "EQUAL", "x", false))); + + try { + servlet.getMessageCount("channel1", filter); + fail("Expected MirthApiException when the channel has no defined metadata columns"); + } catch (MirthApiException e) { + assertEquals(400, e.getResponse().getStatus()); + } + verify(messageController, times(0)).getMessageCount(any(), anyString()); + } + + private static List definedColumns(String... names) { + List columns = new ArrayList(); + for (String name : names) { + columns.add(new MetaDataColumn(name, MetaDataColumnType.STRING, null)); + } + return columns; + } + private static void setupSessionAndRequest(int userId) { session = mock(HttpSession.class); when(session.getAttribute("user")).thenReturn("" + userId); diff --git a/server/src/test/java/com/mirth/connect/server/util/MetaDataColumnValidatorTest.java b/server/src/test/java/com/mirth/connect/server/util/MetaDataColumnValidatorTest.java new file mode 100644 index 0000000000..95ef207c43 --- /dev/null +++ b/server/src/test/java/com/mirth/connect/server/util/MetaDataColumnValidatorTest.java @@ -0,0 +1,112 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: Open Integration Engine + +package com.mirth.connect.server.util; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Supplier; + +import org.junit.Test; + +import com.mirth.connect.donkey.model.channel.MetaDataColumn; +import com.mirth.connect.donkey.model.channel.MetaDataColumnType; +import com.mirth.connect.model.filters.MessageFilter; +import com.mirth.connect.model.filters.elements.MetaDataSearchElement; + +public class MetaDataColumnValidatorTest { + + private static List definedColumns(String... names) { + List columns = new ArrayList(); + for (String name : names) { + columns.add(new MetaDataColumn(name, MetaDataColumnType.STRING, null)); + } + return columns; + } + + private static Supplier> supplier(List columns, AtomicBoolean invoked) { + return () -> { + invoked.set(true); + return columns; + }; + } + + @Test + public void nullFilterReturnsNull() { + assertNull(MetaDataColumnValidator.findUnknownColumn(null, () -> definedColumns("STATUS"))); + } + + @Test + public void noReferencedColumnsReturnsNullAndSkipsLookup() { + AtomicBoolean invoked = new AtomicBoolean(false); + MessageFilter filter = new MessageFilter(); + + assertNull(MetaDataColumnValidator.findUnknownColumn(filter, supplier(definedColumns("STATUS"), invoked))); + assertFalse("Channel columns must not be looked up when the filter references none", invoked.get()); + } + + @Test + public void definedMetaDataSearchColumnReturnsNull() { + AtomicBoolean invoked = new AtomicBoolean(false); + MessageFilter filter = new MessageFilter(); + filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("STATUS", "EQUAL", "x", false))); + + assertNull(MetaDataColumnValidator.findUnknownColumn(filter, supplier(definedColumns("STATUS"), invoked))); + assertTrue("A referenced column must trigger the lookup", invoked.get()); + } + + @Test + public void unknownMetaDataSearchColumnIsReturned() { + MessageFilter filter = new MessageFilter(); + filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("EVIL\" OR '1'='1", "EQUAL", "x", false))); + + assertEquals("EVIL\" OR '1'='1", MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); + } + + @Test + public void nonUpperCaseColumnIsReturned() { + MessageFilter filter = new MessageFilter(); + filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("status", "EQUAL", "x", false))); + + assertEquals("status", MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); + } + + @Test + public void nullColumnNameIsReturnedAsNullString() { + MessageFilter filter = new MessageFilter(); + filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement(null, "EQUAL", "x", false))); + + assertEquals("null", MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); + } + + @Test + public void definedTextSearchColumnReturnsNull() { + MessageFilter filter = new MessageFilter(); + filter.setTextSearchMetaDataColumns(new ArrayList(Arrays.asList("STATUS"))); + + assertNull(MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); + } + + @Test + public void unknownTextSearchColumnIsReturned() { + MessageFilter filter = new MessageFilter(); + filter.setTextSearchMetaDataColumns(new ArrayList(Arrays.asList("BOGUS"))); + + assertEquals("BOGUS", MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); + } + + @Test + public void channelWithNoColumnsRejectsAnyReferencedColumn() { + MessageFilter filter = new MessageFilter(); + filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("STATUS", "EQUAL", "x", false))); + + assertEquals("STATUS", MetaDataColumnValidator.findUnknownColumn(filter, () -> null)); + } +} From 6bf1fc322c29f631e69d0b061ed86258a266902a Mon Sep 17 00:00:00 2001 From: Finnegan's Owner <44065187+pacmano1@users.noreply.github.com> Date: Fri, 24 Jul 2026 10:03:43 -0600 Subject: [PATCH 2/3] Consolidate metadata column validation to a single gate Addresses review feedback on the two-layer approach. Validation now runs once from the FilterOptions constructor, which every message search path (get, count, remove, reprocess; export runs through get) builds exactly once before its batch loop. That is the single choke point a search must pass through, so a new search entry point cannot bypass the check, and it covers direct controller callers such as plugins, not just the REST layer. It does not belong in searchAll: searchAll sits inside the batch loop, which is skipped when no message id range matches (an empty channel, or a min/max filter that excludes everything), so an undefined column could pass unchecked there. FilterOptions is built regardless, so the check runs even then. MetaDataColumnException is a plain domain exception, left to bubble up. On the synchronous API paths the engine turns it into a 500 like any other uncaught controller exception; it is deliberately not mapped to a 400, because the engine has no central exception-to-status convention (every specific status is thrown explicitly at a servlet) and coupling this data-layer type to an HTTP status would be the only such case in the tree. The injection is blocked regardless of status, since the throw happens before any query runs. Reprocess is asynchronous: the throw aborts the run on the background thread and is logged rather than surfaced. The exception message is generic so the untrusted column name is not reflected back in the 500 body; the channel and column are kept as fields for parameterized logging on the reprocess path. Also guards null metadata-search elements and null defined-column entries, which would otherwise NPE in the validator. Signed-off-by: Finnegan's Owner <44065187+pacmano1@users.noreply.github.com> --- .../server/api/servlets/MessageServlet.java | 43 +------ .../controllers/DonkeyMessageController.java | 24 ++-- .../server/util/MetaDataColumnException.java | 46 ++++++++ .../server/util/MetaDataColumnValidator.java | 5 +- .../api/servlets/MessageServletTest.java | 107 ------------------ .../util/MetaDataColumnExceptionTest.java | 31 +++++ .../util/MetaDataColumnValidatorTest.java | 24 ++++ 7 files changed, 124 insertions(+), 156 deletions(-) create mode 100644 server/src/main/java/com/mirth/connect/server/util/MetaDataColumnException.java create mode 100644 server/src/test/java/com/mirth/connect/server/util/MetaDataColumnExceptionTest.java diff --git a/server/src/main/java/com/mirth/connect/server/api/servlets/MessageServlet.java b/server/src/main/java/com/mirth/connect/server/api/servlets/MessageServlet.java index 92316e1033..9cecece5e9 100644 --- a/server/src/main/java/com/mirth/connect/server/api/servlets/MessageServlet.java +++ b/server/src/main/java/com/mirth/connect/server/api/servlets/MessageServlet.java @@ -56,14 +56,13 @@ import com.mirth.connect.server.api.DontCheckAuthorized; import com.mirth.connect.server.api.MirthServlet; import com.mirth.connect.server.api.providers.ResponseCodeFilter; -import com.mirth.connect.server.controllers.ChannelController; import com.mirth.connect.server.controllers.ConfigurationController; import com.mirth.connect.server.controllers.ControllerFactory; import com.mirth.connect.server.controllers.EngineController; import com.mirth.connect.server.controllers.EventController; import com.mirth.connect.server.controllers.MessageController; import com.mirth.connect.server.util.DICOMMessageUtil; -import com.mirth.connect.server.util.MetaDataColumnValidator; +import com.mirth.connect.server.util.MetaDataColumnException; import com.mirth.connect.util.MessageImporter.MessageImportException; import com.mirth.connect.util.messagewriter.EncryptionType; import com.mirth.connect.util.messagewriter.MessageWriterOptions; @@ -75,7 +74,6 @@ public class MessageServlet extends MirthServlet implements MessageServletInterf private static EngineController engineController; private static ConfigurationController configurationController; private static EventController eventController; - private static ChannelController channelController; public MessageServlet(@Context HttpServletRequest request, @Context ContainerRequestContext containerRequestContext, @Context SecurityContext sc) { super(request, containerRequestContext, sc); @@ -92,7 +90,6 @@ protected void initializeControllers() { engineController = controllerFactory.createEngineController(); configurationController = controllerFactory.createConfigurationController(); eventController = controllerFactory.createEventController(); - channelController = controllerFactory.createChannelController(); } @Override @@ -173,7 +170,6 @@ public Long getMaxMessageId(String channelId) { @Override @CheckAuthorizedChannelId public List getMessages(String channelId, MessageFilter filter, Boolean includeContent, Integer offset, Integer limit) { - validateMetaDataColumns(channelId, filter); return messageController.getMessages(filter, channelId, includeContent, offset, limit); } @@ -181,14 +177,12 @@ public List getMessages(String channelId, MessageFilter filter, Boolean @CheckAuthorizedChannelId public List getMessages(String channelId, Long minMessageId, Long maxMessageId, Long minOriginalId, Long maxOriginalId, Long minImportId, Long maxImportId, Calendar startDate, Calendar endDate, String textSearch, Boolean textSearchRegex, Set statuses, Set includedMetaDataIds, Set excludedMetaDataIds, String serverId, Set rawContentSearches, Set processedRawContentSearches, Set transformedContentSearches, Set encodedContentSearches, Set sentContentSearches, Set responseContentSearches, Set responseTransformedContentSearches, Set processedResponseContentSearches, Set connectorMapContentSearches, Set channelMapContentSearches, Set sourceMapContentSearches, Set responseMapContentSearches, Set processingErrorContentSearches, Set postprocessorErrorContentSearches, Set responseErrorContentSearches, Set metaDataSearches, Set metaDataCaseInsensitiveSearches, Set textSearchMetaDataColumns, Integer minSendAttempts, Integer maxSendAttempts, Boolean attachment, Boolean error, Boolean includeContent, Integer offset, Integer limit) { MessageFilter filter = getMessageFilter(minMessageId, maxMessageId, minOriginalId, maxOriginalId, minImportId, maxImportId, startDate, endDate, textSearch, textSearchRegex, statuses, includedMetaDataIds, excludedMetaDataIds, serverId, rawContentSearches, processedRawContentSearches, transformedContentSearches, encodedContentSearches, sentContentSearches, responseContentSearches, responseTransformedContentSearches, processedResponseContentSearches, connectorMapContentSearches, channelMapContentSearches, sourceMapContentSearches, responseMapContentSearches, processingErrorContentSearches, postprocessorErrorContentSearches, responseErrorContentSearches, metaDataSearches, metaDataCaseInsensitiveSearches, textSearchMetaDataColumns, minSendAttempts, maxSendAttempts, attachment, error); - validateMetaDataColumns(channelId, filter); return messageController.getMessages(filter, channelId, includeContent, offset, limit); } @Override @CheckAuthorizedChannelId public Long getMessageCount(String channelId, MessageFilter filter) { - validateMetaDataColumns(channelId, filter); return messageController.getMessageCount(filter, channelId); } @@ -196,14 +190,12 @@ public Long getMessageCount(String channelId, MessageFilter filter) { @CheckAuthorizedChannelId public Long getMessageCount(String channelId, Long minMessageId, Long maxMessageId, Long minOriginalId, Long maxOriginalId, Long minImportId, Long maxImportId, Calendar startDate, Calendar endDate, String textSearch, Boolean textSearchRegex, Set statuses, Set includedMetaDataIds, Set excludedMetaDataIds, String serverId, Set rawContentSearches, Set processedRawContentSearches, Set transformedContentSearches, Set encodedContentSearches, Set sentContentSearches, Set responseContentSearches, Set responseTransformedContentSearches, Set processedResponseContentSearches, Set connectorMapContentSearches, Set channelMapContentSearches, Set sourceMapContentSearches, Set responseMapContentSearches, Set processingErrorContentSearches, Set postprocessorErrorContentSearches, Set responseErrorContentSearches, Set metaDataSearches, Set metaDataCaseInsensitiveSearches, Set textSearchMetaDataColumns, Integer minSendAttempts, Integer maxSendAttempts, Boolean attachment, Boolean error) { MessageFilter filter = getMessageFilter(minMessageId, maxMessageId, minOriginalId, maxOriginalId, minImportId, maxImportId, startDate, endDate, textSearch, textSearchRegex, statuses, includedMetaDataIds, excludedMetaDataIds, serverId, rawContentSearches, processedRawContentSearches, transformedContentSearches, encodedContentSearches, sentContentSearches, responseContentSearches, responseTransformedContentSearches, processedResponseContentSearches, connectorMapContentSearches, channelMapContentSearches, sourceMapContentSearches, responseMapContentSearches, processingErrorContentSearches, postprocessorErrorContentSearches, responseErrorContentSearches, metaDataSearches, metaDataCaseInsensitiveSearches, textSearchMetaDataColumns, minSendAttempts, maxSendAttempts, attachment, error); - validateMetaDataColumns(channelId, filter); return messageController.getMessageCount(filter, channelId); } @Override @CheckAuthorizedChannelId public void reprocessMessages(String channelId, MessageFilter filter, boolean replace, boolean filterDestinations, Set reprocessMetaDataIds) { - validateMetaDataColumns(channelId, filter); doReprocessMessages(channelId, filter, replace, filterDestinations, reprocessMetaDataIds); } @@ -211,7 +203,6 @@ public void reprocessMessages(String channelId, MessageFilter filter, boolean re @CheckAuthorizedChannelId public void reprocessMessages(String channelId, Long minMessageId, Long maxMessageId, Long minOriginalId, Long maxOriginalId, Long minImportId, Long maxImportId, Calendar startDate, Calendar endDate, String textSearch, Boolean textSearchRegex, Set statuses, Set includedMetaDataIds, Set excludedMetaDataIds, String serverId, Set rawContentSearches, Set processedRawContentSearches, Set transformedContentSearches, Set encodedContentSearches, Set sentContentSearches, Set responseContentSearches, Set responseTransformedContentSearches, Set processedResponseContentSearches, Set connectorMapContentSearches, Set channelMapContentSearches, Set sourceMapContentSearches, Set responseMapContentSearches, Set processingErrorContentSearches, Set postprocessorErrorContentSearches, Set responseErrorContentSearches, Set metaDataSearches, Set metaDataCaseInsensitiveSearches, Set textSearchMetaDataColumns, Integer minSendAttempts, Integer maxSendAttempts, Boolean attachment, Boolean error, boolean replace, boolean filterDestinations, Set reprocessMetaDataIds) { final MessageFilter filter = getMessageFilter(minMessageId, maxMessageId, minOriginalId, maxOriginalId, minImportId, maxImportId, startDate, endDate, textSearch, textSearchRegex, statuses, includedMetaDataIds, excludedMetaDataIds, serverId, rawContentSearches, processedRawContentSearches, transformedContentSearches, encodedContentSearches, sentContentSearches, responseContentSearches, responseTransformedContentSearches, processedResponseContentSearches, connectorMapContentSearches, channelMapContentSearches, sourceMapContentSearches, responseMapContentSearches, processingErrorContentSearches, postprocessorErrorContentSearches, responseErrorContentSearches, metaDataSearches, metaDataCaseInsensitiveSearches, textSearchMetaDataColumns, minSendAttempts, maxSendAttempts, attachment, error); - validateMetaDataColumns(channelId, filter); doReprocessMessages(channelId, filter, replace, filterDestinations, reprocessMetaDataIds); } @@ -232,6 +223,11 @@ private void doReprocessMessages(final String channelId, final MessageFilter fil public void run() { try { messageController.reprocessMessages(channelId, filter, replace, metaDataIds); + } catch (MetaDataColumnException e) { + // Reprocess is asynchronous, so the client already received its response; an + // undefined metadata column is rejected here (the run is aborted, nothing is + // reprocessed) and logged rather than surfaced as a response status. + logger.warn("Rejected reprocess for channel {} referencing unknown metadata column: {}", e.getChannelId(), e.getColumnName()); } catch (ControllerException e) { logger.error("Error reprocessing messages for channel " + channelId + ": " + e.getMessage(), e); } @@ -245,7 +241,6 @@ public void run() { @Override @CheckAuthorizedChannelId public void removeMessages(String channelId, MessageFilter filter) { - validateMetaDataColumns(channelId, filter); messageController.removeMessages(channelId, filter); } @@ -253,7 +248,6 @@ public void removeMessages(String channelId, MessageFilter filter) { @CheckAuthorizedChannelId public void removeMessages(String channelId, Long minMessageId, Long maxMessageId, Long minOriginalId, Long maxOriginalId, Long minImportId, Long maxImportId, Calendar startDate, Calendar endDate, String textSearch, Boolean textSearchRegex, Set statuses, Set includedMetaDataIds, Set excludedMetaDataIds, String serverId, Set rawContentSearches, Set processedRawContentSearches, Set transformedContentSearches, Set encodedContentSearches, Set sentContentSearches, Set responseContentSearches, Set responseTransformedContentSearches, Set processedResponseContentSearches, Set connectorMapContentSearches, Set channelMapContentSearches, Set sourceMapContentSearches, Set responseMapContentSearches, Set processingErrorContentSearches, Set postprocessorErrorContentSearches, Set responseErrorContentSearches, Set metaDataSearches, Set metaDataCaseInsensitiveSearches, Set textSearchMetaDataColumns, Integer minSendAttempts, Integer maxSendAttempts, Boolean attachment, Boolean error) { MessageFilter filter = getMessageFilter(minMessageId, maxMessageId, minOriginalId, maxOriginalId, minImportId, maxImportId, startDate, endDate, textSearch, textSearchRegex, statuses, includedMetaDataIds, excludedMetaDataIds, serverId, rawContentSearches, processedRawContentSearches, transformedContentSearches, encodedContentSearches, sentContentSearches, responseContentSearches, responseTransformedContentSearches, processedResponseContentSearches, connectorMapContentSearches, channelMapContentSearches, sourceMapContentSearches, responseMapContentSearches, processingErrorContentSearches, postprocessorErrorContentSearches, responseErrorContentSearches, metaDataSearches, metaDataCaseInsensitiveSearches, textSearchMetaDataColumns, minSendAttempts, maxSendAttempts, attachment, error); - validateMetaDataColumns(channelId, filter); messageController.removeMessages(channelId, filter); } @@ -328,7 +322,6 @@ public MessageImportResult importMessagesServer(String channelId, String path, b @Override @CheckAuthorizedChannelId public int exportMessagesServer(String channelId, MessageFilter filter, int pageSize, MessageWriterOptions writerOptions) { - validateMetaDataColumns(channelId, filter); try { return messageController.exportMessages(channelId, filter, pageSize, writerOptions); } catch (Exception e) { @@ -341,7 +334,6 @@ public int exportMessagesServer(String channelId, MessageFilter filter, int page public int exportMessagesServer(String channelId, Long minMessageId, Long maxMessageId, Long minOriginalId, Long maxOriginalId, Long minImportId, Long maxImportId, Calendar startDate, Calendar endDate, String textSearch, Boolean textSearchRegex, Set statuses, Set includedMetaDataIds, Set excludedMetaDataIds, String serverId, Set rawContentSearches, Set processedRawContentSearches, Set transformedContentSearches, Set encodedContentSearches, Set sentContentSearches, Set responseContentSearches, Set responseTransformedContentSearches, Set processedResponseContentSearches, Set connectorMapContentSearches, Set channelMapContentSearches, Set sourceMapContentSearches, Set responseMapContentSearches, Set processingErrorContentSearches, Set postprocessorErrorContentSearches, Set responseErrorContentSearches, Set metaDataSearches, Set metaDataCaseInsensitiveSearches, Set textSearchMetaDataColumns, Integer minSendAttempts, Integer maxSendAttempts, Boolean attachment, Boolean error, int pageSize, ContentType contentType, boolean destinationContent, boolean encrypt, boolean includeAttachments, String baseFolder, String rootFolder, String filePattern, String archiveFileName, String archiveFormat, String compressFormat, String password, EncryptionType encryptionType) { MessageFilter filter = getMessageFilter(minMessageId, maxMessageId, minOriginalId, maxOriginalId, minImportId, maxImportId, startDate, endDate, textSearch, textSearchRegex, statuses, includedMetaDataIds, excludedMetaDataIds, serverId, rawContentSearches, processedRawContentSearches, transformedContentSearches, encodedContentSearches, sentContentSearches, responseContentSearches, responseTransformedContentSearches, processedResponseContentSearches, connectorMapContentSearches, channelMapContentSearches, sourceMapContentSearches, responseMapContentSearches, processingErrorContentSearches, postprocessorErrorContentSearches, responseErrorContentSearches, metaDataSearches, metaDataCaseInsensitiveSearches, textSearchMetaDataColumns, minSendAttempts, maxSendAttempts, attachment, error); MessageWriterOptions writerOptions = getMessageWriterOptions(contentType, destinationContent, encrypt, includeAttachments, baseFolder, rootFolder, filePattern, archiveFileName, archiveFormat, compressFormat, password, encryptionType); - validateMetaDataColumns(channelId, filter); try { return messageController.exportMessages(channelId, filter, pageSize, writerOptions); } catch (Exception e) { @@ -402,29 +394,6 @@ private void sendServerEventWithAttributes(Map attributes) { eventController.dispatchEvent(event); } - /** - * Validates that every custom metadata column referenced by the filter matches a column actually - * defined on the channel. The message search mappers splice these column names directly into the - * SQL as identifiers (MyBatis ${} substitution), so an unvalidated name is a SQL injection vector. - * Rejecting anything not in the channel's own column set closes that hole; the search value and - * operator are already safe (bound parameter and enum-constrained, respectively). - * - * This runs at the servlet (API) boundary because both the parameter-based and MessageFilter-based - * endpoints for each operation accept these column names from the client. - */ - private void validateMetaDataColumns(String channelId, MessageFilter filter) { - // Primary check at the REST boundary: reject any custom metadata column the filter references - // that is not defined on the channel. Column names are matched exactly against the channel's - // (upper-cased) columns, which come from the in-memory channel cache (no database hit) and are - // only looked up when the filter actually references a custom column. Callers that reach - // DonkeyMessageController directly (e.g. a plugin) are covered by a last-resort backstop there. - String unknownColumn = MetaDataColumnValidator.findUnknownColumn(filter, () -> channelController.getMetaDataColumns(channelId)); - if (unknownColumn != null) { - logger.warn("Rejected message search for channel " + channelId + " referencing unknown metadata column: " + unknownColumn); - throw new MirthApiException(Response.Status.BAD_REQUEST); - } - } - private MessageFilter getMessageFilter(Long minMessageId, Long maxMessageId, Long minOriginalId, Long maxOriginalId, Long minImportId, Long maxImportId, Calendar startDate, Calendar endDate, String textSearch, Boolean textSearchRegex, Set statuses, Set includedMetaDataIds, Set excludedMetaDataIds, String serverId, Set rawContentSearches, Set processedRawContentSearches, Set transformedContentSearches, Set encodedContentSearches, Set sentContentSearches, Set responseContentSearches, Set responseTransformedContentSearches, Set processedResponseContentSearches, Set connectorMapContentSearches, Set channelMapContentSearches, Set sourceMapContentSearches, Set responseMapContentSearches, Set processingErrorContentSearches, Set postprocessorErrorContentSearches, Set responseErrorContentSearches, Set metaDataSearches, Set metaDataCaseInsensitiveSearches, Set textSearchMetaDataColumns, Integer minSendAttempts, Integer maxSendAttempts, Boolean attachment, Boolean error) { MessageFilter filter = new MessageFilter(); filter.setMinMessageId(minMessageId); diff --git a/server/src/main/java/com/mirth/connect/server/controllers/DonkeyMessageController.java b/server/src/main/java/com/mirth/connect/server/controllers/DonkeyMessageController.java index df657ba7f2..07b2b5e86b 100644 --- a/server/src/main/java/com/mirth/connect/server/controllers/DonkeyMessageController.java +++ b/server/src/main/java/com/mirth/connect/server/controllers/DonkeyMessageController.java @@ -63,6 +63,7 @@ import com.mirth.connect.server.util.DICOMMessageUtil; import com.mirth.connect.server.util.ListRangeIterator; import com.mirth.connect.server.util.ListRangeIterator.ListRangeItem; +import com.mirth.connect.server.util.MetaDataColumnException; import com.mirth.connect.server.util.MetaDataColumnValidator; import com.mirth.connect.server.util.SqlConfig; import com.mirth.connect.util.AttachmentUtil; @@ -148,7 +149,6 @@ public long getMinMessageId(String channelId, boolean readOnly) { @Override public Long getMessageCount(MessageFilter filter, String channelId) { - validateMetaDataColumns(channelId, filter); if (filter.getIncludedMetaDataIds() != null && filter.getIncludedMetaDataIds().isEmpty() && filter.getExcludedMetaDataIds() == null) { return 0L; } @@ -192,7 +192,6 @@ public Long getMessageCount(MessageFilter filter, String channelId) { @Override public List getMessages(MessageFilter filter, String channelId, Boolean includeContent, Integer offset, Integer limit) { - validateMetaDataColumns(channelId, filter); // Provide a default value if any of the below 3 parameters are null. includeContent = includeContent == null ? false : includeContent; offset = offset == null ? 0 : offset; @@ -298,7 +297,6 @@ public List getMessageAttachment(String channelId, Long messageId, b @Override public void removeMessages(String channelId, MessageFilter filter) { - validateMetaDataColumns(channelId, filter); EngineController engineController = ControllerFactory.getFactory().createEngineController(); FilterOptions filterOptions = new FilterOptions(filter, channelId, false); @@ -345,7 +343,6 @@ public void removeMessages(String channelId, MessageFilter filter) { } public void reprocessMessages(String channelId, MessageFilter filter, boolean replace, Collection reprocessMetaDataIds) throws ControllerException { - validateMetaDataColumns(channelId, filter); EngineController engineController = ControllerFactory.getFactory().createEngineController(); Channel deployedChannel = engineController.getDeployedChannel(channelId); if (deployedChannel == null) { @@ -682,17 +679,20 @@ private List searchMessages(MessageFilter filter, String ch } /** - * Last-resort backstop against SQL injection through custom metadata column names. The primary - * check runs at the REST boundary (MessageServlet), but a caller that reaches this controller - * directly - notably an external plugin using MessageController.getInstance() - bypasses it. The - * search mappers interpolate these column names into SQL as identifiers, so an undefined column - * must never reach them. This throws (rather than returning a 400) because reaching it means a - * caller skipped the proper gate, which is a programming error. + * Validation gate against SQL injection through custom metadata column names, which the search + * mappers interpolate into SQL as identifiers (MyBatis ${} substitution). It is called from the + * FilterOptions constructor, which every message search path (get, count, remove, reprocess; + * export runs through get) builds exactly once, before its batch loop. That is the single choke + * point through which a search must pass, so a new search entry point cannot bypass the check. + * Throws MetaDataColumnException, a plain domain exception left to bubble up: on the synchronous + * API paths the engine turns it into a 500 like any other uncaught controller exception, and the + * asynchronous reprocess path catches it to log and abort the run. The injection is blocked + * regardless of the resulting status, because the throw happens before any query runs. */ private void validateMetaDataColumns(String channelId, MessageFilter filter) { String unknownColumn = MetaDataColumnValidator.findUnknownColumn(filter, () -> ControllerFactory.getFactory().createChannelController().getMetaDataColumns(channelId)); if (unknownColumn != null) { - throw new IllegalArgumentException("Message search referenced a metadata column that is not defined on channel " + channelId + ": " + unknownColumn); + throw new MetaDataColumnException(channelId, unknownColumn); } } @@ -1168,6 +1168,8 @@ private class FilterOptions { private boolean searchText; public FilterOptions(MessageFilter filter, String channelId, boolean readOnly) { + validateMetaDataColumns(channelId, filter); + if (filter.getMinMessageId() != null && filter.getMaxMessageId() != null && filter.getMinMessageId() > filter.getMaxMessageId()) { /* * If the min message id is greater than the max, use them directly so they fail at diff --git a/server/src/main/java/com/mirth/connect/server/util/MetaDataColumnException.java b/server/src/main/java/com/mirth/connect/server/util/MetaDataColumnException.java new file mode 100644 index 0000000000..b92a064e4d --- /dev/null +++ b/server/src/main/java/com/mirth/connect/server/util/MetaDataColumnException.java @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: Open Integration Engine + +package com.mirth.connect.server.util; + +/** + * Thrown when a message search filter references a custom metadata column that is not defined on the + * channel. Those column names are interpolated into SQL as identifiers by the search mappers (MyBatis + * ${} substitution), so an undefined name must never reach the data layer. + * + *

+ * This is a plain domain exception. It is thrown from the single validation gate in + * DonkeyMessageController - the FilterOptions constructor, which every message search path builds + * before its batch loop - and is left to bubble up. On the synchronous API paths the engine's + * standard handling turns any uncaught controller exception into a 500, the same way every other + * controller-layer rejection surfaces; deliberately not mapped to a 400, because the engine has no + * central exception-to-status convention and coupling this data-layer type to an HTTP status would be + * the only such case in the codebase. The injection is blocked either way, since the throw happens + * before any query runs. The reprocess path is asynchronous and catches this to log and abort the run. + *

+ * + *

+ * The offending channel and column are kept as fields and the message is deliberately generic, so + * callers log the values with parameterized logging rather than concatenating untrusted input into a + * message string. + *

+ */ +public class MetaDataColumnException extends RuntimeException { + + private final String channelId; + private final String columnName; + + public MetaDataColumnException(String channelId, String columnName) { + super("Message search referenced an undefined metadata column"); + this.channelId = channelId; + this.columnName = columnName; + } + + public String getChannelId() { + return channelId; + } + + public String getColumnName() { + return columnName; + } +} diff --git a/server/src/main/java/com/mirth/connect/server/util/MetaDataColumnValidator.java b/server/src/main/java/com/mirth/connect/server/util/MetaDataColumnValidator.java index 7b3ac78645..506cd57535 100644 --- a/server/src/main/java/com/mirth/connect/server/util/MetaDataColumnValidator.java +++ b/server/src/main/java/com/mirth/connect/server/util/MetaDataColumnValidator.java @@ -55,7 +55,7 @@ public static String findUnknownColumn(MessageFilter filter, Supplier allowedColumns = new HashSet(); if (definedColumns != null) { for (MetaDataColumn column : definedColumns) { - if (column.getName() != null) { + if (column != null && column.getName() != null) { allowedColumns.add(column.getName()); } } @@ -63,6 +63,9 @@ public static String findUnknownColumn(MessageFilter filter, Supplier { @@ -220,96 +203,6 @@ public void testRestrictedUserCanRemoveMessagesWithoutClearingStats() { verify(engineController, times(2)).removeAllMessages(any(), anyBoolean(), anyBoolean(), any()); } - @Test - public void testMetaDataSearchRejectsUnknownColumn() { - when(channelController.getMetaDataColumns("channel1")).thenReturn(definedColumns("STATUS")); - MessageServlet servlet = new MessageServlet(request, context, sc, controllerFactory); - - MessageFilter filter = new MessageFilter(); - filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("STATUS\" = '' OR '1'='1' --", "EQUAL", "x", false))); - - try { - servlet.getMessageCount("channel1", filter); - fail("Expected MirthApiException for an unknown metadata column"); - } catch (MirthApiException e) { - assertEquals(400, e.getResponse().getStatus()); - } - verify(messageController, times(0)).getMessageCount(any(), anyString()); - } - - @Test - public void testTextSearchRejectsUnknownColumn() { - when(channelController.getMetaDataColumns("channel1")).thenReturn(definedColumns("STATUS")); - MessageServlet servlet = new MessageServlet(request, context, sc, controllerFactory); - - MessageFilter filter = new MessageFilter(); - filter.setTextSearchMetaDataColumns(new ArrayList(Arrays.asList("BOGUS"))); - - try { - servlet.getMessageCount("channel1", filter); - fail("Expected MirthApiException for an unknown text-search metadata column"); - } catch (MirthApiException e) { - assertEquals(400, e.getResponse().getStatus()); - } - verify(messageController, times(0)).getMessageCount(any(), anyString()); - } - - @Test - public void testMetaDataSearchAllowsDefinedColumn() { - when(channelController.getMetaDataColumns("channel1")).thenReturn(definedColumns("STATUS")); - MessageServlet servlet = new MessageServlet(request, context, sc, controllerFactory); - - MessageFilter filter = new MessageFilter(); - filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("STATUS", "EQUAL", "x", false))); - - servlet.getMessageCount("channel1", filter); - verify(messageController, times(1)).getMessageCount(filter, "channel1"); - } - - @Test - public void testMetaDataSearchRejectsNonUpperCaseColumn() { - when(channelController.getMetaDataColumns("channel1")).thenReturn(definedColumns("STATUS")); - MessageServlet servlet = new MessageServlet(request, context, sc, controllerFactory); - - MessageFilter filter = new MessageFilter(); - // Column names are matched exactly; a non-upper-case name would not match the actual DB - // column either, so it is rejected rather than silently returning nothing. - filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("status", "EQUAL", "x", false))); - - try { - servlet.getMessageCount("channel1", filter); - fail("Expected MirthApiException for a non-upper-case metadata column"); - } catch (MirthApiException e) { - assertEquals(400, e.getResponse().getStatus()); - } - verify(messageController, times(0)).getMessageCount(any(), anyString()); - } - - @Test - public void testRejectsSearchWhenChannelHasNoColumns() { - when(channelController.getMetaDataColumns("channel1")).thenReturn(null); - MessageServlet servlet = new MessageServlet(request, context, sc, controllerFactory); - - MessageFilter filter = new MessageFilter(); - filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("STATUS", "EQUAL", "x", false))); - - try { - servlet.getMessageCount("channel1", filter); - fail("Expected MirthApiException when the channel has no defined metadata columns"); - } catch (MirthApiException e) { - assertEquals(400, e.getResponse().getStatus()); - } - verify(messageController, times(0)).getMessageCount(any(), anyString()); - } - - private static List definedColumns(String... names) { - List columns = new ArrayList(); - for (String name : names) { - columns.add(new MetaDataColumn(name, MetaDataColumnType.STRING, null)); - } - return columns; - } - private static void setupSessionAndRequest(int userId) { session = mock(HttpSession.class); when(session.getAttribute("user")).thenReturn("" + userId); diff --git a/server/src/test/java/com/mirth/connect/server/util/MetaDataColumnExceptionTest.java b/server/src/test/java/com/mirth/connect/server/util/MetaDataColumnExceptionTest.java new file mode 100644 index 0000000000..d1c933cf10 --- /dev/null +++ b/server/src/test/java/com/mirth/connect/server/util/MetaDataColumnExceptionTest.java @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: Open Integration Engine + +package com.mirth.connect.server.util; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +public class MetaDataColumnExceptionTest { + + /** + * A plain domain exception (bubbles up to the engine's standard 500 on the synchronous API + * paths). It must not carry the untrusted column name in its message; that goes in a field so + * the reprocess path can log it with parameterized logging. + */ + @Test + public void isPlainRuntimeExceptionWithGenericMessage() { + MetaDataColumnException e = new MetaDataColumnException("channelId", "EVIL\" OR '1'='1"); + assertTrue(e instanceof RuntimeException); + assertEquals("Message search referenced an undefined metadata column", e.getMessage()); + } + + @Test + public void retainsChannelAndColumn() { + MetaDataColumnException e = new MetaDataColumnException("channelId", "EVIL"); + assertEquals("channelId", e.getChannelId()); + assertEquals("EVIL", e.getColumnName()); + } +} diff --git a/server/src/test/java/com/mirth/connect/server/util/MetaDataColumnValidatorTest.java b/server/src/test/java/com/mirth/connect/server/util/MetaDataColumnValidatorTest.java index 95ef207c43..70232bef0e 100644 --- a/server/src/test/java/com/mirth/connect/server/util/MetaDataColumnValidatorTest.java +++ b/server/src/test/java/com/mirth/connect/server/util/MetaDataColumnValidatorTest.java @@ -109,4 +109,28 @@ public void channelWithNoColumnsRejectsAnyReferencedColumn() { assertEquals("STATUS", MetaDataColumnValidator.findUnknownColumn(filter, () -> null)); } + + @Test + public void nullSearchElementIsRejectedNotThrown() { + MessageFilter filter = new MessageFilter(); + List elements = new ArrayList(); + elements.add(null); + filter.setMetaDataSearch(elements); + + // A null element in the list must be treated as unknown (returned), never an NPE. + assertEquals("null", MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); + } + + @Test + public void nullDefinedColumnEntryIsIgnoredNotThrown() { + MessageFilter filter = new MessageFilter(); + filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("STATUS", "EQUAL", "x", false))); + + List columns = new ArrayList(); + columns.add(null); + columns.add(new MetaDataColumn("STATUS", MetaDataColumnType.STRING, null)); + + // A null entry in the channel's columns must be skipped, not cause an NPE; STATUS still validates. + assertNull(MetaDataColumnValidator.findUnknownColumn(filter, () -> columns)); + } } From 2daa83580f8d89a08dc4d8b61b31ae706bbce4ff Mon Sep 17 00:00:00 2001 From: Finnegan's Owner <44065187+pacmano1@users.noreply.github.com> Date: Mon, 27 Jul 2026 08:10:11 -0600 Subject: [PATCH 3/3] Return Optional from metadata column validator Review feedback: as a new public method, findUnknownColumn should not return a nullable String. It now returns Optional, the javadoc gained @param/@return tags, and the stale class doc describing the removed REST-layer check was corrected. Signed-off-by: Finnegan's Owner <44065187+pacmano1@users.noreply.github.com> --- .../controllers/DonkeyMessageController.java | 7 ++-- .../server/util/MetaDataColumnValidator.java | 34 ++++++++++++------- .../util/MetaDataColumnValidatorTest.java | 24 ++++++------- 3 files changed, 37 insertions(+), 28 deletions(-) diff --git a/server/src/main/java/com/mirth/connect/server/controllers/DonkeyMessageController.java b/server/src/main/java/com/mirth/connect/server/controllers/DonkeyMessageController.java index 07b2b5e86b..ab9f9404ed 100644 --- a/server/src/main/java/com/mirth/connect/server/controllers/DonkeyMessageController.java +++ b/server/src/main/java/com/mirth/connect/server/controllers/DonkeyMessageController.java @@ -20,6 +20,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.NavigableMap; +import java.util.Optional; import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; @@ -690,9 +691,9 @@ private List searchMessages(MessageFilter filter, String ch * regardless of the resulting status, because the throw happens before any query runs. */ private void validateMetaDataColumns(String channelId, MessageFilter filter) { - String unknownColumn = MetaDataColumnValidator.findUnknownColumn(filter, () -> ControllerFactory.getFactory().createChannelController().getMetaDataColumns(channelId)); - if (unknownColumn != null) { - throw new MetaDataColumnException(channelId, unknownColumn); + Optional unknownColumn = MetaDataColumnValidator.findUnknownColumn(filter, () -> ControllerFactory.getFactory().createChannelController().getMetaDataColumns(channelId)); + if (unknownColumn.isPresent()) { + throw new MetaDataColumnException(channelId, unknownColumn.get()); } } diff --git a/server/src/main/java/com/mirth/connect/server/util/MetaDataColumnValidator.java b/server/src/main/java/com/mirth/connect/server/util/MetaDataColumnValidator.java index 506cd57535..80643b28a0 100644 --- a/server/src/main/java/com/mirth/connect/server/util/MetaDataColumnValidator.java +++ b/server/src/main/java/com/mirth/connect/server/util/MetaDataColumnValidator.java @@ -5,6 +5,7 @@ import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.function.Supplier; @@ -21,8 +22,8 @@ * *

* This is a pure check: it never throws and never looks anything up. Callers pass in the channel's - * defined columns and decide what to do with an unknown column - the REST layer returns 400, the - * controller layer throws as a last-resort backstop for callers that bypass the REST layer. + * defined columns and decide what to do with an unknown column - the controller rejects the search + * by throwing a MetaDataColumnException before any query runs. *

*/ public final class MetaDataColumnValidator { @@ -30,25 +31,32 @@ public final class MetaDataColumnValidator { private MetaDataColumnValidator() {} /** - * Returns the first metadata column name referenced by the filter that is not defined on the - * channel, or {@code null} if every referenced column is valid. Column names are matched exactly - * against the channel's (upper-cased) column names; a {@code null} referenced name is treated as - * unknown and returned as the string {@code "null"} so the result stays unambiguous. + * Finds the first metadata column name referenced by the filter that is not defined on the + * channel. Column names are matched exactly against the channel's (upper-cased) column names; a + * {@code null} referenced name is treated as unknown and reported as the string {@code "null"}. * *

* The defined columns are supplied lazily and are only requested when the filter actually * references a custom column, so a search that uses none costs no channel lookup. *

+ * + * @param filter the message search filter to check; may be {@code null}, which validates + * trivially + * @param definedColumnsSupplier supplies the channel's defined metadata columns; only invoked + * when the filter references a custom column, and may return {@code null} for a + * channel with no columns + * @return the first unknown column name referenced by the filter, or {@link Optional#empty()} + * if every referenced column is defined on the channel */ - public static String findUnknownColumn(MessageFilter filter, Supplier> definedColumnsSupplier) { + public static Optional findUnknownColumn(MessageFilter filter, Supplier> definedColumnsSupplier) { if (filter == null) { - return null; + return Optional.empty(); } boolean hasMetaDataSearch = CollectionUtils.isNotEmpty(filter.getMetaDataSearch()); boolean hasTextSearchColumns = CollectionUtils.isNotEmpty(filter.getTextSearchMetaDataColumns()); if (!hasMetaDataSearch && !hasTextSearchColumns) { - return null; + return Optional.empty(); } List definedColumns = definedColumnsSupplier.get(); @@ -64,10 +72,10 @@ public static String findUnknownColumn(MessageFilter filter, Supplier> supplier(List colu @Test public void nullFilterReturnsNull() { - assertNull(MetaDataColumnValidator.findUnknownColumn(null, () -> definedColumns("STATUS"))); + assertEquals(Optional.empty(), MetaDataColumnValidator.findUnknownColumn(null, () -> definedColumns("STATUS"))); } @Test @@ -48,7 +48,7 @@ public void noReferencedColumnsReturnsNullAndSkipsLookup() { AtomicBoolean invoked = new AtomicBoolean(false); MessageFilter filter = new MessageFilter(); - assertNull(MetaDataColumnValidator.findUnknownColumn(filter, supplier(definedColumns("STATUS"), invoked))); + assertEquals(Optional.empty(), MetaDataColumnValidator.findUnknownColumn(filter, supplier(definedColumns("STATUS"), invoked))); assertFalse("Channel columns must not be looked up when the filter references none", invoked.get()); } @@ -58,7 +58,7 @@ public void definedMetaDataSearchColumnReturnsNull() { MessageFilter filter = new MessageFilter(); filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("STATUS", "EQUAL", "x", false))); - assertNull(MetaDataColumnValidator.findUnknownColumn(filter, supplier(definedColumns("STATUS"), invoked))); + assertEquals(Optional.empty(), MetaDataColumnValidator.findUnknownColumn(filter, supplier(definedColumns("STATUS"), invoked))); assertTrue("A referenced column must trigger the lookup", invoked.get()); } @@ -67,7 +67,7 @@ public void unknownMetaDataSearchColumnIsReturned() { MessageFilter filter = new MessageFilter(); filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("EVIL\" OR '1'='1", "EQUAL", "x", false))); - assertEquals("EVIL\" OR '1'='1", MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); + assertEquals(Optional.of("EVIL\" OR '1'='1"), MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); } @Test @@ -75,7 +75,7 @@ public void nonUpperCaseColumnIsReturned() { MessageFilter filter = new MessageFilter(); filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("status", "EQUAL", "x", false))); - assertEquals("status", MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); + assertEquals(Optional.of("status"), MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); } @Test @@ -83,7 +83,7 @@ public void nullColumnNameIsReturnedAsNullString() { MessageFilter filter = new MessageFilter(); filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement(null, "EQUAL", "x", false))); - assertEquals("null", MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); + assertEquals(Optional.of("null"), MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); } @Test @@ -91,7 +91,7 @@ public void definedTextSearchColumnReturnsNull() { MessageFilter filter = new MessageFilter(); filter.setTextSearchMetaDataColumns(new ArrayList(Arrays.asList("STATUS"))); - assertNull(MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); + assertEquals(Optional.empty(), MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); } @Test @@ -99,7 +99,7 @@ public void unknownTextSearchColumnIsReturned() { MessageFilter filter = new MessageFilter(); filter.setTextSearchMetaDataColumns(new ArrayList(Arrays.asList("BOGUS"))); - assertEquals("BOGUS", MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); + assertEquals(Optional.of("BOGUS"), MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); } @Test @@ -107,7 +107,7 @@ public void channelWithNoColumnsRejectsAnyReferencedColumn() { MessageFilter filter = new MessageFilter(); filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("STATUS", "EQUAL", "x", false))); - assertEquals("STATUS", MetaDataColumnValidator.findUnknownColumn(filter, () -> null)); + assertEquals(Optional.of("STATUS"), MetaDataColumnValidator.findUnknownColumn(filter, () -> null)); } @Test @@ -118,7 +118,7 @@ public void nullSearchElementIsRejectedNotThrown() { filter.setMetaDataSearch(elements); // A null element in the list must be treated as unknown (returned), never an NPE. - assertEquals("null", MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); + assertEquals(Optional.of("null"), MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS"))); } @Test @@ -131,6 +131,6 @@ public void nullDefinedColumnEntryIsIgnoredNotThrown() { columns.add(new MetaDataColumn("STATUS", MetaDataColumnType.STRING, null)); // A null entry in the channel's columns must be skipped, not cause an NPE; STATUS still validates. - assertNull(MetaDataColumnValidator.findUnknownColumn(filter, () -> columns)); + assertEquals(Optional.empty(), MetaDataColumnValidator.findUnknownColumn(filter, () -> columns)); } }