From e88ceaf0367b598f579816b03348ab8655abd82c Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Fri, 18 Sep 2026 14:54:34 +0300 Subject: [PATCH] Keep a logged value from forging a debug record A debug record is its prefix line followed by the message and the stack trace, all starting at column 0, so a line break in a logged value - a user name, a RelayState, a SAML attribute - started a line that read as a record of its own. Every SLF4J logger in the product ends there too, through the openam-slf4j binding. DebugRecordFormat now lays out the record for both debug file writers and StdDebugFile.printError: a single-line message is written exactly as before, every line break inside a message continues the record indented, and every line of the stack trace is indented, the exception's own message included. A line at column 0 is a record's prefix and nothing else. FedletLogger writes each parameter on a braced line of its own; a line break inside one is now written as the escape it stands for, so it cannot leave the braces. The audit log's ELFFormatter already escapes line breaks. --- .../plugin/log/impl/FedletLogger.java | 21 ++++- .../plugin/log/impl/FedletLoggerTest.java | 42 +++++++++ .../shared/debug/file/impl/DebugFileImpl.java | 17 +--- .../debug/file/impl/DebugRecordFormat.java | 90 +++++++++++++++++++ .../shared/debug/file/impl/StdDebugFile.java | 24 +---- .../file/impl/DebugRecordFormatTest.java | 74 +++++++++++++++ 6 files changed, 230 insertions(+), 38 deletions(-) create mode 100644 openam-federation/openam-federation-library/src/test/java/com/sun/identity/plugin/log/impl/FedletLoggerTest.java create mode 100644 openam-shared/src/main/java/com/sun/identity/shared/debug/file/impl/DebugRecordFormat.java create mode 100644 openam-shared/src/test/java/com/sun/identity/shared/debug/file/impl/DebugRecordFormatTest.java diff --git a/openam-federation/openam-federation-library/src/main/java/com/sun/identity/plugin/log/impl/FedletLogger.java b/openam-federation/openam-federation-library/src/main/java/com/sun/identity/plugin/log/impl/FedletLogger.java index dda2f6d75f..bf69e2eed2 100644 --- a/openam-federation/openam-federation-library/src/main/java/com/sun/identity/plugin/log/impl/FedletLogger.java +++ b/openam-federation/openam-federation-library/src/main/java/com/sun/identity/plugin/log/impl/FedletLogger.java @@ -24,6 +24,7 @@ * * $Id: FedletLogger.java,v 1.3 2008/08/06 17:28:14 exu Exp $ * + * Portions Copyrighted 2026 3A Systems LLC. */ package com.sun.identity.plugin.log.impl; @@ -124,20 +125,32 @@ public void access( } } - private static String formatMessage(String messageId, String[] param, + static String formatMessage(String messageId, String[] param, Object session) { if ((param == null) || (param.length == 0)) { return messageId; } else { for (int i = 0; i < param.length; i++) { - messageId = messageId + "\n{" + param[i] + "}"; + messageId = messageId + "\n{" + escapeLineBreaks(param[i]) + "}"; } if (session != null) { - messageId = messageId + "\n{" + session.toString() + "}"; + messageId = messageId + "\n{" + escapeLineBreaks(session.toString()) + "}"; } return messageId; } - } + } + + /** + * Each parameter is written on a braced line of its own; a line break inside one is + * request data and is written as the escape it stands for, so it cannot leave the braces + * and start a line that reads as another record. + */ + private static String escapeLineBreaks(String value) { + if (value == null) { + return null; + } + return value.replace("\r", "\\r").replace("\n", "\\n"); + } /** * Logs error messages to the error logs. diff --git a/openam-federation/openam-federation-library/src/test/java/com/sun/identity/plugin/log/impl/FedletLoggerTest.java b/openam-federation/openam-federation-library/src/test/java/com/sun/identity/plugin/log/impl/FedletLoggerTest.java new file mode 100644 index 0000000000..a6922301e0 --- /dev/null +++ b/openam-federation/openam-federation-library/src/test/java/com/sun/identity/plugin/log/impl/FedletLoggerTest.java @@ -0,0 +1,42 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package com.sun.identity.plugin.log.impl; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.testng.annotations.Test; + +public class FedletLoggerTest { + + @Test + public void everyParameterGoesOnItsOwnBracedLine() { + assertThat(FedletLogger.formatMessage("LOGIN_SUCCESS", new String[] {"alice", "sp"}, "sess")) + .isEqualTo("LOGIN_SUCCESS\n{alice}\n{sp}\n{sess}"); + } + + /** A line break inside a parameter must not leave the braces and start a line of its own. */ + @Test + public void aLineBreakInAParameterIsWrittenAsAnEscape() { + assertThat(FedletLogger.formatMessage("LOGIN_FAILED", new String[] {"alice\r\nSEVERE: forged\rx\ny"}, null)) + .isEqualTo("LOGIN_FAILED\n{alice\\r\\nSEVERE: forged\\rx\\ny}"); + } + + @Test + public void aMessageWithoutParametersIsTheMessageId() { + assertThat(FedletLogger.formatMessage("LOGIN_SUCCESS", null, null)).isEqualTo("LOGIN_SUCCESS"); + assertThat(FedletLogger.formatMessage("LOGIN_SUCCESS", new String[0], "sess")).isEqualTo("LOGIN_SUCCESS"); + } +} diff --git a/openam-shared/src/main/java/com/sun/identity/shared/debug/file/impl/DebugFileImpl.java b/openam-shared/src/main/java/com/sun/identity/shared/debug/file/impl/DebugFileImpl.java index 15b8f1296e..89189d634d 100644 --- a/openam-shared/src/main/java/com/sun/identity/shared/debug/file/impl/DebugFileImpl.java +++ b/openam-shared/src/main/java/com/sun/identity/shared/debug/file/impl/DebugFileImpl.java @@ -28,6 +28,7 @@ /** * Portions Copyrighted 2014-2016 ForgeRock AS. + * Portions Copyrighted 2026 3A Systems LLC. */ package com.sun.identity.shared.debug.file.impl; @@ -44,7 +45,6 @@ import java.io.FileWriter; import java.io.IOException; import java.io.PrintWriter; -import java.io.StringWriter; import java.text.DateFormat; import java.text.SimpleDateFormat; import java.util.Date; @@ -142,18 +142,7 @@ private boolean isConfigFileInitialized() { @Override public void writeIt(String prefix, String msg, Throwable th) throws IOException { - StringBuilder buf = new StringBuilder(); - buf.append(prefix); - buf.append('\n'); - buf.append(msg); - if (th != null) { - buf.append('\n'); - StringWriter stBuf = new StringWriter(DebugConstants.MAX_BUFFER_SIZE_EXCEPTION); - PrintWriter stackStream = new PrintWriter(stBuf); - th.printStackTrace(stackStream); - stackStream.flush(); - buf.append(stBuf.toString()); - } + String record = DebugRecordFormat.format(prefix, msg, th); if (isConfigChanged() || !isConfigFileInitialized()) { initialize(); @@ -166,7 +155,7 @@ public void writeIt(String prefix, String msg, Throwable th) throws IOException fileLock.readLock().lock(); try { if (debugWriter != null) { - debugWriter.println(buf.toString()); + debugWriter.println(record); } else { StdDebugFile.printError(prefix, msg, th); } diff --git a/openam-shared/src/main/java/com/sun/identity/shared/debug/file/impl/DebugRecordFormat.java b/openam-shared/src/main/java/com/sun/identity/shared/debug/file/impl/DebugRecordFormat.java new file mode 100644 index 0000000000..24d97abd8b --- /dev/null +++ b/openam-shared/src/main/java/com/sun/identity/shared/debug/file/impl/DebugRecordFormat.java @@ -0,0 +1,90 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package com.sun.identity.shared.debug.file.impl; + +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.regex.Pattern; + +import com.sun.identity.shared.debug.DebugConstants; + +/** + * Lays out one debug record: the prefix line, the message, and the stack trace if there is + * one. A record is told from the next by its prefix line starting at column 0, so nothing + * that is logged may start a line there: every line break inside the message and every + * line of the stack trace (an exception's message is logged data too) continues the record + * indented. A single-line message, the usual case, is written exactly as it always was, and + * a multi-line dump stays readable, indented. + *

+ * This is what keeps a value taken from a request - a user name, a RelayState, a SAML + * attribute - from forging a record of its own, whether it reaches the debug file through + * {@code Debug} directly or through the SLF4J binding. + */ +public final class DebugRecordFormat { + + /** What every continued line is indented with. */ + static final String CONTINUATION = " "; + + private static final Pattern LINE_BREAK = Pattern.compile("\r\n|[\r\n…

]"); + + private DebugRecordFormat() { + } + + /** + * @param prefix the record's prefix line (debug name, timestamp, thread, transaction) + * @param msg the message; {@code null} is written as {@code null} + * @param th the throwable whose stack trace follows the message, or {@code null} + * @return the record, without a trailing line break + */ + public static String format(String prefix, String msg, Throwable th) { + StringBuilder buf = new StringBuilder(prefix); + buf.append('\n'); + buf.append(continued(String.valueOf(msg), false)); + if (th != null) { + StringWriter trace = new StringWriter(DebugConstants.MAX_BUFFER_SIZE_EXCEPTION); + PrintWriter writer = new PrintWriter(trace); + th.printStackTrace(writer); + writer.flush(); + buf.append('\n'); + buf.append(continued(trace.toString(), true)); + } + return buf.toString(); + } + + /** + * {@code text} with every line break turned into a newline followed by the continuation + * indent, the first line indented as well when {@code indentFirst}; trailing line breaks + * are dropped. + */ + private static String continued(String text, boolean indentFirst) { + String[] lines = LINE_BREAK.split(text, -1); + int last = lines.length; + while (last > 1 && lines[last - 1].isEmpty()) { + last--; + } + StringBuilder out = new StringBuilder(text.length() + last * CONTINUATION.length()); + for (int i = 0; i < last; i++) { + if (i > 0) { + out.append('\n'); + } + if (i > 0 || indentFirst) { + out.append(CONTINUATION); + } + out.append(lines[i]); + } + return out.toString(); + } +} diff --git a/openam-shared/src/main/java/com/sun/identity/shared/debug/file/impl/StdDebugFile.java b/openam-shared/src/main/java/com/sun/identity/shared/debug/file/impl/StdDebugFile.java index 08efe48df3..06f6038210 100644 --- a/openam-shared/src/main/java/com/sun/identity/shared/debug/file/impl/StdDebugFile.java +++ b/openam-shared/src/main/java/com/sun/identity/shared/debug/file/impl/StdDebugFile.java @@ -12,17 +12,16 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2014-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package com.sun.identity.shared.debug.file.impl; import static org.forgerock.openam.utils.Time.*; -import com.sun.identity.shared.debug.DebugConstants; import com.sun.identity.shared.debug.file.DebugFile; import java.io.IOException; import java.io.PrintWriter; -import java.io.StringWriter; import java.text.SimpleDateFormat; import java.util.Date; @@ -49,18 +48,7 @@ public static StdDebugFile getInstance() { @Override public void writeIt(String prefix, String msg, Throwable th) throws IOException { - StringBuilder buf = new StringBuilder(prefix); - buf.append('\n'); - buf.append(msg); - if (th != null) { - buf.append('\n'); - StringWriter stBuf = new StringWriter(DebugConstants.MAX_BUFFER_SIZE_EXCEPTION); - PrintWriter stackStream = new PrintWriter(stBuf); - th.printStackTrace(stackStream); - stackStream.flush(); - buf.append(stBuf.toString()); - } - stdoutWriter.println(buf.toString()); + stdoutWriter.println(DebugRecordFormat.format(prefix, msg, th)); } /** @@ -72,13 +60,9 @@ public void writeIt(String prefix, String msg, Throwable th) throws IOException */ public static void printError(String debugName, String message, Throwable ex) { SimpleDateFormat dateFormat = new SimpleDateFormat("MM/dd/yyyy hh:mm:ss:SSS a zzz"); - String prefix = debugName + ":" + dateFormat.format(newDate()) + ": " + Thread.currentThread().toString() + - "\n"; + String prefix = debugName + ":" + dateFormat.format(newDate()) + ": " + Thread.currentThread().toString(); - System.err.println(prefix + message); - if (ex != null) { - ex.printStackTrace(System.err); - } + System.err.println(DebugRecordFormat.format(prefix, message, ex)); } } diff --git a/openam-shared/src/test/java/com/sun/identity/shared/debug/file/impl/DebugRecordFormatTest.java b/openam-shared/src/test/java/com/sun/identity/shared/debug/file/impl/DebugRecordFormatTest.java new file mode 100644 index 0000000000..dbe4e21933 --- /dev/null +++ b/openam-shared/src/test/java/com/sun/identity/shared/debug/file/impl/DebugRecordFormatTest.java @@ -0,0 +1,74 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package com.sun.identity.shared.debug.file.impl; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.testng.annotations.Test; + +public class DebugRecordFormatTest { + + private static final String PREFIX = "amAuth:09/18/2026 10:00:00:000 AM MSK: Thread[main,5,main]: TransactionId[x]"; + + @Test + public void aSingleLineMessageIsWrittenAsBefore() { + assertThat(DebugRecordFormat.format(PREFIX, "login failed for user", null)) + .isEqualTo(PREFIX + "\nlogin failed for user"); + } + + /** A line break carried in a logged value cannot start a line that reads as a new record. */ + @Test + public void aLineBreakInTheMessageContinuesTheRecordIndented() { + String forged = "user\n" + PREFIX + "\nforged message"; + + assertThat(DebugRecordFormat.format(PREFIX, forged, null)) + .isEqualTo(PREFIX + "\nuser\n " + PREFIX + "\n forged message"); + } + + @Test + public void everyKindOfLineBreakIsAContinuation() { + assertThat(DebugRecordFormat.format(PREFIX, "a\r\nb\rc
d
e…f", null)) + .isEqualTo(PREFIX + "\na\n b\n c\n d\n e\n f"); + } + + @Test + public void aMultiLineDumpStaysReadable() { + assertThat(DebugRecordFormat.format(PREFIX, "SAML response:\n\n \n", null)) + .isEqualTo(PREFIX + "\nSAML response:\n \n \n "); + } + + /** The exception's own message is logged data too: no line of the trace may start at column 0. */ + @Test + public void everyLineOfTheStackTraceIsIndented() { + String out = DebugRecordFormat.format(PREFIX, "failed", new IllegalStateException("boom\n" + PREFIX)); + + String[] lines = out.split("\n"); + assertThat(lines[0]).isEqualTo(PREFIX); + assertThat(lines[1]).isEqualTo("failed"); + assertThat(lines[2]).isEqualTo(" java.lang.IllegalStateException: boom"); + assertThat(lines[3]).isEqualTo(" " + PREFIX); + assertThat(lines[4]).startsWith(" \tat "); + for (int i = 2; i < lines.length; i++) { + assertThat(lines[i]).as("line " + i).startsWith(" "); + } + assertThat(out).doesNotEndWith("\n"); + } + + @Test + public void aNullMessageIsWrittenAsTheWordNull() { + assertThat(DebugRecordFormat.format(PREFIX, null, null)).isEqualTo(PREFIX + "\nnull"); + } +}