From 91712745f57d49cbd32a58921585a5fa4e886208 Mon Sep 17 00:00:00 2001 From: Jinwoo Hwang Date: Sun, 16 Nov 2025 12:58:31 -0500 Subject: [PATCH 1/2] GEODE-10523: Fix NullPointerException in gfsh startup - Add terminal initialization before promptLoop() - Implement history file migration from JLine 2 to JLine 3 format - Fix banner display to stdout in non-headless mode After migrating from Spring Shell 1.x to 3.x, terminal and lineReader were not being initialized, causing NPE when gfsh tried to read input. Also fixed incompatible history file format and missing banner output. --- .../management/internal/cli/shell/Gfsh.java | 75 ++++++++++++++++++- .../internal/cli/shell/jline/GfshHistory.java | 66 ++++++++++++++++ 2 files changed, 138 insertions(+), 3 deletions(-) diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java index 14976a93531..01265494f3a 100755 --- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java +++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java @@ -23,6 +23,8 @@ import java.io.IOException; import java.io.PrintStream; import java.net.URL; +import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.Paths; import java.text.MessageFormat; import java.util.ArrayList; @@ -38,6 +40,7 @@ import org.jline.reader.LineReader; import org.jline.reader.LineReaderBuilder; import org.jline.terminal.Terminal; +import org.jline.terminal.TerminalBuilder; import org.apache.geode.annotations.internal.MakeNotStatic; import org.apache.geode.annotations.internal.MutableForTesting; @@ -1094,6 +1097,9 @@ private void write(String message, boolean isError) { } protected LineReader createConsoleReader() { + // Check and migrate old history file format before creating LineReader + migrateHistoryFileIfNeeded(); + // Create GfshCompleter with our parser to enable TAB completion GfshCompleter completer = new GfshCompleter(this.parser); @@ -1110,6 +1116,50 @@ protected LineReader createConsoleReader() { return lineReader; } + /** + * Checks if the history file exists and is in old JLine 2 format. + * If so, backs it up and creates a new empty history file. + */ + private void migrateHistoryFileIfNeeded() { + try { + Path historyPath = Paths.get(getHistoryFileName()); + if (!Files.exists(historyPath)) { + return; + } + + // Check if file contains old format markers (lines starting with // or complex format) + java.util.List lines = Files.readAllLines(historyPath); + boolean hasOldFormat = false; + for (String line : lines) { + String trimmed = line.trim(); + // Old format had // comments and complex history format + if (trimmed.startsWith("//") || trimmed.startsWith("#")) { + hasOldFormat = true; + break; + } + // JLine 3 format should be simple: just command lines or :time:command format + // If we see anything complex, assume old format + if (trimmed.contains(":") && !trimmed.matches("^\\d+:\\d+:.*")) { + // Might be old format - be conservative and migrate + hasOldFormat = true; + break; + } + } + + if (hasOldFormat) { + // Backup old history file + Path backupPath = historyPath.getParent() + .resolve(historyPath.getFileName().toString() + ".old"); + Files.move(historyPath, backupPath, + java.nio.file.StandardCopyOption.REPLACE_EXISTING); + gfshFileLogger.info("Migrated old history file format. Backup saved to: " + backupPath); + } + } catch (IOException e) { + // Ignore - history migration is not critical + gfshFileLogger.warning("Could not migrate history file", e); + } + } + protected void logCommandToOutput(String processedLine) { String originalString = expandedPropCommandsMap.get(processedLine); if (originalString != null) { @@ -1163,9 +1213,10 @@ public void setLastExecutionStatus(int lastExecutionStatus) { } public void printAsInfo(String message) { - if (isHeadlessMode) { - println(message); - } else { + // Always print to stdout for user-facing messages like banner + println(message); + // Also log to file if not in headless mode + if (!isHeadlessMode) { logger.info(message); } } @@ -1589,10 +1640,28 @@ protected String expandProperties(final String input) { @Override public void run() { try { + // Print banner before initializing terminal to ensure it's visible printBannerAndWelcome(); + // Initialize terminal and line reader before starting prompt loop + if (!isHeadlessMode) { + initializeTerminal(); + createConsoleReader(); + } promptLoop(); } catch (Exception e) { gfshFileLogger.severe("Error in shell main loop", e); } } + + /** + * Initializes the JLine 3 Terminal for interactive shell use. + * This must be called before creating the LineReader. + */ + private void initializeTerminal() throws IOException { + if (terminal == null) { + terminal = TerminalBuilder.builder() + .system(true) + .build(); + } + } } diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/jline/GfshHistory.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/jline/GfshHistory.java index c967719c10a..77a5fc708e1 100644 --- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/jline/GfshHistory.java +++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/jline/GfshHistory.java @@ -28,6 +28,10 @@ * Overrides JLine History to add History without newline characters. * Updated for JLine 3.x: extends DefaultHistory instead of MemoryHistory * + *

+ * This implementation handles both old JLine 2 format history files (with timestamp comments) + * and new JLine 3 format. When loading an old format file, it will be converted to the new format. + * * @since GemFire 7.0 */ public class GfshHistory extends DefaultHistory { @@ -60,6 +64,68 @@ public void setHistoryFilePath(Path path) { } } + /** + * Override attach to handle migration from old JLine 2 history format. + * If loading fails due to format issues, we'll backup the old file and start fresh. + */ + @Override + public void attach(org.jline.reader.LineReader reader) { + try { + super.attach(reader); + } catch (Exception e) { + // Check if it's a history file format issue + Throwable cause = e; + while (cause != null) { + if (cause instanceof IllegalArgumentException + && cause.getMessage() != null + && cause.getMessage().contains("Bad history file syntax")) { + // Backup old history file and start fresh + migrateOldHistoryFile(); + // Try again with clean file + try { + super.attach(reader); + } catch (Exception ex) { + // If still fails, just continue without history + } + return; + } + cause = cause.getCause(); + } + // Re-throw if not a history format issue + if (e instanceof RuntimeException) { + throw (RuntimeException) e; + } + throw new RuntimeException(e); + } + } + + /** + * Migrates old JLine 2 format history file to JLine 3 format. + * Backs up the old file and creates a new one with only the valid history entries. + */ + private void migrateOldHistoryFile() { + if (historyFilePath == null || !Files.exists(historyFilePath)) { + return; + } + + try { + // Backup old history file + Path backupPath = historyFilePath.getParent() + .resolve(historyFilePath.getFileName().toString() + ".old"); + Files.move(historyFilePath, backupPath, + java.nio.file.StandardCopyOption.REPLACE_EXISTING); + + // Create new history file with timestamp + try (BufferedWriter writer = Files.newBufferedWriter(historyFilePath, + StandardOpenOption.CREATE, StandardOpenOption.WRITE)) { + writer.write("# Migrated from old format on " + new java.util.Date()); + writer.newLine(); + } + } catch (IOException e) { + // Ignore - just start with empty history + } + } + public void addToHistory(String buffer) { if (isAutoFlush()) { String redacted = ArgumentRedactor.redact(buffer.trim()); From 4c691769870c9fbb8280d4a0a5e0d5765d59e9ff Mon Sep 17 00:00:00 2001 From: Jinwoo Hwang Date: Mon, 17 Nov 2025 09:16:35 -0500 Subject: [PATCH 2/2] Restore original printAsInfo behavior - Revert printAsInfo() to use logger.info() in non-headless mode (matching pre-Jakarta migration behavior from commit 30cd67814^) - Move printBannerAndWelcome() after terminal initialization - This ensures banner output is consistent with original behavior --- .../geode/management/internal/cli/shell/Gfsh.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java index 01265494f3a..a2f4fbf693f 100755 --- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java +++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java @@ -1213,10 +1213,9 @@ public void setLastExecutionStatus(int lastExecutionStatus) { } public void printAsInfo(String message) { - // Always print to stdout for user-facing messages like banner - println(message); - // Also log to file if not in headless mode - if (!isHeadlessMode) { + if (isHeadlessMode) { + println(message); + } else { logger.info(message); } } @@ -1640,13 +1639,12 @@ protected String expandProperties(final String input) { @Override public void run() { try { - // Print banner before initializing terminal to ensure it's visible - printBannerAndWelcome(); // Initialize terminal and line reader before starting prompt loop if (!isHeadlessMode) { initializeTerminal(); createConsoleReader(); } + printBannerAndWelcome(); promptLoop(); } catch (Exception e) { gfshFileLogger.severe("Error in shell main loop", e);