Reject invalid trace levels to avoid obscure exceptions during logging

h2/src/docsrc/html/changelog.html

@@ -21,7 +21,17 @@ <h1>Change Log</h1>
 
 <h2>Next Version (unreleased)</h2>
 <ul>
-<li>Nothing yet...
+<li>Issue #3855: StackOverflowError when using TRACE_LEVEL_SYSTEM_OUT=4
+</li>
+<li>PR #3854: Fix issues with newer JVMs
+</li>
+<li>PR #3851: Remove lift configuration
+</li>
+<li>PR #3847: Fix the NullPointerException caused by accessing 'user' after the session has been closed
+</li>
+<li>PR #3846: Make DB_CLOSE_ON_EXIT safer
+</li>
+<li>PR #3842: Adds support of quotedNulls in CSV handling
 </li>
 </ul>
 

h2/src/main/org/h2/message/TraceSystem.java

@@ -5,12 +5,21 @@
  */
 package org.h2.message;
 
+import static java.time.temporal.ChronoField.HOUR_OF_DAY;
+import static java.time.temporal.ChronoField.MINUTE_OF_HOUR;
+import static java.time.temporal.ChronoField.NANO_OF_SECOND;
+import static java.time.temporal.ChronoField.SECOND_OF_MINUTE;
+
 import java.io.IOException;
 import java.io.PrintStream;
 import java.io.PrintWriter;
 import java.io.Writer;
-import java.text.SimpleDateFormat;
+import java.time.OffsetDateTime;
+import java.time.format.DateTimeFormatter;
+import java.time.format.DateTimeFormatterBuilder;
+import java.util.Locale;
 import java.util.concurrent.atomic.AtomicReferenceArray;
+
 import org.h2.api.ErrorCode;
 import org.h2.engine.Constants;
 import org.h2.jdbc.JdbcException;
@@ -76,14 +85,15 @@ public class TraceSystem implements TraceWriter {
 
     private static final int CHECK_SIZE_EACH_WRITES = 4096;
 
+    private static DateTimeFormatter DATE_TIME_FORMATTER;
+
     private int levelSystemOut = DEFAULT_TRACE_LEVEL_SYSTEM_OUT;
     private int levelFile = DEFAULT_TRACE_LEVEL_FILE;
     private int levelMax;
     private int maxFileSize = DEFAULT_MAX_FILE_SIZE;
     private String fileName;
     private final AtomicReferenceArray<Trace> traces =
             new AtomicReferenceArray<>(Trace.MODULE_NAMES.length);
-    private SimpleDateFormat dateFormat;
     private Writer fileWriter;
     private PrintWriter printWriter;
     /**
@@ -181,6 +191,9 @@ public void setMaxFileSize(int max) {
      * @param level the new level
      */
     public void setLevelSystemOut(int level) {
+        if (level < PARENT || level > DEBUG) {
+            throw DbException.getInvalidValueException("TRACE_LEVEL_SYSTEM_OUT", level);
+        }
         levelSystemOut = level;
         updateLevel();
     }
@@ -211,6 +224,8 @@ public void setLevelFile(int level) {
                 }
                 writer.setName(name);
             }
+        } else if (level < PARENT || level > DEBUG) {
+            throw DbException.getInvalidValueException("TRACE_LEVEL_FILE", level);
         }
         levelFile = level;
         updateLevel();
@@ -220,11 +235,26 @@ public int getLevelFile() {
         return levelFile;
     }
 
-    private synchronized String format(String module, String s) {
-        if (dateFormat == null) {
-            dateFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss ");
+    private static String format(String module, String s) {
+        DateTimeFormatter dateTimeFormatter = DATE_TIME_FORMATTER;
+        if (dateTimeFormatter == null) {
+            dateTimeFormatter = initTimeFormatter();
         }
-        return dateFormat.format(System.currentTimeMillis()) + module + ": " + s;
+        return dateTimeFormatter.format(OffsetDateTime.now()) + ' ' + module + ": " + s;
+    }
+
+    private static DateTimeFormatter initTimeFormatter() {
+        return DATE_TIME_FORMATTER = new DateTimeFormatterBuilder()
+                .append(DateTimeFormatter.ISO_LOCAL_DATE)
+                .appendLiteral(' ')
+                .appendValue(HOUR_OF_DAY, 2)
+                .appendLiteral(':')
+                .appendValue(MINUTE_OF_HOUR, 2)
+                .appendLiteral(':')
+                .appendValue(SECOND_OF_MINUTE, 2)
+                .appendFraction(NANO_OF_SECOND, 6, 6, true)
+                .appendOffsetId()
+                .toFormatter(Locale.ROOT);
     }
 
     @Override
@@ -234,17 +264,20 @@ public void write(int level, int moduleId, String s, Throwable t) {
 
     @Override
     public void write(int level, String module, String s, Throwable t) {
-        if (level <= levelSystemOut || level > this.levelMax) {
-            // level <= levelSystemOut: the system out level is set higher
-            // level > this.level: the level for this module is set higher
-            sysOut.println(format(module, s));
-            if (t != null && levelSystemOut == DEBUG) {
-                t.printStackTrace(sysOut);
+        // level <= levelSystemOut: the system out level is set higher
+        // level > levelMax: the level for this module is set higher
+        boolean logToSystemOut = level <= levelSystemOut || level > levelMax;
+        boolean logToFile = fileName != null && level <= levelFile;
+        if (logToSystemOut || logToFile) {
+            String row = format(module, s);
+            if (logToSystemOut) {
+                sysOut.println(row);
+                if (t != null && levelSystemOut == DEBUG) {
+                    t.printStackTrace(sysOut);
+                }
             }
-        }
-        if (fileName != null) {
-            if (level <= levelFile) {
-                writeFile(format(module, s), t);
+            if (logToFile) {
+                writeFile(row, t);
             }
         }
     }

h2/src/test/org/h2/test/unit/TestTraceSystem.java

@@ -8,6 +8,9 @@
 import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
 import java.nio.charset.StandardCharsets;
+
+import org.h2.api.ErrorCode;
+import org.h2.message.DbException;
 import org.h2.message.TraceSystem;
 import org.h2.store.fs.FileUtils;
 import org.h2.test.TestBase;
@@ -32,6 +35,7 @@ public void test() throws Exception {
         testTraceDebug();
         testReadOnly();
         testAdapter();
+        testInvalidLevel();
     }
 
     private void testAdapter() {
@@ -75,4 +79,27 @@ private void testReadOnly() throws Exception {
         ts.close();
     }
 
+    private void testInvalidLevel() {
+        TraceSystem ts = new TraceSystem(null);
+        testInvalidLevel(ts, false, TraceSystem.PARENT - 1);
+        testInvalidLevel(ts, false, TraceSystem.ADAPTER);
+        testInvalidLevel(ts, false, TraceSystem.ADAPTER + 1);
+        testInvalidLevel(ts, true, TraceSystem.PARENT - 1);
+        testInvalidLevel(ts, true, TraceSystem.ADAPTER + 1);
+        ts.close();
+    }
+
+    private void testInvalidLevel(TraceSystem ts, boolean file, int level) {
+        try {
+            if (file) {
+                ts.setLevelFile(level);
+            } else {
+                ts.setLevelSystemOut(level);
+            }
+            fail("Expected DbException: 90008");
+        } catch (DbException ex) {
+            assertEquals(ErrorCode.INVALID_VALUE_2, ex.getErrorCode());
+        }
+    }
+
 }