fix: parsing and merging of literals in InstantPatternDynamicFormatter

log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatterTest.java

@@ -17,6 +17,7 @@
 package org.apache.logging.log4j.core.util.internal.instant;
 
 import static java.util.Arrays.asList;
+import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
 import static org.apache.logging.log4j.core.util.internal.instant.InstantPatternDynamicFormatter.sequencePattern;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -37,10 +38,12 @@
 import org.apache.logging.log4j.core.util.internal.instant.InstantPatternDynamicFormatter.SecondPatternSequence;
 import org.apache.logging.log4j.core.util.internal.instant.InstantPatternDynamicFormatter.StaticPatternSequence;
 import org.apache.logging.log4j.util.Constants;
+import org.assertj.core.api.Assertions;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.jupiter.params.provider.ValueSource;
+import org.junitpioneer.jupiter.Issue;
 
 class InstantPatternDynamicFormatterTest {
 
@@ -55,8 +58,15 @@ void sequencing_should_work(
     static List<Arguments> sequencingTestCases() {
         final List<Arguments> testCases = new ArrayList<>();
 
+        // Single literals
+        testCases.add(Arguments.of("", ChronoUnit.DAYS, emptyList()));
+        testCases.add(Arguments.of("'foo'", ChronoUnit.DAYS, singletonList(literal("foo"))));
+        testCases.add(Arguments.of("''", ChronoUnit.DAYS, singletonList(literal("'"))));
+        testCases.add(Arguments.of("''''", ChronoUnit.DAYS, singletonList(literal("'"))));
+        testCases.add(Arguments.of("'o''clock'", ChronoUnit.DAYS, singletonList(literal("o'clock"))));
+
         // Merged constants
-        testCases.add(Arguments.of(":'foo',", ChronoUnit.DAYS, singletonList(new StaticPatternSequence(":foo,"))));
+        testCases.add(Arguments.of(":'foo',", ChronoUnit.DAYS, singletonList(literal(":foo,"))));
 
         // `SSSX` should be treated constant for daily updates
         testCases.add(Arguments.of("SSSX", ChronoUnit.DAYS, asList(pMilliSec(), pDyn("X"))));
@@ -108,6 +118,43 @@ static List<Arguments> sequencingTestCases() {
         return testCases;
     }
 
+    @ParameterizedTest
+    @ValueSource(strings = {"'", "'''", "'foo", "'foo''bar"})
+    void sequencing_should_fail_on_unterminated_literal(String pattern) {
+        Assertions.assertThatThrownBy(() -> sequencePattern(pattern, ChronoUnit.DAYS))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessageContaining("incomplete string literal");
+    }
+
+    static Stream<Arguments> merging_of_adjacent_constants_should_work() {
+        return Stream.of(
+                Arguments.of("  ", singletonList(literal("  "))),
+                Arguments.of(" ' ' ", singletonList(literal("   "))),
+                Arguments.of(" '' ", singletonList(literal(" ' "))),
+                Arguments.of("d  ", singletonList(pDyn("d'  '", ChronoUnit.DAYS))),
+                Arguments.of("d ' ' ", singletonList(pDyn("d'   '", ChronoUnit.DAYS))),
+                Arguments.of("d '' ", singletonList(pDyn("d' '' '", ChronoUnit.DAYS))),
+                Arguments.of("  d", singletonList(pDyn("'  'd", ChronoUnit.DAYS))),
+                Arguments.of(" ' ' d", singletonList(pDyn("'   'd", ChronoUnit.DAYS))),
+                Arguments.of(" '' d", singletonList(pDyn("' '' 'd", ChronoUnit.DAYS))),
+                Arguments.of("s  S", singletonList(pSec(1, "  ", 1))),
+                Arguments.of("s ' ' S", singletonList(pSec(1, "   ", 1))),
+                Arguments.of("s '' S", singletonList(pSec(1, " ' ", 1))));
+    }
+
+    @ParameterizedTest
+    @MethodSource
+    @Issue("https://github.com/apache/logging-log4j2/issues/3930")
+    void merging_of_adjacent_constants_should_work(
+            final String pattern, final List<PatternSequence> expectedSequences) {
+        final List<PatternSequence> actualSequences = sequencePattern(pattern, ChronoUnit.DAYS);
+        assertThat(actualSequences).isEqualTo(expectedSequences);
+    }
+
+    private static StaticPatternSequence literal(final String literal) {
+        return new StaticPatternSequence(literal);
+    }
+
     private static DynamicPatternSequence pDyn(final String singlePattern) {
         return new DynamicPatternSequence(singlePattern);
     }

log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java

@@ -253,7 +253,23 @@ private static List<PatternSequence> sequencePattern(final String pattern) {
 
             // Handle single-quotes
             else if (c == '\'') {
-                final int endIndex = pattern.indexOf('\'', startIndex + 1);
+                int endIndex = startIndex + 1;
+                while (endIndex < pattern.length()) {
+                    if (pattern.charAt(endIndex) == '\'') {
+                        if ((endIndex + 1) < pattern.length() && pattern.charAt(endIndex + 1) == '\'') {
+                            // Escaped apostrophe, skip it
+                            endIndex += 2;
+                        } else {
+                            // Closing quote found
+                            break;
+                        }
+                    } else {
+                        endIndex++;
+                    }
+                }
+                if (endIndex >= pattern.length()) {
+                    endIndex = -1; // Signal incomplete literal
+                }
                 final PatternSequence sequence = getStaticPatternSequence(pattern, startIndex, endIndex);
                 sequences.add(sequence);
                 startIndex = endIndex + 1;
@@ -276,7 +292,10 @@ private static PatternSequence getStaticPatternSequence(String pattern, int star
                     startIndex, pattern);
             throw new IllegalArgumentException(message);
         }
-        final String sequenceLiteral = (startIndex + 1) == endIndex ? "'" : pattern.substring(startIndex + 1, endIndex);
+        // Extract the literal, replacing escaped apostrophes with a single apostrophe
+        final String sequenceLiteral = (startIndex + 1) == endIndex
+                ? "'"
+                : pattern.substring(startIndex + 1, endIndex).replace("''", "'");
         return new StaticPatternSequence(sequenceLiteral);
     }
 
@@ -414,8 +433,32 @@ abstract static class PatternSequence {
 
         final ChronoUnit precision;
 
+        /**
+         * Creates a {@code PatternSequence} from a {@link java.time.format.DateTimeFormatter DateTimeFormatter} pattern
+         * and its precision.
+         *
+         * <p><strong>Quoting invariant:</strong> every literal in {@code pattern} must be enclosed in single quotes.
+         * To include a lone apostrophe as a literal, use {@code "''''"} (open quote, escaped apostrophe {@code ''}, close quote).
+         * Never use a bare {@code "''"}: while syntactically valid, it becomes ambiguous at concatenation boundaries.
+         * This contract lets us merge adjacent quoted blocks in a purely context-free way
+         * (drop the left closing quote and the right opening quote).</p>
+         *
+         * <p><b>Examples</b>:
+         * <pre>{@code
+         * "yyyy-MM-dd 'at' HH:mm"    // OK: 'at' is a quoted literal
+         * "HH 'o''clock'"            // OK: apostrophe inside a quoted block is escaped as ''
+         * "yyyy''''MM"               // OK: emits a literal apostrophe between year and month
+         * }</pre>
+         *
+         * @param pattern    a DateTimeFormatter pattern with all literals fully quoted
+         * @param precision  the largest {@link java.time.temporal.ChronoUnit ChronoUnit} interval over which the
+         *                   formatted output remains constant for this pattern
+         * @throws NullPointerException if {@code pattern} or {@code precision} is {@code null}
+         * @throws IllegalArgumentException if {@code pattern} is not a valid {@code DateTimeFormatter} pattern
+         */
         @SuppressWarnings("ReturnValueIgnored")
         PatternSequence(final String pattern, final ChronoUnit precision) {
+            assert !"''".equals(pattern);
             DateTimeFormatter.ofPattern(pattern); // Validate the pattern
             this.pattern = pattern;
             this.precision = precision;
@@ -456,37 +499,40 @@ abstract static class PatternSequence {
          * @return A merged formatter factory or {@code null} if merging is not possible.
          */
         @Nullable
-        PatternSequence tryMerge(PatternSequence other, ChronoUnit thresholdPrecision) {
-            return null;
-        }
+        abstract PatternSequence tryMerge(PatternSequence other, ChronoUnit thresholdPrecision);
 
         boolean isConstantForDurationOf(final ChronoUnit thresholdPrecision) {
             return precision.compareTo(thresholdPrecision) >= 0;
         }
 
         static String escapeLiteral(String literal) {
-            StringBuilder sb = new StringBuilder(literal.length() + 2);
-            boolean inSingleQuotes = false;
-            for (int i = 0; i < literal.length(); i++) {
-                char c = literal.charAt(i);
-                if (c == '\'') {
-                    if (inSingleQuotes) {
-                        sb.append("'");
-                    }
-                    inSingleQuotes = false;
-                    sb.append("''");
-                } else {
-                    if (!inSingleQuotes) {
-                        sb.append("'");
-                    }
-                    inSingleQuotes = true;
-                    sb.append(c);
-                }
-            }
-            if (inSingleQuotes) {
-                sb.append("'");
+            // Ensure that an empty literal is not quoted as "''",
+            // which would be interpreted as an apostrophe-escape sequence.
+            return literal.isEmpty() ? "" : "'" + literal.replace("'", "''") + "'";
+        }
+
+        /**
+         * Concatenates two DateTimeFormatter pattern fragments.
+         * <p>
+         * Precondition (enforced by the caller): every literal is fully quoted.
+         * Even a lone apostrophe is emitted as the quoted literal block "''''"
+         * (open quote, escaped apostrophe, and close quote).
+         * We never use a bare "''".
+         * </
+         */
+        static String mergePatterns(String left, String right) {
+            if (left.isEmpty()) return right;
+            if (right.isEmpty()) return left;
+
+            if (left.charAt(left.length() - 1) == '\'' && right.charAt(0) == '\'') {
+                // Stitch two adjacent quoted-literal blocks into one by removing the
+                // boundary quotes (close-then-open).
+                // Without this, concatenation would yield "...''..." at the join, which would change semantics.
+                //
+                // See: https://github.com/apache/logging-log4j2/issues/3930
+                return left.substring(0, left.length() - 1) + right.substring(1);
             }
-            return sb.toString();
+            return left + right;
         }
 
         @Override
@@ -537,12 +583,12 @@ PatternSequence tryMerge(PatternSequence other, ChronoUnit thresholdPrecision) {
             // We always merge consecutive static pattern factories
             if (other instanceof StaticPatternSequence) {
                 final StaticPatternSequence otherStatic = (StaticPatternSequence) other;
-                return new StaticPatternSequence(this.literal + otherStatic.literal);
+                return new StaticPatternSequence(mergePatterns(this.literal, otherStatic.literal));
             }
             // We also merge a static pattern factory with a DTF factory
             if (other instanceof DynamicPatternSequence) {
                 final DynamicPatternSequence otherDtf = (DynamicPatternSequence) other;
-                return new DynamicPatternSequence(this.pattern + otherDtf.pattern, otherDtf.precision);
+                return new DynamicPatternSequence(mergePatterns(this.pattern, otherDtf.pattern), otherDtf.precision);
             }
             return null;
         }
@@ -591,13 +637,13 @@ PatternSequence tryMerge(PatternSequence other, ChronoUnit thresholdPrecision) {
                     ChronoUnit precision = this.precision.getDuration().compareTo(otherDtf.precision.getDuration()) < 0
                             ? this.precision
                             : otherDtf.precision;
-                    return new DynamicPatternSequence(this.pattern + otherDtf.pattern, precision);
+                    return new DynamicPatternSequence(mergePatterns(this.pattern, otherDtf.pattern), precision);
                 }
             }
             // We merge a static pattern factory
             if (other instanceof StaticPatternSequence) {
                 final StaticPatternSequence otherStatic = (StaticPatternSequence) other;
-                return new DynamicPatternSequence(this.pattern + otherStatic.pattern, this.precision);
+                return new DynamicPatternSequence(mergePatterns(this.pattern, otherStatic.pattern), this.precision);
             }
             return null;
         }

src/changelog/.2.x.x/3930_date-converter.xml

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns="https://logging.apache.org/xml/ns"
+       xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xsi:schemaLocation="
+           https://logging.apache.org/xml/ns
+           https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+       type="fixed">
+  <issue id="3930" link="https://github.com/apache/logging-log4j2/issues/3930"/>
+  <description format="asciidoc">
+    Fix parsing and merging of literals in `InstantPatternDynamicFormatter`.
+  </description>
+</entry>