From 029c7904b3aa3620a60ddb80a2ba9c72808ce0b8 Mon Sep 17 00:00:00 2001 From: Juliet wamalwa Date: Tue, 26 May 2026 15:30:38 +0300 Subject: [PATCH 1/2] TRUNK-6550: Logging should not be vulnerable to injection attacks --- .../openmrs/logging/OpenmrsLoggingUtil.java | 16 ++++ api/src/main/resources/log4j2.xml | 2 +- .../logging/OpenmrsLoggingUtilTest.java | 48 ++++++++++ liquibase/src/main/resources/log4j2.xml | 2 +- .../module/web/ModuleResourcesServlet.java | 14 ++- .../org/openmrs/module/web/ModuleServlet.java | 21 +++-- .../initialization/InitializationFilter.java | 11 ++- .../initialization/TestInstallUtil.java | 12 ++- .../web/filter/update/UpdateFilter.java | 9 +- .../web/ModuleResourcesServletTest.java | 85 +++++++++++++++++ .../openmrs/module/web/ModuleServletTest.java | 91 +++++++++++++++++++ .../InitializationFilterTest.java | 37 ++++++++ .../web/filter/update/UpdateFilterTest.java | 32 +++++++ web/src/test/resources/log4j2.xml | 2 +- webapp/src/main/resources/log4j2.xml | 2 +- 15 files changed, 357 insertions(+), 27 deletions(-) create mode 100644 api/src/test/java/org/openmrs/logging/OpenmrsLoggingUtilTest.java create mode 100644 web/src/test/java/org/openmrs/module/web/ModuleResourcesServletTest.java create mode 100644 web/src/test/java/org/openmrs/module/web/ModuleServletTest.java diff --git a/api/src/main/java/org/openmrs/logging/OpenmrsLoggingUtil.java b/api/src/main/java/org/openmrs/logging/OpenmrsLoggingUtil.java index 52c199d7bd13..95a14e66e498 100644 --- a/api/src/main/java/org/openmrs/logging/OpenmrsLoggingUtil.java +++ b/api/src/main/java/org/openmrs/logging/OpenmrsLoggingUtil.java @@ -198,4 +198,20 @@ public static void reloadLoggingConfiguration() { ((LoggerContext) LogManager.getContext(true)).reconfigure(); } + /** + * Sanitizes a string for logging by replacing newline and carriage return characters with + * underscores. + * + * @param value the string to sanitize + * @return the sanitized string + * @since 2.4.4, 2.5.1, 2.6.0 + */ + @Logging(ignore = true) + public static String sanitize(Object value) { + if (value == null) { + return null; + } + return value.toString().replaceAll("[\n\r]", "_"); + } + } diff --git a/api/src/main/resources/log4j2.xml b/api/src/main/resources/log4j2.xml index b1fd757d9478..dcaf70794de7 100644 --- a/api/src/main/resources/log4j2.xml +++ b/api/src/main/resources/log4j2.xml @@ -15,7 +15,7 @@ - %p - %C{1}.%M(%L) |%d{ISO8601}| %m%n + %p - %C{1}.%M(%L) |%d{ISO8601}| %replace{%m}{[\r\n]}{_}%n diff --git a/api/src/test/java/org/openmrs/logging/OpenmrsLoggingUtilTest.java b/api/src/test/java/org/openmrs/logging/OpenmrsLoggingUtilTest.java new file mode 100644 index 000000000000..a84a73018245 --- /dev/null +++ b/api/src/test/java/org/openmrs/logging/OpenmrsLoggingUtilTest.java @@ -0,0 +1,48 @@ +/** + * This Source Code Form is subject to the terms of the Mozilla Public License, + * v. 2.0. If a copy of the MPL was not distributed with this file, You can + * obtain one at http://mozilla.org/MPL/2.0/. OpenMRS is also distributed under + * the terms of the Healthcare Disclaimer located at http://openmrs.org/license. + * + * Copyright (C) OpenMRS Inc. OpenMRS is a registered trademark and the OpenMRS + * graphic logo is a trademark of OpenMRS Inc. + */ +package org.openmrs.logging; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +/** + * Tests for {@link OpenmrsLoggingUtil}. + */ +public class OpenmrsLoggingUtilTest { + + /** + * @see OpenmrsLoggingUtil#sanitize(Object) + */ + @Test + public void sanitize_shouldReplaceNewlinesWithUnderscores() { + assertEquals("some_data", OpenmrsLoggingUtil.sanitize("some\ndata")); + assertEquals("some_data", OpenmrsLoggingUtil.sanitize("some\rdata")); + assertEquals("some__data", OpenmrsLoggingUtil.sanitize("some\r\ndata")); + assertEquals("no_newlines", OpenmrsLoggingUtil.sanitize("no_newlines")); + } + + /** + * @see OpenmrsLoggingUtil#sanitize(Object) + */ + @Test + public void sanitize_shouldHandleNullValue() { + assertNull(OpenmrsLoggingUtil.sanitize(null)); + } + + /** + * @see OpenmrsLoggingUtil#sanitize(Object) + */ + @Test + public void sanitize_shouldHandleNonStringObjects() { + assertEquals("123", OpenmrsLoggingUtil.sanitize(123)); + } +} diff --git a/liquibase/src/main/resources/log4j2.xml b/liquibase/src/main/resources/log4j2.xml index f5ac70012362..bc00a3a88ff5 100644 --- a/liquibase/src/main/resources/log4j2.xml +++ b/liquibase/src/main/resources/log4j2.xml @@ -14,7 +14,7 @@ - + diff --git a/web/src/main/java/org/openmrs/module/web/ModuleResourcesServlet.java b/web/src/main/java/org/openmrs/module/web/ModuleResourcesServlet.java index 74cd2bb87eca..07801e5e21c7 100644 --- a/web/src/main/java/org/openmrs/module/web/ModuleResourcesServlet.java +++ b/web/src/main/java/org/openmrs/module/web/ModuleResourcesServlet.java @@ -19,6 +19,8 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.apache.commons.lang3.StringUtils; +import org.openmrs.logging.OpenmrsLoggingUtil; import org.openmrs.module.Module; import org.openmrs.module.ModuleUtil; import org.openmrs.util.OpenmrsUtil; @@ -52,7 +54,7 @@ protected long getLastModified(HttpServletRequest req) { @Override protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { - log.debug("In service method for module servlet: " + request.getPathInfo()); + log.debug("In service method for module servlet: " + OpenmrsLoggingUtil.sanitize(request.getPathInfo())); File f = getFile(request); if (f == null) { @@ -82,10 +84,13 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t protected File getFile(HttpServletRequest request) { String path = request.getPathInfo(); + if (StringUtils.isBlank(path) || !path.startsWith("/") || path.lastIndexOf("/") <= 0) { + return null; + } Module module = ModuleUtil.getModuleForPath(path); if (module == null) { - log.warn("No module handles the path: " + path); + log.warn("No module handles the path: " + OpenmrsLoggingUtil.sanitize(path)); return null; } @@ -107,13 +112,14 @@ protected File getFile(HttpServletRequest request) { Path normalizedPath = Path.of(realPath).normalize(); Path normalizedBase = Path.of(basePath).normalize(); if (!normalizedPath.startsWith(normalizedBase)) { - log.warn("Detected attempted directory traversal with path: " + path); + log.warn("Detected attempted directory traversal with path: " + OpenmrsLoggingUtil.sanitize(path)); return null; } File f = normalizedPath.toFile(); if (!f.exists()) { - log.warn("No file with path '" + normalizedPath + "' exists for module '" + module.getModuleId() + "'"); + log.warn("No file with path '" + OpenmrsLoggingUtil.sanitize(normalizedPath) + "' exists for module '" + + OpenmrsLoggingUtil.sanitize(module.getModuleId()) + "'"); return null; } diff --git a/web/src/main/java/org/openmrs/module/web/ModuleServlet.java b/web/src/main/java/org/openmrs/module/web/ModuleServlet.java index 95ca012e267c..79b1274b71c2 100644 --- a/web/src/main/java/org/openmrs/module/web/ModuleServlet.java +++ b/web/src/main/java/org/openmrs/module/web/ModuleServlet.java @@ -21,6 +21,8 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.apache.commons.lang3.StringUtils; +import org.openmrs.logging.OpenmrsLoggingUtil; import org.openmrs.module.Module; import org.openmrs.module.ModuleFactory; import org.slf4j.Logger; @@ -34,8 +36,14 @@ public class ModuleServlet extends HttpServlet { @Override protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { - log.debug("In service method for module servlet: " + request.getPathInfo()); - String servletName = request.getPathInfo(); + log.debug("In service method for module servlet: " + OpenmrsLoggingUtil.sanitize(request.getPathInfo())); + String path = request.getPathInfo(); + if (StringUtils.isBlank(path) || "/".equals(path)) { + response.setStatus(HttpServletResponse.SC_NOT_FOUND); + return; + } + + String servletName = path; int end = servletName.indexOf("/", 1); String moduleId = null; @@ -43,13 +51,14 @@ protected void service(HttpServletRequest request, HttpServletResponse response) moduleId = servletName.substring(1, end); } - log.debug("ModuleId: " + moduleId); + log.debug("ModuleId: " + OpenmrsLoggingUtil.sanitize(moduleId)); Module mod = ModuleFactory.getModuleById(moduleId); // where in the path to start trimming int start = 1; if (mod != null) { - log.debug("Module with id " + moduleId + " found. Looking for servlet name after " + moduleId + " in url path"); + log.debug("Module with id " + OpenmrsLoggingUtil.sanitize(moduleId) + " found. Looking for servlet name after " + + OpenmrsLoggingUtil.sanitize(moduleId) + " in url path"); start = moduleId.length() + 2; // this skips over the moduleId that is in the path } @@ -60,12 +69,12 @@ protected void service(HttpServletRequest request, HttpServletResponse response) } servletName = servletName.substring(start, end); - log.debug("Servlet name: " + servletName); + log.debug("Servlet name: " + OpenmrsLoggingUtil.sanitize(servletName)); HttpServlet servlet = WebModuleUtil.getServlet(servletName); if (servlet == null) { - log.warn("No servlet with name: " + servletName + " was found"); + log.warn("No servlet with name: " + OpenmrsLoggingUtil.sanitize(servletName) + " was found"); response.setStatus(HttpServletResponse.SC_NOT_FOUND); return; } diff --git a/web/src/main/java/org/openmrs/web/filter/initialization/InitializationFilter.java b/web/src/main/java/org/openmrs/web/filter/initialization/InitializationFilter.java index adb6a0f75d26..7c73dd741363 100644 --- a/web/src/main/java/org/openmrs/web/filter/initialization/InitializationFilter.java +++ b/web/src/main/java/org/openmrs/web/filter/initialization/InitializationFilter.java @@ -56,6 +56,7 @@ import org.openmrs.liquibase.ChangeLogDetective; import org.openmrs.liquibase.ChangeLogVersionFinder; import org.openmrs.liquibase.ChangeSetExecutorCallback; +import org.openmrs.logging.OpenmrsLoggingUtil; import org.openmrs.module.MandatoryModuleException; import org.openmrs.module.web.WebModuleUtil; import org.openmrs.util.DatabaseUpdateException; @@ -346,7 +347,8 @@ protected void initializeWizardFromResolvedPropertiesIfPresent() { if (log.isDebugEnabled()) { for (String key : script.stringPropertyNames()) { String value = script.getProperty(key); - log.debug("{} = {}", key, key.toLowerCase().contains("password") ? "*******" : value); + log.debug("{} = {}", OpenmrsLoggingUtil.sanitize(key), + key.toLowerCase().contains("password") ? "*******" : OpenmrsLoggingUtil.sanitize(value)); } } @@ -481,7 +483,8 @@ protected void doPost(HttpServletRequest httpRequest, HttpServletResponse httpRe checkLocaleAttributes(httpRequest); referenceMap.put(FilterUtil.LOCALE_ATTRIBUTE, httpRequest.getSession().getAttribute(FilterUtil.LOCALE_ATTRIBUTE)); - log.info("Locale stored in session is " + httpRequest.getSession().getAttribute(FilterUtil.LOCALE_ATTRIBUTE)); + log.info("Locale stored in session is " + + OpenmrsLoggingUtil.sanitize(httpRequest.getSession().getAttribute(FilterUtil.LOCALE_ATTRIBUTE))); httpResponse.setContentType("text/html"); // otherwise do step one of the wizard @@ -976,7 +979,7 @@ private void checkLocaleAttributes(HttpServletRequest httpRequest) { // if user has changed locale parameter to new one // or chooses it parameter at first page loading if (storedLocale == null || !storedLocale.equals(localeParameter)) { - log.info("Stored locale parameter to session " + localeParameter); + log.info("Stored locale parameter to session " + OpenmrsLoggingUtil.sanitize(localeParameter)); httpRequest.getSession().setAttribute(FilterUtil.LOCALE_ATTRIBUTE, localeParameter); } if (rememberLocale) { @@ -1892,7 +1895,7 @@ public static String loadDriver(String connection, String databaseDriver) { String loadedDriverString = null; try { loadedDriverString = DatabaseUtil.loadDatabaseDriver(connection, databaseDriver); - log.info("using database driver :" + loadedDriverString); + log.info("using database driver :" + OpenmrsLoggingUtil.sanitize(loadedDriverString)); } catch (ClassNotFoundException e) { log.error("The given database driver class was not found. " + "Please ensure that the database driver jar file is on the class path " diff --git a/web/src/main/java/org/openmrs/web/filter/initialization/TestInstallUtil.java b/web/src/main/java/org/openmrs/web/filter/initialization/TestInstallUtil.java index 91129ffe7e46..27dc7a10ac77 100644 --- a/web/src/main/java/org/openmrs/web/filter/initialization/TestInstallUtil.java +++ b/web/src/main/java/org/openmrs/web/filter/initialization/TestInstallUtil.java @@ -35,6 +35,7 @@ import org.openmrs.api.APIAuthenticationException; import org.openmrs.api.APIException; import org.openmrs.api.context.Context; +import org.openmrs.logging.OpenmrsLoggingUtil; import org.openmrs.module.ModuleConstants; import org.openmrs.util.OpenmrsConstants; import org.openmrs.util.OpenmrsUtil; @@ -104,7 +105,7 @@ protected static boolean addTestData(String host, int port, String databaseName, //print out the error messages from the process if (StringUtils.isNotBlank(errorMsg)) { - log.error(errorMsg); + log.error("{}", OpenmrsLoggingUtil.sanitize(errorMsg)); } if (proc.waitFor() == 0) { @@ -147,7 +148,7 @@ protected static boolean addZippedTestModules(InputStream in) { while (entries.hasMoreElements()) { ZipEntry entry = (ZipEntry) entries.nextElement(); if (entry.isDirectory()) { - log.debug("Skipping directory: {}", entry.getName()); + log.debug("Skipping directory: {}", OpenmrsLoggingUtil.sanitize(entry.getName())); continue; } @@ -159,7 +160,7 @@ protected static boolean addZippedTestModules(InputStream in) { fileName = new File(entry.getName()).getName(); } - log.debug("Extracting module file: {}", fileName); + log.debug("Extracting module file: {}", OpenmrsLoggingUtil.sanitize(fileName)); //use the module repository folder GP value if specified String moduleRepositoryFolder = FilterUtil @@ -191,7 +192,7 @@ protected static boolean addZippedTestModules(InputStream in) { OpenmrsUtil.copyFile(zipFile.getInputStream(entry), new BufferedOutputStream(new FileOutputStream(zipEntryFile))); } else { - log.debug("Ignoring file that is not a .omod '{}'", fileName); + log.debug("Ignoring file that is not a .omod '{}'", OpenmrsLoggingUtil.sanitize(fileName)); } } } catch (IOException e) { @@ -255,7 +256,8 @@ protected static InputStream getResourceInputStream(String url, String openmrsUs out.flush(); out.close(); - log.info("Http response message: {}, Code: {}", connection.getResponseMessage(), connection.getResponseCode()); + log.info("Http response message: {}, Code: {}", OpenmrsLoggingUtil.sanitize(connection.getResponseMessage()), + connection.getResponseCode()); if (connection.getResponseCode() == HttpURLConnection.HTTP_UNAUTHORIZED) { throw new APIAuthenticationException("Invalid username or password"); diff --git a/web/src/main/java/org/openmrs/web/filter/update/UpdateFilter.java b/web/src/main/java/org/openmrs/web/filter/update/UpdateFilter.java index 669c0ea9e800..7c676ad3d232 100644 --- a/web/src/main/java/org/openmrs/web/filter/update/UpdateFilter.java +++ b/web/src/main/java/org/openmrs/web/filter/update/UpdateFilter.java @@ -37,6 +37,7 @@ import org.openmrs.liquibase.ChangeLogDetective; import org.openmrs.liquibase.ChangeLogVersionFinder; import org.openmrs.liquibase.ChangeSetExecutorCallback; +import org.openmrs.logging.OpenmrsLoggingUtil; import org.openmrs.util.DatabaseUpdateException; import org.openmrs.util.DatabaseUpdater; import org.openmrs.util.DatabaseUpdaterLiquibaseProvider; @@ -154,7 +155,7 @@ protected synchronized void doPost(HttpServletRequest httpRequest, HttpServletRe String username = httpRequest.getParameter("username"); String password = httpRequest.getParameter("password"); - log.debug("Attempting to authenticate user: " + username); + log.debug("Attempting to authenticate user: " + OpenmrsLoggingUtil.sanitize(username)); if (authenticateAsSuperUser(username, password)) { log.debug("Authentication successful. Redirecting to 'reviewupdates' page."); // set a variable so we know that the user started here @@ -292,13 +293,13 @@ public void checkLocaleAttributesForFirstTime(HttpServletRequest httpRequest) { String systemDefaultLocale = FilterUtil.readSystemDefaultLocale(null); if (CustomResourceLoader.getInstance(httpRequest).getAvailablelocales().contains(locale)) { httpRequest.getSession().setAttribute(FilterUtil.LOCALE_ATTRIBUTE, locale.toString()); - log.info("Used client's locale " + locale.toString()); + log.info("Used client's locale " + OpenmrsLoggingUtil.sanitize(locale)); } else if (StringUtils.isNotBlank(systemDefaultLocale)) { httpRequest.getSession().setAttribute(FilterUtil.LOCALE_ATTRIBUTE, systemDefaultLocale); - log.info("Used system default locale " + systemDefaultLocale); + log.info("Used system default locale " + OpenmrsLoggingUtil.sanitize(systemDefaultLocale)); } else { httpRequest.getSession().setAttribute(FilterUtil.LOCALE_ATTRIBUTE, Locale.ENGLISH.toString()); - log.info("Used default locale " + Locale.ENGLISH.toString()); + log.info("Used default locale " + OpenmrsLoggingUtil.sanitize(Locale.ENGLISH)); } } diff --git a/web/src/test/java/org/openmrs/module/web/ModuleResourcesServletTest.java b/web/src/test/java/org/openmrs/module/web/ModuleResourcesServletTest.java new file mode 100644 index 000000000000..b44bf0a5f453 --- /dev/null +++ b/web/src/test/java/org/openmrs/module/web/ModuleResourcesServletTest.java @@ -0,0 +1,85 @@ +/** + * This Source Code Form is subject to the terms of the Mozilla Public License, + * v. 2.0. If a copy of the MPL was not distributed with this file, You can + * obtain one at http://mozilla.org/MPL/2.0/. OpenMRS is also distributed under + * the terms of the Healthcare Disclaimer located at http://openmrs.org/license. + * + * Copyright (C) OpenMRS Inc. OpenMRS is a registered trademark and the OpenMRS + * graphic logo is a trademark of OpenMRS Inc. + */ +package org.openmrs.module.web; + +import java.io.IOException; + +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Tests for {@link ModuleResourcesServlet} + */ +public class ModuleResourcesServletTest { + + private ModuleResourcesServlet moduleResourcesServlet; + + private HttpServletRequest mockRequest; + + private HttpServletResponse mockResponse; + + @BeforeEach + public void setup() { + moduleResourcesServlet = new ModuleResourcesServlet(); + mockRequest = mock(HttpServletRequest.class); + mockResponse = mock(HttpServletResponse.class); + } + + @Test + public void getLastModified_shouldReturnNegativeOneWhenFileNotFound() { + when(mockRequest.getPathInfo()).thenReturn("/invalid/path"); + long lastModified = moduleResourcesServlet.getLastModified(mockRequest); + assertEquals(-1L, lastModified); + } + + @Test + public void getLastModified_shouldReturnMinusOneForNullFile() { + when(mockRequest.getPathInfo()).thenReturn("/nonexistent/module/resource.txt"); + long lastModified = moduleResourcesServlet.getLastModified(mockRequest); + assertEquals(-1L, lastModified); + } + + @Test + public void doGet_shouldReturn404WhenFileNotFound() throws ServletException, IOException { + when(mockRequest.getPathInfo()).thenReturn("/nonexistent/resource.txt"); + moduleResourcesServlet.doGet(mockRequest, mockResponse); + verify(mockResponse).setStatus(HttpServletResponse.SC_NOT_FOUND); + } + + @Test + public void doGet_shouldReturn404WhenNoModuleHandlesPath() throws ServletException, IOException { + when(mockRequest.getPathInfo()).thenReturn("/unknown/resource.txt"); + moduleResourcesServlet.doGet(mockRequest, mockResponse); + verify(mockResponse).setStatus(HttpServletResponse.SC_NOT_FOUND); + } + + @Test + public void doGet_shouldHandleEmptyPath() throws ServletException, IOException { + when(mockRequest.getPathInfo()).thenReturn(""); + moduleResourcesServlet.doGet(mockRequest, mockResponse); + verify(mockResponse).setStatus(HttpServletResponse.SC_NOT_FOUND); + } + + @Test + public void doGet_shouldHandleNullPath() throws ServletException, IOException { + when(mockRequest.getPathInfo()).thenReturn(null); + moduleResourcesServlet.doGet(mockRequest, mockResponse); + verify(mockResponse).setStatus(HttpServletResponse.SC_NOT_FOUND); + } +} diff --git a/web/src/test/java/org/openmrs/module/web/ModuleServletTest.java b/web/src/test/java/org/openmrs/module/web/ModuleServletTest.java new file mode 100644 index 000000000000..eaa141835db8 --- /dev/null +++ b/web/src/test/java/org/openmrs/module/web/ModuleServletTest.java @@ -0,0 +1,91 @@ +/** + * This Source Code Form is subject to the terms of the Mozilla Public License, + * v. 2.0. If a copy of the MPL was not distributed with this file, You can + * obtain one at http://mozilla.org/MPL/2.0/. OpenMRS is also distributed under + * the terms of the Healthcare Disclaimer located at http://openmrs.org/license. + * + * Copyright (C) OpenMRS Inc. OpenMRS is a registered trademark and the OpenMRS + * graphic logo is a trademark of OpenMRS Inc. + */ +package org.openmrs.module.web; + +import java.io.IOException; + +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Tests for {@link ModuleServlet} + */ +public class ModuleServletTest { + + private ModuleServlet moduleServlet; + + private HttpServletRequest mockRequest; + + private HttpServletResponse mockResponse; + + @BeforeEach + public void setup() { + moduleServlet = new ModuleServlet(); + mockRequest = mock(HttpServletRequest.class); + mockResponse = mock(HttpServletResponse.class); + } + + @Test + public void service_shouldReturn404WhenServletNotFound() throws ServletException, IOException { + when(mockRequest.getPathInfo()).thenReturn("/unknownservlet"); + moduleServlet.service(mockRequest, mockResponse); + verify(mockResponse).setStatus(HttpServletResponse.SC_NOT_FOUND); + } + + @Test + public void service_shouldExtractServletNameFromPath() throws ServletException, IOException { + when(mockRequest.getPathInfo()).thenReturn("/testservlet"); + moduleServlet.service(mockRequest, mockResponse); + verify(mockResponse).setStatus(HttpServletResponse.SC_NOT_FOUND); + } + + @Test + public void service_shouldHandlePathWithMultipleSlashes() throws ServletException, IOException { + when(mockRequest.getPathInfo()).thenReturn("/path/to/servlet/resource/file.txt"); + moduleServlet.service(mockRequest, mockResponse); + verify(mockResponse).setStatus(HttpServletResponse.SC_NOT_FOUND); + } + + @Test + public void service_shouldHandlePathWithoutForwardSlash() throws ServletException, IOException { + when(mockRequest.getPathInfo()).thenReturn("servlet"); + moduleServlet.service(mockRequest, mockResponse); + verify(mockResponse).setStatus(HttpServletResponse.SC_NOT_FOUND); + } + + @Test + public void service_shouldTrimEndIfIndexOfReturnsNegative() throws ServletException, IOException { + when(mockRequest.getPathInfo()).thenReturn("/servlet"); + moduleServlet.service(mockRequest, mockResponse); + verify(mockResponse).setStatus(HttpServletResponse.SC_NOT_FOUND); + } + + @Test + public void service_shouldHandleEmptyPath() throws ServletException, IOException { + when(mockRequest.getPathInfo()).thenReturn(""); + moduleServlet.service(mockRequest, mockResponse); + verify(mockResponse).setStatus(HttpServletResponse.SC_NOT_FOUND); + } + + @Test + public void service_shouldHandleRootPath() throws ServletException, IOException { + when(mockRequest.getPathInfo()).thenReturn("/"); + moduleServlet.service(mockRequest, mockResponse); + verify(mockResponse).setStatus(HttpServletResponse.SC_NOT_FOUND); + } +} diff --git a/web/src/test/java/org/openmrs/web/filter/initialization/InitializationFilterTest.java b/web/src/test/java/org/openmrs/web/filter/initialization/InitializationFilterTest.java index c21dc2afe3f7..5abfd7ead406 100644 --- a/web/src/test/java/org/openmrs/web/filter/initialization/InitializationFilterTest.java +++ b/web/src/test/java/org/openmrs/web/filter/initialization/InitializationFilterTest.java @@ -181,6 +181,43 @@ public void initializeWizardFromResolvedPropertiesIfPresent_shouldInitializeWiza assertEquals("Admin123", model.adminUserPassword); } + @Test + public void initializeWizardFromResolvedPropertiesIfPresent_shouldHandleBlankEnvironmentVariables() { + InitializationFilter spyFilter = spy(filter); + Map fakeEnv = new HashMap<>(); + fakeEnv.put("CONNECTION_URL", ""); + doReturn(fakeEnv).when(spyFilter).getEnvironmentVariables(); + doReturn(new Properties()).when(spyFilter).getInstallationScript(); + spyFilter.initializeWizardFromResolvedPropertiesIfPresent(); + assertNotNull(spyFilter.wizardModel); + } + + @Test + public void initializeWizardFromResolvedPropertiesIfPresent_shouldHandleMultipleEnvironmentVariables() { + InitializationFilter spyFilter = spy(filter); + Map fakeEnv = new HashMap<>(); + fakeEnv.put("CONNECTION_URL", "jdbc:mysql://env-var:3306/openmrs"); + fakeEnv.put("CONNECTION_USERNAME", "env_user"); + fakeEnv.put("CONNECTION_PASSWORD", "env_pass"); + doReturn(fakeEnv).when(spyFilter).getEnvironmentVariables(); + doReturn(new Properties()).when(spyFilter).getInstallationScript(); + spyFilter.initializeWizardFromResolvedPropertiesIfPresent(); + assertEquals("jdbc:mysql://env-var:3306/openmrs", spyFilter.wizardModel.databaseConnection); + assertEquals("env_user", spyFilter.wizardModel.currentDatabaseUsername); + assertEquals("env_pass", spyFilter.wizardModel.currentDatabasePassword); + } + + @Test + public void initializeWizardFromResolvedPropertiesIfPresent_shouldProcessPostgresqlConnections() { + InitializationFilter spyFilter = spy(filter); + Map fakeEnv = new HashMap<>(); + fakeEnv.put("CONNECTION_URL", "jdbc:postgresql://localhost:5432/openmrs"); + doReturn(fakeEnv).when(spyFilter).getEnvironmentVariables(); + doReturn(new Properties()).when(spyFilter).getInstallationScript(); + spyFilter.initializeWizardFromResolvedPropertiesIfPresent(); + assertEquals("jdbc:postgresql://localhost:5432/openmrs", spyFilter.wizardModel.databaseConnection); + } + private static Properties getInstallScript() { Properties installScript = new Properties(); installScript.setProperty("install_method", "auto"); diff --git a/web/src/test/java/org/openmrs/web/filter/update/UpdateFilterTest.java b/web/src/test/java/org/openmrs/web/filter/update/UpdateFilterTest.java index 9a28ba68528a..75b9a711d709 100644 --- a/web/src/test/java/org/openmrs/web/filter/update/UpdateFilterTest.java +++ b/web/src/test/java/org/openmrs/web/filter/update/UpdateFilterTest.java @@ -78,4 +78,36 @@ public void isSuperUser_shouldReturnFalseIfGivenUserDoesNotHaveTheSuperUserRole( assertFalse(new UpdateFilter().isSuperUser(getConnection(), 502)); } + /** + * Test authentication with empty username + */ + @Test + public void authenticateAsSuperUser_shouldReturnFalseIfGivenEmptyUsername() throws ServletException { + assertFalse(new UpdateFilter().authenticateAsSuperUser("", "test")); + } + + /** + * Test authentication with empty password + */ + @Test + public void authenticateAsSuperUser_shouldReturnFalseIfGivenEmptyPassword() throws ServletException { + assertFalse(new UpdateFilter().authenticateAsSuperUser("admin", "")); + } + + /** + * Test authentication with null username + */ + @Test + public void authenticateAsSuperUser_shouldReturnFalseIfGivenNullUsername() throws ServletException { + assertFalse(new UpdateFilter().authenticateAsSuperUser(null, "test")); + } + + /** + * Test authentication with null password + */ + @Test + public void authenticateAsSuperUser_shouldReturnFalseIfGivenNullPassword() throws ServletException { + assertFalse(new UpdateFilter().authenticateAsSuperUser("admin", null)); + } + } diff --git a/web/src/test/resources/log4j2.xml b/web/src/test/resources/log4j2.xml index 39700e14453d..b36e9bef196d 100644 --- a/web/src/test/resources/log4j2.xml +++ b/web/src/test/resources/log4j2.xml @@ -14,7 +14,7 @@ - + diff --git a/webapp/src/main/resources/log4j2.xml b/webapp/src/main/resources/log4j2.xml index 556aeabd4ed5..91a4067a8b0e 100644 --- a/webapp/src/main/resources/log4j2.xml +++ b/webapp/src/main/resources/log4j2.xml @@ -15,7 +15,7 @@ - %p - %C{1}.%M(%L) |%d{ISO8601}| %m%n + %p - %C{1}.%M(%L) |%d{ISO8601}| %replace{%m}{[\r\n]}{_}%n From 2cad134def9b5dc529cc937fa1a5ebedc911fd91 Mon Sep 17 00:00:00 2001 From: Juliet wamalwa Date: Wed, 27 May 2026 16:53:47 +0300 Subject: [PATCH 2/2] Trigger CI rerun