From d9388fb3b00e6ba26964259a59e880589ca5eb53 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Mon, 3 Aug 2026 23:41:09 +0300 Subject: [PATCH] [#828] Fix infinite loop picking the errors-encountered LDIF file name The loop in LDIFConnectionHandler.processLDIFFile() which picks a free .errors-encountered. name never terminated: it had no break and kept appending numeric suffixes, hanging the handler thread and growing the path string until OutOfMemoryError. Replace both name-picking blocks (.applied and .errors-encountered) with a single testable helper which stops at the first free name. The rename and delete of the source file now use StaticUtils.renameFile() and Files.delete(), so a failure is logged and raises an alert instead of being silently ignored. --- .../protocols/LDIFConnectionHandler.java | 62 +++++---- .../LDIFConnectionHandlerTestCase.java | 120 ++++++++++++++++++ 2 files changed, 150 insertions(+), 32 deletions(-) diff --git a/opendj-server-legacy/src/main/java/org/opends/server/protocols/LDIFConnectionHandler.java b/opendj-server-legacy/src/main/java/org/opends/server/protocols/LDIFConnectionHandler.java index 771f625db0..535a5b802a 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/protocols/LDIFConnectionHandler.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/protocols/LDIFConnectionHandler.java @@ -13,11 +13,13 @@ * * Copyright 2008-2009 Sun Microsystems, Inc. * Portions Copyright 2014-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.protocols; import java.io.File; import java.io.IOException; +import java.nio.file.Files; import java.util.Collection; import java.util.Collections; import java.util.LinkedHashMap; @@ -311,21 +313,8 @@ private void processLDIFFile(File ldifFile) importConfig.setInvokeImportPlugins(false); importConfig.setValidateSchema(true); - String outputPath = inputPath + ".applied." + TimeThread.getGMTTime(); - if (new File(outputPath).exists()) - { - int i=2; - while (true) - { - if (! new File(outputPath + "." + i).exists()) - { - outputPath = outputPath + "." + i; - break; - } - - i++; - } - } + String outputPath = selectUnusedPath(inputPath + ".applied." + + TimeThread.getGMTTime()); LDIFExportConfig exportConfig = new LDIFExportConfig(outputPath, ExistingFileBehavior.APPEND); @@ -470,21 +459,8 @@ else if (changeRecord instanceof ModifyDNChangeRecordEntry) if (errorEncountered || !fullyProcessed) { - String renamedPath = inputPath + ".errors-encountered." + - TimeThread.getGMTTime(); - if (new File(renamedPath).exists()) - { - int i=2; - while (true) - { - if (! new File(renamedPath + "." + i).exists()) - { - renamedPath = renamedPath + "." + i; - } - - i++; - } - } + String renamedPath = selectUnusedPath(inputPath + ".errors-encountered." + + TimeThread.getGMTTime()); try { @@ -493,7 +469,7 @@ else if (changeRecord instanceof ModifyDNChangeRecordEntry) logger.trace("Renaming source file to " + renamedPath); } - ldifFile.renameTo(new File(renamedPath)); + renameFile(ldifFile, new File(renamedPath)); } catch (Exception e) { @@ -515,7 +491,7 @@ else if (changeRecord instanceof ModifyDNChangeRecordEntry) logger.trace("Deleting source file"); } - ldifFile.delete(); + Files.delete(ldifFile.toPath()); } catch (Exception e) { @@ -532,6 +508,28 @@ else if (changeRecord instanceof ModifyDNChangeRecordEntry) + /** + * Returns the provided path if no file exists at it. Otherwise, appends + * ".2", ".3", ... to the provided path and returns the first resulting path + * at which no file exists. + * + * @param basePath The desired path for the file. + * + * @return The first path at which no file exists. + */ + static String selectUnusedPath(String basePath) + { + String path = basePath; + for (int i=2; new File(path).exists(); i++) + { + path = basePath + "." + i; + } + + return path; + } + + + @Override public void toString(StringBuilder buffer) { diff --git a/opendj-server-legacy/src/test/java/org/opends/server/protocols/LDIFConnectionHandlerTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/protocols/LDIFConnectionHandlerTestCase.java index 801798d977..e9005efc4a 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/protocols/LDIFConnectionHandlerTestCase.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/protocols/LDIFConnectionHandlerTestCase.java @@ -22,6 +22,9 @@ import java.io.BufferedReader; import java.io.File; import java.io.FileReader; +import java.text.SimpleDateFormat; +import java.util.Date; +import java.util.TimeZone; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; @@ -335,6 +338,123 @@ else if (f.getName().startsWith( + /** + * Tests that the path selected for the applied and errors-encountered files + * skips the names which are already taken and stops at the first free one. + * + * @throws Exception If an unexpected problem occurs. + */ + @Test + public void testSelectUnusedPath() + throws Exception + { + File tempDir = TestCaseUtils.createTemporaryDirectory("testSelectUnusedPath"); + + try + { + String basePath = new File(tempDir, + "test.ldif.errors-encountered.20260803120000Z").getPath(); + + // No file exists at the base path, so it is used as-is. + assertEquals(LDIFConnectionHandler.selectUnusedPath(basePath), basePath); + + // The base path is taken, so ".2" is appended. + assertTrue(new File(basePath).createNewFile()); + assertEquals(LDIFConnectionHandler.selectUnusedPath(basePath), + basePath + ".2"); + + // The base path and several numbered variants are taken, so the first + // free variant is picked, with exactly one suffix appended. + assertTrue(new File(basePath + ".2").createNewFile()); + assertTrue(new File(basePath + ".3").createNewFile()); + assertEquals(LDIFConnectionHandler.selectUnusedPath(basePath), + basePath + ".4"); + } + finally + { + TestCaseUtils.deleteDirectory(tempDir); + } + } + + + + /** + * Tests that an unparseable LDIF file is renamed out of the way even when + * the errors-encountered name is already taken. The name-picking loop used + * to hang forever in that case (issue #828). + * + * @throws Exception If an unexpected problem occurs. + */ + @Test + public void testUnparseableLDIFErrorsNameTaken() + throws Exception + { + TestCaseUtils.initializeTestBackend(false); + + File tempDir = + TestCaseUtils.createTemporaryDirectory("testErrorsNameTaken"); + + TestCaseUtils.dsconfig( + "set-connection-handler-prop", + "--handler-name", "LDIF Connection Handler", + "--set", "ldif-directory:" + tempDir.getAbsolutePath(), + "--set", "enabled:true"); + + try + { + // Occupy every errors-encountered name the handler may pick while this + // test runs, so that it has to fall back to a numbered variant. + SimpleDateFormat timestampFormat = + new SimpleDateFormat("yyyyMMddHHmmss'Z'"); + timestampFormat.setTimeZone(TimeZone.getTimeZone("UTC")); + long now = System.currentTimeMillis(); + for (long t = now - 5000L; t <= now + 30000L; t += 1000L) + { + assertTrue(new File(tempDir, "testTaken.ldif.errors-encountered." + + timestampFormat.format(new Date(t))).createNewFile()); + } + + String path = TestCaseUtils.createTempFile( + "unparseable"); + + File tempFile = new File(path); + File newFile = new File(tempDir, "testTaken.ldif"); + assertTrue(tempFile.renameTo(newFile)); + + boolean numberedErrorsFound = false; + long stopTime = System.currentTimeMillis() + 20000L; + while (System.currentTimeMillis() < stopTime) + { + if (! newFile.exists()) + { + // The file should have been renamed to the first numbered variant + // of the errors-encountered name which was already taken. + for (File f : tempDir.listFiles()) + { + if (f.getName().startsWith("testTaken.ldif.errors-encountered.") && + f.getName().endsWith(".2")) + { + numberedErrorsFound = true; + } + } + + break; + } + + Thread.sleep(10); + } + + assertFalse(newFile.exists()); + assertTrue(numberedErrorsFound); + } + finally + { + TestCaseUtils.deleteDirectory(tempDir); + } + } + + + /** * Tests a number of methods that are part of the generic connection handler * API.