From aa2ae8e7b77480d318744e0e27555a6248de4f82 Mon Sep 17 00:00:00 2001 From: Steve Hu Date: Fri, 30 Oct 2026 15:07:33 -0400 Subject: [PATCH 1/6] fix: report validated server options in module registry --- .../com/networknt/server/ServerConfig.java | 19 +++ .../com/networknt/server/ServerOption.java | 1 + .../server/ServerOptionRegistryTest.java | 120 ++++++++++++++++++ 3 files changed, 140 insertions(+) create mode 100644 server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java diff --git a/server-config/src/main/java/com/networknt/server/ServerConfig.java b/server-config/src/main/java/com/networknt/server/ServerConfig.java index c7197a0dd3..06e2560b5e 100644 --- a/server-config/src/main/java/com/networknt/server/ServerConfig.java +++ b/server-config/src/main/java/com/networknt/server/ServerConfig.java @@ -22,6 +22,7 @@ import org.slf4j.LoggerFactory; import java.util.Arrays; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -427,6 +428,24 @@ public static ServerConfig load(String configName) { return new ServerConfig(configName); } + /** Publish validated startup options without exposing decrypted configuration values. */ + void registerValidatedServerOptions() { + // Named configurations are test instances and must not replace the running server's entry. + if (this != instance) return; + Map config = new HashMap<>( + Config.getNoneDecryptedInstance().getJsonMapConfigNoCache(CONFIG_NAME)); + config.put(IO_THREADS, ioThreads); + config.put(WORKER_THREADS, workerThreads); + config.put(BUFFER_SIZE, bufferSize); + config.put(BACKLOG, backlog); + config.put(SERVER_STRING, serverString); + config.put(ALWAYS_SET_DATE, alwaysSetDate); + config.put(MAX_TRANSFER_FILE_SIZE, maxTransferFileSize); + config.put(ALLOW_UNESCAPED_CHARACTERS_IN_URL, allowUnescapedCharactersInUrl); + config.put(SHUTDOWN_TIMEOUT, shutdownTimeout); + ModuleRegistry.registerModule(CONFIG_NAME, ServerConfig.class.getName(), config, masks); + } + private void loadData() { if(mappedConfig != null) { Object object = mappedConfig.get(IP); diff --git a/server-config/src/main/java/com/networknt/server/ServerOption.java b/server-config/src/main/java/com/networknt/server/ServerOption.java index fc969afbb4..0f3f170bfa 100644 --- a/server-config/src/main/java/com/networknt/server/ServerOption.java +++ b/server-config/src/main/java/com/networknt/server/ServerOption.java @@ -52,6 +52,7 @@ protected static void serverOptionInit(Map mapConfig, ServerConf setToDefaultServerOption(serverOption, serverConfig); } } + serverConfig.registerValidatedServerOptions(); } /** diff --git a/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java b/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java new file mode 100644 index 0000000000..dccb98d081 --- /dev/null +++ b/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java @@ -0,0 +1,120 @@ +/* + * Copyright (c) 2016 Network New Technologies Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.networknt.server; + +import com.networknt.config.Config; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.*; + +class ServerOptionRegistryTest { + private static final String KEY = "server:" + ServerConfig.class.getName(); + + @AfterEach + void clearConfig() { + Config.getInstance().clear(); + ServerConfig.load(); + } + + @Test + void invalidOptionsAreReportedAsValidatedValues() { + Config.getInstance().clear(); + Map source = Config.getInstance().getJsonMapConfig("server"); + for (String name : List.of("ioThreads", "workerThreads", "bufferSize", "backlog", + "maxTransferFileSize", "shutdownTimeout")) { + source.put(name, -1); + } + source.put("serverString", ""); + source.put("maskConfigProperties", true); + ServerConfig server = ServerConfig.load(); + // Seed the same raw values that registration receives from a negative-valued server.yml. + ModuleRegistry.registerModule("server", ServerConfig.class.getName(), + new HashMap<>(source), List.of("keystorePass", "keyPass", "truststorePass", "bootstrapStorePass")); + Map before = reported(); + assertEquals(-1, before.get("maxTransferFileSize")); + + ServerOption.serverOptionInit(source, server); + + assertReportedOptions(server); + assertEquals(1_000_000L, reported().get("maxTransferFileSize")); + assertNull(reported().get("shutdownTimeout")); + assertEquals(-1, source.get("maxTransferFileSize")); + assertEquals(-1, before.get("maxTransferFileSize")); + for (String secret : List.of("keystorePass", "keyPass", "truststorePass", "bootstrapStorePass")) { + assertNotEquals(source.get(secret), reported().get(secret)); + } + // Publishing a new snapshot must also work on subsequent initialization. + ServerOption.serverOptionInit(source, server); + assertReportedOptions(server); + } + + @Test + void absentOptionsAreReportedWithEffectiveDefaults() { + Config.getInstance().clear(); + ServerConfig server = ServerConfig.load(); + for (ServerOption option : ServerOption.values()) { + server.getMappedConfig().remove(option.value()); + } + ServerOption.serverOptionInit(server.getMappedConfig(), server); + assertReportedOptions(server); + assertEquals(1_000_000L, reported().get("maxTransferFileSize")); + } + + @Test + void validOptionsAreReportedAndNamedConfigurationsDoNotReplaceRunningServer() { + Config.getInstance().clear(); + Map source = Config.getInstance().getJsonMapConfig("server"); + source.put("maxTransferFileSize", 1234567L); + source.put("shutdownTimeout", 1234); + ServerConfig server = ServerConfig.load(); + ServerOption.serverOptionInit(source, server); + assertReportedOptions(server); + assertEquals(1234567L, reported().get("maxTransferFileSize")); + assertEquals(1234, reported().get("shutdownTimeout")); + Map nonDecrypted = Config.getNoneDecryptedInstance().getJsonMapConfigNoCache("server"); + for (String secret : List.of("keystorePass", "keyPass", "truststorePass", "bootstrapStorePass")) { + assertEquals(nonDecrypted.get(secret), reported().get(secret)); + } + Map snapshot = reported(); + ServerConfig named = ServerConfig.load("server_invalid_option"); + ServerOption.serverOptionInit(named.getMappedConfig(), named); + assertSame(snapshot, reported()); + } + + @SuppressWarnings("unchecked") + private static Map reported() { + return (Map) ModuleRegistry.getModuleRegistry().get(KEY); + } + + private static void assertReportedOptions(ServerConfig server) { + Map config = reported(); + assertEquals(server.getIoThreads(), config.get("ioThreads")); + assertEquals(server.getWorkerThreads(), config.get("workerThreads")); + assertEquals(server.getBufferSize(), config.get("bufferSize")); + assertEquals(server.getBacklog(), config.get("backlog")); + assertEquals(server.getServerString(), config.get("serverString")); + assertEquals(server.isAlwaysSetDate(), config.get("alwaysSetDate")); + assertEquals(server.getMaxTransferFileSize(), config.get("maxTransferFileSize")); + assertEquals(server.isAllowUnescapedCharactersInUrl(), config.get("allowUnescapedCharactersInUrl")); + assertEquals(server.getShutdownTimeout(), config.get("shutdownTimeout")); + } +} From 3e08d784e5f0a84e5ff9bfd6f40b5389cc7e4841 Mon Sep 17 00:00:00 2001 From: Steve Hu Date: Fri, 30 Oct 2026 15:23:56 -0400 Subject: [PATCH 2/6] fix: validate server options before registration on every load --- .../com/networknt/server/ServerConfig.java | 31 ++- .../com/networknt/server/ServerOption.java | 42 ++-- .../server/ServerOptionRegistryTest.java | 203 +++++++++++++----- .../java/com/networknt/server/Server.java | 11 - 4 files changed, 193 insertions(+), 94 deletions(-) diff --git a/server-config/src/main/java/com/networknt/server/ServerConfig.java b/server-config/src/main/java/com/networknt/server/ServerConfig.java index 06e2560b5e..1cebcd98cc 100644 --- a/server-config/src/main/java/com/networknt/server/ServerConfig.java +++ b/server-config/src/main/java/com/networknt/server/ServerConfig.java @@ -384,6 +384,7 @@ public class ServerConfig { private static final List masks = Arrays.asList("keystorePass", "keyPass", "truststorePass", "bootstrapStorePass"); private Map mappedConfig; + private final Map validatedOptions; private static volatile ServerConfig instance; private ServerConfig() { @@ -400,6 +401,7 @@ private ServerConfig(String configName) { if (mappedConfig != null) { loadData(); } + validatedOptions = ServerOption.validateServerOptions(mappedConfig, this); } public static ServerConfig load() { @@ -421,29 +423,24 @@ public static ServerConfig load(String configName) { return instance; } instance = new ServerConfig(configName); - ModuleRegistry.registerModule(CONFIG_NAME, ServerConfig.class.getName(), Config.getNoneDecryptedInstance().getJsonMapConfigNoCache(CONFIG_NAME), masks); + Map reportedConfig = instance.validatedConfig( + Config.getNoneDecryptedInstance().getJsonMapConfigNoCache(CONFIG_NAME)); + ModuleRegistry.registerModule(CONFIG_NAME, ServerConfig.class.getName(), reportedConfig, masks); return instance; } } return new ServerConfig(configName); } - /** Publish validated startup options without exposing decrypted configuration values. */ - void registerValidatedServerOptions() { - // Named configurations are test instances and must not replace the running server's entry. - if (this != instance) return; - Map config = new HashMap<>( - Config.getNoneDecryptedInstance().getJsonMapConfigNoCache(CONFIG_NAME)); - config.put(IO_THREADS, ioThreads); - config.put(WORKER_THREADS, workerThreads); - config.put(BUFFER_SIZE, bufferSize); - config.put(BACKLOG, backlog); - config.put(SERVER_STRING, serverString); - config.put(ALWAYS_SET_DATE, alwaysSetDate); - config.put(MAX_TRANSFER_FILE_SIZE, maxTransferFileSize); - config.put(ALLOW_UNESCAPED_CHARACTERS_IN_URL, allowUnescapedCharactersInUrl); - config.put(SHUTDOWN_TIMEOUT, shutdownTimeout); - ModuleRegistry.registerModule(CONFIG_NAME, ServerConfig.class.getName(), config, masks); + /** Keep raw values and key presence except where validation corrected a configured option. */ + private Map validatedConfig(Map rawConfig) { + // Never fall back to mappedConfig: it can contain decrypted credentials. + if (rawConfig == null) return null; + Map reportedConfig = new HashMap<>(rawConfig); + validatedOptions.forEach((key, value) -> { + if (reportedConfig.containsKey(key)) reportedConfig.put(key, value); + }); + return reportedConfig; } private void loadData() { diff --git a/server-config/src/main/java/com/networknt/server/ServerOption.java b/server-config/src/main/java/com/networknt/server/ServerOption.java index 0f3f170bfa..001a9679d8 100644 --- a/server-config/src/main/java/com/networknt/server/ServerOption.java +++ b/server-config/src/main/java/com/networknt/server/ServerOption.java @@ -2,7 +2,9 @@ import com.networknt.config.Config; +import java.util.HashMap; import java.util.Map; +import java.util.function.Function; /** * This enum class is used to set and validate server options. @@ -18,20 +20,23 @@ * Note: To set these options, configuring them in server.yml */ public enum ServerOption { - IO_THREADS("ioThreads"), - WORKER_THREADS("workerThreads"), - BUFFER_SIZE("bufferSize"), - BACKLOG("backlog"), - SERVER_STRING("serverString"), - ALWAYS_SET_DATE("alwaysSetDate"), - MAX_TRANSFER_FILE_SIZE("maxTransferFileSize"), - ALLOW_UNESCAPED_CHARACTERS_IN_URL("allowUnescapedCharactersInUrl"), - SHUTDOWN_TIMEOUT("shutdownTimeout"); + IO_THREADS("ioThreads", ServerConfig::getIoThreads), + WORKER_THREADS("workerThreads", ServerConfig::getWorkerThreads), + BUFFER_SIZE("bufferSize", ServerConfig::getBufferSize), + BACKLOG("backlog", ServerConfig::getBacklog), + SERVER_STRING("serverString", ServerConfig::getServerString), + ALWAYS_SET_DATE("alwaysSetDate", ServerConfig::isAlwaysSetDate), + MAX_TRANSFER_FILE_SIZE("maxTransferFileSize", ServerConfig::getMaxTransferFileSize), + ALLOW_UNESCAPED_CHARACTERS_IN_URL("allowUnescapedCharactersInUrl", ServerConfig::isAllowUnescapedCharactersInUrl), + SHUTDOWN_TIMEOUT("shutdownTimeout", ServerConfig::getShutdownTimeout); private final String value; - ServerOption(String serverOption) { + private final Function effectiveValue; + + ServerOption(String serverOption, Function effectiveValue) { this.value = serverOption; + this.effectiveValue = effectiveValue; } public String value() { @@ -43,16 +48,27 @@ public String value() { * @param serverConfig object config generated from server.yml */ protected static void serverOptionInit(Map mapConfig, ServerConfig serverConfig) { + validateServerOptions(mapConfig, serverConfig); + } + + /** Validate fields and return corrections for explicitly configured options only. */ + static Map validateServerOptions(Map mapConfig, ServerConfig serverConfig) { + Map corrections = new HashMap<>(); for (ServerOption serverOption : ServerOption.values()) { - if (mapConfig.containsKey(serverOption.value())) { - if (!setServerOption(serverOption, mapConfig.get(serverOption.value), serverConfig)) { + if (mapConfig != null && mapConfig.containsKey(serverOption.value())) { + Object value = mapConfig.get(serverOption.value()); + boolean valid = setServerOption(serverOption, value, serverConfig); + if (!valid) { ServerConfig.logger.warn("Server option: " + serverOption.value() + " set in server.yml is invalid, has been reset to default value."); } + if (!valid || value == null) { + corrections.put(serverOption.value(), serverOption.effectiveValue.apply(serverConfig)); + } } else { setToDefaultServerOption(serverOption, serverConfig); } } - serverConfig.registerValidatedServerOptions(); + return corrections; } /** diff --git a/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java b/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java index dccb98d081..804f0dd3ce 100644 --- a/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java +++ b/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java @@ -19,85 +19,150 @@ import com.networknt.config.Config; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.util.SetSystemProperty; -import java.util.HashMap; -import java.util.List; +import java.io.ByteArrayInputStream; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.util.Map; import static org.junit.jupiter.api.Assertions.*; +@SetSystemProperty(key = "light_4j_config_password", value = "light") class ServerOptionRegistryTest { private static final String KEY = "server:" + ServerConfig.class.getName(); + private static final String ENCRYPTED = "CRYPT:e6e1b4a099902a2106509e87df909de4:cd9153addc755eb7e00d817c:e499d4501d0372fe37073d88a1025f1970a30cbd9acec7d5"; + private FixtureLoader decrypted; + private FixtureLoader raw; @AfterEach void clearConfig() { + Config.getInstance().setClassLoader(null); + Config.getNoneDecryptedInstance().setClassLoader(null); Config.getInstance().clear(); + Config.getNoneDecryptedInstance().clear(); ServerConfig.load(); } @Test - void invalidOptionsAreReportedAsValidatedValues() { - Config.getInstance().clear(); - Map source = Config.getInstance().getJsonMapConfig("server"); - for (String name : List.of("ioThreads", "workerThreads", "bufferSize", "backlog", - "maxTransferFileSize", "shutdownTimeout")) { - source.put(name, -1); - } - source.put("serverString", ""); - source.put("maskConfigProperties", true); - ServerConfig server = ServerConfig.load(); - // Seed the same raw values that registration receives from a negative-valued server.yml. - ModuleRegistry.registerModule("server", ServerConfig.class.getName(), - new HashMap<>(source), List.of("keystorePass", "keyPass", "truststorePass", "bootstrapStorePass")); - Map before = reported(); - assertEquals(-1, before.get("maxTransferFileSize")); - - ServerOption.serverOptionInit(source, server); - - assertReportedOptions(server); - assertEquals(1_000_000L, reported().get("maxTransferFileSize")); - assertNull(reported().get("shutdownTimeout")); - assertEquals(-1, source.get("maxTransferFileSize")); - assertEquals(-1, before.get("maxTransferFileSize")); - for (String secret : List.of("keystorePass", "keyPass", "truststorePass", "bootstrapStorePass")) { - assertNotEquals(source.get(secret), reported().get(secret)); - } - // Publishing a new snapshot must also work on subsequent initialization. - ServerOption.serverOptionInit(source, server); - assertReportedOptions(server); + void invalidOptionsAreValidatedBeforeRegistrationAndRemainCorrectAfterReload() { + useFixture(""" + ioThreads: -1 + workerThreads: -1 + bufferSize: -1 + backlog: -1 + serverString: '' + alwaysSetDate: null + maxTransferFileSize: -1 + allowUnescapedCharactersInUrl: null + shutdownTimeout: -1 + """, true); + ServerConfig first = ServerConfig.load(); + assertCorrectedOptions(first); + assertEquals(-1, first.getMappedConfig().get("maxTransferFileSize")); + assertEquals("password", first.getKeystorePass()); + assertNotEquals("password", reported().get("keystorePass")); + assertNotEquals(ENCRYPTED, reported().get("keystorePass")); + assertEquals(1, decrypted.reads); + assertEquals(1, raw.reads); + Map snapshot = reported(); + assertSame(first, ServerConfig.load()); + assertSame(snapshot, reported()); + assertEquals(1, raw.reads); + + // Exercise the production cache-clear/reload path without a server bind or manual registration. + Config.getInstance().clearConfigCache("server"); + Config.getNoneDecryptedInstance().clearConfigCache("server"); + ServerConfig reloaded = ServerConfig.load(); + assertNotSame(first, reloaded); + assertNotSame(snapshot, reported()); + assertCorrectedOptions(reloaded); + assertEquals(2, decrypted.reads); + assertEquals(2, raw.reads); } @Test - void absentOptionsAreReportedWithEffectiveDefaults() { - Config.getInstance().clear(); + void missingOptionsKeepTheRawKeySetWhileRuntimeGetsDefaults() { + useFixture("", false); ServerConfig server = ServerConfig.load(); + assertEquals(1_000_000L, server.getMaxTransferFileSize()); + assertTrue(server.getIoThreads() > 0); + assertNull(server.getShutdownTimeout()); + assertEquals(server.getMappedConfig().keySet(), reported().keySet()); for (ServerOption option : ServerOption.values()) { - server.getMappedConfig().remove(option.value()); + assertFalse(reported().containsKey(option.value())); } - ServerOption.serverOptionInit(server.getMappedConfig(), server); - assertReportedOptions(server); - assertEquals(1_000_000L, reported().get("maxTransferFileSize")); } @Test - void validOptionsAreReportedAndNamedConfigurationsDoNotReplaceRunningServer() { - Config.getInstance().clear(); - Map source = Config.getInstance().getJsonMapConfig("server"); - source.put("maxTransferFileSize", 1234567L); - source.put("shutdownTimeout", 1234); + void validRawRepresentationsAndEncryptedSecretsArePreservedWithMaskingDisabled() { + useFixture(""" + ioThreads: '2' + maxTransferFileSize: '1234567' + shutdownTimeout: '1234' + """, false); ServerConfig server = ServerConfig.load(); - ServerOption.serverOptionInit(source, server); - assertReportedOptions(server); - assertEquals(1234567L, reported().get("maxTransferFileSize")); - assertEquals(1234, reported().get("shutdownTimeout")); - Map nonDecrypted = Config.getNoneDecryptedInstance().getJsonMapConfigNoCache("server"); - for (String secret : List.of("keystorePass", "keyPass", "truststorePass", "bootstrapStorePass")) { - assertEquals(nonDecrypted.get(secret), reported().get(secret)); - } + assertEquals(1234567L, server.getMaxTransferFileSize()); + assertEquals(1234, server.getShutdownTimeout()); + assertEquals("2", reported().get("ioThreads")); + assertEquals("1234567", reported().get("maxTransferFileSize")); + assertEquals("1234", reported().get("shutdownTimeout")); + assertEquals(server.getMappedConfig().keySet(), reported().keySet()); + assertEquals("password", server.getKeystorePass()); + assertEquals(ENCRYPTED, reported().get("keystorePass")); + assertEquals(ENCRYPTED, reported().get("keyPass")); + assertEquals(ENCRYPTED, reported().get("truststorePass")); + assertEquals(ENCRYPTED, reported().get("bootstrapStorePass")); + assertEquals(1, raw.reads); + } + + @Test + void missingNonDecryptedConfigDoesNotThrowOrExposeDecryptedValues() { + useFixture("maxTransferFileSize: -1\n", false); + raw.yaml = null; + ServerConfig server = ServerConfig.load(); + assertEquals(1_000_000L, server.getMaxTransferFileSize()); + assertTrue(reported().isEmpty()); + } + + @Test + void missingConfigStillInitializesRuntimeDefaults() { + useFixture("", false); + decrypted.yaml = null; + raw.yaml = null; + ServerConfig server = ServerConfig.load(); + assertNull(server.getMappedConfig()); + assertEquals(1_000_000L, server.getMaxTransferFileSize()); + assertTrue(server.getIoThreads() > 0); + assertTrue(reported().isEmpty()); + } + + @Test + void namedAndStaleInstancesCannotOverwriteTheRunningServerEntry() { + useFixture("maxTransferFileSize: -1\n", false); + ServerConfig stale = ServerConfig.load(); + Config.getInstance().clearConfigCache("server"); + ServerConfig current = ServerConfig.load(); Map snapshot = reported(); + ServerOption.serverOptionInit(stale.getMappedConfig(), stale); ServerConfig named = ServerConfig.load("server_invalid_option"); - ServerOption.serverOptionInit(named.getMappedConfig(), named); + assertTrue(named.getIoThreads() > 0); assertSame(snapshot, reported()); + assertSame(current, ServerConfig.load()); + } + + private void useFixture(String options, boolean mask) { + String yaml = options + "maskConfigProperties: " + mask + "\n" + + "keystorePass: " + ENCRYPTED + "\n" + + "keyPass: " + ENCRYPTED + "\n" + + "truststorePass: " + ENCRYPTED + "\n" + + "bootstrapStorePass: " + ENCRYPTED + "\n"; + decrypted = new FixtureLoader(yaml); + raw = new FixtureLoader(yaml); + Config.getInstance().clear(); + Config.getNoneDecryptedInstance().clear(); + Config.getInstance().setClassLoader(decrypted); + Config.getNoneDecryptedInstance().setClassLoader(raw); } @SuppressWarnings("unchecked") @@ -105,8 +170,17 @@ private static Map reported() { return (Map) ModuleRegistry.getModuleRegistry().get(KEY); } - private static void assertReportedOptions(ServerConfig server) { + private static void assertCorrectedOptions(ServerConfig server) { Map config = reported(); + assertEquals(Runtime.getRuntime().availableProcessors() * 2, server.getIoThreads()); + assertEquals(200, server.getWorkerThreads()); + assertEquals(16384, server.getBufferSize()); + assertEquals(10000, server.getBacklog()); + assertEquals("L", server.getServerString()); + assertTrue(server.isAlwaysSetDate()); + assertEquals(1_000_000L, server.getMaxTransferFileSize()); + assertFalse(server.isAllowUnescapedCharactersInUrl()); + assertNull(server.getShutdownTimeout()); assertEquals(server.getIoThreads(), config.get("ioThreads")); assertEquals(server.getWorkerThreads(), config.get("workerThreads")); assertEquals(server.getBufferSize(), config.get("bufferSize")); @@ -115,6 +189,29 @@ private static void assertReportedOptions(ServerConfig server) { assertEquals(server.isAlwaysSetDate(), config.get("alwaysSetDate")); assertEquals(server.getMaxTransferFileSize(), config.get("maxTransferFileSize")); assertEquals(server.isAllowUnescapedCharactersInUrl(), config.get("allowUnescapedCharactersInUrl")); - assertEquals(server.getShutdownTimeout(), config.get("shutdownTimeout")); + assertTrue(config.containsKey("shutdownTimeout")); + assertNull(config.get("shutdownTimeout")); + } + + private static class FixtureLoader extends ClassLoader { + private String yaml; + private int reads; + + FixtureLoader(String yaml) { + super(ServerOptionRegistryTest.class.getClassLoader()); + this.yaml = yaml; + } + + @Override + public InputStream getResourceAsStream(String name) { + if (name.matches("(?:config/)?server\\.(?:yml|yaml|json)")) { + if (name.equals("server.yml") && yaml != null) { + reads++; + return new ByteArrayInputStream(yaml.getBytes(StandardCharsets.UTF_8)); + } + return null; + } + return super.getResourceAsStream(name); + } } } diff --git a/server/src/main/java/com/networknt/server/Server.java b/server/src/main/java/com/networknt/server/Server.java index 50f492b3eb..84ad1e6aa2 100644 --- a/server/src/main/java/com/networknt/server/Server.java +++ b/server/src/main/java/com/networknt/server/Server.java @@ -260,14 +260,6 @@ private static HttpHandler middlewareInit() { return handler; } - /** - * Method used to initialize server options. If the user has configured a valid server option, - * load it into the server configuration, otherwise use the default value - */ - private static void serverOptionInit() { - ServerOption.serverOptionInit(ServerConfig.load().getMappedConfig(), ServerConfig.load()); - } - static private boolean bind(HttpHandler handler, int port) { ServerConfig serverConfig = ServerConfig.load(); try { @@ -296,9 +288,6 @@ static private boolean bind(HttpHandler handler, int port) { builder.setSocketOption(Options.SSL_CLIENT_AUTH_MODE, SslClientAuthMode.REQUIRED); } - // set and validate server options - serverOptionInit(); - server = builder.setBufferSize(serverConfig.getBufferSize()).setIoThreads(serverConfig.getIoThreads()) // above seems slightly faster in some configurations .setSocketOption(Options.BACKLOG, serverConfig.getBacklog()) From 67d6885f3dbc34ed45b514f93f72a3ccbf5bdcaa Mon Sep 17 00:00:00 2001 From: Steve Hu Date: Fri, 30 Oct 2026 15:39:35 -0400 Subject: [PATCH 3/6] fix: guard server option reporting against source changes --- .../com/networknt/server/ServerConfig.java | 35 +++++++--- .../com/networknt/server/ServerOption.java | 20 +++--- .../networknt/server/ServerConfigTest.java | 4 -- .../server/ServerOptionRegistryTest.java | 70 ++++++++++++++++--- 4 files changed, 94 insertions(+), 35 deletions(-) diff --git a/server-config/src/main/java/com/networknt/server/ServerConfig.java b/server-config/src/main/java/com/networknt/server/ServerConfig.java index 1cebcd98cc..3d2d17b388 100644 --- a/server-config/src/main/java/com/networknt/server/ServerConfig.java +++ b/server-config/src/main/java/com/networknt/server/ServerConfig.java @@ -25,6 +25,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; /** * Server configuration class that maps to server.yml properties. This class is a singleton and can be shared @@ -384,7 +385,6 @@ public class ServerConfig { private static final List masks = Arrays.asList("keystorePass", "keyPass", "truststorePass", "bootstrapStorePass"); private Map mappedConfig; - private final Map validatedOptions; private static volatile ServerConfig instance; private ServerConfig() { @@ -401,7 +401,6 @@ private ServerConfig(String configName) { if (mappedConfig != null) { loadData(); } - validatedOptions = ServerOption.validateServerOptions(mappedConfig, this); } public static ServerConfig load() { @@ -422,23 +421,39 @@ public static ServerConfig load(String configName) { if (instance != null && instance.getMappedConfig() == Config.getInstance().getJsonMapConfig(configName)) { return instance; } - instance = new ServerConfig(configName); - Map reportedConfig = instance.validatedConfig( - Config.getNoneDecryptedInstance().getJsonMapConfigNoCache(CONFIG_NAME)); + ServerConfig loaded = new ServerConfig(configName); + Map corrections = ServerOption.validateServerOptions(loaded.mappedConfig, loaded, true); + instance = loaded; + Map reportedConfig = validatedConfig( + Config.getNoneDecryptedInstance().getJsonMapConfigNoCache(CONFIG_NAME), loaded.mappedConfig, corrections); ModuleRegistry.registerModule(CONFIG_NAME, ServerConfig.class.getName(), reportedConfig, masks); return instance; } } - return new ServerConfig(configName); + ServerConfig loaded = new ServerConfig(configName); + ServerOption.validateServerOptions(loaded.mappedConfig, loaded, false); + return loaded; } /** Keep raw values and key presence except where validation corrected a configured option. */ - private Map validatedConfig(Map rawConfig) { + private static Map validatedConfig(Map rawConfig, + Map mappedConfig, + Map corrections) { // Never fall back to mappedConfig: it can contain decrypted credentials. - if (rawConfig == null) return null; + if (rawConfig == null) { + if (mappedConfig != null) { + logger.warn("Non-decrypted server configuration is unavailable; server-info configuration will be empty."); + } + return null; + } Map reportedConfig = new HashMap<>(rawConfig); - validatedOptions.forEach((key, value) -> { - if (reportedConfig.containsKey(key)) reportedConfig.put(key, value); + corrections.forEach((key, correction) -> { + if (reportedConfig.containsKey(key) && Objects.equals(rawConfig.get(key), correction.originalValue())) { + reportedConfig.put(key, correction.effectiveValue()); + } else { + // Separate Config reads may observe a source change. Do not overwrite a newer raw value. + logger.warn("Server option {} differs between validated and non-decrypted configuration; retaining the raw reporting value.", key); + } }); return reportedConfig; } diff --git a/server-config/src/main/java/com/networknt/server/ServerOption.java b/server-config/src/main/java/com/networknt/server/ServerOption.java index 001a9679d8..3f206f3706 100644 --- a/server-config/src/main/java/com/networknt/server/ServerOption.java +++ b/server-config/src/main/java/com/networknt/server/ServerOption.java @@ -43,26 +43,19 @@ public String value() { return this.value; } - /** - * @param mapConfig map config generated from server.yml - * @param serverConfig object config generated from server.yml - */ - protected static void serverOptionInit(Map mapConfig, ServerConfig serverConfig) { - validateServerOptions(mapConfig, serverConfig); - } - /** Validate fields and return corrections for explicitly configured options only. */ - static Map validateServerOptions(Map mapConfig, ServerConfig serverConfig) { - Map corrections = new HashMap<>(); + static Map validateServerOptions(Map mapConfig, ServerConfig serverConfig, + boolean warnInvalid) { + Map corrections = new HashMap<>(); for (ServerOption serverOption : ServerOption.values()) { if (mapConfig != null && mapConfig.containsKey(serverOption.value())) { Object value = mapConfig.get(serverOption.value()); boolean valid = setServerOption(serverOption, value, serverConfig); - if (!valid) { + if (!valid && value != null && warnInvalid) { ServerConfig.logger.warn("Server option: " + serverOption.value() + " set in server.yml is invalid, has been reset to default value."); } if (!valid || value == null) { - corrections.put(serverOption.value(), serverOption.effectiveValue.apply(serverConfig)); + corrections.put(serverOption.value(), new Correction(value, serverOption.effectiveValue.apply(serverConfig))); } } else { setToDefaultServerOption(serverOption, serverConfig); @@ -71,6 +64,9 @@ static Map validateServerOptions(Map mapConfig, return corrections; } + /** Capture the validated input so a later mutable-map update cannot change the comparison. */ + record Correction(Object originalValue, Object effectiveValue) {} + /** * Method used to validate and set server options * diff --git a/server-config/src/test/java/com/networknt/server/ServerConfigTest.java b/server-config/src/test/java/com/networknt/server/ServerConfigTest.java index 4e6a74819d..13673f30f8 100644 --- a/server-config/src/test/java/com/networknt/server/ServerConfigTest.java +++ b/server-config/src/test/java/com/networknt/server/ServerConfigTest.java @@ -40,7 +40,6 @@ public void testNullEnv() { public void testDefaultServerOptions() { config.clear(); ServerConfig serverConfig = ServerConfig.load("server"); - ServerOption.serverOptionInit(serverConfig.getMappedConfig(),serverConfig); Assertions.assertEquals(1024*16, serverConfig.getBufferSize()); Assertions.assertEquals(Runtime.getRuntime().availableProcessors() * 2, serverConfig.getIoThreads()); Assertions.assertEquals(200, serverConfig.getWorkerThreads()); @@ -53,7 +52,6 @@ public void testDefaultServerOptions() { public void testInvalidServerOptions() { config.clear(); ServerConfig serverConfig = ServerConfig.load("server_invalid_option"); - ServerOption.serverOptionInit(serverConfig.getMappedConfig(),serverConfig); Assertions.assertEquals(1024*16, serverConfig.getBufferSize()); Assertions.assertEquals(Runtime.getRuntime().availableProcessors() * 2, serverConfig.getIoThreads()); Assertions.assertEquals(200, serverConfig.getWorkerThreads()); @@ -67,7 +65,6 @@ public void testInvalidServerOptions() { public void testValidServerOptions() { config.clear(); ServerConfig serverConfig = ServerConfig.load("server_valid_option"); - ServerOption.serverOptionInit(serverConfig.getMappedConfig(),serverConfig); Assertions.assertEquals(10000, serverConfig.getBufferSize()); Assertions.assertEquals(1, serverConfig.getIoThreads()); Assertions.assertEquals(100, serverConfig.getWorkerThreads()); @@ -81,7 +78,6 @@ public void testValidServerOptions() { public void testMaxTransferFileSize() { config.clear(); ServerConfig serverConfig = ServerConfig.load(); - ServerOption.serverOptionInit(serverConfig.getMappedConfig(), serverConfig); Assertions.assertEquals(1000000, serverConfig.getMaxTransferFileSize()); } } diff --git a/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java b/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java index 804f0dd3ce..b044290a0f 100644 --- a/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java +++ b/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java @@ -17,7 +17,12 @@ package com.networknt.server; import com.networknt.config.Config; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.util.SetSystemProperty; @@ -34,9 +39,20 @@ class ServerOptionRegistryTest { private static final String ENCRYPTED = "CRYPT:e6e1b4a099902a2106509e87df909de4:cd9153addc755eb7e00d817c:e499d4501d0372fe37073d88a1025f1970a30cbd9acec7d5"; private FixtureLoader decrypted; private FixtureLoader raw; + private final Logger logger = (Logger) ServerConfig.logger; + private ListAppender logs; + + @BeforeEach + void captureLogs() { + logs = new ListAppender<>(); + logs.start(); + logger.addAppender(logs); + } @AfterEach void clearConfig() { + logger.detachAppender(logs); + logs.stop(); Config.getInstance().setClassLoader(null); Config.getNoneDecryptedInstance().setClassLoader(null); Config.getInstance().clear(); @@ -123,6 +139,8 @@ void missingNonDecryptedConfigDoesNotThrowOrExposeDecryptedValues() { ServerConfig server = ServerConfig.load(); assertEquals(1_000_000L, server.getMaxTransferFileSize()); assertTrue(reported().isEmpty()); + assertTrue(logs.list.stream().anyMatch(event -> event.getLevel() == Level.WARN + && event.getFormattedMessage().contains("server-info configuration will be empty"))); } @Test @@ -138,17 +156,49 @@ void missingConfigStillInitializesRuntimeDefaults() { } @Test - void namedAndStaleInstancesCannotOverwriteTheRunningServerEntry() { + void changedRawValuesAreNotOverwrittenByStaleCorrections() { useFixture("maxTransferFileSize: -1\n", false); - ServerConfig stale = ServerConfig.load(); - Config.getInstance().clearConfigCache("server"); - ServerConfig current = ServerConfig.load(); - Map snapshot = reported(); - ServerOption.serverOptionInit(stale.getMappedConfig(), stale); + raw.yaml = raw.yaml.replace("maxTransferFileSize: -1", "maxTransferFileSize: 7654321"); + raw.beforeRead = () -> Config.getInstance().getJsonMapConfig("server").put("maxTransferFileSize", 7654321); + ServerConfig server = ServerConfig.load(); + assertEquals(7654321, server.getMappedConfig().get("maxTransferFileSize")); + assertEquals(1_000_000L, server.getMaxTransferFileSize()); + assertEquals(7654321, reported().get("maxTransferFileSize")); + assertTrue(logs.list.stream().anyMatch(event -> event.getLevel() == Level.WARN + && event.getFormattedMessage().contains("retaining the raw reporting value"))); + assertEquals(ENCRYPTED, reported().get("keystorePass")); + } + + @Test + void blankDefaultsAndNamedLoadsDoNotProduceInvalidOptionWarnings() { + useFixture("shutdownTimeout: null\n", false); + ServerConfig server = ServerConfig.load(); + assertNull(server.getShutdownTimeout()); + assertTrue(reported().containsKey("shutdownTimeout")); + assertNull(reported().get("shutdownTimeout")); ServerConfig named = ServerConfig.load("server_invalid_option"); - assertTrue(named.getIoThreads() > 0); - assertSame(snapshot, reported()); - assertSame(current, ServerConfig.load()); + assertEquals(200, named.getWorkerThreads()); + assertEquals(16384, named.getBufferSize()); + assertEquals(0, invalidWarnings()); + } + + @Test + void explicitInvalidOptionsWarnOnlyOnFreshCanonicalLoads() { + useFixture("maxTransferFileSize: -1\n", false); + ServerConfig first = ServerConfig.load(); + assertEquals(1, invalidWarnings()); + assertSame(first, ServerConfig.load()); + assertEquals(1, invalidWarnings()); + Config.getInstance().clearConfigCache("server"); + ServerConfig second = ServerConfig.load(); + assertNotSame(first, second); + assertEquals(1_000_000L, second.getMaxTransferFileSize()); + assertEquals(2, invalidWarnings()); + } + + private long invalidWarnings() { + return logs.list.stream().filter(event -> event.getLevel() == Level.WARN + && event.getFormattedMessage().contains("has been reset to default value")).count(); } private void useFixture(String options, boolean mask) { @@ -196,6 +246,7 @@ private static void assertCorrectedOptions(ServerConfig server) { private static class FixtureLoader extends ClassLoader { private String yaml; private int reads; + private Runnable beforeRead; FixtureLoader(String yaml) { super(ServerOptionRegistryTest.class.getClassLoader()); @@ -207,6 +258,7 @@ public InputStream getResourceAsStream(String name) { if (name.matches("(?:config/)?server\\.(?:yml|yaml|json)")) { if (name.equals("server.yml") && yaml != null) { reads++; + if (beforeRead != null) beforeRead.run(); return new ByteArrayInputStream(yaml.getBytes(StandardCharsets.UTF_8)); } return null; From 7f47636badddbc4c60be55782472a0e8e85110a4 Mon Sep 17 00:00:00 2001 From: Steve Hu Date: Fri, 30 Oct 2026 15:51:26 -0400 Subject: [PATCH 4/6] fix: compare all server option inputs across config reads --- .../com/networknt/server/ServerConfig.java | 46 ++++++---- .../com/networknt/server/ServerOption.java | 53 +++++++----- .../server/ServerOptionRegistryTest.java | 86 +++++++++++++++++-- 3 files changed, 138 insertions(+), 47 deletions(-) diff --git a/server-config/src/main/java/com/networknt/server/ServerConfig.java b/server-config/src/main/java/com/networknt/server/ServerConfig.java index 3d2d17b388..136e0dd8f7 100644 --- a/server-config/src/main/java/com/networknt/server/ServerConfig.java +++ b/server-config/src/main/java/com/networknt/server/ServerConfig.java @@ -25,7 +25,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; /** * Server configuration class that maps to server.yml properties. This class is a singleton and can be shared @@ -421,38 +420,47 @@ public static ServerConfig load(String configName) { if (instance != null && instance.getMappedConfig() == Config.getInstance().getJsonMapConfig(configName)) { return instance; } - ServerConfig loaded = new ServerConfig(configName); - Map corrections = ServerOption.validateServerOptions(loaded.mappedConfig, loaded, true); - instance = loaded; + ValidatedConfig loaded = loadValidated(configName); + instance = loaded.config(); Map reportedConfig = validatedConfig( - Config.getNoneDecryptedInstance().getJsonMapConfigNoCache(CONFIG_NAME), loaded.mappedConfig, corrections); + Config.getNoneDecryptedInstance().getJsonMapConfigNoCache(CONFIG_NAME), loaded); ModuleRegistry.registerModule(CONFIG_NAME, ServerConfig.class.getName(), reportedConfig, masks); return instance; } } - ServerConfig loaded = new ServerConfig(configName); - ServerOption.validateServerOptions(loaded.mappedConfig, loaded, false); - return loaded; + return loadValidated(configName).config(); } - /** Keep raw values and key presence except where validation corrected a configured option. */ - private static Map validatedConfig(Map rawConfig, - Map mappedConfig, - Map corrections) { + private static ValidatedConfig loadValidated(String configName) { + ServerConfig config = new ServerConfig(configName); + return new ValidatedConfig(config, ServerOption.validateServerOptions( + config.mappedConfig, config, CONFIG_NAME.equals(configName))); + } + + private record ValidatedConfig(ServerConfig config, Map options) {} + + /** Preserve raw key presence and values unless the same input was corrected by validation. */ + private static Map validatedConfig(Map rawConfig, ValidatedConfig loaded) { // Never fall back to mappedConfig: it can contain decrypted credentials. if (rawConfig == null) { - if (mappedConfig != null) { + if (loaded.config().mappedConfig != null) { logger.warn("Non-decrypted server configuration is unavailable; server-info configuration will be empty."); } return null; } Map reportedConfig = new HashMap<>(rawConfig); - corrections.forEach((key, correction) -> { - if (reportedConfig.containsKey(key) && Objects.equals(rawConfig.get(key), correction.originalValue())) { - reportedConfig.put(key, correction.effectiveValue()); - } else { - // Separate Config reads may observe a source change. Do not overwrite a newer raw value. - logger.warn("Server option {} differs between validated and non-decrypted configuration; retaining the raw reporting value.", key); + loaded.options().forEach((key, option) -> { + boolean rawPresent = rawConfig.containsKey(key); + if (option.present() && !rawPresent) { + logger.warn("Server option {} is absent from non-decrypted reporting configuration; retaining its absence.", key); + } else if (!option.present() && rawPresent) { + logger.warn("Server option {} is present only in non-decrypted reporting configuration; retaining the raw reporting value.", key); + } else if (rawPresent) { + if (!option.matches(rawConfig.get(key))) { + logger.warn("Server option {} differs between validated and non-decrypted configuration; retaining the raw reporting value.", key); + } else if (option.corrected()) { + reportedConfig.put(key, option.effectiveValue()); + } } }); return reportedConfig; diff --git a/server-config/src/main/java/com/networknt/server/ServerOption.java b/server-config/src/main/java/com/networknt/server/ServerOption.java index 3f206f3706..2e2ed0e876 100644 --- a/server-config/src/main/java/com/networknt/server/ServerOption.java +++ b/server-config/src/main/java/com/networknt/server/ServerOption.java @@ -1,9 +1,12 @@ package com.networknt.server; import com.networknt.config.Config; +import com.networknt.config.ConfigInjection; +import com.networknt.decrypt.Decryptor; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.function.Function; /** @@ -43,29 +46,39 @@ public String value() { return this.value; } - /** Validate fields and return corrections for explicitly configured options only. */ - static Map validateServerOptions(Map mapConfig, ServerConfig serverConfig, - boolean warnInvalid) { - Map corrections = new HashMap<>(); + /** Validate fields and capture the presence and input of every option for reporting comparisons. */ + static Map validateServerOptions(Map mapConfig, ServerConfig serverConfig, + boolean warnInvalid) { + Map options = new HashMap<>(); for (ServerOption serverOption : ServerOption.values()) { - if (mapConfig != null && mapConfig.containsKey(serverOption.value())) { - Object value = mapConfig.get(serverOption.value()); - boolean valid = setServerOption(serverOption, value, serverConfig); - if (!valid && value != null && warnInvalid) { - ServerConfig.logger.warn("Server option: " + serverOption.value() + " set in server.yml is invalid, has been reset to default value."); - } - if (!valid || value == null) { - corrections.put(serverOption.value(), new Correction(value, serverOption.effectiveValue.apply(serverConfig))); - } - } else { - setToDefaultServerOption(serverOption, serverConfig); + boolean present = mapConfig != null && mapConfig.containsKey(serverOption.value()); + Object value = present ? mapConfig.get(serverOption.value()) : null; + boolean valid = setServerOption(serverOption, value, serverConfig); + if (present && !valid && value != null && warnInvalid) { + ServerConfig.logger.warn("Server option: " + serverOption.value() + " set in server.yml is invalid, has been reset to default value."); } + options.put(serverOption.value(), new ValidatedOption(present, value, + serverOption.effectiveValue.apply(serverConfig), present && (!valid || value == null))); } - return corrections; + return options; } - /** Capture the validated input so a later mutable-map update cannot change the comparison. */ - record Correction(Object originalValue, Object effectiveValue) {} + /** Capture scalar inputs without retaining validation state on ServerConfig. */ + record ValidatedOption(boolean present, Object originalValue, Object effectiveValue, boolean corrected) { + boolean matches(Object rawValue) { + if (rawValue instanceof String encrypted && encrypted.startsWith(Decryptor.CRYPT_PREFIX)) { + try { + // Compare the scalar only; reporting must retain its original ciphertext. + return Objects.equals(ConfigInjection.getDecryptor().decrypt(encrypted), + originalValue == null ? null : originalValue.toString()); + } catch (RuntimeException ignored) { + // A newer encrypted source may be unreadable. Do not expose values or override it. + return false; + } + } + return Objects.equals(rawValue, originalValue); + } + } /** * Method used to validate and set server options @@ -141,8 +154,4 @@ private static boolean setServerOption(ServerOption serverOption, Object value, default -> true; }; } - - private static void setToDefaultServerOption(ServerOption serverOption, ServerConfig serverConfig) { - setServerOption(serverOption, null, serverConfig); - } } diff --git a/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java b/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java index b044290a0f..95176d4011 100644 --- a/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java +++ b/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java @@ -156,12 +156,13 @@ void missingConfigStillInitializesRuntimeDefaults() { } @Test - void changedRawValuesAreNotOverwrittenByStaleCorrections() { + void invalidToValidSourceChangeKeepsLaterRawValueAndWarns() { useFixture("maxTransferFileSize: -1\n", false); - raw.yaml = raw.yaml.replace("maxTransferFileSize: -1", "maxTransferFileSize: 7654321"); - raw.beforeRead = () -> Config.getInstance().getJsonMapConfig("server").put("maxTransferFileSize", 7654321); + Map cached = changeSourceAfterCachedRead( + raw.yaml.replace("maxTransferFileSize: -1", "maxTransferFileSize: 7654321")); ServerConfig server = ServerConfig.load(); - assertEquals(7654321, server.getMappedConfig().get("maxTransferFileSize")); + assertSame(cached, server.getMappedConfig()); + assertEquals(-1, server.getMappedConfig().get("maxTransferFileSize")); assertEquals(1_000_000L, server.getMaxTransferFileSize()); assertEquals(7654321, reported().get("maxTransferFileSize")); assertTrue(logs.list.stream().anyMatch(event -> event.getLevel() == Level.WARN @@ -169,6 +170,73 @@ void changedRawValuesAreNotOverwrittenByStaleCorrections() { assertEquals(ENCRYPTED, reported().get("keystorePass")); } + @Test + void validToInvalidSourceChangeKeepsLaterRawValueAndWarns() { + useFixture("maxTransferFileSize: 7654321\n", false); + Map cached = changeSourceAfterCachedRead( + raw.yaml.replace("maxTransferFileSize: 7654321", "maxTransferFileSize: -1")); + ServerConfig server = ServerConfig.load(); + assertSame(cached, server.getMappedConfig()); + assertEquals(7654321L, server.getMaxTransferFileSize()); + assertEquals(7654321, server.getMappedConfig().get("maxTransferFileSize")); + assertEquals(-1, reported().get("maxTransferFileSize")); + assertTrue(logs.list.stream().anyMatch(event -> event.getLevel() == Level.WARN + && event.getFormattedMessage().contains("maxTransferFileSize differs"))); + assertEquals(ENCRYPTED, reported().get("keystorePass")); + } + + @Test + void missingCorrectedRawKeyHasAnAbsenceWarningAndRemainsAbsent() { + useFixture("maxTransferFileSize: -1\n", false); + Map cached = changeSourceAfterCachedRead(raw.yaml.replace("maxTransferFileSize: -1\n", "")); + ServerConfig server = ServerConfig.load(); + assertSame(cached, server.getMappedConfig()); + assertEquals(-1, server.getMappedConfig().get("maxTransferFileSize")); + assertEquals(1_000_000L, server.getMaxTransferFileSize()); + assertFalse(reported().containsKey("maxTransferFileSize")); + assertTrue(logs.list.stream().anyMatch(event -> event.getLevel() == Level.WARN + && event.getFormattedMessage().contains("maxTransferFileSize is absent"))); + assertFalse(logs.list.stream().anyMatch(event -> event.getFormattedMessage().contains("maxTransferFileSize differs"))); + } + + @Test + void rawOnlyOptionHasAPresenceWarningAndKeepsItsRawValue() { + useFixture("", false); + Map cached = changeSourceAfterCachedRead("maxTransferFileSize: -1\n" + raw.yaml); + ServerConfig server = ServerConfig.load(); + assertSame(cached, server.getMappedConfig()); + assertFalse(server.getMappedConfig().containsKey("maxTransferFileSize")); + assertEquals(1_000_000L, server.getMaxTransferFileSize()); + assertEquals(-1, reported().get("maxTransferFileSize")); + assertTrue(logs.list.stream().anyMatch(event -> event.getLevel() == Level.WARN + && event.getFormattedMessage().contains("maxTransferFileSize is present only"))); + assertFalse(logs.list.stream().anyMatch(event -> event.getFormattedMessage().contains("maxTransferFileSize differs"))); + } + + @Test + void encryptedOptionIsComparedWithoutChangingItsCiphertextOrWarning() { + useFixture("serverString: " + ENCRYPTED + "\n", false); + ServerConfig server = ServerConfig.load(); + assertEquals("password", server.getServerString()); + assertEquals("password", server.getMappedConfig().get("serverString")); + assertEquals(ENCRYPTED, reported().get("serverString")); + assertFalse(logs.list.stream().anyMatch(event -> event.getLevel() == Level.WARN)); + } + + @Test + void unreadableNewEncryptedOptionIsRetainedWithAWarning() { + useFixture("serverString: L\n", false); + Map cached = changeSourceAfterCachedRead( + raw.yaml.replace("serverString: L", "serverString: CRYPT:unreadable-test-value")); + ServerConfig server = ServerConfig.load(); + assertSame(cached, server.getMappedConfig()); + assertEquals("L", server.getServerString()); + assertEquals("CRYPT:unreadable-test-value", reported().get("serverString")); + assertTrue(logs.list.stream().anyMatch(event -> event.getLevel() == Level.WARN + && event.getFormattedMessage().contains("serverString differs"))); + assertFalse(logs.list.stream().anyMatch(event -> event.getFormattedMessage().contains("unreadable-test-value"))); + } + @Test void blankDefaultsAndNamedLoadsDoNotProduceInvalidOptionWarnings() { useFixture("shutdownTimeout: null\n", false); @@ -196,6 +264,14 @@ void explicitInvalidOptionsWarnOnlyOnFreshCanonicalLoads() { assertEquals(2, invalidWarnings()); } + private Map changeSourceAfterCachedRead(String updatedYaml) { + // A real source edit leaves the existing decrypted cache untouched; the later no-cache read sees it. + Map cached = Config.getInstance().getJsonMapConfig("server"); + decrypted.yaml = updatedYaml; + raw.yaml = updatedYaml; + return cached; + } + private long invalidWarnings() { return logs.list.stream().filter(event -> event.getLevel() == Level.WARN && event.getFormattedMessage().contains("has been reset to default value")).count(); @@ -246,7 +322,6 @@ private static void assertCorrectedOptions(ServerConfig server) { private static class FixtureLoader extends ClassLoader { private String yaml; private int reads; - private Runnable beforeRead; FixtureLoader(String yaml) { super(ServerOptionRegistryTest.class.getClassLoader()); @@ -258,7 +333,6 @@ public InputStream getResourceAsStream(String name) { if (name.matches("(?:config/)?server\\.(?:yml|yaml|json)")) { if (name.equals("server.yml") && yaml != null) { reads++; - if (beforeRead != null) beforeRead.run(); return new ByteArrayInputStream(yaml.getBytes(StandardCharsets.UTF_8)); } return null; From e5b8193bbb80ed2f2a67f85dea42f9b32a96cf09 Mon Sep 17 00:00:00 2001 From: Steve Hu Date: Fri, 30 Oct 2026 16:03:56 -0400 Subject: [PATCH 5/6] fix: distinguish server option decryption failures --- .../com/networknt/server/ServerConfig.java | 14 +++-- .../com/networknt/server/ServerOption.java | 22 ++++---- .../server/ServerOptionRegistryTest.java | 53 ++++++++++++++++++- 3 files changed, 72 insertions(+), 17 deletions(-) diff --git a/server-config/src/main/java/com/networknt/server/ServerConfig.java b/server-config/src/main/java/com/networknt/server/ServerConfig.java index 136e0dd8f7..5cba01c76b 100644 --- a/server-config/src/main/java/com/networknt/server/ServerConfig.java +++ b/server-config/src/main/java/com/networknt/server/ServerConfig.java @@ -421,6 +421,11 @@ public static ServerConfig load(String configName) { return instance; } ValidatedConfig loaded = loadValidated(configName); + loaded.options().forEach((key, option) -> { + if (option.corrected() && option.originalValue() != null) { + logger.warn("Server option: {} set in server.yml is invalid, has been reset to default value.", key); + } + }); instance = loaded.config(); Map reportedConfig = validatedConfig( Config.getNoneDecryptedInstance().getJsonMapConfigNoCache(CONFIG_NAME), loaded); @@ -433,8 +438,7 @@ public static ServerConfig load(String configName) { private static ValidatedConfig loadValidated(String configName) { ServerConfig config = new ServerConfig(configName); - return new ValidatedConfig(config, ServerOption.validateServerOptions( - config.mappedConfig, config, CONFIG_NAME.equals(configName))); + return new ValidatedConfig(config, ServerOption.validateServerOptions(config.mappedConfig, config)); } private record ValidatedConfig(ServerConfig config, Map options) {} @@ -456,7 +460,11 @@ private static Map validatedConfig(Map rawConfig } else if (!option.present() && rawPresent) { logger.warn("Server option {} is present only in non-decrypted reporting configuration; retaining the raw reporting value.", key); } else if (rawPresent) { - if (!option.matches(rawConfig.get(key))) { + ServerOption.Comparison comparison = option.compare(rawConfig.get(key)); + if (comparison.decryptionFailureType() != null) { + logger.warn("Server option {} could not be decrypted for reporting comparison ({}); check the configured key and ciphertext. Retaining the raw reporting value.", + key, comparison.decryptionFailureType()); + } else if (!comparison.matches()) { logger.warn("Server option {} differs between validated and non-decrypted configuration; retaining the raw reporting value.", key); } else if (option.corrected()) { reportedConfig.put(key, option.effectiveValue()); diff --git a/server-config/src/main/java/com/networknt/server/ServerOption.java b/server-config/src/main/java/com/networknt/server/ServerOption.java index 2e2ed0e876..690274d71b 100644 --- a/server-config/src/main/java/com/networknt/server/ServerOption.java +++ b/server-config/src/main/java/com/networknt/server/ServerOption.java @@ -47,16 +47,12 @@ public String value() { } /** Validate fields and capture the presence and input of every option for reporting comparisons. */ - static Map validateServerOptions(Map mapConfig, ServerConfig serverConfig, - boolean warnInvalid) { + static Map validateServerOptions(Map mapConfig, ServerConfig serverConfig) { Map options = new HashMap<>(); for (ServerOption serverOption : ServerOption.values()) { boolean present = mapConfig != null && mapConfig.containsKey(serverOption.value()); Object value = present ? mapConfig.get(serverOption.value()) : null; boolean valid = setServerOption(serverOption, value, serverConfig); - if (present && !valid && value != null && warnInvalid) { - ServerConfig.logger.warn("Server option: " + serverOption.value() + " set in server.yml is invalid, has been reset to default value."); - } options.put(serverOption.value(), new ValidatedOption(present, value, serverOption.effectiveValue.apply(serverConfig), present && (!valid || value == null))); } @@ -65,21 +61,23 @@ static Map validateServerOptions(Map ma /** Capture scalar inputs without retaining validation state on ServerConfig. */ record ValidatedOption(boolean present, Object originalValue, Object effectiveValue, boolean corrected) { - boolean matches(Object rawValue) { + Comparison compare(Object rawValue) { if (rawValue instanceof String encrypted && encrypted.startsWith(Decryptor.CRYPT_PREFIX)) { try { // Compare the scalar only; reporting must retain its original ciphertext. - return Objects.equals(ConfigInjection.getDecryptor().decrypt(encrypted), - originalValue == null ? null : originalValue.toString()); - } catch (RuntimeException ignored) { - // A newer encrypted source may be unreadable. Do not expose values or override it. - return false; + return new Comparison(Objects.equals(ConfigInjection.getDecryptor().decrypt(encrypted), + originalValue == null ? null : originalValue.toString()), null); + } catch (RuntimeException failure) { + // Only retain the type: messages and causes can contain configuration values. + return new Comparison(false, failure.getClass().getName()); } } - return Objects.equals(rawValue, originalValue); + return new Comparison(Objects.equals(rawValue, originalValue), null); } } + record Comparison(boolean matches, String decryptionFailureType) {} + /** * Method used to validate and set server options * diff --git a/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java b/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java index 95176d4011..0aa88d39c7 100644 --- a/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java +++ b/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java @@ -29,6 +29,7 @@ import java.io.ByteArrayInputStream; import java.io.InputStream; import java.nio.charset.StandardCharsets; +import java.util.HashMap; import java.util.Map; import static org.junit.jupiter.api.Assertions.*; @@ -37,6 +38,11 @@ class ServerOptionRegistryTest { private static final String KEY = "server:" + ServerConfig.class.getName(); private static final String ENCRYPTED = "CRYPT:e6e1b4a099902a2106509e87df909de4:cd9153addc755eb7e00d817c:e499d4501d0372fe37073d88a1025f1970a30cbd9acec7d5"; + // Synthetic AES-GCM scalar fixtures; WRONG_KEY uses a deliberately different test master. + private static final String ENCRYPTED_EIGHT = "CRYPT:2b4b3f72b8740340a84c29ce0309288e:47901ccf96ccd4ce8397ef07:83f132252f5d8edb546d52e01c148f98e4"; + private static final String ENCRYPTED_TRUE = "CRYPT:9c652eddc1ea8a62fd7376acc46c646b:e27ccc2edc2784d824052815:3d7be00f94b3e7c47225d9f3fee470c15783545b"; + private static final String ENCRYPTED_FALSE = "CRYPT:22942d8c857c75d7c80f748cc7305896:2dd5e864fd94ee12be55bfa3:9d015fd607ebf5fcb1026dfe807fda497a94510b70"; + private static final String ENCRYPTED_WRONG_KEY = "CRYPT:f720c9468e492a43cfb8ab9c07f5eea8:eaf11088c3380fed4a7d0c90:07d763a84c82441caea1936de6e072ff6e5bc8a2c1431676"; private FixtureLoader decrypted; private FixtureLoader raw; private final Logger logger = (Logger) ServerConfig.logger; @@ -224,7 +230,7 @@ void encryptedOptionIsComparedWithoutChangingItsCiphertextOrWarning() { } @Test - void unreadableNewEncryptedOptionIsRetainedWithAWarning() { + void corruptEncryptedOptionHasADecryptionDiagnosticInsteadOfASourceChangeWarning() { useFixture("serverString: L\n", false); Map cached = changeSourceAfterCachedRead( raw.yaml.replace("serverString: L", "serverString: CRYPT:unreadable-test-value")); @@ -233,10 +239,53 @@ void unreadableNewEncryptedOptionIsRetainedWithAWarning() { assertEquals("L", server.getServerString()); assertEquals("CRYPT:unreadable-test-value", reported().get("serverString")); assertTrue(logs.list.stream().anyMatch(event -> event.getLevel() == Level.WARN - && event.getFormattedMessage().contains("serverString differs"))); + && event.getFormattedMessage().contains("serverString could not be decrypted"))); + assertFalse(logs.list.stream().anyMatch(event -> event.getFormattedMessage().contains("serverString differs"))); assertFalse(logs.list.stream().anyMatch(event -> event.getFormattedMessage().contains("unreadable-test-value"))); } + @Test + void encryptedNumericAndBooleanOptionsMatchTypedCachedInputs() { + useFixture("ioThreads: " + ENCRYPTED_EIGHT + "\n" + + "maxTransferFileSize: " + ENCRYPTED_EIGHT + "\n" + + "alwaysSetDate: " + ENCRYPTED_TRUE + "\n" + + "allowUnescapedCharactersInUrl: " + ENCRYPTED_FALSE + "\n", false); + // Injection and custom loaders may supply typed scalars instead of decrypted strings. + Map typed = new HashMap<>(Config.getInstance().getJsonMapConfig("server")); + typed.put("ioThreads", 8); + typed.put("maxTransferFileSize", 8L); + typed.put("alwaysSetDate", true); + typed.put("allowUnescapedCharactersInUrl", false); + Config.getInstance().putInConfigCache("server", typed); + ServerConfig server = ServerConfig.load(); + assertSame(typed, server.getMappedConfig()); + assertEquals(8, server.getIoThreads()); + assertEquals(8L, server.getMaxTransferFileSize()); + assertTrue(server.isAlwaysSetDate()); + assertFalse(server.isAllowUnescapedCharactersInUrl()); + assertEquals(ENCRYPTED_EIGHT, reported().get("ioThreads")); + assertEquals(ENCRYPTED_EIGHT, reported().get("maxTransferFileSize")); + assertEquals(ENCRYPTED_TRUE, reported().get("alwaysSetDate")); + assertEquals(ENCRYPTED_FALSE, reported().get("allowUnescapedCharactersInUrl")); + assertFalse(logs.list.stream().anyMatch(event -> event.getLevel() == Level.WARN)); + } + + @Test + void wrongEncryptionKeyHasADecryptionDiagnosticWithoutLeakingValues() { + useFixture("serverString: L\n", false); + Map cached = changeSourceAfterCachedRead( + raw.yaml.replace("serverString: L", "serverString: " + ENCRYPTED_WRONG_KEY)); + ServerConfig server = ServerConfig.load(); + assertSame(cached, server.getMappedConfig()); + assertEquals("L", server.getServerString()); + assertEquals(ENCRYPTED_WRONG_KEY, reported().get("serverString")); + assertTrue(logs.list.stream().anyMatch(event -> event.getLevel() == Level.WARN + && event.getFormattedMessage().contains("serverString could not be decrypted"))); + assertFalse(logs.list.stream().anyMatch(event -> event.getFormattedMessage().contains("serverString differs"))); + assertFalse(logs.list.stream().anyMatch(event -> event.getFormattedMessage().contains(ENCRYPTED_WRONG_KEY) + || event.getFormattedMessage().contains("password"))); + } + @Test void blankDefaultsAndNamedLoadsDoNotProduceInvalidOptionWarnings() { useFixture("shutdownTimeout: null\n", false); From 3024be68f7f9e012d88d901f99120177b4765843 Mon Sep 17 00:00:00 2001 From: Steve Hu Date: Fri, 30 Oct 2026 16:18:00 -0400 Subject: [PATCH 6/6] fix: preserve server option warning declaration order --- .../com/networknt/server/ServerOption.java | 4 +-- .../server/ServerOptionRegistryTest.java | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/server-config/src/main/java/com/networknt/server/ServerOption.java b/server-config/src/main/java/com/networknt/server/ServerOption.java index 690274d71b..fe9663a1e6 100644 --- a/server-config/src/main/java/com/networknt/server/ServerOption.java +++ b/server-config/src/main/java/com/networknt/server/ServerOption.java @@ -4,7 +4,7 @@ import com.networknt.config.ConfigInjection; import com.networknt.decrypt.Decryptor; -import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Objects; import java.util.function.Function; @@ -48,7 +48,7 @@ public String value() { /** Validate fields and capture the presence and input of every option for reporting comparisons. */ static Map validateServerOptions(Map mapConfig, ServerConfig serverConfig) { - Map options = new HashMap<>(); + Map options = new LinkedHashMap<>(); for (ServerOption serverOption : ServerOption.values()) { boolean present = mapConfig != null && mapConfig.containsKey(serverOption.value()); Object value = present ? mapConfig.get(serverOption.value()) : null; diff --git a/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java b/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java index 0aa88d39c7..fccf95cef4 100644 --- a/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java +++ b/server-config/src/test/java/com/networknt/server/ServerOptionRegistryTest.java @@ -30,6 +30,7 @@ import java.io.InputStream; import java.nio.charset.StandardCharsets; import java.util.HashMap; +import java.util.List; import java.util.Map; import static org.junit.jupiter.api.Assertions.*; @@ -313,6 +314,31 @@ void explicitInvalidOptionsWarnOnlyOnFreshCanonicalLoads() { assertEquals(2, invalidWarnings()); } + @Test + void invalidOptionWarningsFollowDeclarationOrderRegardlessOfSourceOrder() { + useFixture(""" + shutdownTimeout: -1 + allowUnescapedCharactersInUrl: null + maxTransferFileSize: -1 + alwaysSetDate: null + serverString: '' + backlog: -1 + bufferSize: -1 + workerThreads: -1 + ioThreads: -1 + """, false); + ServerConfig.load(); + var warnings = logs.list.stream() + .filter(event -> event.getLevel() == Level.WARN + && event.getFormattedMessage().contains("has been reset to default value")) + .map(ILoggingEvent::getFormattedMessage).toList(); + var expected = List.of("ioThreads", "workerThreads", "bufferSize", "backlog", "serverString", + "maxTransferFileSize", "shutdownTimeout").stream() + .map(name -> "Server option: " + name + " set in server.yml is invalid, has been reset to default value.") + .toList(); + assertEquals(expected, warnings); + } + private Map changeSourceAfterCachedRead(String updatedYaml) { // A real source edit leaves the existing decrypted cache untouched; the later no-cache read sees it. Map cached = Config.getInstance().getJsonMapConfig("server");