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 e22b6d62..f104995b 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 @@ -1678,127 +1678,199 @@ protected static File getLongestCommonDir(File dir1, File dir2) { * @return the command string to use to start the container */ private String[] getContainerCommand() throws IOException, PluginExecutionException { - // Use List for building the command. This will be converted to a String[] when passed to ProcessBuilder. + // Use List for building the command. This will be converted to a String[] when passed to ProcessBuilder. // The ProcessBuilder will handle quoting of paths (for blanks) as needed for various OS. List commandElements = new ArrayList(); commandElements.add(getContainerCommandPrefix().trim()); commandElements.add("run"); commandElements.add("--rm"); - if (!skipDefaultPorts) { - int httpPortToUse, httpsPortToUse; - try { - httpPortToUse = findAvailablePort(LIBERTY_DEFAULT_HTTP_PORT, false); - httpsPortToUse = findAvailablePort(LIBERTY_DEFAULT_HTTPS_PORT, false); - } catch (IOException x) { - error("An error occurred while trying to find an available network port. Using default port numbers.", x); - httpPortToUse = LIBERTY_DEFAULT_HTTP_PORT; - httpsPortToUse = LIBERTY_DEFAULT_HTTPS_PORT; + // Select host-side ports and hold them open for the entire command-build phase. + // Holding all three sockets simultaneously means a concurrent module's + // findAvailablePort() will see each chosen port as already bound and advance to + // the next one, eliminating the TOCTOU race without any shared state. + // The try/finally below ensures all sockets are released even if an exception + // is thrown while building the rest of the command. + 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); + commandElements.add("-p"); + commandElements.add(httpPortToUse+":"+LIBERTY_DEFAULT_HTTP_PORT); + + commandElements.add("-p"); + commandElements.add(httpsPortToUse+":"+LIBERTY_DEFAULT_HTTPS_PORT); } - commandElements.add("-p"); - commandElements.add(httpPortToUse+":"+LIBERTY_DEFAULT_HTTP_PORT); - commandElements.add("-p"); - commandElements.add(httpsPortToUse+":"+LIBERTY_DEFAULT_HTTPS_PORT); - } - - if (libertyDebug) { - // map debug port - int containerDebugPort, hostDebugPort; - try { + if (libertyDebug) { + // map debug port + int containerDebugPort, hostDebugPort; if (alternativeDebugPort == -1) { // it is possible another JVM has grabbed our port since dev mode last checked - hostDebugPort = findAvailablePort(libertyDebugPort, true); + hostDebugPort = findAndHoldPort(libertyDebugPort, true, heldSockets); containerDebugPort = libertyDebugPort; } else { // dev mode has already selected an ephemeral port containerDebugPort = hostDebugPort = alternativeDebugPort; } - } catch (IOException x) { - containerDebugPort = hostDebugPort = libertyDebugPort; - } - commandElements.add("-p"); - commandElements.add(hostDebugPort+":"+containerDebugPort); - // set environment variables in the container to ensure debug mode does not suspend the server, and to enable a custom debug port to be used - // and to allow remote debugging into the container - commandElements.add("-e"); - commandElements.add("WLP_DEBUG_SUSPEND=n"); - commandElements.add("-e"); - commandElements.add("WLP_DEBUG_ADDRESS=" + containerDebugPort); - commandElements.add("-e"); - commandElements.add("WLP_DEBUG_REMOTE=y"); - } - - // mount potential directories containing .war.xml from devc specific folder - override /config/apps and /config/dropins - File tempApps = new File(buildDirectory, DEVC_HIDDEN_FOLDER + "/apps"); - File tempDropins = new File(buildDirectory, DEVC_HIDDEN_FOLDER + "/dropins"); - commandElements.add("-v"); - commandElements.add(tempApps + ":/config/apps"); - - commandElements.add("-v"); - commandElements.add(tempDropins + ":/config/dropins"); - - // mount the loose application resources in the container using the appropriate project root - File looseApplicationProjectRoot = getLooseAppProjectRoot(projectDirectory, multiModuleProjectDirectory); - commandElements.add("-v"); - commandElements.add(looseApplicationProjectRoot.getAbsolutePath() + ":" + DEVMODE_DIR_NAME); - - // 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"); - commandElements.add(logsDir + ":/logs"); - - // mount the Maven .m2 cache directory for featureUtility to use. For now, featureUtility does not support Gradle cache. - commandElements.add("-v"); - commandElements.add(mavenCacheLocation + ":/devmode-maven-cache"); - - // mount all files from COPY commands in the Containerfile to allow for hot deployment - addCopiedFiles(commandElements); - - // Add a --user option when running Linux - addUserId(commandElements); - - // Do not generate a name if the user has specified a name - String name = getContainerOption("--name"); - if (name == null || name.isEmpty()) { - if (name != null && name.isEmpty()) { - error("The container option --name is specified with an unsupported value: empty string."); - // now generate a name so that the container errors make some sense to the user. - } - containerName = generateNewContainerName(); - commandElements.add("--name"); - commandElements.add(containerName); - } else { - containerName = name; - } - debug("containerName: " + containerName + "."); + commandElements.add("-p"); + commandElements.add(hostDebugPort+":"+containerDebugPort); + // set environment variables in the container to ensure debug mode does not suspend the server, and to enable a custom debug port to be used + // and to allow remote debugging into the container + commandElements.add("-e"); + commandElements.add("WLP_DEBUG_SUSPEND=n"); + commandElements.add("-e"); + commandElements.add("WLP_DEBUG_ADDRESS=" + containerDebugPort); + commandElements.add("-e"); + commandElements.add("WLP_DEBUG_REMOTE=y"); + } + + // mount potential directories containing .war.xml from devc specific folder - override /config/apps and /config/dropins + File tempApps = new File(buildDirectory, DEVC_HIDDEN_FOLDER + "/apps"); + File tempDropins = new File(buildDirectory, DEVC_HIDDEN_FOLDER + "/dropins"); + commandElements.add("-v"); + commandElements.add(tempApps + ":/config/apps"); + + commandElements.add("-v"); + commandElements.add(tempDropins + ":/config/dropins"); + + // mount the loose application resources in the container using the appropriate project root + File looseApplicationProjectRoot = getLooseAppProjectRoot(projectDirectory, multiModuleProjectDirectory); + commandElements.add("-v"); + commandElements.add(looseApplicationProjectRoot.getAbsolutePath() + ":" + DEVMODE_DIR_NAME); + + // 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"); + commandElements.add(logsDir + ":/logs"); + + // mount the Maven .m2 cache directory for featureUtility to use. For now, featureUtility does not support Gradle cache. + commandElements.add("-v"); + commandElements.add(mavenCacheLocation + ":/devmode-maven-cache"); + + // mount all files from COPY commands in the Containerfile to allow for hot deployment + addCopiedFiles(commandElements); + + // Add a --user option when running Linux + addUserId(commandElements); + + // Do not generate a name if the user has specified a name + String name = getContainerOption("--name"); + if (name == null || name.isEmpty()) { + if (name != null && name.isEmpty()) { + error("The container option --name is specified with an unsupported value: empty string."); + // now generate a name so that the container errors make some sense to the user. + } + containerName = generateNewContainerName(); + commandElements.add("--name"); + commandElements.add(containerName); + } else { + containerName = name; + } + debug("containerName: " + containerName + "."); - // Allow the user to add their own options to this command via a system property. - if (containerRunOpts != null) { - addContainerRunOpts(containerRunOpts, commandElements); - } + // Allow the user to add their own options to this command via a system property. + if (containerRunOpts != null) { + addContainerRunOpts(containerRunOpts, commandElements); + } - // Options must precede this in any order. Image name and command code follows. - commandElements.add(imageName); + // Options must precede this in any order. Image name and command code follows. + commandElements.add(imageName); - // Command to start the server - commandElements.add("server"); - if (libertyDebug) { - commandElements.add("debug"); - } else { - commandElements.add("run"); + // Command to start the server + commandElements.add("server"); + if (libertyDebug) { + commandElements.add("debug"); + } else { + commandElements.add("run"); + } + commandElements.add("defaultServer"); + + // All the Liberty variable definitions must appear after the -- option. + // Important: other Liberty options must appear before -- + commandElements.add("--"); + commandElements.add("--"+DEVMODE_PROJECT_ROOT+"="+DEVMODE_DIR_NAME); + + //return command.toString(); + String[] newCommand = commandElements.toArray(new String[commandElements.size()]); + info("Container command: "+String.join(" ", newCommand)); + // Release the held port sockets as late as possible so the container engine + // can bind those exact ports immediately after this command is issued. + return newCommand; + } finally { + for (ServerSocket s : heldSockets) { + closeQuietly(s); + } } - commandElements.add("defaultServer"); + } - // All the Liberty variable definitions must appear after the -- option. - // Important: other Liberty options must appear before -- - commandElements.add("--"); - commandElements.add("--"+DEVMODE_PROJECT_ROOT+"="+DEVMODE_DIR_NAME); + /** + * 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 + * the container command. The bound socket is added to {@code heldSockets} so the + * caller can release all of them in a single loop. + * + *

If {@code findAvailablePort} throws, the default port is returned without + * holding any socket (consistent with the pre-existing error-recovery behaviour). + * + * @param preferred the preferred port number to try first + * @param isDebug true if this is the debug port (affects ephemeral-port fallback) + * @param heldSockets accumulator list; the new socket is appended if binding succeeds + * @return the selected port number + */ + private int findAndHoldPort(int preferred, boolean isDebug, List heldSockets) + throws IOException { + int port; + try { + port = findAvailablePort(preferred, isDebug); + } catch (IOException x) { + error("An error occurred while trying to find an available network port. Using default port " + preferred + ".", x); + return preferred; + } + ServerSocket s = bindPortSocket(port); + if (s != null) { + heldSockets.add(s); + } + return port; + } - //return command.toString(); - String[] newCommand = commandElements.toArray(new String[commandElements.size()]); - info("Container command: "+String.join(" ", newCommand)); - return newCommand; + /** + * Closes a {@link ServerSocket}, logging any {@link IOException} at debug level + * rather than propagating it. Safe to call with a {@code null} argument. + */ + private void closeQuietly(ServerSocket s) { + if (s != null) { + try { + s.close(); + } catch (IOException e) { + debug("closeQuietly: error closing port socket: " + e.getMessage()); + } + } + } + + /** + * Attempts to bind the given port on the loopback interface to hold it open. + * Returns the bound ServerSocket, or null if binding fails (in which case the + * caller continues without holding a socket). + *

+ * SO_REUSEADDR is explicitly set to false so that the OS will not allow another + * process to bind the same port while this socket is open. + */ + ServerSocket bindPortSocket(int port) { + try { + if (OSUtil.isWindows()) { + return new ServerSocket(port); + } else { + ServerSocket s = new ServerSocket(); + s.setReuseAddress(false); + s.bind(new InetSocketAddress(InetAddress.getByName(null), port), 1); + return s; + } + } catch (IOException e) { + debug("bindPortSocket: could not hold port " + port + ": " + e.getMessage()); + return null; + } } /** @@ -1823,38 +1895,89 @@ private String getContainerOption(String optionName) { return null; } - private String generateNewContainerName() throws PluginExecutionException { + /** + * Generates a unique container name for this dev mode instance. + * + *

The preferred name is {@code "liberty-dev-"} (or just + * {@code "liberty-dev"} when {@code applicationId} is null or blank). Each module in a + * multi-module project therefore gets a stable, distinct default name that does not + * depend on the order in which modules start, eliminating the race condition that + * occurred when multiple modules computed the same numeric suffix from a shared + * container list. + * + *

If the preferred name is already in use by an existing container (e.g. a second + * instance of the same module, or two different apps that happen to share the same + * application ID), an incrementing numeric suffix is appended until an unused name is + * found (e.g. {@code "liberty-dev-modulea-1"}). + * + * @return a unique container name that is not currently used by any running or stopped container + * @throws PluginExecutionException if the container command cannot be executed + */ + String generateNewContainerName() throws PluginExecutionException { + // Build a sanitized name segment from applicationId so each module gets a stable, + // unique default name (e.g. "liberty-dev-modulea") that does not depend on the + // order in which modules start. This eliminates the race condition that occurred + // when multiple modules computed the same numeric suffix from a shared container list. + String appSegment = sanitizeContainerNameSegment(applicationId); + String preferredName = DEVMODE_CONTAINER_BASE_NAME + (appSegment.isEmpty() ? "" : "-" + appSegment); + String containerContNamesCmd = "ps -a --format \"{{.Names}}\""; debug("container names list command: " + containerContNamesCmd); String result = execContainerCmdWithPrefix(containerContNamesCmd, CONTAINER_TIMEOUT); - if (result == null) { - return DEVMODE_CONTAINER_BASE_NAME; - } - String[] containerNames = result.split(" "); - int highestNum = -1; - for(int i = 0; i < containerNames.length; i++) { - String name = removeSurroundingQuotes(containerNames[i]); - int num = -1; - if (name.equals(DEVMODE_CONTAINER_BASE_NAME)) { - num = 0; - } else if (name.startsWith(DEVMODE_CONTAINER_BASE_NAME + "-")) { - String[] nameSegments = name.split("-"); - // if DEVMODE_CONTAINER_BASE_NAME changes, the logic below may need to change - if (nameSegments.length == 3) { - String lastSegment = nameSegments[nameSegments.length - 1]; - try { - num = Integer.parseInt(lastSegment); - } catch (NumberFormatException e) { - debug("Last segment of container name is not a number."); - } - } - } - if (num > highestNum) { - highestNum = num; + + Set existingNames = new HashSet(); + if (result != null) { + for (String rawName : result.split(" ")) { + existingNames.add(removeSurroundingQuotes(rawName).trim()); } } - - return DEVMODE_CONTAINER_BASE_NAME + ((highestNum != -1) ? "-" + ++highestNum : ""); + + if (!existingNames.contains(preferredName)) { + return preferredName; + } + + // Preferred name is already in use (e.g. a second instance of the same module). + // Append an incrementing suffix until we find an unused name. + int suffix = 1; + String candidate; + do { + candidate = preferredName + "-" + suffix; + suffix++; + } while (existingNames.contains(candidate)); + return candidate; + } + + /** + * Sanitizes a string so it can be used as part of a container name. + * Container names must match {@code [a-zA-Z0-9][a-zA-Z0-9_.-]*}. + * The input is lower-cased, characters outside the allowed set are replaced + * with hyphens, consecutive separator runs are collapsed to a single hyphen, + * leading/trailing separators are removed, and the result is truncated to + * 110 characters so that the full {@code "liberty-dev-"} name stays + * within the 128-char limit that most container engines impose on container names. + * + * @param input the raw string to sanitize (e.g. applicationId) + * @return a sanitized, lower-case string suitable for use in a container name, + * or an empty string if input is null or blank + */ + static String sanitizeContainerNameSegment(String input) { + if (input == null || input.trim().isEmpty()) { + return ""; + } + // Lower-case the input and replace any character that is not alphanumeric, + // hyphen, underscore, or dot with a hyphen. + String sanitized = input.trim().toLowerCase().replaceAll("[^a-z0-9_.\\-]", "-"); + // Collapse consecutive separators (hyphens, dots, underscores) to a single hyphen. + sanitized = sanitized.replaceAll("[-_.]{2,}", "-"); + // Remove leading and trailing separators (hyphens, dots, underscores). + sanitized = sanitized.replaceAll("^[-_.]+|[-_.]+$", ""); + // Truncate to 110 characters so the full "liberty-dev-" name stays + // within the 128-char limit that most container engines impose on container names. + if (sanitized.length() > 110) { + sanitized = sanitized.substring(0, 110); + sanitized = sanitized.replaceAll("[-_.]+$", ""); + } + return sanitized; } /** @@ -6054,7 +6177,10 @@ public void writeDevcMetadata(boolean alive) { private void writeElement(XMLStreamWriter writer, String element, String optional) throws XMLStreamException { writer.writeStartElement(element); - if (optional != null) writer.writeCharacters(optional); + // Always write the characters (empty string when null) so the XML stream writer + // emits rather than the self-closing form. + // The test assertions depend on the open+close form being present. + writer.writeCharacters(optional != null ? optional : ""); writer.writeEndElement(); } } diff --git a/src/test/java/io/openliberty/tools/common/plugins/util/BaseDevUtilTest.java b/src/test/java/io/openliberty/tools/common/plugins/util/BaseDevUtilTest.java index 50f7de39..e34a1677 100644 --- a/src/test/java/io/openliberty/tools/common/plugins/util/BaseDevUtilTest.java +++ b/src/test/java/io/openliberty/tools/common/plugins/util/BaseDevUtilTest.java @@ -18,9 +18,10 @@ import java.io.File; import java.io.IOException; +import java.net.ServerSocket; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Collection; -import java.nio.file.Path; import java.util.Collections; import java.util.List; import java.util.Set; @@ -38,6 +39,8 @@ public class BaseDevUtilTest { public class DevTestUtil extends DevUtil { + private String containerCmdOutput = null; + public DevTestUtil(File serverDirectory, File sourceDirectory, File testSourceDirectory, File configDirectory, List resourceDirs, List webResourceDirs, boolean hotTests, boolean skipTests) throws IOException { super(temp.newFolder(), serverDirectory, sourceDirectory, testSourceDirectory, configDirectory, null, null, @@ -57,12 +60,28 @@ public DevTestUtil(File serverDirectory, File buildDir, boolean container) { container, null, null, null, 0, false, null, false, null, null, false, null, null, null, false, null, null, null, Collections.emptyMap()); } + public DevTestUtil(File buildDir, String applicationId, String containerCmdOutput) throws IOException { + super(buildDir, null, null, null, null, null, null, + null, false, false, false, false, false, false, applicationId, 30, 30, 5, 500, true, false, false, false, + false, null, null, null, 0, false, null, false, null, null, false, null, null, null, false, null, null, null, Collections.emptyMap()); + this.containerCmdOutput = containerCmdOutput; + } + /** - * Returns null for all container CLI commands to avoid requiring a running container engine. + * Returns null for all container CLI commands to avoid requiring a running container engine, + * unless a fake output has been set via the applicationId constructor. */ @Override protected String execContainerCmdWithPrefix(String command, int timeout, boolean throwExceptionOnError) { - return null; + return containerCmdOutput; + } + + public ServerSocket callBindPortSocket(int port) { + return bindPortSocket(port); + } + + public String callGenerateNewContainerName() throws PluginExecutionException { + return generateNewContainerName(); } /** @@ -307,4 +326,8 @@ public DevUtil getNewDevUtil(File serverDirectory, File buildDir) { public DevTestUtil getNewContainerUtil() { return new DevTestUtil(null, null, true); } + + public DevTestUtil getNewDevUtil(File buildDir, String applicationId, String containerCmdOutput) throws IOException { + return new DevTestUtil(buildDir, applicationId, containerCmdOutput); + } } diff --git a/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilContainerNameTest.java b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilContainerNameTest.java new file mode 100644 index 00000000..73f7c04d --- /dev/null +++ b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilContainerNameTest.java @@ -0,0 +1,126 @@ +/** + * (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 static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +public class DevUtilContainerNameTest extends BaseDevUtilTest { + + @Test + public void testSanitize_null() { + assertEquals("", DevUtil.sanitizeContainerNameSegment(null)); + } + + @Test + public void testSanitize_emptyString() { + assertEquals("", DevUtil.sanitizeContainerNameSegment("")); + } + + @Test + public void testSanitize_blankString() { + assertEquals("", DevUtil.sanitizeContainerNameSegment(" ")); + } + + @Test + public void testSanitize_simpleAlphanumeric() { + assertEquals("myapp", DevUtil.sanitizeContainerNameSegment("myApp")); + } + + @Test + public void testSanitize_hyphenAndDot() { + assertEquals("my-app.v1", DevUtil.sanitizeContainerNameSegment("my-app.v1")); + } + + @Test + public void testSanitize_specialCharactersReplacedWithHyphen() { + assertEquals("com.example-myapp", DevUtil.sanitizeContainerNameSegment("com.example:myApp")); + assertEquals("my-app", DevUtil.sanitizeContainerNameSegment("my app")); + assertEquals("a-b", DevUtil.sanitizeContainerNameSegment("a/b")); + } + + @Test + public void testSanitize_consecutiveSeparatorsCollapsed() { + assertEquals("a-b", DevUtil.sanitizeContainerNameSegment("a..b")); + assertEquals("a-b", DevUtil.sanitizeContainerNameSegment("a--b")); + assertEquals("a-b", DevUtil.sanitizeContainerNameSegment("a_.b")); + } + + @Test + public void testSanitize_leadingAndTrailingSeparatorsRemoved() { + assertEquals("app", DevUtil.sanitizeContainerNameSegment("-app-")); + assertEquals("app", DevUtil.sanitizeContainerNameSegment("..app..")); + } + + @Test + public void testSanitize_truncatesLongInput() { + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < 120; i++) sb.append('a'); + String result = DevUtil.sanitizeContainerNameSegment(sb.toString()); + assertTrue("Sanitized segment must be <= 110 chars", result.length() <= 110); + } + + @Test + public void testSanitize_upperCaseLowered() { + assertEquals("moduleabc", DevUtil.sanitizeContainerNameSegment("ModuleABC")); + } + + @Test + public void testGenerateContainerName_noExistingContainers() throws Exception { + DevTestUtil util = getNewDevUtil(temp.newFolder(), "moduleA", null); + assertEquals("liberty-dev-modulea", util.callGenerateNewContainerName()); + } + + @Test + public void testGenerateContainerName_preferredNameFree() throws Exception { + DevTestUtil util = getNewDevUtil(temp.newFolder(), "moduleA", "some-other-container liberty-dev-moduleb"); + assertEquals("liberty-dev-modulea", util.callGenerateNewContainerName()); + } + + @Test + public void testGenerateContainerName_preferredNameTaken_firstSuffix() throws Exception { + DevTestUtil util = getNewDevUtil(temp.newFolder(), "moduleA", "liberty-dev-modulea"); + assertEquals("liberty-dev-modulea-1", util.callGenerateNewContainerName()); + } + + @Test + public void testGenerateContainerName_preferredNameAndFirstSuffixTaken() throws Exception { + DevTestUtil util = getNewDevUtil(temp.newFolder(), "moduleA", "liberty-dev-modulea liberty-dev-modulea-1"); + assertEquals("liberty-dev-modulea-2", util.callGenerateNewContainerName()); + } + + @Test + public void testGenerateContainerName_differentModulesGetDifferentNames() throws Exception { + DevTestUtil utilA = getNewDevUtil(temp.newFolder(), "moduleA", null); + DevTestUtil utilB = getNewDevUtil(temp.newFolder(), "moduleB", null); + assertNotEquals(utilA.callGenerateNewContainerName(), utilB.callGenerateNewContainerName()); + } + + @Test + public void testGenerateContainerName_nullApplicationId() throws Exception { + DevTestUtil util = getNewDevUtil(temp.newFolder(), null, null); + assertEquals("liberty-dev", util.callGenerateNewContainerName()); + } + + @Test + public void testGenerateContainerName_applicationIdWithSpecialChars() throws Exception { + DevTestUtil util = getNewDevUtil(temp.newFolder(), "com.example:my-App", null); + assertEquals("liberty-dev-com.example-my-app", util.callGenerateNewContainerName()); + } +} diff --git a/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilPortHoldingTest.java b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilPortHoldingTest.java new file mode 100644 index 00000000..be5734dc --- /dev/null +++ b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilPortHoldingTest.java @@ -0,0 +1,108 @@ +/** + * (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 static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.ServerSocket; + +import org.junit.Test; + +public class DevUtilPortHoldingTest extends BaseDevUtilTest { + + private static int findFreePort() throws IOException { + try (ServerSocket s = new ServerSocket(0)) { + s.setReuseAddress(true); + return s.getLocalPort(); + } + } + + @Test + public void testFindAvailablePort_returnsFreePort() throws Exception { + DevTestUtil util = getNewDevUtil(temp.newFolder(), "test-app", null); + int freePort = findFreePort(); + int found = util.findAvailablePort(freePort, false); + assertTrue("findAvailablePort must return a port >= 1024", found >= 1024); + } + + @Test + public void testBindPortSocket_succeeds_onFreePort() throws Exception { + DevTestUtil util = getNewDevUtil(temp.newFolder(), "test-app", null); + int port = findFreePort(); + ServerSocket sock = util.callBindPortSocket(port); + try { + assertNotNull("bindPortSocket should return a non-null socket for a free port", sock); + assertEquals("Bound port must match requested port", port, sock.getLocalPort()); + } finally { + if (sock != null) sock.close(); + } + } + + @Test + public void testBindPortSocket_returnsNull_whenPortAlreadyBound() throws Exception { + DevTestUtil util = getNewDevUtil(temp.newFolder(), "test-app", null); + int port = findFreePort(); + try (ServerSocket occupier = new ServerSocket()) { + occupier.setReuseAddress(false); + occupier.bind(new InetSocketAddress(InetAddress.getByName(null), port), 1); + + ServerSocket result = util.callBindPortSocket(port); + assertNull("bindPortSocket must return null when the port is already in use", result); + } + } + + @Test + public void testPortHeldByBindSocket_notReturnedByFindAvailablePort() throws Exception { + DevTestUtil util = getNewDevUtil(temp.newFolder(), "test-app", null); + int port = findFreePort(); + ServerSocket held = util.callBindPortSocket(port); + assertNotNull("Pre-condition: bindPortSocket should succeed on a free port", held); + try { + int next = util.findAvailablePort(port, false); + assertNotEquals( + "findAvailablePort must not return a port that is already held open", + port, next); + } finally { + held.close(); + } + } + + @Test + public void testTwoSequentialFindAvailablePort_returnDifferentPorts_whenFirstIsHeld() throws Exception { + DevTestUtil util = getNewDevUtil(temp.newFolder(), "test-app", null); + int preferredPort = findFreePort(); + + int portA = util.findAvailablePort(preferredPort, false); + ServerSocket holdA = util.callBindPortSocket(portA); + assertNotNull("Module A must be able to hold its chosen port", holdA); + + try { + int portB = util.findAvailablePort(preferredPort, false); + assertNotEquals( + "Module B must receive a different port from module A's held port", + portA, portB); + } finally { + holdA.close(); + } + } +}