From b099a2ca7a442e03407303c450b7a056e45c8e28 Mon Sep 17 00:00:00 2001 From: Sajeer Date: Wed, 9 Sep 2026 17:07:58 +0530 Subject: [PATCH 01/13] Resolving httpEndpoint variables via ServerConfigDocument before building the container -p flags --- .../tools/common/CommonLoggerI.java | 21 ++ .../tools/common/plugins/util/DevUtil.java | 133 +++++++- .../plugins/util/DevUtilResolvePortTest.java | 311 ++++++++++++++++++ 3 files changed, 461 insertions(+), 4 deletions(-) create mode 100644 src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java diff --git a/src/main/java/io/openliberty/tools/common/CommonLoggerI.java b/src/main/java/io/openliberty/tools/common/CommonLoggerI.java index 62e859974..b39b4e726 100644 --- a/src/main/java/io/openliberty/tools/common/CommonLoggerI.java +++ b/src/main/java/io/openliberty/tools/common/CommonLoggerI.java @@ -17,6 +17,27 @@ public interface CommonLoggerI { + /** + * Returns a no-op {@code CommonLoggerI} that silently discards all log calls. + * Useful when a logger instance is required by an API but no output is desired. + */ + static CommonLoggerI noop() { + return NoopLogger.INSTANCE; + } + + /** Singleton backing {@link #noop()}. */ + final class NoopLogger implements CommonLoggerI { + static final NoopLogger INSTANCE = new NoopLogger(); + private NoopLogger() {} + @Override public void debug(String msg) {} + @Override public void debug(String msg, Throwable e) {} + @Override public void debug(Throwable e) {} + @Override public void warn(String msg) {} + @Override public void info(String msg) {} + @Override public void error(String msg) {} + @Override public boolean isDebugEnabled() { return false; } + } + /** * Log debug * diff --git a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java index c746a584f..28b3d2b04 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java +++ b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java @@ -82,6 +82,8 @@ import com.sun.nio.file.SensitivityWatchEventModifier; import io.openliberty.tools.ant.ServerTask; +import io.openliberty.tools.common.CommonLoggerI; +import io.openliberty.tools.common.plugins.config.ServerConfigDocument; import io.openliberty.tools.common.plugins.util.ServerFeatureUtil.FeaturesPlatforms; import javax.xml.stream.XMLOutputFactory; @@ -1694,13 +1696,15 @@ private String[] getContainerCommand() throws IOException, PluginExecutionExcept List heldSockets = new ArrayList(); try { if (!skipDefaultPorts) { - int httpPortToUse = findAndHoldPort(LIBERTY_DEFAULT_HTTP_PORT, false, heldSockets); - int httpsPortToUse = findAndHoldPort(LIBERTY_DEFAULT_HTTPS_PORT, false, heldSockets); + int effectiveHttpPort = resolveEffectiveContainerPort(LIBERTY_DEFAULT_HTTP_PORT, "httpPort"); + int effectiveHttpsPort = resolveEffectiveContainerPort(LIBERTY_DEFAULT_HTTPS_PORT, "httpsPort"); + int httpPortToUse = findAndHoldPort(effectiveHttpPort, false, heldSockets); + int httpsPortToUse = findAndHoldPort(effectiveHttpsPort, false, heldSockets); commandElements.add("-p"); - commandElements.add(httpPortToUse+":"+LIBERTY_DEFAULT_HTTP_PORT); + commandElements.add(httpPortToUse + ":" + effectiveHttpPort); commandElements.add("-p"); - commandElements.add(httpsPortToUse+":"+LIBERTY_DEFAULT_HTTPS_PORT); + commandElements.add(httpsPortToUse + ":" + effectiveHttpsPort); } if (libertyDebug) { @@ -1740,6 +1744,17 @@ private String[] getContainerCommand() throws IOException, PluginExecutionExcept commandElements.add("-v"); commandElements.add(looseApplicationProjectRoot.getAbsolutePath() + ":" + DEVMODE_DIR_NAME); + // Mount configDropins/defaults and configDropins/overrides into the container + // so Liberty sees the same variable overrides (e.g. liberty-plugin-variable-config.xml) + // that port resolution used, ensuring Liberty starts on the published port. + for (String subDir : new String[]{"defaults", "overrides"}) { + File configDropinsDir = new File(serverDirectory, "configDropins/" + subDir); + if (configDropinsDir.isDirectory()) { + commandElements.add("-v"); + commandElements.add(configDropinsDir.getAbsolutePath() + ":/config/configDropins/" + subDir); + } + } + // mount the server logs directory over the /logs used by the open liberty container as defined by the LOG_DIR env. var. File logsDir = new File(serverDirectory.getAbsolutePath(), "logs"); commandElements.add("-v"); @@ -1805,6 +1820,116 @@ private String[] getContainerCommand() throws IOException, PluginExecutionExcept } } + /** + * Resolves the effective Liberty HTTP or HTTPS port by reading the server configuration + * using {@link ServerConfigDocument}. + * Returns {@code defaultPort} on any failure (missing files, parse errors, absent + * {@code }, or non-integer resolved value). + * + * @param defaultPort the Liberty default to fall back to (9080 or 9443) + * @param endpointAttr the {@code httpEndpoint} attribute name: {@code "httpPort"} or + * {@code "httpsPort"} + * @return the resolved effective port, or {@code defaultPort} on any failure + */ + // package-private for unit testing + int resolveEffectiveContainerPort(int defaultPort, String endpointAttr) { + // Prefer serverXmlFile set by watchFiles(); fall back to configDirectory/server.xml + // for calls from startContainer() that happen before watchFiles() runs. + File effectiveServerXml = (serverXmlFile != null && serverXmlFile.isFile()) + ? serverXmlFile + : (configDirectory != null ? new File(configDirectory, "server.xml") : null); + if (effectiveServerXml == null || !effectiveServerXml.isFile()) { + return defaultPort; + } + try { + File pluginConfigXml = (buildDirectory != null) + ? new File(buildDirectory, "liberty-plugin-config.xml") : null; + File installDir = readTextElement(pluginConfigXml, "installDirectory"); + File userDir = readTextElement(pluginConfigXml, "userDirectory"); + + // Fall back to serverDirectory so that at minimum server.env and + // bootstrap.properties are still picked up by ServerConfigDocument. + if (installDir == null) installDir = serverDirectory; + if (userDir == null) userDir = serverDirectory; + + // serverDirectory is where the plugin writes configDropins/overrides/ + // liberty-plugin-variable-config.xml (liberty.var.* values). configDirectory + // is the source config dir and does not contain configDropins at runtime. + ServerConfigDocument scd = new ServerConfigDocument( + CommonLoggerI.noop(), effectiveServerXml, + installDir, userDir, serverDirectory, serverDirectory); + + // Read the raw httpEndpoint attribute value from server.xml. + Document doc = scd.parseDocument(effectiveServerXml); + if (doc == null) { + return defaultPort; + } + javax.xml.xpath.XPath xp = javax.xml.xpath.XPathFactory.newInstance().newXPath(); + org.w3c.dom.Element endpoint = (org.w3c.dom.Element) + xp.compile("/server/httpEndpoint").evaluate(doc, javax.xml.xpath.XPathConstants.NODE); + if (endpoint == null) { + return defaultPort; + } + String raw = endpoint.getAttribute(endpointAttr); + if (raw == null || raw.trim().isEmpty()) { + return defaultPort; + } + raw = raw.trim(); + + // Literal integer — no variable resolution needed. + try { + return Integer.parseInt(raw); + } catch (NumberFormatException ignored) { /* fall through */ } + + // Variable reference — resolve through the fully-loaded variable maps. + String resolved = VariableUtility.resolveVariables( + CommonLoggerI.noop(), raw, null, + scd.getProperties(), scd.getDefaultProperties(), + scd.getLibertyDirPropertyFiles()); + if (resolved == null) { + return defaultPort; + } + try { + return Integer.parseInt(resolved.trim()); + } catch (NumberFormatException e) { + return defaultPort; + } + } catch (Exception e) { + debug("resolveEffectiveContainerPort: could not resolve port, using default " + defaultPort + ": " + e.getMessage()); + return defaultPort; + } + } + + /** + * Reads the text content of the first element matching {@code tagName} in an XML file, + * returning a {@link File} for that path, or {@code null} if absent or unreadable. + */ + private File readTextElement(File xmlFile, String tagName) { + if (xmlFile == null || !xmlFile.isFile()) { + return null; + } + try { + DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); + dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false); + dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + dbf.setFeature("http://xml.org/sax/features/external-general-entities", false); + dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + dbf.setXIncludeAware(false); + dbf.setExpandEntityReferences(false); + Document doc = dbf.newDocumentBuilder().parse(xmlFile); + NodeList nodes = doc.getElementsByTagName(tagName); + if (nodes.getLength() == 0) { + return null; + } + String text = nodes.item(0).getTextContent(); + return (text != null && !text.trim().isEmpty()) ? new File(text.trim()) : null; + } catch (Exception e) { + return null; + } + } + /** * Finds an available port starting from {@code preferred} and immediately binds a * {@code ServerSocket} on it to hold that port open until the caller is done building diff --git a/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java new file mode 100644 index 000000000..ffe74fc9f --- /dev/null +++ b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java @@ -0,0 +1,311 @@ +/** + * (C) Copyright IBM Corporation 2026. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.openliberty.tools.common.plugins.util; + +import static org.junit.Assert.assertEquals; + +import java.io.File; +import java.io.FileWriter; +import java.io.IOException; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +/** + * Unit tests for {@link DevUtil#resolveEffectiveContainerPort}. + * + * Each test creates a minimal on-disk Liberty server structure inside a + * {@link TemporaryFolder} and calls the package-private method directly + * through the {@link BaseDevUtilTest.DevTestUtil} subclass. + */ +public class DevUtilResolvePortTest extends BaseDevUtilTest { + + @Rule + public TemporaryFolder tmp = new TemporaryFolder(); + + // ------------------------------------------------------------------ + // Helpers + // ------------------------------------------------------------------ + + /** Creates a server.xml with a literal httpEndpoint. */ + private File writeServerXml(File serverDir, String httpPort, String httpsPort) throws IOException { + File serverXml = new File(serverDir, "server.xml"); + String port = httpPort != null ? " httpPort=\"" + httpPort + "\"" : ""; + String sport = httpsPort != null ? " httpsPort=\"" + httpsPort + "\"" : ""; + write(serverXml, + "\n" + + " \n" + + ""); + return serverXml; + } + + /** Creates a server.xml with a variable reference in httpEndpoint. */ + private File writeServerXmlWithVar(File serverDir, String varName) throws IOException { + File serverXml = new File(serverDir, "server.xml"); + write(serverXml, + "\n" + + " \n" + + " \n" + + ""); + return serverXml; + } + + private void write(File f, String content) throws IOException { + f.getParentFile().mkdirs(); + try (FileWriter fw = new FileWriter(f)) { fw.write(content); } + } + + private DevTestUtil util(File serverDir, File buildDir, File serverXmlFile) throws IOException { + DevTestUtil u = new DevTestUtil(serverDir, buildDir); + u.serverXmlFile = serverXmlFile; + return u; + } + + // ------------------------------------------------------------------ + // Tests + // ------------------------------------------------------------------ + + @Test + public void testLiteralHttpPort() throws Exception { + File serverDir = tmp.newFolder("server"); + File serverXml = writeServerXml(serverDir, "9090", null); + DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); + + assertEquals(9090, u.resolveEffectiveContainerPort(9080, "httpPort")); + } + + @Test + public void testLiteralHttpsPort() throws Exception { + File serverDir = tmp.newFolder("server"); + File serverXml = writeServerXml(serverDir, null, "9453"); + DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); + + assertEquals(9453, u.resolveEffectiveContainerPort(9443, "httpsPort")); + } + + @Test + public void testDefaultPortReturnedWhenNoHttpEndpoint() throws Exception { + File serverDir = tmp.newFolder("server"); + File serverXml = new File(serverDir, "server.xml"); + write(serverXml, "servlet-4.0"); + DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); + + assertEquals(9080, u.resolveEffectiveContainerPort(9080, "httpPort")); + } + + @Test + public void testDefaultPortReturnedWhenServerXmlMissing() throws Exception { + File serverDir = tmp.newFolder("server"); + DevTestUtil u = util(serverDir, tmp.newFolder("build"), new File(serverDir, "nonexistent.xml")); + + assertEquals(9080, u.resolveEffectiveContainerPort(9080, "httpPort")); + } + + @Test + public void testVariableResolvedFromDefaultValue() throws Exception { + File serverDir = tmp.newFolder("server"); + File serverXml = writeServerXmlWithVar(serverDir, "myHttpPort"); + DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); + + assertEquals(9090, u.resolveEffectiveContainerPort(9080, "httpPort")); + } + + @Test + public void testVariableResolvedFromBootstrapProperties() throws Exception { + File serverDir = tmp.newFolder("server"); + // server.xml references a variable with no defaultValue + File serverXml = new File(serverDir, "server.xml"); + write(serverXml, + "\n" + + " \n" + + ""); + // bootstrap.properties defines the variable + write(new File(serverDir, "bootstrap.properties"), "http.port=9091\n"); + DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); + + assertEquals(9091, u.resolveEffectiveContainerPort(9080, "httpPort")); + } + + @Test + public void testVariableResolvedFromServerEnv() throws Exception { + File serverDir = tmp.newFolder("server"); + File serverXml = new File(serverDir, "server.xml"); + write(serverXml, + "\n" + + " \n" + + ""); + write(new File(serverDir, "server.env"), "HTTP_PORT=9092\n"); + DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); + + assertEquals(9092, u.resolveEffectiveContainerPort(9080, "httpPort")); + } + + @Test + public void testVariableResolvedFromConfigDropinsOverrides() throws Exception { + File serverDir = tmp.newFolder("server"); + File serverXml = new File(serverDir, "server.xml"); + write(serverXml, + "\n" + + " \n" + + ""); + // configDropins/overrides sets the variable at the highest precedence + File overrides = new File(serverDir, "configDropins/overrides"); + overrides.mkdirs(); + write(new File(overrides, "port-override.xml"), + "\n" + + " \n" + + ""); + DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); + + assertEquals(9095, u.resolveEffectiveContainerPort(9080, "httpPort")); + } + + @Test + public void testOverridePrecedenceOverDefault() throws Exception { + File serverDir = tmp.newFolder("server"); + // server.xml has a defaultValue of 9090; bootstrap.properties overrides to 9093 + File serverXml = new File(serverDir, "server.xml"); + write(serverXml, + "\n" + + " \n" + + " \n" + + ""); + write(new File(serverDir, "bootstrap.properties"), "http.port=9093\n"); + DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); + + // bootstrap.properties (step 3) overrides defaultValue (step 1) + assertEquals(9093, u.resolveEffectiveContainerPort(9080, "httpPort")); + } + + @Test + public void testDefaultPortWhenPortIsNotANumber() throws Exception { + File serverDir = tmp.newFolder("server"); + File serverXml = new File(serverDir, "server.xml"); + write(serverXml, + "\n" + + " \n" + + ""); + DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); + + // Variable is not defined anywhere — should fall back to default + assertEquals(9080, u.resolveEffectiveContainerPort(9080, "httpPort")); + } + + @Test + public void testFallsBackToConfigDirectoryWhenServerXmlFileIsNull() throws Exception { + // serverXmlFile is null — resolveEffectiveContainerPort must fall back to configDirectory/server.xml. + // Use separate dirs matching the real Maven layout: + // configDirectory = src/main/liberty/config (source — this is where the fallback reads from) + // serverDirectory = target/.../defaultServer (runtime — SCD looks for server.xml here) + // The plugin copies server.xml from source to server dir at deploy time, so both dirs have it. + File configDir = tmp.newFolder("src-config"); + File serverDir = tmp.newFolder("server"); + String content = "\n" + + " \n" + + ""; + write(new File(configDir, "server.xml"), content); + write(new File(serverDir, "server.xml"), content); // deployed copy for ServerConfigDocument + DevTestUtil u = new DevTestUtil(serverDir, null, null, configDir, + java.util.Collections.emptyList(), java.util.Collections.emptyList(), false, false); + + assertEquals(9097, u.resolveEffectiveContainerPort(9080, "httpPort")); + } + + @Test + public void testPluginConfigXmlProvidesInstallAndUserDir() throws Exception { + File serverDir = tmp.newFolder("server"); + File installDir = tmp.newFolder("wlp"); + File userDir = tmp.newFolder("usr"); + File buildDir = tmp.newFolder("build"); + + // Write server.env to the install dir (install/etc/server.env) — highest priority path + File etcDir = new File(installDir, "etc"); + etcDir.mkdirs(); + File serverXml = new File(serverDir, "server.xml"); + write(serverXml, + "\n" + + " \n" + + ""); + write(new File(etcDir, "server.env"), "env.port=9096\n"); + + // liberty-plugin-config.xml in buildDir points to installDir and userDir + File pluginConfig = new File(buildDir, "liberty-plugin-config.xml"); + write(pluginConfig, + "\n" + + " " + installDir.getAbsolutePath() + "\n" + + " " + userDir.getAbsolutePath() + "\n" + + ""); + + DevTestUtil u = util(serverDir, buildDir, serverXml); + + assertEquals(9096, u.resolveEffectiveContainerPort(9080, "httpPort")); + } + + @Test + public void testVariableResolvedFromServerDirConfigDropinsOverrides() throws Exception { + // Mirrors the real Maven/Gradle scenario: + // configDirectory = src/main/liberty/config (source config — no configDropins here) + // serverDirectory = target/.../defaultServer (runtime — plugin copies server.xml here + // and writes liberty-plugin-variable-config.xml + // into configDropins/overrides/) + // ServerConfigDocument uses serverDirectory as SERVER_CONFIG_DIR, so it finds + // both server.xml and configDropins/overrides/ under the same root. + File srcConfigDir = tmp.newFolder("src-config"); // DevUtil.configDirectory (source) + File serverDir = tmp.newFolder("server"); // DevUtil.serverDirectory (runtime) + + // The plugin copies server.xml from srcConfigDir into serverDir at deploy time. + // serverXmlFile points to the source copy; serverDir also has the deployed copy. + String serverXmlContent = + "\n" + + " \n" + + ""; + File srcServerXml = new File(srcConfigDir, "server.xml"); + File deployedServerXml = new File(serverDir, "server.xml"); + write(srcServerXml, serverXmlContent); + write(deployedServerXml, serverXmlContent); // deployed copy — SCD finds this via SERVER_CONFIG_DIR + + // liberty-plugin-variable-config.xml written by plugin into serverDir/configDropins/overrides/ + File overrides = new File(serverDir, "configDropins/overrides"); + overrides.mkdirs(); + write(new File(overrides, "liberty-plugin-variable-config.xml"), + "\n" + + " \n" + + ""); + + // DevUtil: configDirectory = srcConfigDir, serverDirectory = serverDir + // serverXmlFile points to source server.xml (set by watchFiles in real usage) + DevTestUtil u = new DevTestUtil(serverDir, null, null, srcConfigDir, + java.util.Collections.emptyList(), java.util.Collections.emptyList(), false, false); + u.serverXmlFile = srcServerXml; + + assertEquals(9090, u.resolveEffectiveContainerPort(9080, "httpPort")); + } + + @Test + public void testVariableResolvedForHttpsPort() throws Exception { + File serverDir = tmp.newFolder("server"); + File serverXml = new File(serverDir, "server.xml"); + write(serverXml, + "\n" + + " \n" + + " \n" + + ""); + DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); + + assertEquals(9453, u.resolveEffectiveContainerPort(9443, "httpsPort")); + } +} From 9eef6a5428b1b2cca1987c40aa0bbd69611f4d92 Mon Sep 17 00:00:00 2001 From: Sajeer Date: Wed, 9 Sep 2026 17:47:13 +0530 Subject: [PATCH 02/13] Imports fixed --- .../openliberty/tools/common/plugins/util/DevUtil.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java index 28b3d2b04..c63f3d7d1 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java +++ b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java @@ -89,6 +89,9 @@ import javax.xml.stream.XMLOutputFactory; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamWriter; +import javax.xml.xpath.XPath; +import javax.xml.xpath.XPathConstants; +import javax.xml.xpath.XPathFactory; import org.apache.commons.io.FileUtils; import org.apache.commons.io.filefilter.NameFileFilter; @@ -100,6 +103,7 @@ import org.json.JSONException; import org.json.JSONObject; import org.w3c.dom.Document; +import org.w3c.dom.Element; import org.w3c.dom.Node; import org.w3c.dom.NodeList; import org.xml.sax.SAXException; @@ -1864,9 +1868,9 @@ int resolveEffectiveContainerPort(int defaultPort, String endpointAttr) { if (doc == null) { return defaultPort; } - javax.xml.xpath.XPath xp = javax.xml.xpath.XPathFactory.newInstance().newXPath(); - org.w3c.dom.Element endpoint = (org.w3c.dom.Element) - xp.compile("/server/httpEndpoint").evaluate(doc, javax.xml.xpath.XPathConstants.NODE); + XPath xp = XPathFactory.newInstance().newXPath(); + Element endpoint = (Element) + xp.compile("/server/httpEndpoint").evaluate(doc, XPathConstants.NODE); if (endpoint == null) { return defaultPort; } From 91152157121a6b75c420a6bb8dc859098189c09e Mon Sep 17 00:00:00 2001 From: Sajeer Date: Wed, 9 Sep 2026 18:07:30 +0530 Subject: [PATCH 03/13] pluginConfigXml is now read first --- .../tools/common/plugins/util/DevUtil.java | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java index c63f3d7d1..ace56e549 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java +++ b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java @@ -1837,19 +1837,28 @@ private String[] getContainerCommand() throws IOException, PluginExecutionExcept */ // package-private for unit testing int resolveEffectiveContainerPort(int defaultPort, String endpointAttr) { - // Prefer serverXmlFile set by watchFiles(); fall back to configDirectory/server.xml - // for calls from startContainer() that happen before watchFiles() runs. - File effectiveServerXml = (serverXmlFile != null && serverXmlFile.isFile()) - ? serverXmlFile - : (configDirectory != null ? new File(configDirectory, "server.xml") : null); - if (effectiveServerXml == null || !effectiveServerXml.isFile()) { - return defaultPort; - } try { + // Read liberty-plugin-config.xml first — it contains the resolved paths for + // configFile (custom server.xml), installDirectory, and userDirectory. File pluginConfigXml = (buildDirectory != null) ? new File(buildDirectory, "liberty-plugin-config.xml") : null; File installDir = readTextElement(pluginConfigXml, "installDirectory"); File userDir = readTextElement(pluginConfigXml, "userDirectory"); + // Use the configFile path from the plugin config if available; that is the + // user-specified server.xml (serverXmlFile parameter). Fall back to + // serverXmlFile set by watchFiles(), then to configDirectory/server.xml. + File configFileFromPlugin = readTextElement(pluginConfigXml, "configFile"); + File effectiveServerXml; + if (configFileFromPlugin != null && configFileFromPlugin.isFile()) { + effectiveServerXml = configFileFromPlugin; + } else if (serverXmlFile != null && serverXmlFile.isFile()) { + effectiveServerXml = serverXmlFile; + } else { + effectiveServerXml = (configDirectory != null ? new File(configDirectory, "server.xml") : null); + } + if (effectiveServerXml == null || !effectiveServerXml.isFile()) { + return defaultPort; + } // Fall back to serverDirectory so that at minimum server.env and // bootstrap.properties are still picked up by ServerConfigDocument. From 03de02e0ee4c29ce0d5ace16b7beac0e57383e96 Mon Sep 17 00:00:00 2001 From: Sajeer Date: Fri, 11 Sep 2026 14:19:22 +0530 Subject: [PATCH 04/13] Moved readTextElementFromXmlFile to XmlDocument --- .../common/plugins/config/XmlDocument.java | 31 ++++++++++++++++ .../tools/common/plugins/util/DevUtil.java | 37 +++---------------- 2 files changed, 37 insertions(+), 31 deletions(-) diff --git a/src/main/java/io/openliberty/tools/common/plugins/config/XmlDocument.java b/src/main/java/io/openliberty/tools/common/plugins/config/XmlDocument.java index 559acfa2f..1fc6a66f7 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/config/XmlDocument.java +++ b/src/main/java/io/openliberty/tools/common/plugins/config/XmlDocument.java @@ -38,6 +38,7 @@ import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.Node; +import org.w3c.dom.NodeList; import org.w3c.dom.Text; import org.xml.sax.SAXException; @@ -112,6 +113,36 @@ protected boolean isWhitespace(Node node) { return node != null && node instanceof Text && ((Text)node).getData().trim().isEmpty(); } + /** + * Reads the text content of the first element matching {@code tagName} in an XML file, + * or {@code null} if the file is absent, the tag is missing, or any parse error occurs. + */ + public static String readTextElementFromXmlFile(File xmlFile, String tagName) { + if (xmlFile == null || !xmlFile.isFile()) { + return null; + } + try { + DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); + dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false); + dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + dbf.setFeature("http://xml.org/sax/features/external-general-entities", false); + dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + dbf.setXIncludeAware(false); + dbf.setExpandEntityReferences(false); + Document doc = dbf.newDocumentBuilder().parse(xmlFile); + NodeList nodes = doc.getElementsByTagName(tagName); + if (nodes.getLength() == 0) { + return null; + } + String text = nodes.item(0).getTextContent(); + return (text != null && !text.trim().isEmpty()) ? text.trim() : null; + } catch (Exception e) { + return null; + } + } + public static void addNewlineBeforeFirstElement(File f) throws IOException { // look for "" and add a newline byte[] contents = Files.readAllBytes(f.toPath()); diff --git a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java index ace56e549..992b889a4 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java +++ b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java @@ -84,6 +84,7 @@ import io.openliberty.tools.ant.ServerTask; import io.openliberty.tools.common.CommonLoggerI; import io.openliberty.tools.common.plugins.config.ServerConfigDocument; +import io.openliberty.tools.common.plugins.config.XmlDocument; import io.openliberty.tools.common.plugins.util.ServerFeatureUtil.FeaturesPlatforms; import javax.xml.stream.XMLOutputFactory; @@ -1842,12 +1843,12 @@ int resolveEffectiveContainerPort(int defaultPort, String endpointAttr) { // configFile (custom server.xml), installDirectory, and userDirectory. File pluginConfigXml = (buildDirectory != null) ? new File(buildDirectory, "liberty-plugin-config.xml") : null; - File installDir = readTextElement(pluginConfigXml, "installDirectory"); - File userDir = readTextElement(pluginConfigXml, "userDirectory"); + File installDir = toFile(XmlDocument.readTextElementFromXmlFile(pluginConfigXml, "installDirectory")); + File userDir = toFile(XmlDocument.readTextElementFromXmlFile(pluginConfigXml, "userDirectory")); // Use the configFile path from the plugin config if available; that is the // user-specified server.xml (serverXmlFile parameter). Fall back to // serverXmlFile set by watchFiles(), then to configDirectory/server.xml. - File configFileFromPlugin = readTextElement(pluginConfigXml, "configFile"); + File configFileFromPlugin = toFile(XmlDocument.readTextElementFromXmlFile(pluginConfigXml, "configFile")); File effectiveServerXml; if (configFileFromPlugin != null && configFileFromPlugin.isFile()) { effectiveServerXml = configFileFromPlugin; @@ -1913,34 +1914,8 @@ int resolveEffectiveContainerPort(int defaultPort, String endpointAttr) { } } - /** - * Reads the text content of the first element matching {@code tagName} in an XML file, - * returning a {@link File} for that path, or {@code null} if absent or unreadable. - */ - private File readTextElement(File xmlFile, String tagName) { - if (xmlFile == null || !xmlFile.isFile()) { - return null; - } - try { - DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); - dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false); - dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - dbf.setFeature("http://xml.org/sax/features/external-general-entities", false); - dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - dbf.setXIncludeAware(false); - dbf.setExpandEntityReferences(false); - Document doc = dbf.newDocumentBuilder().parse(xmlFile); - NodeList nodes = doc.getElementsByTagName(tagName); - if (nodes.getLength() == 0) { - return null; - } - String text = nodes.item(0).getTextContent(); - return (text != null && !text.trim().isEmpty()) ? new File(text.trim()) : null; - } catch (Exception e) { - return null; - } + private static File toFile(String path) { + return (path != null) ? new File(path) : null; } /** From 260f3999d9f6461664f6139c38facf2dfa490390 Mon Sep 17 00:00:00 2001 From: Sajeer Date: Tue, 15 Sep 2026 10:24:19 +0530 Subject: [PATCH 05/13] Import issue fixed and added two new test cases --- .../tools/common/plugins/util/DevUtil.java | 57 +++++++++++++- .../plugins/util/DevUtilResolvePortTest.java | 75 ++++++++++++++++++- 2 files changed, 129 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java index 992b889a4..4866f9ac5 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java +++ b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java @@ -1873,7 +1873,8 @@ int resolveEffectiveContainerPort(int defaultPort, String endpointAttr) { CommonLoggerI.noop(), effectiveServerXml, installDir, userDir, serverDirectory, serverDirectory); - // Read the raw httpEndpoint attribute value from server.xml. + // Read the raw httpEndpoint attribute value from server.xml, including + // any files pulled in via elements. Document doc = scd.parseDocument(effectiveServerXml); if (doc == null) { return defaultPort; @@ -1881,6 +1882,10 @@ int resolveEffectiveContainerPort(int defaultPort, String endpointAttr) { XPath xp = XPathFactory.newInstance().newXPath(); Element endpoint = (Element) xp.compile("/server/httpEndpoint").evaluate(doc, XPathConstants.NODE); + if (endpoint == null) { + // Not in the top-level document — walk files. + endpoint = findHttpEndpointInIncludes(doc, effectiveServerXml.getParentFile(), xp, scd); + } if (endpoint == null) { return defaultPort; } @@ -1918,6 +1923,56 @@ private static File toFile(String path) { return (path != null) ? new File(path) : null; } + /** + * Recursively walks {@code } elements in {@code doc} to find an + * {@code } element that is not present in the top-level document. + * Relative include locations are resolved against {@code parentDir}. + * + * @return the first {@code httpEndpoint} {@link Element} found in any included + * document, or {@code null} if none is found + */ + private Element findHttpEndpointInIncludes(Document doc, File parentDir, + XPath xp, ServerConfigDocument scd) { + try { + NodeList includes = (NodeList) + xp.compile("/server/include").evaluate(doc, XPathConstants.NODESET); + for (int i = 0; i < includes.getLength(); i++) { + if (!(includes.item(i) instanceof Element)) { + continue; + } + String loc = ((Element) includes.item(i)).getAttribute("location"); + if (loc == null || loc.trim().isEmpty()) { + continue; + } + // Resolve relative paths against the parent dir of the including file. + File inclFile = new File(loc); + if (!inclFile.isAbsolute()) { + inclFile = new File(parentDir, loc); + } + if (!inclFile.isFile()) { + continue; + } + Document inclDoc = scd.parseDocument(inclFile); + if (inclDoc == null) { + continue; + } + Element endpoint = (Element) + xp.compile("/server/httpEndpoint").evaluate(inclDoc, XPathConstants.NODE); + if (endpoint != null) { + return endpoint; + } + // Recurse into nested includes. + endpoint = findHttpEndpointInIncludes(inclDoc, inclFile.getParentFile(), xp, scd); + if (endpoint != null) { + return endpoint; + } + } + } catch (Exception e) { + debug("findHttpEndpointInIncludes: error walking includes: " + e.getMessage()); + } + return null; + } + /** * Finds an available port starting from {@code preferred} and immediately binds a * {@code ServerSocket} on it to hold that port open until the caller is done building diff --git a/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java index ffe74fc9f..aa9fe571a 100644 --- a/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java +++ b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java @@ -20,6 +20,7 @@ import java.io.File; import java.io.FileWriter; import java.io.IOException; +import java.util.Collections; import org.junit.Rule; import org.junit.Test; @@ -220,7 +221,7 @@ public void testFallsBackToConfigDirectoryWhenServerXmlFileIsNull() throws Excep write(new File(configDir, "server.xml"), content); write(new File(serverDir, "server.xml"), content); // deployed copy for ServerConfigDocument DevTestUtil u = new DevTestUtil(serverDir, null, null, configDir, - java.util.Collections.emptyList(), java.util.Collections.emptyList(), false, false); + Collections.emptyList(), Collections.emptyList(), false, false); assertEquals(9097, u.resolveEffectiveContainerPort(9080, "httpPort")); } @@ -289,7 +290,7 @@ public void testVariableResolvedFromServerDirConfigDropinsOverrides() throws Exc // DevUtil: configDirectory = srcConfigDir, serverDirectory = serverDir // serverXmlFile points to source server.xml (set by watchFiles in real usage) DevTestUtil u = new DevTestUtil(serverDir, null, null, srcConfigDir, - java.util.Collections.emptyList(), java.util.Collections.emptyList(), false, false); + Collections.emptyList(), Collections.emptyList(), false, false); u.serverXmlFile = srcServerXml; assertEquals(9090, u.resolveEffectiveContainerPort(9080, "httpPort")); @@ -308,4 +309,74 @@ public void testVariableResolvedForHttpsPort() throws Exception { assertEquals(9453, u.resolveEffectiveContainerPort(9443, "httpsPort")); } + + /** + * A custom/external server.xml whose httpPort is defined in a sibling file + * pulled in via a relative {@code }. + * + * Layout: + * serverDir/server.xml — contains {@code } + * serverDir/ports.xml — contains the httpEndpoint with the literal port + * + * ServerConfigDocument resolves the relative include against configDirectory + * (= serverDir), so it finds ports.xml and picks up the port value. + */ + @Test + public void testPortDefinedViaRelativeInclude() throws Exception { + File serverDir = tmp.newFolder("server"); + + // ports.xml — the included file that defines the actual port + write(new File(serverDir, "ports.xml"), + "\n" + + " \n" + + ""); + + // server.xml delegates to ports.xml via a relative include + File serverXml = new File(serverDir, "server.xml"); + write(serverXml, + "\n" + + " \n" + + ""); + + DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); + + assertEquals(9094, u.resolveEffectiveContainerPort(9080, "httpPort")); + } + + /** + * A custom/external server.xml with a sibling {@code configDropins} directory + * that supplies the port variable. + * + * Layout: + * serverDir/server.xml — references ${ext.http.port} + * serverDir/configDropins/overrides/port.xml — defines ext.http.port = 9098 + * + * {@code ServerConfigDocument} uses {@code serverDirectory} as its config directory, so + * it finds the sibling {@code configDropins} next to the deployed server.xml. + * This test validates that variables from that sibling {@code configDropins} are + * picked up and used to resolve the port, mirroring the real deployment layout. + */ + @Test + public void testPortFromSiblingConfigDropins() throws Exception { + File serverDir = tmp.newFolder("server"); + + // server.xml references a variable defined only in the sibling configDropins + File serverXml = new File(serverDir, "server.xml"); + write(serverXml, + "\n" + + " \n" + + ""); + + // Sibling configDropins/overrides defines the variable + File overrides = new File(serverDir, "configDropins/overrides"); + overrides.mkdirs(); + write(new File(overrides, "port.xml"), + "\n" + + " \n" + + ""); + + DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); + + assertEquals(9098, u.resolveEffectiveContainerPort(9080, "httpPort")); + } } From 28cfab5c6991a22bffd2f58aec86bd0ee3863fbf Mon Sep 17 00:00:00 2001 From: Sajeer Date: Thu, 17 Sep 2026 14:42:56 +0530 Subject: [PATCH 06/13] Copyright years updated and refactored the document builder logic --- .../tools/common/CommonLoggerI.java | 2 +- .../plugins/config/ServerConfigDocument.java | 42 +++-------- .../common/plugins/config/XmlDocument.java | 72 ++++++++++++------- 3 files changed, 56 insertions(+), 60 deletions(-) diff --git a/src/main/java/io/openliberty/tools/common/CommonLoggerI.java b/src/main/java/io/openliberty/tools/common/CommonLoggerI.java index b39b4e726..809c6a11d 100644 --- a/src/main/java/io/openliberty/tools/common/CommonLoggerI.java +++ b/src/main/java/io/openliberty/tools/common/CommonLoggerI.java @@ -1,5 +1,5 @@ /** - * (C) Copyright IBM Corporation 2019. + * (C) Copyright IBM Corporation 2019, 2026. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java b/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java index 05ce7f487..082892e49 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java +++ b/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java @@ -202,30 +202,8 @@ public ServerConfigDocument(CommonLoggerI log, File originalServerXMLFile, Map docs) * @throws SAXException */ public Document parseDocument(File file) throws FileNotFoundException, IOException { - try (FileInputStream is = new FileInputStream(file)) { - Document document= parseDocument(is); - document.setDocumentURI(file.getCanonicalPath()); - return document; + try { + return XmlDocument.parseDocument(file); } catch (SAXException ex) { // If the file was not valid XML, assume it was some other non XML // file in dropins. - log.info("Skipping parsing " + file.getAbsolutePath() + " because it was not recognized as XML."); + if (log != null) { + log.info("Skipping parsing " + file.getAbsolutePath() + " because it was not recognized as XML."); + } return null; } } @@ -824,14 +802,12 @@ public Document parseDocument(File file) throws FileNotFoundException, IOExcepti private Document parseDocument(URL url) throws IOException, SAXException { URLConnection connection = url.openConnection(); try (InputStream is = connection.getInputStream()) { - return parseDocument(is); + return XmlDocument.parseDocument(is); } } private Document parseDocument(InputStream in) throws SAXException, IOException { - try (InputStream ins = in) { // ins will be auto-closed - return getDocumentBuilder().parse(ins); - } + return XmlDocument.parseDocument(in); } public void parsePropertiesFromFile(File propertiesFile) throws Exception, FileNotFoundException { diff --git a/src/main/java/io/openliberty/tools/common/plugins/config/XmlDocument.java b/src/main/java/io/openliberty/tools/common/plugins/config/XmlDocument.java index 1fc6a66f7..365a4b3d4 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/config/XmlDocument.java +++ b/src/main/java/io/openliberty/tools/common/plugins/config/XmlDocument.java @@ -1,5 +1,5 @@ /** - * (C) Copyright IBM Corporation 2017, 2024. + * (C) Copyright IBM Corporation 2017, 2026. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; import java.io.OutputStreamWriter; import java.nio.charset.StandardCharsets; @@ -56,20 +57,7 @@ public void createDocument(String rootElement) throws ParserConfigurationExcepti } public void createDocument(File xmlFile) throws ParserConfigurationException, SAXException, IOException { - DocumentBuilderFactory builderFactory = DocumentBuilderFactory.newInstance(); - builderFactory.setCoalescing(true); - builderFactory.setIgnoringElementContentWhitespace(true); - builderFactory.setValidating(false); - builderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false); - builderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - builderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - builderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - builderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false); - builderFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - builderFactory.setXIncludeAware(false); - builderFactory.setExpandEntityReferences(false); - DocumentBuilder builder = builderFactory.newDocumentBuilder(); - doc = builder.parse(xmlFile); + doc = parseDocument(xmlFile); } public void writeXMLDocument(String fileName) throws IOException, TransformerException { @@ -114,7 +102,48 @@ protected boolean isWhitespace(Node node) { } /** - * Reads the text content of the first element matching {@code tagName} in an XML file, + * Creates and returns a securely configured {@link DocumentBuilder}. + */ + public static DocumentBuilder getDocumentBuilder() { + DocumentBuilder docBuilder; + DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance(); + docBuilderFactory.setIgnoringComments(true); + docBuilderFactory.setCoalescing(true); + docBuilderFactory.setIgnoringElementContentWhitespace(true); + docBuilderFactory.setValidating(false); + try { + docBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false); + docBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + docBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + docBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + docBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false); + docBuilderFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + docBuilderFactory.setXIncludeAware(false); + docBuilderFactory.setExpandEntityReferences(false); + docBuilder = docBuilderFactory.newDocumentBuilder(); + } catch (ParserConfigurationException e) { + // fail if we can't create a document builder + throw new RuntimeException(e); + } + return docBuilder; + } + + public static Document parseDocument(File file) throws IOException, SAXException { + try (InputStream is = Files.newInputStream(file.toPath())) { + Document document = parseDocument(is); + document.setDocumentURI(file.getCanonicalPath()); + return document; + } + } + + public static Document parseDocument(InputStream in) throws SAXException, IOException { + try (InputStream ins = in) { + return getDocumentBuilder().parse(ins); + } + } + + /** + * Returns the text content of the first element matching {@code tagName} in an XML file, * or {@code null} if the file is absent, the tag is missing, or any parse error occurs. */ public static String readTextElementFromXmlFile(File xmlFile, String tagName) { @@ -122,16 +151,7 @@ public static String readTextElementFromXmlFile(File xmlFile, String tagName) { return null; } try { - DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); - dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false); - dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - dbf.setFeature("http://xml.org/sax/features/external-general-entities", false); - dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - dbf.setXIncludeAware(false); - dbf.setExpandEntityReferences(false); - Document doc = dbf.newDocumentBuilder().parse(xmlFile); + Document doc = parseDocument(xmlFile); NodeList nodes = doc.getElementsByTagName(tagName); if (nodes.getLength() == 0) { return null; From bcecc54936309301e2b9bdd8a0b510d62988db53 Mon Sep 17 00:00:00 2001 From: Sajeer Date: Thu, 17 Sep 2026 16:13:30 +0530 Subject: [PATCH 07/13] Handled duplications --- .../plugins/config/ServerConfigDocument.java | 36 +++++ .../tools/common/plugins/util/DevUtil.java | 142 ++++++------------ .../plugins/util/DevUtilResolvePortTest.java | 15 ++ 3 files changed, 100 insertions(+), 93 deletions(-) diff --git a/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java b/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java index 082892e49..ae9744f95 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java +++ b/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java @@ -53,6 +53,8 @@ import org.apache.commons.io.comparator.NameFileComparator; import org.w3c.dom.Document; import org.w3c.dom.Element; +import org.w3c.dom.NamedNodeMap; +import org.w3c.dom.Node; import org.w3c.dom.NodeList; import org.xml.sax.SAXException; @@ -75,6 +77,7 @@ public class ServerConfigDocument { private Set namelessLocations; private Set locations; private HashMap locationsAndNames; + private Map httpEndpointAttributes; private Properties props; private Properties defaultProps; private Map libertyDirectoryPropertyToFile = null; @@ -86,6 +89,7 @@ public class ServerConfigDocument { private static final XPathExpression XPATH_SERVER_WEB_APPLICATION; private static final XPathExpression XPATH_SERVER_SPRINGBOOT_APPLICATION; private static final XPathExpression XPATH_SERVER_ENTERPRISE_APPLICATION; + private static final XPathExpression XPATH_SERVER_HTTP_ENDPOINT; private static final XPathExpression XPATH_SERVER_INCLUDE; public static final XPathExpression XPATH_SERVER_VARIABLE; private static final XPathExpression XPATH_ALL_SERVER_APPLICATIONS; @@ -102,6 +106,7 @@ public class ServerConfigDocument { XPATH_SERVER_WEB_APPLICATION = xPath.compile("/server/webApplication"); XPATH_SERVER_SPRINGBOOT_APPLICATION = xPath.compile("/server/springBootApplication"); XPATH_SERVER_ENTERPRISE_APPLICATION = xPath.compile("/server/enterpriseApplication"); + XPATH_SERVER_HTTP_ENDPOINT = xPath.compile("/server/httpEndpoint"); XPATH_SERVER_INCLUDE = xPath.compile("/server/include"); XPATH_SERVER_VARIABLE = xPath.compile("/server/variable"); XPATH_ALL_SERVER_APPLICATIONS = xPath.compile("/server/application | /server/webApplication | /server/enterpriseApplication | /server/springBootApplication"); @@ -127,6 +132,10 @@ public Set getNamelessLocations() { return namelessLocations; } + public Map getHttpEndpointAttributes() { + return httpEndpointAttributes; + } + public Properties getProperties() { return props; } @@ -165,6 +174,7 @@ public ServerConfigDocument(CommonLoggerI log, File originalServerXMLFile, Map(); namelessLocations = new HashSet(); locationsAndNames = new HashMap(); + httpEndpointAttributes = new HashMap(); props = new Properties(); defaultProps = new Properties(); this.originalServerXMLFile = originalServerXMLFile; @@ -196,6 +206,7 @@ public ServerConfigDocument(CommonLoggerI log, File originalServerXMLFile, Map(); namelessLocations = new HashSet(); locationsAndNames = new HashMap(); + httpEndpointAttributes = new HashMap(); props = new Properties(); if (initProperties != null) props.putAll(initProperties); defaultProps = new Properties(); @@ -268,6 +279,7 @@ public void initializeAppsLocation() throws PluginExecutionException { parseApplication(doc, XPATH_SERVER_ENTERPRISE_APPLICATION); parseApplication(doc, XPATH_SERVER_SPRINGBOOT_APPLICATION); parseNames(doc, XPATH_ALL_SERVER_APPLICATIONS); + parseHttpEndpoint(doc); parseInclude(doc); parseConfigDropinsDir(); @@ -560,6 +572,28 @@ private void parseNames(Document doc, XPathExpression expression) throws XPathEx } } + private void parseHttpEndpoint(Document doc) throws XPathExpressionException { + if (doc == null) { + return; + } + NodeList nodeList = (NodeList) XPATH_SERVER_HTTP_ENDPOINT.evaluate(doc, XPathConstants.NODESET); + for (int i = 0; i < nodeList.getLength(); i++) { + Node node = nodeList.item(i); + if (node instanceof Element) { + Element elem = (Element) node; + NamedNodeMap attributes = elem.getAttributes(); + if (attributes != null) { + for (int j = 0; j < attributes.getLength(); j++) { + Node attr = attributes.item(j); + if (!httpEndpointAttributes.containsKey(attr.getNodeName())) { + httpEndpointAttributes.put(attr.getNodeName(), attr.getNodeValue()); + } + } + } + } + } + } + public String findNameForLocation(String location) { String appName = locationsAndNames.get(location); @@ -634,6 +668,7 @@ private void parseInclude(Document doc) throws XPathExpressionException, IOExcep parseApplication(inclDoc, XPATH_SERVER_SPRINGBOOT_APPLICATION); parseApplication(inclDoc, XPATH_SERVER_ENTERPRISE_APPLICATION); parseNames(inclDoc, XPATH_ALL_SERVER_APPLICATIONS); + parseHttpEndpoint(inclDoc); // handle nested include elements parseInclude(inclDoc); } @@ -677,6 +712,7 @@ private void parseDropinsFile(File file) throws IOException, XPathExpressionExce parseApplication(doc, XPATH_SERVER_SPRINGBOOT_APPLICATION); parseApplication(doc, XPATH_SERVER_ENTERPRISE_APPLICATION); parseNames(doc, XPATH_ALL_SERVER_APPLICATIONS); + parseHttpEndpoint(doc); parseInclude(doc); } } diff --git a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java index 4866f9ac5..d7569e74f 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java +++ b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java @@ -1701,8 +1701,12 @@ private String[] getContainerCommand() throws IOException, PluginExecutionExcept List heldSockets = new ArrayList(); try { if (!skipDefaultPorts) { - int effectiveHttpPort = resolveEffectiveContainerPort(LIBERTY_DEFAULT_HTTP_PORT, "httpPort"); - int effectiveHttpsPort = resolveEffectiveContainerPort(LIBERTY_DEFAULT_HTTPS_PORT, "httpsPort"); + Map defaultPorts = new HashMap(); + defaultPorts.put("httpPort", LIBERTY_DEFAULT_HTTP_PORT); + defaultPorts.put("httpsPort", LIBERTY_DEFAULT_HTTPS_PORT); + Map effectivePorts = resolveEffectiveContainerPorts(defaultPorts); + int effectiveHttpPort = effectivePorts.get("httpPort"); + int effectiveHttpsPort = effectivePorts.get("httpsPort"); int httpPortToUse = findAndHoldPort(effectiveHttpPort, false, heldSockets); int httpsPortToUse = findAndHoldPort(effectiveHttpsPort, false, heldSockets); commandElements.add("-p"); @@ -1838,6 +1842,21 @@ private String[] getContainerCommand() throws IOException, PluginExecutionExcept */ // package-private for unit testing int resolveEffectiveContainerPort(int defaultPort, String endpointAttr) { + Map defaultPorts = new HashMap(); + defaultPorts.put(endpointAttr, defaultPort); + Map result = resolveEffectiveContainerPorts(defaultPorts); + return result.getOrDefault(endpointAttr, defaultPort); + } + + /** + * Resolves the effective container ports for the given map of endpoint attributes and their defaults + * in a single pass using {@link ServerConfigDocument}. + * + * @param defaultPortsByAttr a map of endpoint attribute names to their default port integers (e.g. "httpPort" -> 9080, "httpsPort" -> 9443) + * @return a map containing resolved effective ports for each attribute key + */ + Map resolveEffectiveContainerPorts(Map defaultPortsByAttr) { + Map resolvedPorts = new HashMap(defaultPortsByAttr); try { // Read liberty-plugin-config.xml first — it contains the resolved paths for // configFile (custom server.xml), installDirectory, and userDirectory. @@ -1858,7 +1877,7 @@ int resolveEffectiveContainerPort(int defaultPort, String endpointAttr) { effectiveServerXml = (configDirectory != null ? new File(configDirectory, "server.xml") : null); } if (effectiveServerXml == null || !effectiveServerXml.isFile()) { - return defaultPort; + return resolvedPorts; } // Fall back to serverDirectory so that at minimum server.env and @@ -1873,106 +1892,43 @@ int resolveEffectiveContainerPort(int defaultPort, String endpointAttr) { CommonLoggerI.noop(), effectiveServerXml, installDir, userDir, serverDirectory, serverDirectory); - // Read the raw httpEndpoint attribute value from server.xml, including - // any files pulled in via elements. - Document doc = scd.parseDocument(effectiveServerXml); - if (doc == null) { - return defaultPort; - } - XPath xp = XPathFactory.newInstance().newXPath(); - Element endpoint = (Element) - xp.compile("/server/httpEndpoint").evaluate(doc, XPathConstants.NODE); - if (endpoint == null) { - // Not in the top-level document — walk files. - endpoint = findHttpEndpointInIncludes(doc, effectiveServerXml.getParentFile(), xp, scd); - } - if (endpoint == null) { - return defaultPort; - } - String raw = endpoint.getAttribute(endpointAttr); - if (raw == null || raw.trim().isEmpty()) { - return defaultPort; - } - raw = raw.trim(); - - // Literal integer — no variable resolution needed. - try { - return Integer.parseInt(raw); - } catch (NumberFormatException ignored) { /* fall through */ } + Map endpointAttrs = scd.getHttpEndpointAttributes(); + for (Map.Entry entry : defaultPortsByAttr.entrySet()) { + String attrName = entry.getKey(); + int defaultPort = entry.getValue(); + String raw = endpointAttrs.get(attrName); + if (raw == null || raw.trim().isEmpty()) { + continue; + } + raw = raw.trim(); - // Variable reference — resolve through the fully-loaded variable maps. - String resolved = VariableUtility.resolveVariables( - CommonLoggerI.noop(), raw, null, - scd.getProperties(), scd.getDefaultProperties(), - scd.getLibertyDirPropertyFiles()); - if (resolved == null) { - return defaultPort; - } - try { - return Integer.parseInt(resolved.trim()); - } catch (NumberFormatException e) { - return defaultPort; + // Literal integer — no variable resolution needed. + try { + resolvedPorts.put(attrName, Integer.parseInt(raw)); + continue; + } catch (NumberFormatException ignored) { /* fall through */ } + + // Variable reference — resolve through the fully-loaded variable maps. + String resolved = VariableUtility.resolveVariables( + CommonLoggerI.noop(), raw, null, + scd.getProperties(), scd.getDefaultProperties(), + scd.getLibertyDirPropertyFiles()); + if (resolved != null) { + try { + resolvedPorts.put(attrName, Integer.parseInt(resolved.trim())); + } catch (NumberFormatException ignored) { /* fall back to default */ } + } } } catch (Exception e) { - debug("resolveEffectiveContainerPort: could not resolve port, using default " + defaultPort + ": " + e.getMessage()); - return defaultPort; + debug("resolveEffectiveContainerPorts: could not resolve ports, using defaults: " + e.getMessage()); } + return resolvedPorts; } private static File toFile(String path) { return (path != null) ? new File(path) : null; } - /** - * Recursively walks {@code } elements in {@code doc} to find an - * {@code } element that is not present in the top-level document. - * Relative include locations are resolved against {@code parentDir}. - * - * @return the first {@code httpEndpoint} {@link Element} found in any included - * document, or {@code null} if none is found - */ - private Element findHttpEndpointInIncludes(Document doc, File parentDir, - XPath xp, ServerConfigDocument scd) { - try { - NodeList includes = (NodeList) - xp.compile("/server/include").evaluate(doc, XPathConstants.NODESET); - for (int i = 0; i < includes.getLength(); i++) { - if (!(includes.item(i) instanceof Element)) { - continue; - } - String loc = ((Element) includes.item(i)).getAttribute("location"); - if (loc == null || loc.trim().isEmpty()) { - continue; - } - // Resolve relative paths against the parent dir of the including file. - File inclFile = new File(loc); - if (!inclFile.isAbsolute()) { - inclFile = new File(parentDir, loc); - } - if (!inclFile.isFile()) { - continue; - } - Document inclDoc = scd.parseDocument(inclFile); - if (inclDoc == null) { - continue; - } - Element endpoint = (Element) - xp.compile("/server/httpEndpoint").evaluate(inclDoc, XPathConstants.NODE); - if (endpoint != null) { - return endpoint; - } - // Recurse into nested includes. - endpoint = findHttpEndpointInIncludes(inclDoc, inclFile.getParentFile(), xp, scd); - if (endpoint != null) { - return endpoint; - } - } - } catch (Exception e) { - debug("findHttpEndpointInIncludes: error walking includes: " + e.getMessage()); - } - return null; - } - /** * Finds an available port starting from {@code preferred} and immediately binds a * {@code ServerSocket} on it to hold that port open until the caller is done building diff --git a/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java index aa9fe571a..afc3e5b47 100644 --- a/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java +++ b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java @@ -379,4 +379,19 @@ public void testPortFromSiblingConfigDropins() throws Exception { assertEquals(9098, u.resolveEffectiveContainerPort(9080, "httpPort")); } + + @Test + public void testResolveBothPortsInSinglePass() throws Exception { + File serverDir = tmp.newFolder("server"); + File serverXml = writeServerXml(serverDir, "9090", "9453"); + DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); + + java.util.Map defaults = new java.util.HashMap(); + defaults.put("httpPort", 9080); + defaults.put("httpsPort", 9443); + + java.util.Map resolved = u.resolveEffectiveContainerPorts(defaults); + assertEquals(Integer.valueOf(9090), resolved.get("httpPort")); + assertEquals(Integer.valueOf(9453), resolved.get("httpsPort")); + } } From 4b3daf0d2cf72002f7c341f90edf1a98771d4c40 Mon Sep 17 00:00:00 2001 From: Sajeer Date: Fri, 18 Sep 2026 12:10:13 +0530 Subject: [PATCH 08/13] Removed redundant codes and refactored --- .../plugins/config/ServerConfigDocument.java | 16 +------ .../common/plugins/config/XmlDocument.java | 3 +- .../tools/common/plugins/util/DevUtil.java | 36 +-------------- .../plugins/util/DevUtilResolvePortTest.java | 44 ++++++++++++------- 4 files changed, 31 insertions(+), 68 deletions(-) diff --git a/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java b/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java index ae9744f95..d3f82c1bd 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java +++ b/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java @@ -37,10 +37,6 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -import javax.xml.XMLConstants; -import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.ParserConfigurationException; import javax.xml.xpath.XPath; import javax.xml.xpath.XPathConstants; import javax.xml.xpath.XPathExpression; @@ -213,10 +209,6 @@ public ServerConfigDocument(CommonLoggerI log, File originalServerXMLFile, Map}, or non-integer resolved value). - * - * @param defaultPort the Liberty default to fall back to (9080 or 9443) - * @param endpointAttr the {@code httpEndpoint} attribute name: {@code "httpPort"} or - * {@code "httpsPort"} - * @return the resolved effective port, or {@code defaultPort} on any failure - */ - // package-private for unit testing - int resolveEffectiveContainerPort(int defaultPort, String endpointAttr) { - Map defaultPorts = new HashMap(); - defaultPorts.put(endpointAttr, defaultPort); - Map result = resolveEffectiveContainerPorts(defaultPorts); - return result.getOrDefault(endpointAttr, defaultPort); - } - /** * Resolves the effective container ports for the given map of endpoint attributes and their defaults * in a single pass using {@link ServerConfigDocument}. @@ -3940,17 +3918,7 @@ protected Collection getOmitFilesList(File looseAppFile, String srcDirecto Collection omitFiles = new ArrayList(); try { if (looseAppFile != null && looseAppFile.exists()) { - DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); - dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false); - dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - dbf.setFeature("http://xml.org/sax/features/external-general-entities", false); - dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - dbf.setXIncludeAware(false); - dbf.setExpandEntityReferences(false); - DocumentBuilder db = dbf.newDocumentBuilder(); - Document document = db.parse(looseAppFile); + Document document = XmlDocument.parseDocument(looseAppFile); NodeList archiveList = document.getElementsByTagName("archive"); for (int i = 0; i < archiveList.getLength(); i++) { NodeList ar = archiveList.item(i).getChildNodes(); @@ -3971,7 +3939,7 @@ protected Collection getOmitFilesList(File looseAppFile, String srcDirecto } } } - } catch (ParserConfigurationException | SAXException | IOException e) { + } catch (SAXException | IOException e) { error("Unable to read loose application configuration file: " + looseAppFile.toString()); return omitFiles; } diff --git a/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java index afc3e5b47..9ed9bd107 100644 --- a/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java +++ b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java @@ -21,13 +21,15 @@ import java.io.FileWriter; import java.io.IOException; import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; /** - * Unit tests for {@link DevUtil#resolveEffectiveContainerPort}. + * Unit tests for {@link DevUtil#resolveEffectiveContainerPorts}. * * Each test creates a minimal on-disk Liberty server structure inside a * {@link TemporaryFolder} and calls the package-private method directly @@ -76,6 +78,14 @@ private DevTestUtil util(File serverDir, File buildDir, File serverXmlFile) thro return u; } + /** + * Helper to resolve a single endpoint port for testing convenience by delegating + * to {@link DevUtil#resolveEffectiveContainerPorts(Map)}. + */ + private int resolvePort(DevTestUtil u, int defaultPort, String attrName) { + return u.resolveEffectiveContainerPorts(Collections.singletonMap(attrName, defaultPort)).get(attrName); + } + // ------------------------------------------------------------------ // Tests // ------------------------------------------------------------------ @@ -86,7 +96,7 @@ public void testLiteralHttpPort() throws Exception { File serverXml = writeServerXml(serverDir, "9090", null); DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9090, u.resolveEffectiveContainerPort(9080, "httpPort")); + assertEquals(9090, resolvePort(u, 9080, "httpPort")); } @Test @@ -95,7 +105,7 @@ public void testLiteralHttpsPort() throws Exception { File serverXml = writeServerXml(serverDir, null, "9453"); DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9453, u.resolveEffectiveContainerPort(9443, "httpsPort")); + assertEquals(9453, resolvePort(u, 9443, "httpsPort")); } @Test @@ -105,7 +115,7 @@ public void testDefaultPortReturnedWhenNoHttpEndpoint() throws Exception { write(serverXml, "servlet-4.0"); DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9080, u.resolveEffectiveContainerPort(9080, "httpPort")); + assertEquals(9080, resolvePort(u, 9080, "httpPort")); } @Test @@ -113,7 +123,7 @@ public void testDefaultPortReturnedWhenServerXmlMissing() throws Exception { File serverDir = tmp.newFolder("server"); DevTestUtil u = util(serverDir, tmp.newFolder("build"), new File(serverDir, "nonexistent.xml")); - assertEquals(9080, u.resolveEffectiveContainerPort(9080, "httpPort")); + assertEquals(9080, resolvePort(u, 9080, "httpPort")); } @Test @@ -122,7 +132,7 @@ public void testVariableResolvedFromDefaultValue() throws Exception { File serverXml = writeServerXmlWithVar(serverDir, "myHttpPort"); DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9090, u.resolveEffectiveContainerPort(9080, "httpPort")); + assertEquals(9090, resolvePort(u, 9080, "httpPort")); } @Test @@ -138,7 +148,7 @@ public void testVariableResolvedFromBootstrapProperties() throws Exception { write(new File(serverDir, "bootstrap.properties"), "http.port=9091\n"); DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9091, u.resolveEffectiveContainerPort(9080, "httpPort")); + assertEquals(9091, resolvePort(u, 9080, "httpPort")); } @Test @@ -152,7 +162,7 @@ public void testVariableResolvedFromServerEnv() throws Exception { write(new File(serverDir, "server.env"), "HTTP_PORT=9092\n"); DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9092, u.resolveEffectiveContainerPort(9080, "httpPort")); + assertEquals(9092, resolvePort(u, 9080, "httpPort")); } @Test @@ -172,7 +182,7 @@ public void testVariableResolvedFromConfigDropinsOverrides() throws Exception { ""); DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9095, u.resolveEffectiveContainerPort(9080, "httpPort")); + assertEquals(9095, resolvePort(u, 9080, "httpPort")); } @Test @@ -189,7 +199,7 @@ public void testOverridePrecedenceOverDefault() throws Exception { DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); // bootstrap.properties (step 3) overrides defaultValue (step 1) - assertEquals(9093, u.resolveEffectiveContainerPort(9080, "httpPort")); + assertEquals(9093, resolvePort(u, 9080, "httpPort")); } @Test @@ -203,7 +213,7 @@ public void testDefaultPortWhenPortIsNotANumber() throws Exception { DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); // Variable is not defined anywhere — should fall back to default - assertEquals(9080, u.resolveEffectiveContainerPort(9080, "httpPort")); + assertEquals(9080, resolvePort(u, 9080, "httpPort")); } @Test @@ -223,7 +233,7 @@ public void testFallsBackToConfigDirectoryWhenServerXmlFileIsNull() throws Excep DevTestUtil u = new DevTestUtil(serverDir, null, null, configDir, Collections.emptyList(), Collections.emptyList(), false, false); - assertEquals(9097, u.resolveEffectiveContainerPort(9080, "httpPort")); + assertEquals(9097, resolvePort(u, 9080, "httpPort")); } @Test @@ -253,7 +263,7 @@ public void testPluginConfigXmlProvidesInstallAndUserDir() throws Exception { DevTestUtil u = util(serverDir, buildDir, serverXml); - assertEquals(9096, u.resolveEffectiveContainerPort(9080, "httpPort")); + assertEquals(9096, resolvePort(u, 9080, "httpPort")); } @Test @@ -293,7 +303,7 @@ public void testVariableResolvedFromServerDirConfigDropinsOverrides() throws Exc Collections.emptyList(), Collections.emptyList(), false, false); u.serverXmlFile = srcServerXml; - assertEquals(9090, u.resolveEffectiveContainerPort(9080, "httpPort")); + assertEquals(9090, resolvePort(u, 9080, "httpPort")); } @Test @@ -307,7 +317,7 @@ public void testVariableResolvedForHttpsPort() throws Exception { ""); DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9453, u.resolveEffectiveContainerPort(9443, "httpsPort")); + assertEquals(9453, resolvePort(u, 9443, "httpsPort")); } /** @@ -340,7 +350,7 @@ public void testPortDefinedViaRelativeInclude() throws Exception { DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9094, u.resolveEffectiveContainerPort(9080, "httpPort")); + assertEquals(9094, resolvePort(u, 9080, "httpPort")); } /** @@ -377,7 +387,7 @@ public void testPortFromSiblingConfigDropins() throws Exception { DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9098, u.resolveEffectiveContainerPort(9080, "httpPort")); + assertEquals(9098, resolvePort(u, 9080, "httpPort")); } @Test From 5b18195099a4ecbfc4dd3916cdf2d9509deefdf1 Mon Sep 17 00:00:00 2001 From: Sajeer Date: Fri, 18 Sep 2026 12:41:26 +0530 Subject: [PATCH 09/13] Removed redundant codes and refactored --- .../tools/common/plugins/config/ServerConfigDocument.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java b/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java index d3f82c1bd..cbe7fb9da 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java +++ b/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java @@ -577,9 +577,7 @@ private void parseHttpEndpoint(Document doc) throws XPathExpressionException { if (attributes != null) { for (int j = 0; j < attributes.getLength(); j++) { Node attr = attributes.item(j); - if (!httpEndpointAttributes.containsKey(attr.getNodeName())) { - httpEndpointAttributes.put(attr.getNodeName(), attr.getNodeValue()); - } + httpEndpointAttributes.put(attr.getNodeName(), attr.getNodeValue()); } } } From e5741f643d9671f82fca2eaec53c3b1302d2976f Mon Sep 17 00:00:00 2001 From: Sajeer Date: Fri, 18 Sep 2026 17:54:11 +0530 Subject: [PATCH 10/13] New test cases for testing port overrides by and configDropins/overrides --- .../plugins/util/DevUtilResolvePortTest.java | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java index 9ed9bd107..1d92fab30 100644 --- a/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java +++ b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java @@ -390,6 +390,59 @@ public void testPortFromSiblingConfigDropins() throws Exception { assertEquals(9098, resolvePort(u, 9080, "httpPort")); } + /** + * An included file overrides the {@code httpPort} attribute defined in {@code server.xml}. + */ + @Test + public void testIncludeOverridesHttpEndpointPort() throws Exception { + File serverDir = tmp.newFolder("server"); + + // included file that overrides the port + write(new File(serverDir, "ports.xml"), + "\n" + + " \n" + + ""); + + // server.xml defines httpPort=9090 but includes ports.xml which overrides it to 9094 + File serverXml = new File(serverDir, "server.xml"); + write(serverXml, + "\n" + + " \n" + + " \n" + + ""); + + DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); + + assertEquals(9094, resolvePort(u, 9080, "httpPort")); + } + + /** + * A {@code configDropins/overrides} file overrides the {@code httpPort} attribute defined in {@code server.xml}. + */ + @Test + public void testConfigDropinsOverridesHttpEndpointPort() throws Exception { + File serverDir = tmp.newFolder("server"); + + // server.xml defines httpPort=9090 + File serverXml = new File(serverDir, "server.xml"); + write(serverXml, + "\n" + + " \n" + + ""); + + // configDropins/overrides/override.xml defines httpPort=9099 which should take precedence + File overrides = new File(serverDir, "configDropins/overrides"); + overrides.mkdirs(); + write(new File(overrides, "override.xml"), + "\n" + + " \n" + + ""); + + DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); + + assertEquals(9099, resolvePort(u, 9080, "httpPort")); + } + @Test public void testResolveBothPortsInSinglePass() throws Exception { File serverDir = tmp.newFolder("server"); From b8f1485e39f8c6d089d71ad4186a4fb8d30d3f5b Mon Sep 17 00:00:00 2001 From: Sajeer Date: Fri, 18 Sep 2026 18:09:03 +0530 Subject: [PATCH 11/13] Removed toFile from DevUtil and created getFileElementFromXmlFile in XmlDocument --- .../tools/common/plugins/config/XmlDocument.java | 9 +++++++++ .../openliberty/tools/common/plugins/util/DevUtil.java | 10 +++------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/main/java/io/openliberty/tools/common/plugins/config/XmlDocument.java b/src/main/java/io/openliberty/tools/common/plugins/config/XmlDocument.java index cbabff430..95bf81606 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/config/XmlDocument.java +++ b/src/main/java/io/openliberty/tools/common/plugins/config/XmlDocument.java @@ -162,6 +162,15 @@ public static String readTextElementFromXmlFile(File xmlFile, String tagName) { } } + /** + * Returns a {@link File} for the text content of the first element matching {@code tagName} + * in an XML file, or {@code null} if the file is absent, the tag is missing, or any parse error occurs. + */ + public static File getFileElementFromXmlFile(File xmlFile, String tagName) { + String path = readTextElementFromXmlFile(xmlFile, tagName); + return (path != null) ? new File(path) : null; + } + public static void addNewlineBeforeFirstElement(File f) throws IOException { // look for "" and add a newline byte[] contents = Files.readAllBytes(f.toPath()); diff --git a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java index 8aaa86e66..775dbed5b 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java +++ b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java @@ -1840,12 +1840,12 @@ Map resolveEffectiveContainerPorts(Map default // configFile (custom server.xml), installDirectory, and userDirectory. File pluginConfigXml = (buildDirectory != null) ? new File(buildDirectory, "liberty-plugin-config.xml") : null; - File installDir = toFile(XmlDocument.readTextElementFromXmlFile(pluginConfigXml, "installDirectory")); - File userDir = toFile(XmlDocument.readTextElementFromXmlFile(pluginConfigXml, "userDirectory")); + File installDir = XmlDocument.getFileElementFromXmlFile(pluginConfigXml, "installDirectory"); + File userDir = XmlDocument.getFileElementFromXmlFile(pluginConfigXml, "userDirectory"); // Use the configFile path from the plugin config if available; that is the // user-specified server.xml (serverXmlFile parameter). Fall back to // serverXmlFile set by watchFiles(), then to configDirectory/server.xml. - File configFileFromPlugin = toFile(XmlDocument.readTextElementFromXmlFile(pluginConfigXml, "configFile")); + File configFileFromPlugin = XmlDocument.getFileElementFromXmlFile(pluginConfigXml, "configFile"); File effectiveServerXml; if (configFileFromPlugin != null && configFileFromPlugin.isFile()) { effectiveServerXml = configFileFromPlugin; @@ -1903,10 +1903,6 @@ Map resolveEffectiveContainerPorts(Map default return resolvedPorts; } - private static File toFile(String path) { - return (path != null) ? new File(path) : null; - } - /** * Finds an available port starting from {@code preferred} and immediately binds a * {@code ServerSocket} on it to hold that port open until the caller is done building From aeb994b444f06f65a11d5d992ac79e2a167fde31 Mon Sep 17 00:00:00 2001 From: Sajeer Date: Fri, 18 Sep 2026 20:08:25 +0530 Subject: [PATCH 12/13] Created and used constants for httpPort and httpsPort --- .../plugins/config/ServerConfigDocument.java | 3 ++ .../tools/common/plugins/util/DevUtil.java | 8 ++-- .../plugins/util/DevUtilResolvePortTest.java | 38 ++++++++++--------- 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java b/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java index cbe7fb9da..56671e718 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java +++ b/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java @@ -81,6 +81,9 @@ public class ServerConfigDocument { Optional springBootAppNodeLocation = Optional.empty(); Optional springBootAppNodeDocumentURI = Optional.empty(); + public static final String HTTP_PORT_ATTR = "httpPort"; + public static final String HTTPS_PORT_ATTR = "httpsPort"; + private static final XPathExpression XPATH_SERVER_APPLICATION; private static final XPathExpression XPATH_SERVER_WEB_APPLICATION; private static final XPathExpression XPATH_SERVER_SPRINGBOOT_APPLICATION; diff --git a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java index 775dbed5b..dd6c7f286 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java +++ b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java @@ -1699,11 +1699,11 @@ private String[] getContainerCommand() throws IOException, PluginExecutionExcept try { if (!skipDefaultPorts) { Map defaultPorts = new HashMap(); - defaultPorts.put("httpPort", LIBERTY_DEFAULT_HTTP_PORT); - defaultPorts.put("httpsPort", LIBERTY_DEFAULT_HTTPS_PORT); + defaultPorts.put(ServerConfigDocument.HTTP_PORT_ATTR, LIBERTY_DEFAULT_HTTP_PORT); + defaultPorts.put(ServerConfigDocument.HTTPS_PORT_ATTR, LIBERTY_DEFAULT_HTTPS_PORT); Map effectivePorts = resolveEffectiveContainerPorts(defaultPorts); - int effectiveHttpPort = effectivePorts.get("httpPort"); - int effectiveHttpsPort = effectivePorts.get("httpsPort"); + int effectiveHttpPort = effectivePorts.get(ServerConfigDocument.HTTP_PORT_ATTR); + int effectiveHttpsPort = effectivePorts.get(ServerConfigDocument.HTTPS_PORT_ATTR); int httpPortToUse = findAndHoldPort(effectiveHttpPort, false, heldSockets); int httpsPortToUse = findAndHoldPort(effectiveHttpsPort, false, heldSockets); commandElements.add("-p"); diff --git a/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java index 1d92fab30..11a98f0ac 100644 --- a/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java +++ b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilResolvePortTest.java @@ -28,6 +28,8 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; +import io.openliberty.tools.common.plugins.config.ServerConfigDocument; + /** * Unit tests for {@link DevUtil#resolveEffectiveContainerPorts}. * @@ -96,7 +98,7 @@ public void testLiteralHttpPort() throws Exception { File serverXml = writeServerXml(serverDir, "9090", null); DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9090, resolvePort(u, 9080, "httpPort")); + assertEquals(9090, resolvePort(u, 9080, ServerConfigDocument.HTTP_PORT_ATTR)); } @Test @@ -105,7 +107,7 @@ public void testLiteralHttpsPort() throws Exception { File serverXml = writeServerXml(serverDir, null, "9453"); DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9453, resolvePort(u, 9443, "httpsPort")); + assertEquals(9453, resolvePort(u, 9443, ServerConfigDocument.HTTPS_PORT_ATTR)); } @Test @@ -115,7 +117,7 @@ public void testDefaultPortReturnedWhenNoHttpEndpoint() throws Exception { write(serverXml, "servlet-4.0"); DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9080, resolvePort(u, 9080, "httpPort")); + assertEquals(9080, resolvePort(u, 9080, ServerConfigDocument.HTTP_PORT_ATTR)); } @Test @@ -123,7 +125,7 @@ public void testDefaultPortReturnedWhenServerXmlMissing() throws Exception { File serverDir = tmp.newFolder("server"); DevTestUtil u = util(serverDir, tmp.newFolder("build"), new File(serverDir, "nonexistent.xml")); - assertEquals(9080, resolvePort(u, 9080, "httpPort")); + assertEquals(9080, resolvePort(u, 9080, ServerConfigDocument.HTTP_PORT_ATTR)); } @Test @@ -132,7 +134,7 @@ public void testVariableResolvedFromDefaultValue() throws Exception { File serverXml = writeServerXmlWithVar(serverDir, "myHttpPort"); DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9090, resolvePort(u, 9080, "httpPort")); + assertEquals(9090, resolvePort(u, 9080, ServerConfigDocument.HTTP_PORT_ATTR)); } @Test @@ -148,7 +150,7 @@ public void testVariableResolvedFromBootstrapProperties() throws Exception { write(new File(serverDir, "bootstrap.properties"), "http.port=9091\n"); DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9091, resolvePort(u, 9080, "httpPort")); + assertEquals(9091, resolvePort(u, 9080, ServerConfigDocument.HTTP_PORT_ATTR)); } @Test @@ -162,7 +164,7 @@ public void testVariableResolvedFromServerEnv() throws Exception { write(new File(serverDir, "server.env"), "HTTP_PORT=9092\n"); DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9092, resolvePort(u, 9080, "httpPort")); + assertEquals(9092, resolvePort(u, 9080, ServerConfigDocument.HTTP_PORT_ATTR)); } @Test @@ -182,7 +184,7 @@ public void testVariableResolvedFromConfigDropinsOverrides() throws Exception { ""); DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9095, resolvePort(u, 9080, "httpPort")); + assertEquals(9095, resolvePort(u, 9080, ServerConfigDocument.HTTP_PORT_ATTR)); } @Test @@ -199,7 +201,7 @@ public void testOverridePrecedenceOverDefault() throws Exception { DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); // bootstrap.properties (step 3) overrides defaultValue (step 1) - assertEquals(9093, resolvePort(u, 9080, "httpPort")); + assertEquals(9093, resolvePort(u, 9080, ServerConfigDocument.HTTP_PORT_ATTR)); } @Test @@ -213,7 +215,7 @@ public void testDefaultPortWhenPortIsNotANumber() throws Exception { DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); // Variable is not defined anywhere — should fall back to default - assertEquals(9080, resolvePort(u, 9080, "httpPort")); + assertEquals(9080, resolvePort(u, 9080, ServerConfigDocument.HTTP_PORT_ATTR)); } @Test @@ -233,7 +235,7 @@ public void testFallsBackToConfigDirectoryWhenServerXmlFileIsNull() throws Excep DevTestUtil u = new DevTestUtil(serverDir, null, null, configDir, Collections.emptyList(), Collections.emptyList(), false, false); - assertEquals(9097, resolvePort(u, 9080, "httpPort")); + assertEquals(9097, resolvePort(u, 9080, ServerConfigDocument.HTTP_PORT_ATTR)); } @Test @@ -263,7 +265,7 @@ public void testPluginConfigXmlProvidesInstallAndUserDir() throws Exception { DevTestUtil u = util(serverDir, buildDir, serverXml); - assertEquals(9096, resolvePort(u, 9080, "httpPort")); + assertEquals(9096, resolvePort(u, 9080, ServerConfigDocument.HTTP_PORT_ATTR)); } @Test @@ -303,7 +305,7 @@ public void testVariableResolvedFromServerDirConfigDropinsOverrides() throws Exc Collections.emptyList(), Collections.emptyList(), false, false); u.serverXmlFile = srcServerXml; - assertEquals(9090, resolvePort(u, 9080, "httpPort")); + assertEquals(9090, resolvePort(u, 9080, ServerConfigDocument.HTTP_PORT_ATTR)); } @Test @@ -317,7 +319,7 @@ public void testVariableResolvedForHttpsPort() throws Exception { ""); DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); - assertEquals(9453, resolvePort(u, 9443, "httpsPort")); + assertEquals(9453, resolvePort(u, 9443, ServerConfigDocument.HTTPS_PORT_ATTR)); } /** @@ -450,11 +452,11 @@ public void testResolveBothPortsInSinglePass() throws Exception { DevTestUtil u = util(serverDir, tmp.newFolder("build"), serverXml); java.util.Map defaults = new java.util.HashMap(); - defaults.put("httpPort", 9080); - defaults.put("httpsPort", 9443); + defaults.put(ServerConfigDocument.HTTP_PORT_ATTR, 9080); + defaults.put(ServerConfigDocument.HTTPS_PORT_ATTR, 9443); java.util.Map resolved = u.resolveEffectiveContainerPorts(defaults); - assertEquals(Integer.valueOf(9090), resolved.get("httpPort")); - assertEquals(Integer.valueOf(9453), resolved.get("httpsPort")); + assertEquals(Integer.valueOf(9090), resolved.get(ServerConfigDocument.HTTP_PORT_ATTR)); + assertEquals(Integer.valueOf(9453), resolved.get(ServerConfigDocument.HTTPS_PORT_ATTR)); } } From 477cc4cae2e8a0115d8924b779df3b1169121918 Mon Sep 17 00:00:00 2001 From: Sajeer Date: Mon, 21 Sep 2026 14:00:36 +0530 Subject: [PATCH 13/13] Only store http and https attributes into the map on parsing the HTTP Endpoint --- .../plugins/config/ServerConfigDocument.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java b/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java index 56671e718..0dca466fd 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java +++ b/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java @@ -49,7 +49,6 @@ import org.apache.commons.io.comparator.NameFileComparator; import org.w3c.dom.Document; import org.w3c.dom.Element; -import org.w3c.dom.NamedNodeMap; import org.w3c.dom.Node; import org.w3c.dom.NodeList; import org.xml.sax.SAXException; @@ -576,12 +575,15 @@ private void parseHttpEndpoint(Document doc) throws XPathExpressionException { Node node = nodeList.item(i); if (node instanceof Element) { Element elem = (Element) node; - NamedNodeMap attributes = elem.getAttributes(); - if (attributes != null) { - for (int j = 0; j < attributes.getLength(); j++) { - Node attr = attributes.item(j); - httpEndpointAttributes.put(attr.getNodeName(), attr.getNodeValue()); - } + + String httpAttribute = elem.getAttribute(HTTP_PORT_ATTR); + if (!httpAttribute.isEmpty()) { + httpEndpointAttributes.put(HTTP_PORT_ATTR, httpAttribute); + } + + String httpsAttribute = elem.getAttribute(HTTPS_PORT_ATTR); + if (!httpsAttribute.isEmpty()) { + httpEndpointAttributes.put(HTTPS_PORT_ATTR, httpsAttribute); } } }