From be53376f280cd646d096c67a6feb930b2bf56f01 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 8 Dec 2025 18:04:12 -0800 Subject: [PATCH 01/13] GUACAMOLE-2181: Include extension information and line continuations in log messages. --- guacamole/.ratignore | 1 + .../AuthenticationProviderFacade.java | 89 +++++- .../apache/guacamole/log/ForkableHashMap.java | 267 ++++++++++++++++++ .../log/GuacamoleLogbackServiceProvider.java | 109 +++++++ .../guacamole/log/InheritableMDCAdapter.java | 155 ++++++++++ .../org/apache/guacamole/log/LogLevel.java | 43 ++- .../org/apache/guacamole/log/LogModule.java | 97 +++++++ .../rest/session/SessionResource.java | 34 ++- .../org.slf4j.spi.SLF4JServiceProvider | 1 + 9 files changed, 768 insertions(+), 28 deletions(-) create mode 100644 guacamole/src/main/java/org/apache/guacamole/log/ForkableHashMap.java create mode 100644 guacamole/src/main/java/org/apache/guacamole/log/GuacamoleLogbackServiceProvider.java create mode 100644 guacamole/src/main/java/org/apache/guacamole/log/InheritableMDCAdapter.java create mode 100644 guacamole/src/main/resources/META-INF/services/org.slf4j.spi.SLF4JServiceProvider diff --git a/guacamole/.ratignore b/guacamole/.ratignore index 548fcb76c..6ced18f45 100644 --- a/guacamole/.ratignore +++ b/guacamole/.ratignore @@ -1 +1,2 @@ src/main/frontend/dist/**/* +src/main/resources/META-INF/services/* diff --git a/guacamole/src/main/java/org/apache/guacamole/extension/AuthenticationProviderFacade.java b/guacamole/src/main/java/org/apache/guacamole/extension/AuthenticationProviderFacade.java index b89b2ada6..e2122fdef 100644 --- a/guacamole/src/main/java/org/apache/guacamole/extension/AuthenticationProviderFacade.java +++ b/guacamole/src/main/java/org/apache/guacamole/extension/AuthenticationProviderFacade.java @@ -23,12 +23,14 @@ import java.util.Set; import java.util.UUID; import org.apache.guacamole.GuacamoleClientException; import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.log.LogModule; import org.apache.guacamole.net.auth.AuthenticatedUser; import org.apache.guacamole.net.auth.AuthenticationProvider; import org.apache.guacamole.net.auth.Credentials; import org.apache.guacamole.net.auth.UserContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.slf4j.MDC; /** * Provides a safe wrapper around an AuthenticationProvider subclass, such that @@ -40,7 +42,7 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { /** * Logger for this class. */ - private Logger logger = LoggerFactory.getLogger(AuthenticationProviderFacade.class); + private final Logger logger = LoggerFactory.getLogger(AuthenticationProviderFacade.class); /** * The underlying authentication provider, or null if the authentication @@ -58,6 +60,12 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { */ private final Set tolerateFailures; + /** + * The logging context that should be provided to SLF4J's MDC for all log + * messages originating from the underlying authentication provider. + */ + private final String logContext; + /** * The identifier to provide for the underlying authentication provider if * the authentication provider could not be loaded. @@ -86,9 +94,16 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { public AuthenticationProviderFacade( Class authProviderClass, Set tolerateFailures) { + this.tolerateFailures = tolerateFailures; this.authProvider = ProviderFactory.newInstance("authentication provider", authProviderClass); + + if (this.authProvider != null) + this.logContext = LogModule.getMDCContext(authProvider); + else + this.logContext = LogModule.MDC_CONTEXT_ROOT; + } @Override @@ -115,7 +130,13 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { } // Delegate to underlying auth provider - return authProvider.getResource(); + try { + MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); + return authProvider.getResource(); + } + finally { + MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + } } @@ -127,7 +148,13 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { return credentials; // Delegate to underlying auth provider - return authProvider.updateCredentials(credentials); + try { + MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); + return authProvider.updateCredentials(credentials); + } + finally { + MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + } } @@ -199,6 +226,7 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { // Delegate to underlying auth provider try { + MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); return authProvider.authenticateUser(credentials); } @@ -224,6 +252,10 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { } + finally { + MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + } + } @Override @@ -239,7 +271,13 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { } // Delegate to underlying auth provider - return authProvider.updateAuthenticatedUser(authenticatedUser, credentials); + try { + MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); + return authProvider.updateAuthenticatedUser(authenticatedUser, credentials); + } + finally { + MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + } } @@ -257,6 +295,7 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { // Delegate to underlying auth provider try { + MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); return authProvider.getUserContext(authenticatedUser); } @@ -282,6 +321,10 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { } + finally { + MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + } + } @Override @@ -298,8 +341,14 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { } // Delegate to underlying auth provider - return authProvider.updateUserContext(context, authenticatedUser, credentials); - + try { + MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); + return authProvider.updateUserContext(context, authenticatedUser, credentials); + } + finally { + MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + } + } @Override @@ -312,7 +361,13 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { return context; // Delegate to underlying auth provider - return authProvider.decorate(context, authenticatedUser, credentials); + try { + MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); + return authProvider.decorate(context, authenticatedUser, credentials); + } + finally { + MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + } } @@ -326,15 +381,27 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { return context; // Delegate to underlying auth provider - return authProvider.redecorate(decorated, context, - authenticatedUser, credentials); + try { + MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); + return authProvider.redecorate(decorated, context, + authenticatedUser, credentials); + } + finally { + MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + } } @Override public void shutdown() { - if (authProvider != null) - authProvider.shutdown(); + try { + MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); + if (authProvider != null) + authProvider.shutdown(); + } + finally { + MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + } } } diff --git a/guacamole/src/main/java/org/apache/guacamole/log/ForkableHashMap.java b/guacamole/src/main/java/org/apache/guacamole/log/ForkableHashMap.java new file mode 100644 index 000000000..2ef5e1c47 --- /dev/null +++ b/guacamole/src/main/java/org/apache/guacamole/log/ForkableHashMap.java @@ -0,0 +1,267 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.guacamole.log; + +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.function.BiConsumer; +import java.util.function.BiFunction; +import java.util.function.Function; + +/** + * HashMap implementation that uses copy-on-write to allow efficient cloning. + * When an instance of ForkableHashMap has been copied using {@link #fork()}, + * the parent and child both continue to refer to the same copy of their + * underlying data. If the parent (or child) is modified, only then does the + * parent (or child) create an independent copy of that underlying data. + *

+ * For simplicity, this implementation DOES NOT permit modification via the + * collections/sets returned by {@link #values()}, {@link #keySet()}, or + * {@link #entrySet()}. + *

+ * As with the standard HashMap, this implementation IS NOT threadsafe. + * + * @param + * The type of key used to store/retrieve values within this map. + * + * @param + * The type of values stored by this map. + */ +public class ForkableHashMap implements Map { + + /** + * The underlying content of this ForkableHashMap. This content is shared + * with at least one other ForkableHashMap if {@link #shared} is true, and + * is an independent copy that may safely be modified if {@link #shared} is + * false. + */ + private Map content; + + /** + * Whether the Map referenced by {@link #content} is shared with at least + * one other ForkableHashMap. + */ + private boolean shared; + + /** + * Creates a new, empty ForkableHashMap. + */ + public ForkableHashMap() { + this.content = new HashMap<>(); + this.shared = false; + } + + /** + * Creates a new ForkableHashMap that contains an independent copy of the + * entries in the provided map. + *

+ * This is not a deep copy; only the references to keys and values are + * copied. + * + * @param map + * The map to copy. + */ + public ForkableHashMap(Map map) { + this.content = new HashMap<>(map); + this.shared = false; + } + + /** + * Creates a new ForkableHashMap that shares the content of the given + * ForkableHashMap. It is the responsibility of the caller to ensure that + * the given ForkableHashMap has its content marked as shared. + * + * @param parent + * The ForkableHashMap to lazily copy. + */ + private ForkableHashMap(ForkableHashMap parent) { + this.content = parent.content; + this.shared = true; + } + + /** + * Creates a new ForkableHashMap that shares the content of this map. No + * actual copy operation will be performed until this map or the new map + * are modified. + * + * @return + * A new ForkableHashMap that lazily shares the content of this map. + */ + public ForkableHashMap fork() { + shared = true; + return new ForkableHashMap<>(this); + } + + /** + * Returns the map providing content for this ForkableHashMap while + * ensuring that map is safe to modify. If the map is shared with another + * ForkableHashMap, the map is first replaced with an independent copy. + * + * @return + * The map providing content for this ForkableHashMap, which shall be + * safe to modify after this function returns. + */ + private Map writableContent() { + + // Transform into independent copy if other instances of ForkableHashMap + // share the same underlying data + if (shared) { + content = new HashMap<>(content); + shared = false; + } + + return content; + + } + + @Override + public int size() { + return content.size(); + } + + @Override + public boolean isEmpty() { + return content.isEmpty(); + } + + @Override + public boolean containsKey(Object o) { + return content.containsKey(o); + } + + @Override + public boolean containsValue(Object o) { + return content.containsValue(o); + } + + @Override + public V get(Object o) { + return content.get(o); + } + + @Override + public V put(K k, V v) { + return writableContent().put(k, v); + } + + @Override + public V remove(Object o) { + return writableContent().remove(o); + } + + @Override + public void putAll(Map map) { + writableContent().putAll(map); + } + + @Override + public void clear() { + writableContent().clear(); + } + + @Override + public Set keySet() { + return Collections.unmodifiableSet(content.keySet()); + } + + @Override + public Collection values() { + return Collections.unmodifiableCollection(content.values()); + } + + @Override + public Set> entrySet() { + return Collections.unmodifiableSet(content.entrySet()); + } + + @Override + public boolean equals(Object o) { + return content.equals(o); + } + + @Override + public int hashCode() { + return content.hashCode(); + } + + @Override + public V getOrDefault(Object key, V defaultValue) { + return content.getOrDefault(key, defaultValue); + } + + @Override + public void forEach(BiConsumer action) { + content.forEach(action); + } + + @Override + public void replaceAll(BiFunction function) { + writableContent().replaceAll(function); + } + + @Override + public V putIfAbsent(K key, V value) { + return writableContent().putIfAbsent(key, value); + } + + @Override + public boolean remove(Object key, Object value) { + return writableContent().remove(key, value); + } + + @Override + public boolean replace(K key, V oldValue, V newValue) { + return writableContent().replace(key, oldValue, newValue); + } + + @Override + public V replace(K key, V value) { + return writableContent().replace(key, value); + } + + @Override + public V computeIfAbsent(K key, Function mappingFunction) { + return content.computeIfAbsent(key, mappingFunction); + } + + @Override + public V computeIfPresent(K key, BiFunction remappingFunction) { + return content.computeIfPresent(key, remappingFunction); + } + + @Override + public V compute(K key, BiFunction remappingFunction) { + return content.compute(key, remappingFunction); + } + + @Override + public V merge(K key, V value, BiFunction remappingFunction) { + return writableContent().merge(key, value, remappingFunction); + } + + @Override + public String toString() { + return content.toString(); + } + +} diff --git a/guacamole/src/main/java/org/apache/guacamole/log/GuacamoleLogbackServiceProvider.java b/guacamole/src/main/java/org/apache/guacamole/log/GuacamoleLogbackServiceProvider.java new file mode 100644 index 000000000..c1414c554 --- /dev/null +++ b/guacamole/src/main/java/org/apache/guacamole/log/GuacamoleLogbackServiceProvider.java @@ -0,0 +1,109 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.guacamole.log; + +import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.util.ContextInitializer; +import ch.qos.logback.core.joran.spi.JoranException; +import org.slf4j.ILoggerFactory; +import org.slf4j.IMarkerFactory; +import org.slf4j.helpers.BasicMarkerFactory; +import org.slf4j.spi.MDCAdapter; +import org.slf4j.spi.SLF4JServiceProvider; + +/** + * SLF4JServiceProvider that overrides the default behavior of Logback's + * service provider, forcing the use of a custom MDCAdapter. + */ +public class GuacamoleLogbackServiceProvider implements SLF4JServiceProvider { + + /** + * The highest version of SLF4J that this service provider can support. + * This is intentionally not actually a released version, but a reasonable + * upper bound. This value is identical to the value used by Logback. + */ + private static final String SLF4J_API_MAX_VERSION = "2.0.99"; + + /** + * The name of the LoggerContext maintained by this service provider. + */ + private static final String LOGGER_CONTEXT_NAME = "guacamole"; + + /** + * The singleton instance of the LoggerContext maintained by this service + * provider. The LoggerContext is actually the LoggerFactory. + */ + private LoggerContext context; + + /** + * The singleton instance of the MDCAdapter implementation that should be + * used by Logback and SLF4J. The custom implementation provided here + * allows the context map to be inherited by child threads. + */ + private final MDCAdapter mdcAdapter = new InheritableMDCAdapter(); + + /** + * The singleton MarkerFactory instance maintained by this service + * provider. + */ + private final IMarkerFactory markerFactory = new BasicMarkerFactory(); + + @Override + public void initialize() { + + context = new LoggerContext(); + context.setName(LOGGER_CONTEXT_NAME); + context.setMDCAdapter(mdcAdapter); + + // Perform basic init (prior to loading any Guacamole-specific + // configuration, which is loaded by the LogModule) + try { + new ContextInitializer(context).autoConfig(); + } + catch (JoranException | RuntimeException | Error e) { + System.err.println("Logging system could not be initialized: " + e.getMessage()); + e.printStackTrace(System.err); + } + + context.start(); + + } + + @Override + public MDCAdapter getMDCAdapter() { + return mdcAdapter; + } + + @Override + public ILoggerFactory getLoggerFactory() { + return context; + } + + @Override + public IMarkerFactory getMarkerFactory() { + return markerFactory; + } + + @Override + public String getRequestedApiVersion() { + return SLF4J_API_MAX_VERSION; + } + +} diff --git a/guacamole/src/main/java/org/apache/guacamole/log/InheritableMDCAdapter.java b/guacamole/src/main/java/org/apache/guacamole/log/InheritableMDCAdapter.java new file mode 100644 index 000000000..493925476 --- /dev/null +++ b/guacamole/src/main/java/org/apache/guacamole/log/InheritableMDCAdapter.java @@ -0,0 +1,155 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.guacamole.log; + +import java.util.Deque; +import java.util.HashMap; +import java.util.LinkedList; +import java.util.Map; +import org.slf4j.spi.MDCAdapter; + +/** + * MDCAdapter implementation that allows individual string context values to be + * inherited by child threads. Deque context values (as manipulated with + * {@link #pushByKey(java.lang.String, java.lang.String)}, etc.) are not + * inherited. + */ +public class InheritableMDCAdapter implements MDCAdapter { + + /** + * MDC context map that is automatically inherited by child threads. + */ + private final InheritableThreadLocal> context = + new InheritableThreadLocal>() { + + @Override + protected ForkableHashMap initialValue() { + return new ForkableHashMap<>(); + } + + @Override + protected ForkableHashMap childValue(ForkableHashMap parentValue) { + // This function is invoked by the JVM for every thread created, + // so simply creating a new copy is expensive. We instead create a + // copy-on-write reference. + return parentValue.fork(); + } + + }; + + /** + * MDC deque map that is NOT automatically inherited by child threads. We + * choose not to inherit the deque map as that would result in pushed values + * possibly never getting popped. + *

+ * NOTE: The Deques have no relation to the single values maintained using + * {@link #put(java.lang.String, java.lang.String)} and retrieved using + * {@link #get(java.lang.String)} which are intentionally stored + * separately in {@link #context}. This matches the behavior of the Logback + * implementation of MDC. + */ + private final ThreadLocal>> contextDeques = + new ThreadLocal>>() { + + @Override + protected Map> initialValue() { + return new HashMap<>(); + } + + }; + + @Override + public void put(String key, String value) { + context.get().put(key, value); + } + + @Override + public String get(String key) { + return context.get().get(key); + } + + @Override + public void remove(String key) { + context.get().remove(key); + } + + @Override + public void clear() { + context.get().clear(); + } + + @Override + public Map getCopyOfContextMap() { + // This function is invoked by Logback for every log message produced, + // so simply copying the context map is expensive. We instead create a + // copy-on-write reference to the context map. + return context.get().fork(); + } + + @Override + public void setContextMap(Map contextMap) { + context.set(new ForkableHashMap<>(contextMap)); + } + + /** + * Returns the Deque of pushed values associated with the given key, as + * required for {@link #pushByKey(java.lang.String, java.lang.String)} and + * {@link #popByKey(java.lang.String)}. If no Deque yet exists for the + * given key, it is first created. + * + * @param key + * The key to retrieve the Deque for. + * + * @return + * The Deque of pushed values for the given key. + */ + private Deque getDeque(String key) { + + Deque values = contextDeques.get().get(key); + if (values == null) { + values = new LinkedList<>(); + contextDeques.get().put(key, values); + } + + return values; + + } + + @Override + public void pushByKey(String key, String value) { + getDeque(key).push(value); + } + + @Override + public String popByKey(String key) { + return getDeque(key).pop(); + } + + @Override + public Deque getCopyOfDequeByKey(String key) { + return new LinkedList<>(getDeque(key)); + } + + @Override + public void clearDequeByKey(String key) { + getDeque(key).clear(); + } + +} diff --git a/guacamole/src/main/java/org/apache/guacamole/log/LogLevel.java b/guacamole/src/main/java/org/apache/guacamole/log/LogLevel.java index 27bf0fe88..7d3c118d2 100644 --- a/guacamole/src/main/java/org/apache/guacamole/log/LogLevel.java +++ b/guacamole/src/main/java/org/apache/guacamole/log/LogLevel.java @@ -56,14 +56,14 @@ public enum LogLevel { * messages, while verbose, will not affect performance. */ @PropertyValue("debug") - DEBUG("debug"), + DEBUG("debug", "${guac_verbose_pattern}"), /** * Informational messages that may be useful for debugging, but which are * so low-level that they may affect performance. */ @PropertyValue("trace") - TRACE("trace"); + TRACE("trace", "${guac_verbose_pattern}"); /** * Format string whose sole format argument is a String containing the @@ -77,7 +77,7 @@ public enum LogLevel { + " \n" + " \n" + " \n" - + " %%d{HH:mm:ss.SSS} [%%thread] %%-5level %%logger{36} - %%msg%%n\n" + + " %s\n" + " \n" + " \n" + "\n" @@ -102,9 +102,10 @@ public enum LogLevel { private final byte[] logbackConfig; /** - * Creates a new LogLevel with the given names. The pair of names provided - * correspond to the name used within Guacamole's configuration and the - * name used within Logback's configuration. + * Creates a new LogLevel with the given names and pattern. The pair of + * names provided correspond to the name used within Guacamole's + * configuration and the name used within Logback's configuration, while + * the pattern determines how Logback will convert log messages to text. * * @param canonicalName * The name that should be used for this log level when configuring @@ -113,10 +114,14 @@ public enum LogLevel { * @param logbackLogLevel * The name that would be provided to Logback to log at this level if * manually configuring Logback using "logback.xml". + * + * @param pattern + * The Logback encoder pattern that should be use for messages logged + * at this level. */ - private LogLevel(String canonicalName, String logbackLogLevel) { + private LogLevel(String canonicalName, String logbackLogLevel, String pattern) { this.canonicalName = canonicalName; - this.logbackConfig = String.format(LOGBACK_XML_TEMPLATE, logbackLogLevel).getBytes(StandardCharsets.UTF_8); + this.logbackConfig = String.format(LOGBACK_XML_TEMPLATE, pattern, logbackLogLevel).getBytes(StandardCharsets.UTF_8); } /** @@ -131,7 +136,27 @@ public enum LogLevel { * "logback.xml" configuration file. */ private LogLevel(String logLevel) { - this(logLevel, logLevel); + this(logLevel, logLevel, "${guac_pattern}"); + } + + /** + * Creates a new LogLevel with the given name and pattern. The provided name + * corresponds to both the name used within Guacamole's configuration and + * the name used within Logback's configuration, while the pattern + * determines how Logback will convert log messages to text. + * + * @param logLevel + * The name that should be used for this log level when configuring + * Guacamole to log at this level using the "log-level" property AND + * when manually configuring Logback to log at this level using a + * "logback.xml" configuration file. + * + * @param pattern + * The Logback encoder pattern that should be use for messages logged + * at this level. + */ + private LogLevel(String logLevel, String pattern) { + this(logLevel, logLevel, pattern); } /** diff --git a/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java b/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java index a9e4ac3bc..9497ec0f4 100644 --- a/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java +++ b/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java @@ -31,6 +31,7 @@ import java.io.IOException; import java.io.InputStream; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.environment.Environment; +import org.apache.guacamole.net.auth.AuthenticationProvider; import org.apache.guacamole.properties.EnumGuacamoleProperty; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -50,6 +51,95 @@ public class LogModule extends AbstractModule { */ private final Environment environment; + /** + * The name of the MDC key that should be used to store Guacamole-specific + * context information. If an extension logs a message, this information + * will include the identifier of the relevant authentication provider. + */ + public static final String MDC_CONTEXT_KEY = "context"; + + /** + * The name that should be considered the top-level context when populating + * the MDC entry pointed to by {@link #MDC_CONTEXT_KEY}. + */ + public static final String MDC_CONTEXT_ROOT = "guacamole"; + + /** + * Returns the context information that should be stored in MDC under + * {@link #MDC_CONTEXT_KEY} for any message logged by the given + * authentication provider. + * + * @param authProvider + * The authentication provider to provide context for. + * + * @return + * The context information that should be stored in MDC for any message + * logged by the given authentication provider. + */ + public static String getMDCContext(AuthenticationProvider authProvider) { + return LogModule.MDC_CONTEXT_ROOT + "/" + authProvider.getIdentifier(); + } + + /** + * Regular expression that matches one or more newlines, regardless of style + * (Windows, Linux, etc.), preserving only the first such newline. This + * expression is intended to reliably match newlines that may be interpreted + * by the shell/viewer, regardless of the platform producing the log + * message. + */ + private static final String NEWLINE_REGEX = + + // Locate the right edge of the first newline sequence, whether + // that is CR, LF, CRLF, or LFCR + "(?<=" + + // Do not match newline sequences that are not the first + // newline in the sequence + + "(? Date: Mon, 8 Dec 2025 18:05:17 -0800 Subject: [PATCH 02/13] GUACAMOLE-2181: Include full exception when logging at debug level. --- .../guacamole/event/EventLoggingListener.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/guacamole/src/main/java/org/apache/guacamole/event/EventLoggingListener.java b/guacamole/src/main/java/org/apache/guacamole/event/EventLoggingListener.java index 16bee6ae8..ec7a484dc 100644 --- a/guacamole/src/main/java/org/apache/guacamole/event/EventLoggingListener.java +++ b/guacamole/src/main/java/org/apache/guacamole/event/EventLoggingListener.java @@ -183,30 +183,33 @@ public class EventLoggingListener implements Listener { if (creds.isEmpty()) logger.debug("Empty authentication attempt (login screen " + "initialization) from {} failed: {}", - new RemoteAddress(creds), new Failure(event)); + new RemoteAddress(creds), new Failure(event), event.getFailure()); else if (username == null || username.isEmpty()) logger.debug("Anonymous authentication attempt from {} failed: {}", - new RemoteAddress(creds), new Failure(event)); + new RemoteAddress(creds), new Failure(event), event.getFailure()); else if (event.getFailure() instanceof GuacamoleInsufficientCredentialsException) { if (authProvider != null) logger.debug("Authentication attempt from {} for user \"{}\" " + "requires additional credentials to continue: {} " + "(requested by \"{}\")", new RemoteAddress(creds), - username, new Failure(event), authProvider.getIdentifier()); + username, new Failure(event), authProvider.getIdentifier(), + event.getFailure()); else logger.debug("Authentication attempt from {} for user \"{}\" " + "requires additional credentials to continue: {}", - new RemoteAddress(creds), username, new Failure(event)); + new RemoteAddress(creds), username, new Failure(event), + event.getFailure()); } else { if (authProvider != null) logger.warn("Authentication attempt from {} for user \"{}\" " + "failed: {} (rejected by \"{}\")", new RemoteAddress(creds), - username, new Failure(event), authProvider.getIdentifier()); + username, new Failure(event), authProvider.getIdentifier(), + event.getFailure()); else logger.warn("Authentication attempt from {} for user \"{}\" " + "failed: {}", new RemoteAddress(creds), username, - new Failure(event)); + new Failure(event), event.getFailure()); } } From ea06e01ddde3330f991bb20c2687571ce5dae1f2 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 8 Dec 2025 20:24:10 -0800 Subject: [PATCH 03/13] GUACAMOLE-2181: Rely on automatic stack traces for debug-level logs. --- .../jdbc/connection/ModeledConnection.java | 12 ++++------ .../ModeledConnectionGroup.java | 6 ++--- .../AbstractGuacamoleTunnelService.java | 5 ++-- .../guacamole/auth/jdbc/user/ModeledUser.java | 12 ++++------ .../auth/jdbc/user/ModeledUserContext.java | 10 +++----- .../auth/json/RequestValidationService.java | 13 +++++++---- .../auth/json/user/UserDataService.java | 12 ++++------ .../auth/ldap/LDAPConnectionService.java | 7 +++--- .../auth/ldap/ObjectQueryService.java | 3 +-- .../auth/ldap/conf/ConfigurationService.java | 2 +- .../ldap/connection/ConnectionService.java | 15 ++++-------- .../auth/ldap/group/UserGroupService.java | 7 ++---- .../radius/AuthenticationProviderService.java | 11 +++------ .../auth/radius/RadiusConnectionService.java | 9 +++----- .../guacamole/host/HostRestrictionParser.java | 3 +-- .../openid/token/TokenValidationService.java | 15 ++++-------- .../acs/AssertionConsumerServiceResource.java | 3 +-- .../ssl/SSLClientAuthenticationResource.java | 12 +++++----- .../totp/user/UserVerificationService.java | 10 +++----- .../connection/HistoryConnectionRecord.java | 13 +++++------ .../vault/ksm/conf/KsmAttributeService.java | 13 ++++------- .../guacamole/vault/ksm/secret/KsmClient.java | 2 +- .../guacamole/vault/ksm/user/KsmUser.java | 13 ++++------- .../servlet/GuacamoleHTTPTunnelServlet.java | 5 ++-- .../GuacamoleWebSocketTunnelEndpoint.java | 9 +++----- .../environment/DefaultEnvironment.java | 3 +-- .../environment/LocalEnvironment.java | 10 ++++---- .../GuacamoleServletContextListener.java | 15 ++++-------- .../apache/guacamole/GuacamoleSession.java | 3 +-- ...temFileEnvironmentGuacamoleProperties.java | 4 ++-- .../auth/file/FileAuthenticationProvider.java | 14 +++++------ .../AuthenticationProviderFacade.java | 6 +---- .../guacamole/extension/ExtensionModule.java | 14 ++++++----- .../extension/LanguageResourceService.java | 9 +++----- .../guacamole/extension/ProviderFactory.java | 23 +++++++------------ .../org/apache/guacamole/log/LogModule.java | 10 ++++---- .../guacamole/rest/RESTExceptionMapper.java | 16 +++++-------- .../rest/auth/AuthenticationService.java | 10 ++++---- .../rest/auth/HashTokenSessionMap.java | 3 +-- .../tunnel/InterceptedStreamMap.java | 3 +-- .../OutputStreamInterceptingFilter.java | 3 +-- .../GuacamoleWebSocketTunnelServlet.java | 9 +++----- .../GuacamoleWebSocketTunnelListener.java | 9 +++----- .../GuacamoleWebSocketTunnelServlet.java | 9 +++----- 44 files changed, 152 insertions(+), 243 deletions(-) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ModeledConnection.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ModeledConnection.java index 20861bae9..ccbfb43d3 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ModeledConnection.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ModeledConnection.java @@ -343,15 +343,13 @@ public class ModeledConnection extends ModeledChildDirectoryObject implements User { if (attributes.containsKey(ACCESS_WINDOW_START_ATTRIBUTE_NAME)) { try { getModel().setAccessWindowStart(parseTime(attributes.get(ACCESS_WINDOW_START_ATTRIBUTE_NAME))); } catch (ParseException e) { - logger.warn("Not setting start time of user access window: {}", e.getMessage()); - logger.debug("Unable to parse time attribute.", e); + logger.warn("Not setting start time of user access window: {}", e.getMessage(), e); } } @@ -451,8 +450,7 @@ public class ModeledUser extends ModeledPermissions implements User { if (attributes.containsKey(ACCESS_WINDOW_END_ATTRIBUTE_NAME)) { try { getModel().setAccessWindowEnd(parseTime(attributes.get(ACCESS_WINDOW_END_ATTRIBUTE_NAME))); } catch (ParseException e) { - logger.warn("Not setting end time of user access window: {}", e.getMessage()); - logger.debug("Unable to parse time attribute.", e); + logger.warn("Not setting end time of user access window: {}", e.getMessage(), e); } } @@ -460,8 +458,7 @@ public class ModeledUser extends ModeledPermissions implements User { if (attributes.containsKey(VALID_FROM_ATTRIBUTE_NAME)) { try { getModel().setValidFrom(parseDate(attributes.get(VALID_FROM_ATTRIBUTE_NAME))); } catch (ParseException e) { - logger.warn("Not setting user validity start date: {}", e.getMessage()); - logger.debug("Unable to parse date attribute.", e); + logger.warn("Not setting user validity start date: {}", e.getMessage(), e); } } @@ -469,8 +466,7 @@ public class ModeledUser extends ModeledPermissions implements User { if (attributes.containsKey(VALID_UNTIL_ATTRIBUTE_NAME)) { try { getModel().setValidUntil(parseDate(attributes.get(VALID_UNTIL_ATTRIBUTE_NAME))); } catch (ParseException e) { - logger.warn("Not setting user validity end date: {}", e.getMessage()); - logger.debug("Unable to parse date attribute.", e); + logger.warn("Not setting user validity end date: {}", e.getMessage(), e); } } diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledUserContext.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledUserContext.java index 6064ddaf2..57bf2ddbf 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledUserContext.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledUserContext.java @@ -325,13 +325,9 @@ public class ModeledUserContext extends RestrictedObject } catch (GuacamoleException e) { - - logger.warn( - "Unable to determine if access window enforcement is" - + " enabled for active sessions; enforcing by default: {}" - , e.getMessage()); - logger.debug("Unable to determine access window enforcement policy.", e); - + logger.warn("Unable to determine if access window enforcement is" + + " enabled for active sessions; enforcing by default: {}", + e.getMessage(), e); } // A user context is valid if the associated user's account is valid diff --git a/extensions/guacamole-auth-json/src/main/java/org/apache/guacamole/auth/json/RequestValidationService.java b/extensions/guacamole-auth-json/src/main/java/org/apache/guacamole/auth/json/RequestValidationService.java index 12cd1c6ab..f7a591407 100644 --- a/extensions/guacamole-auth-json/src/main/java/org/apache/guacamole/auth/json/RequestValidationService.java +++ b/extensions/guacamole-auth-json/src/main/java/org/apache/guacamole/auth/json/RequestValidationService.java @@ -78,14 +78,15 @@ public class RequestValidationService { // Deny all requests if restrictions cannot be parsed catch (GuacamoleException e) { - logger.warn("Authentication request from \"{}\" is DENIED due to parse error: {}", credentials.getRemoteAddress(), e.getMessage()); - logger.debug("Error parsing authentication request restrictions from guacamole.properties.", e); + logger.warn("Authentication request from \"{}\" is DENIED due to " + + "parse error: {}", credentials.getRemoteAddress(), e.getMessage(), e); return false; } // All requests are allowed if no restrictions are defined if (trustedNetworks.isEmpty()) { - logger.debug("Authentication request from \"{}\" is ALLOWED (no restrictions).", credentials.getRemoteAddress()); + logger.debug("Authentication request from \"{}\" is ALLOWED (no " + + "restrictions).", credentials.getRemoteAddress()); return true; } @@ -94,14 +95,16 @@ public class RequestValidationService { // Request is allowed if any subnet matches if (new IPAddressString(network).contains(new IPAddressString(credentials.getRemoteAddress()))) { - logger.debug("Authentication request from \"{}\" is ALLOWED (matched subnet).", credentials.getRemoteAddress()); + logger.debug("Authentication request from \"{}\" is ALLOWED " + + "(matched subnet).", credentials.getRemoteAddress()); return true; } } // Otherwise request is denied - no subnets matched - logger.debug("Authentication request from \"{}\" is DENIED (did not match subnet).", credentials.getRemoteAddress()); + logger.debug("Authentication request from \"{}\" is DENIED (did not " + + "match subnet).", credentials.getRemoteAddress()); return false; } diff --git a/extensions/guacamole-auth-json/src/main/java/org/apache/guacamole/auth/json/user/UserDataService.java b/extensions/guacamole-auth-json/src/main/java/org/apache/guacamole/auth/json/user/UserDataService.java index 33b5beb7d..717abfe07 100644 --- a/extensions/guacamole-auth-json/src/main/java/org/apache/guacamole/auth/json/user/UserDataService.java +++ b/extensions/guacamole-auth-json/src/main/java/org/apache/guacamole/auth/json/user/UserDataService.java @@ -168,22 +168,19 @@ public class UserDataService { // Fail if base64 data is not valid catch (IllegalArgumentException e) { - logger.warn("Submitted data is not proper base64."); - logger.debug("Invalid base64 data.", e); + logger.warn("Submitted data is not proper base64: {}", e.getMessage(), e); return null; } // Handle lack of standard UTF-8 support (should never happen) catch (UnsupportedEncodingException e) { - logger.error("Unexpected lack of support for UTF-8: {}", e.getMessage()); - logger.debug("Unable to decode base64 data as UTF-8.", e); + logger.error("Unexpected lack of support for UTF-8: {}", e.getMessage(), e); return null; } // Fail if decryption or key retrieval fails for any reason catch (GuacamoleException e) { - logger.error("Decryption of received data failed: {}", e.getMessage()); - logger.debug("Unable to decrypt received data.", e); + logger.error("Decryption of received data failed: {}", e.getMessage(), e); return null; } @@ -205,8 +202,7 @@ public class UserDataService { // Fail UserData creation if JSON is invalid/unreadable catch (IOException e) { - logger.error("Received JSON is invalid: {}", e.getMessage()); - logger.debug("Error parsing UserData JSON.", e); + logger.error("Received JSON is invalid: {}", e.getMessage(), e); return null; } diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java index 06f83b5eb..f9f76082e 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java @@ -322,8 +322,7 @@ public class LDAPConnectionService { ldapConnection.close(); logger.error("Binding with the LDAP server at \"{}\" as user " + "\"{}\" failed: {}", config.getLdapHost(), bindUser, - e.getMessage()); - logger.debug("Unable to bind to LDAP server.", e); + e.getMessage(), e); return null; } @@ -369,8 +368,8 @@ public class LDAPConnectionService { } catch (LdapInvalidDnException e) { logger.error("Credentials of existing connection cannot be used. " - + "The username used (\"{}\") is not a valid DN.", username); - logger.debug("Cannot bind using invalid DN.", e); + + "The username used (\"{}\") is not a valid DN.", + username, e); ldapConnection.close(); return null; } diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ObjectQueryService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ObjectQueryService.java index 02814ac9c..42d57d19f 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ObjectQueryService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ObjectQueryService.java @@ -265,8 +265,7 @@ public class ObjectQueryService { + "URL \"{}\".", url); } catch (GuacamoleException e) { - logger.warn("Referral to \"{}\" could not be followed: {}", url, e.getMessage()); - logger.debug("Failed to follow LDAP referral.", e); + logger.warn("Referral to \"{}\" could not be followed: {}", url, e.getMessage(), e); } } diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/conf/ConfigurationService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/conf/ConfigurationService.java index 15bd0026c..d0e838c75 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/conf/ConfigurationService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/conf/ConfigurationService.java @@ -122,7 +122,7 @@ public class ConfigurationService { } catch (IOException e) { - logger.error("\"{}\" could not be read/parsed: {}", ldapServers, e.getMessage()); + logger.error("\"{}\" could not be read/parsed: {}", ldapServers, e.getMessage(), e); } } else diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java index 50626ca3b..a40824dd4 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java @@ -202,8 +202,7 @@ public class ConnectionService { } catch (LdapInvalidAttributeValueException e) { logger.error("Invalid value for {} attribute: {}", - LDAP_ATTRIBUTE_NAME_ID, e.getMessage()); - logger.debug("LDAP exception while getting CN attribute.", e); + LDAP_ATTRIBUTE_NAME_ID, e.getMessage(), e); return null; } @@ -223,8 +222,7 @@ public class ConnectionService { config.setProtocol(protocol.getString()); } catch (LdapInvalidAttributeValueException e) { - logger.error("Invalid value of the protocol entry: {}", e.getMessage()); - logger.debug("LDAP exception when getting protocol value.", e); + logger.error("Invalid value of the protocol entry: {}", e.getMessage(), e); return null; } @@ -234,8 +232,7 @@ public class ConnectionService { proxyConfig = getProxyConfiguration(entry); } catch (GuacamoleException e) { - logger.error("Failed to retrieve proxy configuration.", e.getMessage()); - logger.debug("Guacamole Exception when retrieving proxy configuration.", e); + logger.error("Failed to retrieve proxy configuration.", e.getMessage(), e); return null; } @@ -250,8 +247,7 @@ public class ConnectionService { parameter = parameterAttribute.getString(); } catch (LdapInvalidAttributeValueException e) { - logger.warn("Parameter value not valid for {}: {}", cnName, e.getMessage()); - logger.debug("LDAP exception when getting parameter value.", e); + logger.warn("Parameter value not valid for {}: {}", cnName, e.getMessage(), e); return null; } parameterAttribute.remove(parameter); @@ -394,8 +390,7 @@ public class ConnectionService { return new GuacamoleProxyConfiguration(proxyHostname, proxyPort, proxyEncryption); } catch (LdapInvalidAttributeValueException e) { - logger.error("Invalid value in proxy configuration: {}", e.getMessage()); - logger.debug("LDAP exception fetching proxy attribute value.", e); + logger.error("Invalid value in proxy configuration: {}", e.getMessage(), e); throw new GuacamoleServerException("Invalid LDAP value in proxy configuration.", e); } } diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/group/UserGroupService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/group/UserGroupService.java index b38fc9198..89049a00b 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/group/UserGroupService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/group/UserGroupService.java @@ -214,9 +214,7 @@ public class UserGroupService { } catch (LdapInvalidAttributeValueException e) { logger.error("User group missing identifier: {}", - e.getMessage()); - logger.debug("LDAP exception while getting " - + "group identifier.", e); + e.getMessage(), e); } } } @@ -280,8 +278,7 @@ public class UserGroupService { } catch (LdapInvalidAttributeValueException e) { logger.error("User group missing identifier: {}", - e.getMessage()); - logger.debug("LDAP exception while getting group identifier.", e); + e.getMessage(), e); } }); diff --git a/extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/AuthenticationProviderService.java b/extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/AuthenticationProviderService.java index 8568ced7e..23c72911b 100644 --- a/extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/AuthenticationProviderService.java +++ b/extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/AuthenticationProviderService.java @@ -95,7 +95,6 @@ public class AuthenticationProviderService { RadiusAttribute stateAttr = challengePacket.findAttribute(Attr_State.TYPE); if (stateAttr == null) { logger.error("Something went wrong, state attribute not present."); - logger.debug("State Attribute turned up null, which shouldn't happen in AccessChallenge."); return null; } @@ -103,7 +102,6 @@ public class AuthenticationProviderService { RadiusAttribute replyAttr = challengePacket.findAttribute(Attr_ReplyMessage.TYPE); if (replyAttr == null) { logger.error("No reply message received from the server."); - logger.debug("Expecting a Attr_ReplyMessage attribute on this packet, and did not get one."); return null; } @@ -162,8 +160,7 @@ public class AuthenticationProviderService { null); } catch (GuacamoleException e) { - logger.error("Cannot configure RADIUS server: {}", e.getMessage()); - logger.debug("Error configuring RADIUS server.", e); + logger.error("Cannot configure RADIUS server: {}", e.getMessage(), e); throw new GuacamoleInvalidCredentialsException("Authentication error.", CredentialsInfo.USERNAME_PASSWORD); } } @@ -184,13 +181,11 @@ public class AuthenticationProviderService { stateBytes); } catch (IllegalArgumentException e) { - logger.warn("Illegal hexadecimal value while parsing RADIUS state string: {}", e.getMessage()); - logger.debug("Encountered exception while attempting to parse the hexidecimal state value.", e); + logger.warn("Illegal hexadecimal value while parsing RADIUS state string: {}", e.getMessage(), e); throw new GuacamoleInvalidCredentialsException("Authentication error.", CredentialsInfo.USERNAME_PASSWORD); } catch (GuacamoleException e) { - logger.error("Cannot configure RADIUS server: {}", e.getMessage()); - logger.debug("Error configuring RADIUS server.", e); + logger.error("Cannot configure RADIUS server: {}", e.getMessage(), e); throw new GuacamoleInvalidCredentialsException("Authentication error.", CredentialsInfo.USERNAME_PASSWORD); } } diff --git a/extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/RadiusConnectionService.java b/extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/RadiusConnectionService.java index 800e451a5..5dad9edd3 100644 --- a/extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/RadiusConnectionService.java +++ b/extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/RadiusConnectionService.java @@ -245,18 +245,15 @@ public class RadiusConnectionService { } catch (RadiusException e) { - logger.error("Unable to complete authentication.", e.getMessage()); - logger.debug("Authentication with RADIUS failed.", e); + logger.error("Unable to complete authentication: {}", e.getMessage(), e); return null; } catch (NoSuchAlgorithmException e) { - logger.error("No such RADIUS algorithm: {}", e.getMessage()); - logger.debug("Unknown RADIUS algorithm.", e); + logger.error("No such RADIUS algorithm: {}", e.getMessage(), e); return null; } catch (UnknownHostException e) { - logger.error("Could not resolve address: {}", e.getMessage()); - logger.debug("Exception resolving host address.", e); + logger.error("Could not resolve address: {}", e.getMessage(), e); return null; } finally { diff --git a/extensions/guacamole-auth-restrict/src/main/java/org/apache/guacamole/host/HostRestrictionParser.java b/extensions/guacamole-auth-restrict/src/main/java/org/apache/guacamole/host/HostRestrictionParser.java index d368c807a..3e5a14ec7 100644 --- a/extensions/guacamole-auth-restrict/src/main/java/org/apache/guacamole/host/HostRestrictionParser.java +++ b/extensions/guacamole-auth-restrict/src/main/java/org/apache/guacamole/host/HostRestrictionParser.java @@ -64,8 +64,7 @@ public class HostRestrictionParser { addressList.add(hostName); } catch (HostNameException e) { - LOGGER.warn("Invalid host name or IP: {}", host); - LOGGER.debug("HostNameException.", e.getMessage()); + LOGGER.warn("Invalid host name or IP: {}", host, e); } } diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java index 139ab2021..35a515837 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java @@ -122,12 +122,10 @@ public class TokenValidationService { } // Log any failures to validate/parse the JWT catch (MalformedClaimException e) { - logger.info("Rejected OpenID token with malformed claim: {}", e.getMessage()); - logger.debug("Malformed claim within received JWT.", e); + logger.info("Rejected OpenID token with malformed claim: {}", e.getMessage(), e); } catch (InvalidJwtException e) { - logger.info("Rejected invalid OpenID token: {}", e.getMessage()); - logger.debug("Invalid JWT received.", e); + logger.info("Rejected invalid OpenID token: {}", e.getMessage(), e); } return null; @@ -160,8 +158,7 @@ public class TokenValidationService { return username; } catch (MalformedClaimException e) { - logger.info("Rejected OpenID token with malformed claim: {}", e.getMessage()); - logger.debug("Malformed claim within received JWT.", e); + logger.info("Rejected OpenID token with malformed claim: {}", e.getMessage(), e); } // Warn if username was not present in token, as it likely means @@ -203,8 +200,7 @@ public class TokenValidationService { return Collections.unmodifiableSet(new HashSet<>(oidcGroups)); } catch (MalformedClaimException e) { - logger.info("Rejected OpenID token with malformed claim: {}", e.getMessage()); - logger.debug("Malformed claim within received JWT.", e); + logger.info("Rejected OpenID token with malformed claim: {}", e.getMessage(), e); } } @@ -262,8 +258,7 @@ public class TokenValidationService { return Collections.unmodifiableMap(tokens); } catch (MalformedClaimException e) { - logger.info("Rejected OpenID token with malformed claim: {}", e.getMessage()); - logger.debug("Malformed claim within received JWT.", e); + logger.info("Rejected OpenID token with malformed claim: {}", e.getMessage(), e); } } diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/acs/AssertionConsumerServiceResource.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/acs/AssertionConsumerServiceResource.java index 7aadc73f5..492dc64a6 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/acs/AssertionConsumerServiceResource.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/acs/AssertionConsumerServiceResource.java @@ -125,8 +125,7 @@ public class AssertionConsumerServiceResource extends SSOResource { // If invalid, redirect back to main page to re-attempt authentication catch (GuacamoleException e) { - logger.warn("Authentication attempted with an invalid SAML response: {}", e.getMessage()); - logger.debug("Received SAML response failed validation.", e); + logger.warn("Authentication attempted with an invalid SAML response: {}", e.getMessage(), e); return Response.seeOther(guacBase).build(); } diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/java/org/apache/guacamole/auth/ssl/SSLClientAuthenticationResource.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/java/org/apache/guacamole/auth/ssl/SSLClientAuthenticationResource.java index e9a798466..37977876f 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/java/org/apache/guacamole/auth/ssl/SSLClientAuthenticationResource.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-ssl/src/main/java/org/apache/guacamole/auth/ssl/SSLClientAuthenticationResource.java @@ -351,16 +351,16 @@ public class SSLClientAuthenticationResource extends SSOResource { } catch (GuacamoleClientException e) { - logger.warn("SSL/TLS client authentication attempt rejected: {}", e.getMessage()); - logger.debug("SSL/TLS client authentication failed.", e); + logger.warn("SSL/TLS client authentication attempt rejected: {}", + e.getMessage(), e); } catch (GuacamoleException e) { - logger.error("SSL/TLS client authentication attempt could not be processed: {}", e.getMessage()); - logger.debug("SSL/TLS client authentication failed.", e); + logger.error("SSL/TLS client authentication attempt could not be " + + "processed: {}", e.getMessage(), e); } catch (RuntimeException | Error e) { - logger.error("SSL/TLS client authentication attempt failed internally: {}", e.getMessage()); - logger.debug("Internal failure processing SSL/TLS client authentication attempt.", e); + logger.error("SSL/TLS client authentication attempt failed " + + "internally: {}", e.getMessage(), e); } return sessionManager.generateInvalid(); diff --git a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java index 3ebd85b5b..03e482103 100644 --- a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java +++ b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java @@ -128,8 +128,7 @@ public class UserVerificationService { // If key is not valid base32, warn but otherwise pretend the key does // not exist catch (IllegalArgumentException e) { - logger.warn("TOTP key of user \"{}\" is not valid base32.", self.getIdentifier()); - logger.debug("TOTP key is not valid base32.", e); + logger.warn("TOTP key of user \"{}\" is not valid base32.", self.getIdentifier(), e); return null; } @@ -218,9 +217,7 @@ public class UserVerificationService { + "lack permission to update their own account and the " + "TOTP extension was unable to obtain privileged access. " + "TOTP will be disabled for this user.", - self.getIdentifier()); - logger.debug("Permission denied to set TOTP key of user " - + "account.", e); + self.getIdentifier(), e); return false; } catch (GuacamoleUnsupportedException e) { @@ -422,8 +419,7 @@ public class UserVerificationService { } catch (InvalidKeyException e) { - logger.warn("User \"{}\" is associated with an invalid TOTP key.", username); - logger.debug("TOTP key is not valid.", e); + logger.warn("User \"{}\" is associated with an invalid TOTP key.", username, e); } // Provided code is not valid diff --git a/extensions/guacamole-history-recording-storage/src/main/java/org/apache/guacamole/history/connection/HistoryConnectionRecord.java b/extensions/guacamole-history-recording-storage/src/main/java/org/apache/guacamole/history/connection/HistoryConnectionRecord.java index f1c53ff87..91f2ce0e6 100644 --- a/extensions/guacamole-history-recording-storage/src/main/java/org/apache/guacamole/history/connection/HistoryConnectionRecord.java +++ b/extensions/guacamole-history-recording-storage/src/main/java/org/apache/guacamole/history/connection/HistoryConnectionRecord.java @@ -147,8 +147,8 @@ public class HistoryConnectionRecord extends DelegatingConnectionRecord { } catch (IOException e) { logger.warn("Possible session recording \"{}\" could not be " - + "identified as it cannot be read: {}", file, e.getMessage()); - logger.debug("Possible session recording \"{}\" could not be read.", file, e); + + "identified as it cannot be read: {}", + file, e.getMessage(), e); } finally { @@ -161,9 +161,7 @@ public class HistoryConnectionRecord extends DelegatingConnectionRecord { catch (IOException e) { logger.warn("Unexpected error closing recording file \"{}\": {}", - file, e.getMessage()); - logger.debug("Session recording file \"{}\" could not be closed.", - file, e); + file, e.getMessage(), e); } } @@ -297,8 +295,9 @@ public class HistoryConnectionRecord extends DelegatingConnectionRecord { .array()); } catch (MalformedURLException e) { - logger.warn("Ignoring file \"{}\" as a unique URL and UUID for that file could not be generated: {}", e.getMessage()); - logger.debug("URL for file \"{}\" could not be determined.", file, e); + logger.warn("Ignoring file \"{}\" as a unique URL and UUID for " + + "that file could not be generated: {}", + e.getMessage(), e); return; } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java index 8b3c5d938..c1aee2121 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmAttributeService.java @@ -155,15 +155,12 @@ public class KsmAttributeService implements VaultAttributeService { catch (GuacamoleException e) { - logger.warn( - "Unable to determine if KSM user attributes " - + "should be exposed due to config parsing error: {}.", e.getMessage()); - logger.debug( - "Config parsing error prevented checking user attribute configuration", - e); - - // If the configuration can't be parsed, default to not exposing the attributes + // If the configuration can't be parsed, default to not exposing + // the attributes + logger.warn("Unable to determine if KSM user attributes should be " + + "exposed due to config parsing error: {}.", e.getMessage(), e); return Collections.emptyList(); + } } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java index f811e9421..bd412a720 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java @@ -781,7 +781,7 @@ public class KsmClient { // Show the warning only if there is no fallback function // and this was the last attempt logger.warn("Keeper notation \"{}\" could not be resolved " - + "to a record: {}", notation, e.getMessage()); + + "to a record: {}", notation, e.getMessage(), e); return CompletableFuture.completedFuture(null); } diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java index 9c002eb4d..6c7b6413a 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/user/KsmUser.java @@ -87,13 +87,10 @@ import com.google.inject.assistedinject.AssistedInject; boolean userKsmConfigEnabled = false; try { userKsmConfigEnabled = configurationService.getAllowUserConfig(); - } catch (GuacamoleException e) { - - logger.warn( - "Disabling user KSM config due to exception: {}" - , e.getMessage()); - logger.debug("Error looking up if user KSM config is enabled.", e); - + } + catch (GuacamoleException e) { + logger.warn("Disabling user KSM config due to exception: {}", + e.getMessage(), e); } // If user-specific KSM configuration is not enabled, do not expose the @@ -112,4 +109,4 @@ import com.google.inject.assistedinject.AssistedInject; return attributes; } - } \ No newline at end of file + } diff --git a/guacamole-common/src/main/java/org/apache/guacamole/servlet/GuacamoleHTTPTunnelServlet.java b/guacamole-common/src/main/java/org/apache/guacamole/servlet/GuacamoleHTTPTunnelServlet.java index 492076933..e6c1ac3eb 100644 --- a/guacamole-common/src/main/java/org/apache/guacamole/servlet/GuacamoleHTTPTunnelServlet.java +++ b/guacamole-common/src/main/java/org/apache/guacamole/servlet/GuacamoleHTTPTunnelServlet.java @@ -328,13 +328,12 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet { // Catch any thrown guacamole exception and attempt to pass within the // HTTP response, logging each error appropriately. catch (GuacamoleClientException e) { - logger.warn("HTTP tunnel request rejected: {}", e.getMessage()); + logger.warn("HTTP tunnel request rejected: {}", e.getMessage(), e); sendError(response, e.getStatus().getGuacamoleStatusCode(), e.getStatus().getHttpStatusCode(), e.getMessage()); } catch (GuacamoleException e) { - logger.error("HTTP tunnel request failed: {}", e.getMessage()); - logger.debug("Internal error in HTTP tunnel.", e); + logger.error("HTTP tunnel request failed: {}", e.getMessage(), e); sendError(response, e.getStatus().getGuacamoleStatusCode(), e.getStatus().getHttpStatusCode(), "Internal server error."); } diff --git a/guacamole-common/src/main/java/org/apache/guacamole/websocket/GuacamoleWebSocketTunnelEndpoint.java b/guacamole-common/src/main/java/org/apache/guacamole/websocket/GuacamoleWebSocketTunnelEndpoint.java index 772ce64b2..405e6ed90 100644 --- a/guacamole-common/src/main/java/org/apache/guacamole/websocket/GuacamoleWebSocketTunnelEndpoint.java +++ b/guacamole-common/src/main/java/org/apache/guacamole/websocket/GuacamoleWebSocketTunnelEndpoint.java @@ -205,8 +205,7 @@ public abstract class GuacamoleWebSocketTunnelEndpoint extends Endpoint { } catch (GuacamoleException e) { - logger.error("Creation of WebSocket tunnel to guacd failed: {}", e.getMessage()); - logger.debug("Error connecting WebSocket tunnel.", e); + logger.error("Creation of WebSocket tunnel to guacd failed: {}", e.getMessage(), e); closeConnection(session, e.getStatus().getGuacamoleStatusCode(), e.getWebSocketCode()); return; @@ -265,8 +264,7 @@ public abstract class GuacamoleWebSocketTunnelEndpoint extends Endpoint { // to pass within the WebSocket connection, logging // each error appropriately. catch (GuacamoleClientException e) { - logger.info("WebSocket connection terminated: {}", e.getMessage()); - logger.debug("WebSocket connection terminated due to client error.", e); + logger.info("WebSocket connection terminated: {}", e.getMessage(), e); closeConnection(session, e.getStatus().getGuacamoleStatusCode(), e.getWebSocketCode()); } @@ -275,8 +273,7 @@ public abstract class GuacamoleWebSocketTunnelEndpoint extends Endpoint { closeConnection(session, GuacamoleStatus.SUCCESS); } catch (GuacamoleException e) { - logger.error("Connection to guacd terminated abnormally: {}", e.getMessage()); - logger.debug("Internal error during connection to guacd.", e); + logger.error("Connection to guacd terminated abnormally: {}", e.getMessage(), e); closeConnection(session, e.getStatus().getGuacamoleStatusCode(), e.getWebSocketCode()); } diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/environment/DefaultEnvironment.java b/guacamole-ext/src/main/java/org/apache/guacamole/environment/DefaultEnvironment.java index a3db06c19..272a42ee3 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/environment/DefaultEnvironment.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/environment/DefaultEnvironment.java @@ -158,9 +158,8 @@ class DefaultEnvironment extends DelegatingEnvironment { logger.error("Defaulting to case-sensitive handling of " + "usernames and group names as the desired case " + "sensitivity configuration could not be read: {}", - e.getMessage()); + e.getMessage(), e); - logger.debug("Error reading case sensitivity configuration.", e); return CaseSensitivity.ENABLED; } diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/environment/LocalEnvironment.java b/guacamole-ext/src/main/java/org/apache/guacamole/environment/LocalEnvironment.java index bd0f3f152..d323e2bfd 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/environment/LocalEnvironment.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/environment/LocalEnvironment.java @@ -266,8 +266,9 @@ public class LocalEnvironment implements Environment { } catch (IOException e) { - logger.error("Unable to read connection parameter information from \"{}\": {}", file.getAbsolutePath(), e.getMessage()); - logger.debug("Error reading protocol JSON.", e); + logger.error("Unable to read connection parameter " + + "information from \"{}\": {}", + file.getAbsolutePath(), e.getMessage(), e); } } @@ -290,8 +291,9 @@ public class LocalEnvironment implements Environment { protocols.put(protocol, readProtocol(stream)); } catch (IOException e) { - logger.error("Unable to read pre-defined connection parameter information for protocol \"{}\": {}", protocol, e.getMessage()); - logger.debug("Error reading pre-defined protocol JSON.", e); + logger.error("Unable to read pre-defined connection " + + "parameter information for protocol \"{}\": {}", + protocol, e.getMessage(), e); } } diff --git a/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContextListener.java b/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContextListener.java index d8e3f3d58..aef48592a 100644 --- a/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContextListener.java +++ b/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContextListener.java @@ -166,8 +166,7 @@ public class GuacamoleServletContextListener extends GuiceServletContextListener logger.info("Read configuration parameters from \"{}\".", guacProperties); } catch (GuacamoleException e) { - logger.error("Unable to read guacamole.properties: {}", e.getMessage()); - logger.debug("Error reading guacamole.properties.", e); + logger.error("Unable to read guacamole.properties: {}", e.getMessage(), e); } // For any values not defined in GUACAMOLE_HOME/guacamole.properties, @@ -181,8 +180,7 @@ public class GuacamoleServletContextListener extends GuiceServletContextListener } } catch (GuacamoleException e) { - logger.error("Unable to configure support for environment properties: {}", e.getMessage()); - logger.debug("Error reading \"{}\" property from guacamole.properties.", ENABLE_ENVIRONMENT_PROPERTIES.getName(), e); + logger.error("Unable to configure support for environment properties: {}", e.getMessage(), e); } // For any values not defined in GUACAMOLE_HOME/guacamole.properties @@ -198,8 +196,7 @@ public class GuacamoleServletContextListener extends GuiceServletContextListener } } catch (GuacamoleException e) { - logger.error("Unable to configure support for file environment properties: {}", e.getMessage()); - logger.debug("Error reading \"{}\" property from guacamole.properties.", ENABLE_FILE_ENVIRONMENT_PROPERTIES.getName(), e); + logger.error("Unable to configure support for file environment properties: {}", e.getMessage(), e); } // Now that at least the main guacamole.properties source of @@ -225,8 +222,7 @@ public class GuacamoleServletContextListener extends GuiceServletContextListener }); } catch (GuacamoleException e) { - logger.error("An extension listening for application startup failed: {}", e.getMessage()); - logger.debug("Extension failed internally while handling the application startup event.", e); + logger.error("An extension listening for application startup failed: {}", e.getMessage(), e); } } @@ -307,8 +303,7 @@ public class GuacamoleServletContextListener extends GuiceServletContextListener }); } catch (GuacamoleException e) { - logger.error("An extension listening for application shutdown failed: {}", e.getMessage()); - logger.debug("Extension failed internally while handling the application shutdown event.", e); + logger.error("An extension listening for application shutdown failed: {}", e.getMessage(), e); } } diff --git a/guacamole/src/main/java/org/apache/guacamole/GuacamoleSession.java b/guacamole/src/main/java/org/apache/guacamole/GuacamoleSession.java index 5bf0371b3..50ee2a13d 100644 --- a/guacamole/src/main/java/org/apache/guacamole/GuacamoleSession.java +++ b/guacamole/src/main/java/org/apache/guacamole/GuacamoleSession.java @@ -320,8 +320,7 @@ public class GuacamoleSession { }); } catch (GuacamoleException e) { - logger.error("An extension listening for session invalidation failed: {}", e.getMessage()); - logger.debug("Extension failed internally while handling the session invalidation event.", e); + logger.error("An extension listening for session invalidation failed: {}", e.getMessage(), e); } } diff --git a/guacamole/src/main/java/org/apache/guacamole/SystemFileEnvironmentGuacamoleProperties.java b/guacamole/src/main/java/org/apache/guacamole/SystemFileEnvironmentGuacamoleProperties.java index 8b0825292..440051c65 100644 --- a/guacamole/src/main/java/org/apache/guacamole/SystemFileEnvironmentGuacamoleProperties.java +++ b/guacamole/src/main/java/org/apache/guacamole/SystemFileEnvironmentGuacamoleProperties.java @@ -51,8 +51,8 @@ public class SystemFileEnvironmentGuacamoleProperties implements GuacamoleProper return Files.asCharSource(new File(filename), StandardCharsets.UTF_8).read(); } catch (IOException e) { - logger.error("Property \"{}\" could not be read from file \"{}\": {}", name, filename, e.getMessage()); - logger.debug("Error reading property value from file.", e); + logger.error("Property \"{}\" could not be read from file \"{}\": {}", name, + filename, e.getMessage(), e); } } diff --git a/guacamole/src/main/java/org/apache/guacamole/auth/file/FileAuthenticationProvider.java b/guacamole/src/main/java/org/apache/guacamole/auth/file/FileAuthenticationProvider.java index 3099b3b92..0e24d8534 100644 --- a/guacamole/src/main/java/org/apache/guacamole/auth/file/FileAuthenticationProvider.java +++ b/guacamole/src/main/java/org/apache/guacamole/auth/file/FileAuthenticationProvider.java @@ -106,13 +106,13 @@ public class FileAuthenticationProvider extends SimpleAuthenticationProvider { parser = SAXParserFactory.newInstance().newSAXParser(); } catch (ParserConfigurationException e) { - logger.error("Unable to create XML parser for reading \"{}\": {}", USER_MAPPING_FILENAME, e.getMessage()); - logger.debug("An instance of SAXParser could not be created.", e); + logger.error("Unable to create XML parser for reading \"{}\": {}", + USER_MAPPING_FILENAME, e.getMessage(), e); return null; } catch (SAXException e) { - logger.error("Unable to create XML parser for reading \"{}\": {}", USER_MAPPING_FILENAME, e.getMessage()); - logger.debug("An instance of SAXParser could not be created.", e); + logger.error("Unable to create XML parser for reading \"{}\": {}", + USER_MAPPING_FILENAME, e.getMessage(), e); return null; } @@ -138,15 +138,13 @@ public class FileAuthenticationProvider extends SimpleAuthenticationProvider { // If the file is unreadable, return no mapping catch (IOException e) { - logger.warn("Unable to read user mapping file \"{}\": {}", userMappingFile, e.getMessage()); - logger.debug("Error reading user mapping file.", e); + logger.warn("Unable to read user mapping file \"{}\": {}", userMappingFile, e.getMessage(), e); return null; } // If the file cannot be parsed, return no mapping catch (SAXException e) { - logger.warn("User mapping file \"{}\" is not valid: {}", userMappingFile, e.getMessage()); - logger.debug("Error parsing user mapping file.", e); + logger.warn("User mapping file \"{}\" is not valid: {}", userMappingFile, e.getMessage(), e); return null; } diff --git a/guacamole/src/main/java/org/apache/guacamole/extension/AuthenticationProviderFacade.java b/guacamole/src/main/java/org/apache/guacamole/extension/AuthenticationProviderFacade.java index e2122fdef..e51ea76cf 100644 --- a/guacamole/src/main/java/org/apache/guacamole/extension/AuthenticationProviderFacade.java +++ b/guacamole/src/main/java/org/apache/guacamole/extension/AuthenticationProviderFacade.java @@ -182,15 +182,11 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { * authentication provider being skipped. */ private void warnAuthProviderSkipped(Throwable e) { - logger.warn("The \"{}\" authentication provider has been skipped due " + "to an internal error. If this is unexpected or you are the " + "developer of this authentication provider, you may wish to " + "enable debug-level logging: {}", - getIdentifier(), e.getMessage()); - - logger.debug("Authentication provider skipped due to an internal failure.", e); - + getIdentifier(), e.getMessage(), e); } /** diff --git a/guacamole/src/main/java/org/apache/guacamole/extension/ExtensionModule.java b/guacamole/src/main/java/org/apache/guacamole/extension/ExtensionModule.java index c4744e2d9..21adbe81e 100644 --- a/guacamole/src/main/java/org/apache/guacamole/extension/ExtensionModule.java +++ b/guacamole/src/main/java/org/apache/guacamole/extension/ExtensionModule.java @@ -434,8 +434,9 @@ public class ExtensionModule extends ServletModule { // Use empty set by default if property cannot be parsed catch (GuacamoleException e) { - logger.warn("The list of authentication providers specified via the \"{}\" property could not be parsed: {}", SKIP_IF_UNAVAILABLE.getName(), e.getMessage()); - logger.debug("Unable to parse \"{}\" property.", SKIP_IF_UNAVAILABLE.getName(), e); + logger.warn("The list of authentication providers specified via " + + "the \"{}\" property could not be parsed: {}", + SKIP_IF_UNAVAILABLE.getName(), e.getMessage(), e); return Collections.emptySet(); } @@ -457,8 +458,9 @@ public class ExtensionModule extends ServletModule { // Sort by filename if the desired order cannot be read catch (GuacamoleException e) { - logger.warn("The list of extensions specified via the \"{}\" property could not be parsed: {}", EXTENSION_PRIORITY.getName(), e.getMessage()); - logger.debug("Unable to parse \"{}\" property.", EXTENSION_PRIORITY.getName(), e); + logger.warn("The list of extensions specified via the \"{}\" " + + "property could not be parsed: {}", + EXTENSION_PRIORITY.getName(), e.getMessage(), e); return ExtensionOrderProperty.DEFAULT_COMPARATOR; } @@ -522,8 +524,8 @@ public class ExtensionModule extends ServletModule { } catch (GuacamoleException e) { - logger.error("Extension \"{}\" could not be loaded: {}", extensionFile.getName(), e.getMessage()); - logger.debug("Unable to load extension.", e); + logger.error("Extension \"{}\" could not be loaded: {}", + extensionFile.getName(), e.getMessage(), e); } } diff --git a/guacamole/src/main/java/org/apache/guacamole/extension/LanguageResourceService.java b/guacamole/src/main/java/org/apache/guacamole/extension/LanguageResourceService.java index 3cb2acbb9..53e329426 100644 --- a/guacamole/src/main/java/org/apache/guacamole/extension/LanguageResourceService.java +++ b/guacamole/src/main/java/org/apache/guacamole/extension/LanguageResourceService.java @@ -125,8 +125,7 @@ public class LanguageResourceService { // Warn of failure to parse catch (GuacamoleException e) { parsedAllowedLanguages = null; - logger.error("Unable to parse list of allowed languages: {}", e.getMessage()); - logger.debug("Error parsing list of allowed languages.", e); + logger.error("Unable to parse list of allowed languages: {}", e.getMessage(), e); } this.allowedLanguages = parsedAllowedLanguages; @@ -308,8 +307,7 @@ public class LanguageResourceService { } catch (IOException e) { - logger.error("Unable to merge language resource \"{}\": {}", key, e.getMessage()); - logger.debug("Error merging language resource.", e); + logger.error("Unable to merge language resource \"{}\": {}", key, e.getMessage(), e); } } @@ -437,8 +435,7 @@ public class LanguageResourceService { // Continue with next language if unable to read catch (IOException e) { - logger.warn("Unable to read language resource \"{}\".", languageKey); - logger.debug("Error reading language resource.", e); + logger.warn("Unable to read language resource \"{}\".", languageKey, e); } } diff --git a/guacamole/src/main/java/org/apache/guacamole/extension/ProviderFactory.java b/guacamole/src/main/java/org/apache/guacamole/extension/ProviderFactory.java index 01fda5719..9eeab14f9 100644 --- a/guacamole/src/main/java/org/apache/guacamole/extension/ProviderFactory.java +++ b/guacamole/src/main/java/org/apache/guacamole/extension/ProviderFactory.java @@ -61,35 +61,29 @@ class ProviderFactory { catch (NoSuchMethodException e) { logger.error("The {} extension in use is not properly defined. " + "Please contact the developers of the extension or, if you " - + "are the developer, turn on debug-level logging.", typeName); - logger.debug("{} is missing a default constructor.", - providerClass.getName(), e); + + "are the developer, turn on debug-level logging.", + typeName, e); } catch (SecurityException e) { logger.error("The Java security manager is preventing extensions " + "from being loaded. Please check the configuration of Java or your " - + "servlet container."); - logger.debug("Creation of {} disallowed by security manager.", - providerClass.getName(), e); + + "servlet container.", e); } catch (InstantiationException e) { logger.error("The {} extension in use is not properly defined. " + "Please contact the developers of the extension or, if you " - + "are the developer, turn on debug-level logging.", typeName); - logger.debug("{} cannot be instantiated.", providerClass.getName(), e); + + "are the developer, turn on debug-level logging.", + typeName, e); } catch (IllegalAccessException e) { logger.error("The {} extension in use is not properly defined. " + "Please contact the developers of the extension or, if you " - + "are the developer, turn on debug-level logging."); - logger.debug("Default constructor of {} is not public.", typeName, e); + + "are the developer, turn on debug-level logging.", e); } catch (IllegalArgumentException e) { logger.error("The {} extension in use is not properly defined. " + "Please contact the developers of the extension or, if you " - + "are the developer, turn on debug-level logging.", typeName); - logger.debug("Default constructor of {} cannot accept zero arguments.", - providerClass.getName(), e); + + "are the developer, turn on debug-level logging.", typeName, e); } catch (InvocationTargetException e) { // Obtain causing error - create relatively-informative stub error if cause is unknown @@ -97,8 +91,7 @@ class ProviderFactory { if (cause == null) cause = new GuacamoleException("Error encountered during initialization."); - logger.error("{} extension failed to start: {}", typeName, cause.getMessage()); - logger.debug("{} instantiation failed.", providerClass.getName(), e); + logger.error("{} extension failed to start: {}", typeName, cause.getMessage(), cause); } return instance; diff --git a/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java b/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java index 9497ec0f4..e3b7d8b23 100644 --- a/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java +++ b/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java @@ -187,7 +187,7 @@ public class LogModule extends AbstractModule { } catch (FileNotFoundException e) { logger.info("Logback configuration could not be read " - + "from \"{}\": {}", logbackFile, e.getMessage()); + + "from \"{}\": {}", logbackFile, e.getMessage(), e); } } @@ -200,7 +200,7 @@ public class LogModule extends AbstractModule { } catch (GuacamoleException e) { level = LogLevel.INFO; - logger.error("Falling back to \"{}\" log level: {}", level.getCanonicalName(), e.getMessage()); + logger.error("Falling back to \"{}\" log level: {}", level.getCanonicalName(), e.getMessage(), e); } return level.getLogbackConfiguration(); @@ -232,12 +232,10 @@ public class LogModule extends AbstractModule { } catch (JoranException e) { - logger.error("Initialization of logback failed: {}", e.getMessage()); - logger.debug("Unable to load logback configuration.", e); + logger.error("Initialization of logback failed: {}", e.getMessage(), e); } catch (IOException e) { - logger.warn("Logback configuration file could not be cleanly closed: {}", e.getMessage()); - logger.debug("Failed to close logback configuration file.", e); + logger.warn("Logback configuration file could not be cleanly closed: {}", e.getMessage(), e); } } diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/RESTExceptionMapper.java b/guacamole/src/main/java/org/apache/guacamole/rest/RESTExceptionMapper.java index ecb99a473..e84510453 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/RESTExceptionMapper.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/RESTExceptionMapper.java @@ -75,7 +75,7 @@ public class RESTExceptionMapper implements ExceptionMapper { if (t instanceof GuacamoleUnauthorizedException) { String token = authenticationService.getAuthenticationToken(request); if (authenticationService.destroyGuacamoleSession(token)) - logger.debug("Implicitly invalidated session for token \"{}\"", token); + logger.debug("Implicitly invalidated session for token \"{}\"", token, t); } // Translate GuacamoleException subclasses to HTTP error codes @@ -84,11 +84,9 @@ public class RESTExceptionMapper implements ExceptionMapper { // Always log the human-readable details of GuacacamoleExceptions // for the benefit of the administrator if (t instanceof GuacamoleClientException) - logger.debug("Client request rejected: {}", t.getMessage()); - else { - logger.error("Request could not be processed: {}", t.getMessage()); - logger.debug("Processing of request aborted by extension.", t); - } + logger.debug("Client request rejected: {}", t.getMessage(), t); + else + logger.error("Request could not be processed: {}", t.getMessage(), t); return Response .status(((GuacamoleException) t).getHttpStatusCode()) @@ -100,13 +98,11 @@ public class RESTExceptionMapper implements ExceptionMapper { // Wrap unchecked exceptions String message = t.getMessage(); if (message != null) - logger.error("Unexpected internal error: {}", message); + logger.error("Unexpected internal error: {}", message, t); else logger.error("An internal error occurred, but did not contain " + "an error message. Enable debug-level logging for " - + "details."); - - logger.debug("Unexpected error in REST endpoint.", t); + + "details.", t); return Response .status(Response.Status.INTERNAL_SERVER_ERROR) diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/auth/AuthenticationService.java b/guacamole/src/main/java/org/apache/guacamole/rest/auth/AuthenticationService.java index 5a0e4c5fe..7037bb789 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/auth/AuthenticationService.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/auth/AuthenticationService.java @@ -433,12 +433,12 @@ public class AuthenticationService { // This line SHOULD be unreachable unless a bug causes // rethrowCause() to not actually rethrow the underlying failure Throwable cause = e.getCause(); - if (cause != null) { - logger.warn("An underlying internal error was not correctly rethrown by rethrowCause(): {}", cause.getMessage()); - logger.debug("Internal error not rethrown by rethrowCause().", cause); - } + if (cause != null) + logger.warn("An underlying internal error was not correctly " + + "rethrown by rethrowCause(): {}", cause.getMessage(), cause); else - logger.warn("An underlying internal error was not correctly rethrown by rethrowCause()."); + logger.warn("An underlying internal error was not correctly " + + "rethrown by rethrowCause()."); throw e.getCauseAsGuacamoleException(); diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/auth/HashTokenSessionMap.java b/guacamole/src/main/java/org/apache/guacamole/rest/auth/HashTokenSessionMap.java index cabbb78ee..745362d21 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/auth/HashTokenSessionMap.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/auth/HashTokenSessionMap.java @@ -81,8 +81,7 @@ public class HashTokenSessionMap implements TokenSessionMap { sessionTimeoutValue = environment.getProperty(API_SESSION_TIMEOUT, 60); } catch (GuacamoleException e) { - logger.error("Unable to read guacamole.properties: {}", e.getMessage()); - logger.debug("Error while reading session timeout value.", e); + logger.error("Unable to read guacamole.properties: {}", e.getMessage(), e); sessionTimeoutValue = 60; } diff --git a/guacamole/src/main/java/org/apache/guacamole/tunnel/InterceptedStreamMap.java b/guacamole/src/main/java/org/apache/guacamole/tunnel/InterceptedStreamMap.java index 6ddfdf976..e06ded01b 100644 --- a/guacamole/src/main/java/org/apache/guacamole/tunnel/InterceptedStreamMap.java +++ b/guacamole/src/main/java/org/apache/guacamole/tunnel/InterceptedStreamMap.java @@ -69,8 +69,7 @@ public class InterceptedStreamMap { stream.close(); } catch (IOException e) { - logger.warn("Unable to close intercepted stream: {}", e.getMessage()); - logger.debug("I/O error prevented closure of intercepted stream.", e); + logger.warn("Unable to close intercepted stream: {}", e.getMessage(), e); } // Notify waiting threads that the stream has ended diff --git a/guacamole/src/main/java/org/apache/guacamole/tunnel/OutputStreamInterceptingFilter.java b/guacamole/src/main/java/org/apache/guacamole/tunnel/OutputStreamInterceptingFilter.java index 99cf1538f..7fab4ff87 100644 --- a/guacamole/src/main/java/org/apache/guacamole/tunnel/OutputStreamInterceptingFilter.java +++ b/guacamole/src/main/java/org/apache/guacamole/tunnel/OutputStreamInterceptingFilter.java @@ -130,8 +130,7 @@ public class OutputStreamInterceptingFilter blob = BaseEncoding.base64().decode(data); } catch (IllegalArgumentException e) { - logger.warn("Received base64 data for intercepted stream was invalid."); - logger.debug("Decoding base64 data for intercepted stream failed.", e); + logger.warn("Received base64 data for intercepted stream was invalid.", e); return null; } diff --git a/guacamole/src/main/java/org/apache/guacamole/tunnel/websocket/jetty8/GuacamoleWebSocketTunnelServlet.java b/guacamole/src/main/java/org/apache/guacamole/tunnel/websocket/jetty8/GuacamoleWebSocketTunnelServlet.java index 304e1dd95..11e10c873 100644 --- a/guacamole/src/main/java/org/apache/guacamole/tunnel/websocket/jetty8/GuacamoleWebSocketTunnelServlet.java +++ b/guacamole/src/main/java/org/apache/guacamole/tunnel/websocket/jetty8/GuacamoleWebSocketTunnelServlet.java @@ -240,8 +240,7 @@ public abstract class GuacamoleWebSocketTunnelServlet extends WebSocketServlet { tunnel = doConnect(tunnelRequest); } catch (GuacamoleException e) { - logger.error("Creation of WebSocket tunnel to guacd failed: {}", e.getMessage()); - logger.debug("Error connecting WebSocket tunnel.", e); + logger.error("Creation of WebSocket tunnel to guacd failed: {}", e.getMessage(), e); closeConnection(connection, e.getStatus().getGuacamoleStatusCode(), e.getWebSocketCode()); return; @@ -295,8 +294,7 @@ public abstract class GuacamoleWebSocketTunnelServlet extends WebSocketServlet { // to pass within the WebSocket connection, logging // each error appropriately. catch (GuacamoleClientException e) { - logger.info("WebSocket connection terminated: {}", e.getMessage()); - logger.debug("WebSocket connection terminated due to client error.", e); + logger.info("WebSocket connection terminated: {}", e.getMessage(), e); closeConnection(connection, e.getStatus().getGuacamoleStatusCode(), e.getWebSocketCode()); } @@ -305,8 +303,7 @@ public abstract class GuacamoleWebSocketTunnelServlet extends WebSocketServlet { closeConnection(connection, GuacamoleStatus.SUCCESS); } catch (GuacamoleException e) { - logger.error("Connection to guacd terminated abnormally: {}", e.getMessage()); - logger.debug("Internal error during connection to guacd.", e); + logger.error("Connection to guacd terminated abnormally: {}", e.getMessage(), e); closeConnection(connection, e.getStatus().getGuacamoleStatusCode(), e.getWebSocketCode()); } diff --git a/guacamole/src/main/java/org/apache/guacamole/tunnel/websocket/jetty9/GuacamoleWebSocketTunnelListener.java b/guacamole/src/main/java/org/apache/guacamole/tunnel/websocket/jetty9/GuacamoleWebSocketTunnelListener.java index 6422f5718..7ba42d903 100644 --- a/guacamole/src/main/java/org/apache/guacamole/tunnel/websocket/jetty9/GuacamoleWebSocketTunnelListener.java +++ b/guacamole/src/main/java/org/apache/guacamole/tunnel/websocket/jetty9/GuacamoleWebSocketTunnelListener.java @@ -196,8 +196,7 @@ public abstract class GuacamoleWebSocketTunnelListener implements WebSocketListe } catch (GuacamoleException e) { - logger.error("Creation of WebSocket tunnel to guacd failed: {}", e.getMessage()); - logger.debug("Error connecting WebSocket tunnel.", e); + logger.error("Creation of WebSocket tunnel to guacd failed: {}", e.getMessage(), e); closeConnection(session, e.getStatus().getGuacamoleStatusCode(), e.getWebSocketCode()); return; } @@ -245,8 +244,7 @@ public abstract class GuacamoleWebSocketTunnelListener implements WebSocketListe // to pass within the WebSocket connection, logging // each error appropriately. catch (GuacamoleClientException e) { - logger.info("WebSocket connection terminated: {}", e.getMessage()); - logger.debug("WebSocket connection terminated due to client error.", e); + logger.info("WebSocket connection terminated: {}", e.getMessage(), e); closeConnection(session, e.getStatus().getGuacamoleStatusCode(), e.getWebSocketCode()); } @@ -255,8 +253,7 @@ public abstract class GuacamoleWebSocketTunnelListener implements WebSocketListe closeConnection(session, GuacamoleStatus.SUCCESS); } catch (GuacamoleException e) { - logger.error("Connection to guacd terminated abnormally: {}", e.getMessage()); - logger.debug("Internal error during connection to guacd.", e); + logger.error("Connection to guacd terminated abnormally: {}", e.getMessage(), e); closeConnection(session, e.getStatus().getGuacamoleStatusCode(), e.getWebSocketCode()); } diff --git a/guacamole/src/main/java/org/apache/guacamole/tunnel/websocket/tomcat/GuacamoleWebSocketTunnelServlet.java b/guacamole/src/main/java/org/apache/guacamole/tunnel/websocket/tomcat/GuacamoleWebSocketTunnelServlet.java index 215cc8f05..25e8059ee 100644 --- a/guacamole/src/main/java/org/apache/guacamole/tunnel/websocket/tomcat/GuacamoleWebSocketTunnelServlet.java +++ b/guacamole/src/main/java/org/apache/guacamole/tunnel/websocket/tomcat/GuacamoleWebSocketTunnelServlet.java @@ -268,8 +268,7 @@ public abstract class GuacamoleWebSocketTunnelServlet extends WebSocketServlet { tunnel = doConnect(tunnelRequest); } catch (GuacamoleException e) { - logger.error("Creation of WebSocket tunnel to guacd failed: {}", e.getMessage()); - logger.debug("Error connecting WebSocket tunnel.", e); + logger.error("Creation of WebSocket tunnel to guacd failed: {}", e.getMessage(), e); closeConnection(outbound, e.getStatus().getGuacamoleStatusCode(), e.getWebSocketCode()); return; @@ -323,8 +322,7 @@ public abstract class GuacamoleWebSocketTunnelServlet extends WebSocketServlet { // to pass within the WebSocket connection, logging // each error appropriately. catch (GuacamoleClientException e) { - logger.info("WebSocket connection terminated: {}", e.getMessage()); - logger.debug("WebSocket connection terminated due to client error.", e); + logger.info("WebSocket connection terminated: {}", e.getMessage(), e); closeConnection(outbound, e.getStatus().getGuacamoleStatusCode(), e.getWebSocketCode()); } @@ -333,8 +331,7 @@ public abstract class GuacamoleWebSocketTunnelServlet extends WebSocketServlet { closeConnection(outbound, GuacamoleStatus.SUCCESS); } catch (GuacamoleException e) { - logger.error("Connection to guacd terminated abnormally: {}", e.getMessage()); - logger.debug("Internal error during connection to guacd.", e); + logger.error("Connection to guacd terminated abnormally: {}", e.getMessage(), e); closeConnection(outbound, e.getStatus().getGuacamoleStatusCode(), e.getWebSocketCode()); } From cf3902ea8c2533ea79800ed34655663b8c5e503c Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 8 Dec 2025 23:34:49 -0800 Subject: [PATCH 04/13] GUACAMOLE-2181: Ensure ThreadLocals are cleaned up when empty. --- .../AuthenticationProviderFacade.java | 18 +-- .../guacamole/log/InheritableMDCAdapter.java | 112 ++++++++++++++++-- .../rest/session/SessionResource.java | 4 +- 3 files changed, 112 insertions(+), 22 deletions(-) diff --git a/guacamole/src/main/java/org/apache/guacamole/extension/AuthenticationProviderFacade.java b/guacamole/src/main/java/org/apache/guacamole/extension/AuthenticationProviderFacade.java index e51ea76cf..79d757bdd 100644 --- a/guacamole/src/main/java/org/apache/guacamole/extension/AuthenticationProviderFacade.java +++ b/guacamole/src/main/java/org/apache/guacamole/extension/AuthenticationProviderFacade.java @@ -135,7 +135,7 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { return authProvider.getResource(); } finally { - MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + MDC.remove(LogModule.MDC_CONTEXT_KEY); } } @@ -153,7 +153,7 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { return authProvider.updateCredentials(credentials); } finally { - MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + MDC.remove(LogModule.MDC_CONTEXT_KEY); } } @@ -249,7 +249,7 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { } finally { - MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + MDC.remove(LogModule.MDC_CONTEXT_KEY); } } @@ -272,7 +272,7 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { return authProvider.updateAuthenticatedUser(authenticatedUser, credentials); } finally { - MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + MDC.remove(LogModule.MDC_CONTEXT_KEY); } } @@ -318,7 +318,7 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { } finally { - MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + MDC.remove(LogModule.MDC_CONTEXT_KEY); } } @@ -342,7 +342,7 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { return authProvider.updateUserContext(context, authenticatedUser, credentials); } finally { - MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + MDC.remove(LogModule.MDC_CONTEXT_KEY); } } @@ -362,7 +362,7 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { return authProvider.decorate(context, authenticatedUser, credentials); } finally { - MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + MDC.remove(LogModule.MDC_CONTEXT_KEY); } } @@ -383,7 +383,7 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { authenticatedUser, credentials); } finally { - MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + MDC.remove(LogModule.MDC_CONTEXT_KEY); } } @@ -396,7 +396,7 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { authProvider.shutdown(); } finally { - MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + MDC.remove(LogModule.MDC_CONTEXT_KEY); } } diff --git a/guacamole/src/main/java/org/apache/guacamole/log/InheritableMDCAdapter.java b/guacamole/src/main/java/org/apache/guacamole/log/InheritableMDCAdapter.java index 493925476..e75586b47 100644 --- a/guacamole/src/main/java/org/apache/guacamole/log/InheritableMDCAdapter.java +++ b/guacamole/src/main/java/org/apache/guacamole/log/InheritableMDCAdapter.java @@ -33,8 +33,26 @@ import org.slf4j.spi.MDCAdapter; */ public class InheritableMDCAdapter implements MDCAdapter { + // + // IMPORTANT: Thread-local values MUST be removed at web application + // shutdown or a memory leak may result. To avoid this, care must be + // taken in this implementation: + // + // * Ensure that a balanced remove() for every put() is sufficient to + // clear and remove the thread-local itself. + // + // * Calls to ThreadLocal.get() implicitly assign a value to the + // thread-local if no value was present before. This value needs to be + // removed to avoid leaking references at web application shutdown. + // + /** * MDC context map that is automatically inherited by child threads. + *

+ * IMPORTANT: As a thread-local value, it is critical that this value be + * removed at web application shutdown or a memory leak may result. Here, + * we maintain a reference within the thread-local only as long as at least + * one value is stored. Once the map is empty, the thread-local is removed. */ private final InheritableThreadLocal> context = new InheritableThreadLocal>() { @@ -64,6 +82,12 @@ public class InheritableMDCAdapter implements MDCAdapter { * {@link #get(java.lang.String)} which are intentionally stored * separately in {@link #context}. This matches the behavior of the Logback * implementation of MDC. + *

+ * IMPORTANT: As a thread-local value, it is critical that this value be + * removed at web application shutdown or a memory leak may result. Here, + * we maintain a reference within the thread-local only as long as at least + * one non-empty deque is stored. Once no non-empty deques are present + * empty, the thread-local is removed. */ private final ThreadLocal>> contextDeques = new ThreadLocal>>() { @@ -82,30 +106,67 @@ public class InheritableMDCAdapter implements MDCAdapter { @Override public String get(String key) { - return context.get().get(key); + + ForkableHashMap contextMap = context.get(); + + // Do not maintain a thread-local reference to an empty map + if (contextMap.isEmpty()) { + context.remove(); + return null; + } + + return contextMap.get(key); + } @Override public void remove(String key) { - context.get().remove(key); + + ForkableHashMap contextMap = context.get(); + contextMap.remove(key); + + // Do not maintain a thread-local reference to an empty map + if (contextMap.isEmpty()) + context.remove(); + } @Override public void clear() { - context.get().clear(); + // Do not maintain a thread-local reference to an empty map + context.remove(); } @Override public Map getCopyOfContextMap() { - // This function is invoked by Logback for every log message produced, - // so simply copying the context map is expensive. We instead create a - // copy-on-write reference to the context map. - return context.get().fork(); + + ForkableHashMap contextMap = context.get(); + + // Do not maintain a thread-local reference to an empty map - instead + // repurpose the new empty map as the "copy" + if (contextMap.isEmpty()) { + context.remove(); + return contextMap; + } + + // Universally copying the context map would be expensive, as the + // getCopyOfContextMap() function is invoked by Logback for every log + // message produced. Instead, we create a copy-on-write reference to + // the context map. + return contextMap.fork(); + } @Override public void setContextMap(Map contextMap) { - context.set(new ForkableHashMap<>(contextMap)); + + // Do not maintain a thread-local reference to an empty map + if (contextMap.isEmpty()) + context.remove(); + + else + context.set(new ForkableHashMap<>(contextMap)); + } /** @@ -139,17 +200,46 @@ public class InheritableMDCAdapter implements MDCAdapter { @Override public String popByKey(String key) { - return getDeque(key).pop(); + + Deque deque = getDeque(key); + String value = deque.pop(); + + // Do not maintain any references to empty deques (so that it's easy to + // detect when there are no items in any deques) + if (deque.isEmpty()) + clearDequeByKey(key); + + return value; + } @Override public Deque getCopyOfDequeByKey(String key) { - return new LinkedList<>(getDeque(key)); + + Deque deque = getDeque(key); + + // Do not maintain a thread-local reference to an empty deque - instead + // repurpose the new empty deque as the "copy" + if (deque.isEmpty()) { + clearDequeByKey(key); // ... and ensure the deque map is removed if empty, too + return deque; + } + + return new LinkedList<>(deque); + } @Override public void clearDequeByKey(String key) { - getDeque(key).clear(); + + // Clear by entirely removing + Map> dequeMap = contextDeques.get(); + dequeMap.remove(key); + + // Do not maintain a thread-local reference to an empty map + if (dequeMap.isEmpty()) + contextDeques.remove(); + } } diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/session/SessionResource.java b/guacamole/src/main/java/org/apache/guacamole/rest/session/SessionResource.java index 9e9707e10..7d69d180e 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/session/SessionResource.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/session/SessionResource.java @@ -126,7 +126,7 @@ public class SessionResource { } finally { - MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + MDC.remove(LogModule.MDC_CONTEXT_KEY); } } @@ -168,7 +168,7 @@ public class SessionResource { } finally { - MDC.put(LogModule.MDC_CONTEXT_KEY, LogModule.MDC_CONTEXT_ROOT); + MDC.remove(LogModule.MDC_CONTEXT_KEY); } } From 751ca979ce0783663b97ff48f44c1378cd291275 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Thu, 11 Dec 2025 22:37:55 -0800 Subject: [PATCH 05/13] GUACAMOLE-2181: Migrate to deriving log context from call stack. --- .../AuthenticationProviderFacade.java | 83 +--- .../extension/DirectoryClassLoader.java | 7 +- .../apache/guacamole/extension/Extension.java | 2 +- .../extension/ExtensionClassLoader.java | 20 +- .../extension/LoggerContextProvider.java | 41 ++ .../guacamole/log/ContextAwareLogger.java | 429 ++++++++++++++++++ .../log/ContextAwareLoggerFactory.java | 355 +++++++++++++++ .../apache/guacamole/log/ForkableHashMap.java | 267 ----------- .../log/GuacamoleLogbackServiceProvider.java | 47 +- .../guacamole/log/InheritableMDCAdapter.java | 245 ---------- .../org/apache/guacamole/log/LogModule.java | 122 +---- .../log/ReconfigurableLoggerFactory.java | 46 ++ .../rest/session/SessionResource.java | 34 +- 13 files changed, 937 insertions(+), 761 deletions(-) create mode 100644 guacamole/src/main/java/org/apache/guacamole/extension/LoggerContextProvider.java create mode 100644 guacamole/src/main/java/org/apache/guacamole/log/ContextAwareLogger.java create mode 100644 guacamole/src/main/java/org/apache/guacamole/log/ContextAwareLoggerFactory.java delete mode 100644 guacamole/src/main/java/org/apache/guacamole/log/ForkableHashMap.java delete mode 100644 guacamole/src/main/java/org/apache/guacamole/log/InheritableMDCAdapter.java create mode 100644 guacamole/src/main/java/org/apache/guacamole/log/ReconfigurableLoggerFactory.java diff --git a/guacamole/src/main/java/org/apache/guacamole/extension/AuthenticationProviderFacade.java b/guacamole/src/main/java/org/apache/guacamole/extension/AuthenticationProviderFacade.java index 79d757bdd..d922f851e 100644 --- a/guacamole/src/main/java/org/apache/guacamole/extension/AuthenticationProviderFacade.java +++ b/guacamole/src/main/java/org/apache/guacamole/extension/AuthenticationProviderFacade.java @@ -23,14 +23,12 @@ import java.util.Set; import java.util.UUID; import org.apache.guacamole.GuacamoleClientException; import org.apache.guacamole.GuacamoleException; -import org.apache.guacamole.log.LogModule; import org.apache.guacamole.net.auth.AuthenticatedUser; import org.apache.guacamole.net.auth.AuthenticationProvider; import org.apache.guacamole.net.auth.Credentials; import org.apache.guacamole.net.auth.UserContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.slf4j.MDC; /** * Provides a safe wrapper around an AuthenticationProvider subclass, such that @@ -60,12 +58,6 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { */ private final Set tolerateFailures; - /** - * The logging context that should be provided to SLF4J's MDC for all log - * messages originating from the underlying authentication provider. - */ - private final String logContext; - /** * The identifier to provide for the underlying authentication provider if * the authentication provider could not be loaded. @@ -99,11 +91,6 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { this.authProvider = ProviderFactory.newInstance("authentication provider", authProviderClass); - if (this.authProvider != null) - this.logContext = LogModule.getMDCContext(authProvider); - else - this.logContext = LogModule.MDC_CONTEXT_ROOT; - } @Override @@ -130,13 +117,7 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { } // Delegate to underlying auth provider - try { - MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); - return authProvider.getResource(); - } - finally { - MDC.remove(LogModule.MDC_CONTEXT_KEY); - } + return authProvider.getResource(); } @@ -148,13 +129,7 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { return credentials; // Delegate to underlying auth provider - try { - MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); - return authProvider.updateCredentials(credentials); - } - finally { - MDC.remove(LogModule.MDC_CONTEXT_KEY); - } + return authProvider.updateCredentials(credentials); } @@ -222,7 +197,6 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { // Delegate to underlying auth provider try { - MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); return authProvider.authenticateUser(credentials); } @@ -248,10 +222,6 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { } - finally { - MDC.remove(LogModule.MDC_CONTEXT_KEY); - } - } @Override @@ -267,13 +237,7 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { } // Delegate to underlying auth provider - try { - MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); - return authProvider.updateAuthenticatedUser(authenticatedUser, credentials); - } - finally { - MDC.remove(LogModule.MDC_CONTEXT_KEY); - } + return authProvider.updateAuthenticatedUser(authenticatedUser, credentials); } @@ -291,7 +255,6 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { // Delegate to underlying auth provider try { - MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); return authProvider.getUserContext(authenticatedUser); } @@ -317,10 +280,6 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { } - finally { - MDC.remove(LogModule.MDC_CONTEXT_KEY); - } - } @Override @@ -337,13 +296,7 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { } // Delegate to underlying auth provider - try { - MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); - return authProvider.updateUserContext(context, authenticatedUser, credentials); - } - finally { - MDC.remove(LogModule.MDC_CONTEXT_KEY); - } + return authProvider.updateUserContext(context, authenticatedUser, credentials); } @@ -357,13 +310,7 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { return context; // Delegate to underlying auth provider - try { - MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); - return authProvider.decorate(context, authenticatedUser, credentials); - } - finally { - MDC.remove(LogModule.MDC_CONTEXT_KEY); - } + return authProvider.decorate(context, authenticatedUser, credentials); } @@ -377,27 +324,15 @@ public class AuthenticationProviderFacade implements AuthenticationProvider { return context; // Delegate to underlying auth provider - try { - MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); - return authProvider.redecorate(decorated, context, - authenticatedUser, credentials); - } - finally { - MDC.remove(LogModule.MDC_CONTEXT_KEY); - } + return authProvider.redecorate(decorated, context, + authenticatedUser, credentials); } @Override public void shutdown() { - try { - MDC.put(LogModule.MDC_CONTEXT_KEY, logContext); - if (authProvider != null) - authProvider.shutdown(); - } - finally { - MDC.remove(LogModule.MDC_CONTEXT_KEY); - } + if (authProvider != null) + authProvider.shutdown(); } } diff --git a/guacamole/src/main/java/org/apache/guacamole/extension/DirectoryClassLoader.java b/guacamole/src/main/java/org/apache/guacamole/extension/DirectoryClassLoader.java index 83ad9be73..71508c47d 100644 --- a/guacamole/src/main/java/org/apache/guacamole/extension/DirectoryClassLoader.java +++ b/guacamole/src/main/java/org/apache/guacamole/extension/DirectoryClassLoader.java @@ -32,7 +32,7 @@ import org.apache.guacamole.GuacamoleException; * A ClassLoader implementation which finds classes within .jar files within a * given directory. */ -public class DirectoryClassLoader extends URLClassLoader { +public class DirectoryClassLoader extends URLClassLoader implements LoggerContextProvider { /** * Returns all .jar files within the given directory as an array of URLs. @@ -105,4 +105,9 @@ public class DirectoryClassLoader extends URLClassLoader { super(getJarURLs(dir), DirectoryClassLoader.class.getClassLoader()); } + @Override + public String getLoggerContext() { + return "lib"; + } + } diff --git a/guacamole/src/main/java/org/apache/guacamole/extension/Extension.java b/guacamole/src/main/java/org/apache/guacamole/extension/Extension.java index be67a384b..355dc50e4 100644 --- a/guacamole/src/main/java/org/apache/guacamole/extension/Extension.java +++ b/guacamole/src/main/java/org/apache/guacamole/extension/Extension.java @@ -397,7 +397,7 @@ public class Extension { } // Create isolated classloader for this extension - classLoader = new ExtensionClassLoader(file, temporaryFiles, parent); + classLoader = new ExtensionClassLoader(manifest, file, temporaryFiles, parent); } diff --git a/guacamole/src/main/java/org/apache/guacamole/extension/ExtensionClassLoader.java b/guacamole/src/main/java/org/apache/guacamole/extension/ExtensionClassLoader.java index 6473aa230..4f79678e2 100644 --- a/guacamole/src/main/java/org/apache/guacamole/extension/ExtensionClassLoader.java +++ b/guacamole/src/main/java/org/apache/guacamole/extension/ExtensionClassLoader.java @@ -46,7 +46,7 @@ import org.slf4j.LoggerFactory; * the given .jar. If classes are defined in both the parent and the extension * .jar, the versions defined within the extension .jar are used. */ -public class ExtensionClassLoader extends URLClassLoader { +public class ExtensionClassLoader extends URLClassLoader implements LoggerContextProvider { /** * Logger for this class. @@ -71,6 +71,11 @@ public class ExtensionClassLoader extends URLClassLoader { */ private final ClassLoader parent; + /** + * The parsed manifest file of the extension that this class loader is for. + */ + private final ExtensionManifest manifest; + /** * Returns the URL that refers to the given file. If the given file refers * to a directory, an exception is thrown. @@ -251,6 +256,9 @@ public class ExtensionClassLoader extends URLClassLoader { * multiple times will not affect previously-returned instances of * ExtensionClassLoader. * + * @param manifest + * The parsed manifest of the extension. + * * @param extension * The extension .jar file from which classes should be loaded. * @@ -267,10 +275,11 @@ public class ExtensionClassLoader extends URLClassLoader { * If the given file is not actually a file, or the contents of the * file cannot be read. */ - public ExtensionClassLoader(File extension, List temporaryFiles, - ClassLoader parent) throws GuacamoleException { + public ExtensionClassLoader(ExtensionManifest manifest, File extension, + List temporaryFiles, ClassLoader parent) throws GuacamoleException { super(getExtensionURLs(extension, temporaryFiles), null); this.parent = parent; + this.manifest = manifest; } @Override @@ -288,4 +297,9 @@ public class ExtensionClassLoader extends URLClassLoader { } + @Override + public String getLoggerContext() { + return "ext:" + manifest.getNamespace(); + } + } diff --git a/guacamole/src/main/java/org/apache/guacamole/extension/LoggerContextProvider.java b/guacamole/src/main/java/org/apache/guacamole/extension/LoggerContextProvider.java new file mode 100644 index 000000000..3cf37f4bf --- /dev/null +++ b/guacamole/src/main/java/org/apache/guacamole/extension/LoggerContextProvider.java @@ -0,0 +1,41 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.guacamole.extension; + +/** + * Provider of general, human-readable text that describes the context of a + * logger. This is currently used to allow our various custom class loaders to + * provide contextual information, which our custom logger factory can pull from + * the call stack when loggers are created. + */ +public interface LoggerContextProvider { + + /** + * Returns human-readable text describing the context of events logged by + * a logger. The text should describe the general context of the source of + * the entry. + * + * @return + * Human-readable text describing the context of events logged by any + * logger created/maintained by the class implementing this interface. + */ + String getLoggerContext(); + +} diff --git a/guacamole/src/main/java/org/apache/guacamole/log/ContextAwareLogger.java b/guacamole/src/main/java/org/apache/guacamole/log/ContextAwareLogger.java new file mode 100644 index 000000000..68ab7c973 --- /dev/null +++ b/guacamole/src/main/java/org/apache/guacamole/log/ContextAwareLogger.java @@ -0,0 +1,429 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.guacamole.log; + +import java.util.Arrays; +import org.slf4j.Logger; +import org.slf4j.Marker; +import org.slf4j.MarkerFactory; +import org.slf4j.event.Level; +import org.slf4j.spi.LoggingEventBuilder; + +/** + * {@link Logger} that automatically includes arbitrary context. The additional + * context is included as a {@link Marker}, and all logging functions are + * overridden to include at least this Marker. If a logging function is given a + * Marker by the caller, it is wrapped as a reference of Marker created by the + * ContextAwareLogger's Marker. + */ +public class ContextAwareLogger implements Logger { + + /** + * The wrapped Logger. + */ + private final Logger logger; + + /** + * The arbitrary context that should be included as a Marker in all log + * messages. + */ + private final String context; + + /** + * Creates a new ContextAwareLogger that wraps given Logger, automatically + * including the provided context as a Marker for each logged message. + * + * @param context + * The arbitrary context to include as a Marker with each + * logged message. + * + * @param logger + * The Logger to wrap. + */ + public ContextAwareLogger(String context, Logger logger) { + this.logger = logger; + this.context = context; + } + + /** + * Returns a Marker containing the arbitrary context provided when this + * ContextAwareLogger was created. If Markers are provided to this function, + * they are included as references within the returned Marker. + * + * @param refs + * The Markers to include in the returned Marker as references, if any. + * + * @return + * A Marker containing the context provided when this + * ContextAwareLogger was created, as well as any provided Marker + * references. + */ + private Marker getContextMarker(Marker... refs) { + Marker marker = MarkerFactory.getMarker(context); + Arrays.stream(refs).forEach(marker::add); + return marker; + } + + @Override + public String getName() { + return logger.getName(); + } + + @Override + public LoggingEventBuilder makeLoggingEventBuilder(Level level) { + return logger.makeLoggingEventBuilder(level); + } + + @Override + public LoggingEventBuilder atLevel(Level level) { + return logger.atLevel(level); + } + + @Override + public boolean isEnabledForLevel(Level level) { + return logger.isEnabledForLevel(level); + } + + @Override + public boolean isTraceEnabled() { + return logger.isTraceEnabled(); + } + + @Override + public void trace(String msg) { + logger.trace(getContextMarker(), msg); + } + + @Override + public void trace(String format, Object arg) { + logger.trace(getContextMarker(), format, arg); + } + + @Override + public void trace(String format, Object arg1, Object arg2) { + logger.trace(getContextMarker(), format, arg1, arg2); + } + + @Override + public void trace(String format, Object... arguments) { + logger.trace(getContextMarker(), format, arguments); + } + + @Override + public void trace(String msg, Throwable t) { + logger.trace(getContextMarker(), msg, t); + } + + @Override + public boolean isTraceEnabled(Marker marker) { + return logger.isTraceEnabled(marker); + } + + @Override + public LoggingEventBuilder atTrace() { + return logger.atTrace(); + } + + @Override + public void trace(Marker marker, String msg) { + logger.trace(getContextMarker(marker), msg); + } + + @Override + public void trace(Marker marker, String format, Object arg) { + logger.trace(getContextMarker(marker), format, arg); + } + + @Override + public void trace(Marker marker, String format, Object arg1, Object arg2) { + logger.trace(getContextMarker(marker), format, arg1, arg2); + } + + @Override + public void trace(Marker marker, String format, Object... argArray) { + logger.trace(getContextMarker(marker), format, argArray); + } + + @Override + public void trace(Marker marker, String msg, Throwable t) { + logger.trace(getContextMarker(marker), msg, t); + } + + @Override + public boolean isDebugEnabled() { + return logger.isDebugEnabled(); + } + + @Override + public void debug(String msg) { + logger.debug(getContextMarker(), msg); + } + + @Override + public void debug(String format, Object arg) { + logger.debug(getContextMarker(), format, arg); + } + + @Override + public void debug(String format, Object arg1, Object arg2) { + logger.debug(getContextMarker(), format, arg1, arg2); + } + + @Override + public void debug(String format, Object... arguments) { + logger.debug(getContextMarker(), format, arguments); + } + + @Override + public void debug(String msg, Throwable t) { + logger.debug(getContextMarker(), msg, t); + } + + @Override + public boolean isDebugEnabled(Marker marker) { + return logger.isDebugEnabled(marker); + } + + @Override + public void debug(Marker marker, String msg) { + logger.debug(getContextMarker(marker), msg); + } + + @Override + public void debug(Marker marker, String format, Object arg) { + logger.debug(getContextMarker(marker), format, arg); + } + + @Override + public void debug(Marker marker, String format, Object arg1, Object arg2) { + logger.debug(getContextMarker(marker), format, arg1, arg2); + } + + @Override + public void debug(Marker marker, String format, Object... arguments) { + logger.debug(getContextMarker(marker), format, arguments); + } + + @Override + public void debug(Marker marker, String msg, Throwable t) { + logger.debug(getContextMarker(marker), msg, t); + } + + @Override + public LoggingEventBuilder atDebug() { + return logger.atDebug(); + } + + @Override + public boolean isInfoEnabled() { + return logger.isInfoEnabled(); + } + + @Override + public void info(String msg) { + logger.info(getContextMarker(), msg); + } + + @Override + public void info(String format, Object arg) { + logger.info(getContextMarker(), format, arg); + } + + @Override + public void info(String format, Object arg1, Object arg2) { + logger.info(getContextMarker(), format, arg1, arg2); + } + + @Override + public void info(String format, Object... arguments) { + logger.info(getContextMarker(), format, arguments); + } + + @Override + public void info(String msg, Throwable t) { + logger.info(getContextMarker(), msg, t); + } + + @Override + public boolean isInfoEnabled(Marker marker) { + return logger.isInfoEnabled(marker); + } + + @Override + public void info(Marker marker, String msg) { + logger.info(getContextMarker(marker), msg); + } + + @Override + public void info(Marker marker, String format, Object arg) { + logger.info(getContextMarker(marker), format, arg); + } + + @Override + public void info(Marker marker, String format, Object arg1, Object arg2) { + logger.info(getContextMarker(marker), format, arg1, arg2); + } + + @Override + public void info(Marker marker, String format, Object... arguments) { + logger.info(getContextMarker(marker), format, arguments); + } + + @Override + public void info(Marker marker, String msg, Throwable t) { + logger.info(getContextMarker(marker), msg, t); + } + + @Override + public LoggingEventBuilder atInfo() { + return logger.atInfo(); + } + + @Override + public boolean isWarnEnabled() { + return logger.isWarnEnabled(); + } + + @Override + public void warn(String msg) { + logger.warn(getContextMarker(), msg); + } + + @Override + public void warn(String format, Object arg) { + logger.warn(getContextMarker(), format, arg); + } + + @Override + public void warn(String format, Object... arguments) { + logger.warn(getContextMarker(), format, arguments); + } + + @Override + public void warn(String format, Object arg1, Object arg2) { + logger.warn(getContextMarker(), format, arg1, arg2); + } + + @Override + public void warn(String msg, Throwable t) { + logger.warn(getContextMarker(), msg, t); + } + + @Override + public boolean isWarnEnabled(Marker marker) { + return logger.isWarnEnabled(marker); + } + + @Override + public void warn(Marker marker, String msg) { + logger.warn(getContextMarker(marker), msg); + } + + @Override + public void warn(Marker marker, String format, Object arg) { + logger.warn(getContextMarker(marker), format, arg); + } + + @Override + public void warn(Marker marker, String format, Object arg1, Object arg2) { + logger.warn(getContextMarker(marker), format, arg1, arg2); + } + + @Override + public void warn(Marker marker, String format, Object... arguments) { + logger.warn(getContextMarker(marker), format, arguments); + } + + @Override + public void warn(Marker marker, String msg, Throwable t) { + logger.warn(getContextMarker(marker), msg, t); + } + + @Override + public LoggingEventBuilder atWarn() { + return logger.atWarn(); + } + + @Override + public boolean isErrorEnabled() { + return logger.isErrorEnabled(); + } + + @Override + public void error(String msg) { + logger.error(getContextMarker(), msg); + } + + @Override + public void error(String format, Object arg) { + logger.error(getContextMarker(), format, arg); + } + + @Override + public void error(String format, Object arg1, Object arg2) { + logger.error(getContextMarker(), format, arg1, arg2); + } + + @Override + public void error(String format, Object... arguments) { + logger.error(getContextMarker(), format, arguments); + } + + @Override + public void error(String msg, Throwable t) { + logger.error(getContextMarker(), msg, t); + } + + @Override + public boolean isErrorEnabled(Marker marker) { + return logger.isErrorEnabled(marker); + } + + @Override + public void error(Marker marker, String msg) { + logger.error(getContextMarker(marker), msg); + } + + @Override + public void error(Marker marker, String format, Object arg) { + logger.error(getContextMarker(marker), format, arg); + } + + @Override + public void error(Marker marker, String format, Object arg1, Object arg2) { + logger.error(getContextMarker(marker), format, arg1, arg2); + } + + @Override + public void error(Marker marker, String format, Object... arguments) { + logger.error(getContextMarker(marker), format, arguments); + } + + @Override + public void error(Marker marker, String msg, Throwable t) { + logger.error(getContextMarker(marker), msg, t); + } + + @Override + public LoggingEventBuilder atError() { + return logger.atError(); + } + +} diff --git a/guacamole/src/main/java/org/apache/guacamole/log/ContextAwareLoggerFactory.java b/guacamole/src/main/java/org/apache/guacamole/log/ContextAwareLoggerFactory.java new file mode 100644 index 000000000..e751e71eb --- /dev/null +++ b/guacamole/src/main/java/org/apache/guacamole/log/ContextAwareLoggerFactory.java @@ -0,0 +1,355 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.guacamole.log; + +import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.joran.JoranConfigurator; +import ch.qos.logback.classic.util.ContextInitializer; +import ch.qos.logback.core.joran.spi.JoranException; +import ch.qos.logback.core.util.StatusPrinter; +import java.io.InputStream; +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.List; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.apache.guacamole.extension.LoggerContextProvider; +import org.slf4j.ILoggerFactory; +import org.slf4j.Logger; +import org.slf4j.MDC; +import org.slf4j.Marker; +import org.slf4j.spi.MDCAdapter; + +/** + * {@link ILoggerFactory} implementation that automatically includes additional + * {@link Marker} context based on the origin of each log message. The origin + * of each message is determined by investigating the caller classes, with + * the actual context provided by the associated classloader if it implements + * {@link LoggerContextProvider}. + */ +public class ContextAwareLoggerFactory implements ReconfigurableLoggerFactory { + + /** + * The name of the LoggerContext maintained by this logger factory. + */ + private static final String LOGGER_CONTEXT_NAME = "guacamole"; + + /** + * The default text that should be displayed if no other context can be + * derived from any caller. + */ + private static final String DEFAULT_CONTEXT = "app"; + + /** + * Regular expression that matches one or more newlines, regardless of style + * (Windows, Linux, etc.), preserving only the first such newline. This + * expression is intended to reliably match newlines that may be interpreted + * by the shell/viewer, regardless of the platform producing the log + * message. + *

+ * This specific regular expression is intended for use with content that is + * being included inline (concatenated with an existing line of output). + */ + private static final String INLINE_NEWLINE_REGEX = + + // Locate the right edge of the first newline sequence, whether + // that is CR, LF, CRLF, or LFCR + "(?<=" + + // Do not match newline sequences that are not the first + // newline in the sequence + + "(? + * NOTE: This method is only supported by Java 9 and late and will throw a + * ClassNotFoundException otherwise.. + * + * @return + * A List of all classes currently on the call stack, in order of + * decreasing depth. + * + * @throws UnsupportedOperationException + * If support for StackWalker is not present (Java 8). + */ + @SuppressWarnings("unchecked") + private List> getCallersFromStackWalker() + throws UnsupportedOperationException { + + // + // NOTE: The following is the reflection equivalent of: + // + // return StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE).walk((stack) -> + // stack.map((frame) -> frame.getDeclaringClass()).collect(Collectors.toList()) + // ); + // + + try { + + // Retrieve StackWalker definition and related classes + Class stackWalkerClass = Class.forName("java.lang.StackWalker"); + Class stackFrameClass = Class.forName("java.lang.StackWalker$StackFrame"); + Class optionEnum = (Class) Class.forName("java.lang.StackWalker$Option"); + + // Retrieve references to functions we will be needing + Method stackWalkerGetInstanceMethod = stackWalkerClass.getMethod("getInstance", optionEnum); + Method walkMethod = stackWalkerClass.getMethod("walk", Function.class); + Method getDeclaringClassMethod = stackFrameClass.getMethod("getDeclaringClass"); + + // Equivalent to: + // StackWalker stackWalker = StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE) + Object stackWalker = stackWalkerGetInstanceMethod.invoke(null, + Enum.valueOf(optionEnum, "RETAIN_CLASS_REFERENCE")); + + // Equivalent to: + // (stack) -> stack.map((frame) -> frame.getDeclaringClass()).collect(Collectors.toList()) + Function, List>> walker = + (stack) -> stack.map((frame) -> { + try { + return (Class) getDeclaringClassMethod.invoke(frame); + } + catch (ReflectiveOperationException e) { + throw new UnsupportedOperationException("getDeclaringClass() " + + "could not be invoked for stack frame ", e); + } + }).collect(Collectors.toList()); + + // Equivalent to: stackWalker.walk(...) + return (List>) walkMethod.invoke(stackWalker, walker); + + } + catch (ReflectiveOperationException e) { + throw new UnsupportedOperationException("StackWalker is not " + + "supported by this version of Java", e); + } + + } + + /** + * Uses the deprecated {@link SecurityManager} to retrieve all classes + * currently on the call stack for the current function, in order of + * decreasing depth. The current function (the deepest function in terms of + * call depth) is the first item in the returned list. + *

+ * NOTE: This is the only method that is supported by Java 8 that includes + * direct Class references. + * + * @return + * A List of all classes currently on the call stack, in order of + * decreasing depth. + * + * @throws UnsupportedOperationException + * If support for SecurityManager is not present (expected for future + * Java versions). + */ + @SuppressWarnings("removal") + private List> getCallersFromSecurityManager() { + try { + + // Atempt to locate SecurityManager class and grab its + // getClassContext() method (which returns a stack trace) + Class securityManagerClass = Class.forName("java.lang.SecurityManager"); + Object securityManager = securityManagerClass.getDeclaredConstructor().newInstance(); + Method getClassContextMethod = securityManagerClass.getDeclaredMethod("getClassContext"); + + // getClassContext() is protected + getClassContextMethod.setAccessible(true); + + return Arrays.asList((Class[]) getClassContextMethod.invoke(securityManager)); + + } + catch (ReflectiveOperationException e) { + throw new UnsupportedOperationException("SecurityManager is not " + + "supported by this version of Java", e); + } + } + + /** + * Returns all classes currently on the call stack for the current function, + * in order of decreasing depth. The current function (the deepest function + * in terms of call depth) is the first item in the returned list. + * + * @return + * A List of all classes currently on the call stack, in order of + * decreasing depth. + * + * @throws UnsupportedOperationException + * If no supported mechanism exists for retrieving the call stack. + */ + private List> getCallers() throws UnsupportedOperationException { + try { + return getCallersFromStackWalker(); + } + catch (UnsupportedOperationException e) { + return getCallersFromSecurityManager(); + } + } + + /** + * Derives the context of the message currently being logged based on the + * content of the call stack. + * + * @return + * The context of the message currently being logged. + */ + private String getCallerLogContext() { + + // Derive context from the most recent caller that explicitly provides + // their own logging context, if any + for (Class caller : getCallers()) { + + ClassLoader classloader = caller.getClassLoader(); + if (classloader instanceof LoggerContextProvider) + return ((LoggerContextProvider) classloader).getLoggerContext(); + + } + + // Use default context if no callers provide their own + return DEFAULT_CONTEXT; + + } + + @Override + public Logger getLogger(String name) { + return new ContextAwareLogger(getCallerLogContext(), context.getLogger(name)); + } + + @Override + public void reconfigure(InputStream logbackConfiguration) throws JoranException { + + context.reset(); + + // Include convenience properties for building off Guacamole's + // built-in configuration with a custom logback.xml + context.putProperty("guac_timestamp_pattern", LOG_PATTERN_FRAGMENT_TIMESTAMP); + context.putProperty("guac_message_pattern", LOG_PATTERN_FRAGMENT_MESSAGE_BODY); + context.putProperty("guac_pattern", LOG_PATTERN_DEFAULT); + context.putProperty("guac_verbose_pattern", LOG_PATTERN_VERBOSE); + + // Initialize logback + JoranConfigurator configurator = new JoranConfigurator(); + configurator.setContext(context); + configurator.doConfigure(logbackConfiguration); + + // Dump any errors that occur during logback init + StatusPrinter.printInCaseOfErrorsOrWarnings(context); + + } + +} diff --git a/guacamole/src/main/java/org/apache/guacamole/log/ForkableHashMap.java b/guacamole/src/main/java/org/apache/guacamole/log/ForkableHashMap.java deleted file mode 100644 index 2ef5e1c47..000000000 --- a/guacamole/src/main/java/org/apache/guacamole/log/ForkableHashMap.java +++ /dev/null @@ -1,267 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you 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 org.apache.guacamole.log; - -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; -import java.util.function.BiConsumer; -import java.util.function.BiFunction; -import java.util.function.Function; - -/** - * HashMap implementation that uses copy-on-write to allow efficient cloning. - * When an instance of ForkableHashMap has been copied using {@link #fork()}, - * the parent and child both continue to refer to the same copy of their - * underlying data. If the parent (or child) is modified, only then does the - * parent (or child) create an independent copy of that underlying data. - *

- * For simplicity, this implementation DOES NOT permit modification via the - * collections/sets returned by {@link #values()}, {@link #keySet()}, or - * {@link #entrySet()}. - *

- * As with the standard HashMap, this implementation IS NOT threadsafe. - * - * @param - * The type of key used to store/retrieve values within this map. - * - * @param - * The type of values stored by this map. - */ -public class ForkableHashMap implements Map { - - /** - * The underlying content of this ForkableHashMap. This content is shared - * with at least one other ForkableHashMap if {@link #shared} is true, and - * is an independent copy that may safely be modified if {@link #shared} is - * false. - */ - private Map content; - - /** - * Whether the Map referenced by {@link #content} is shared with at least - * one other ForkableHashMap. - */ - private boolean shared; - - /** - * Creates a new, empty ForkableHashMap. - */ - public ForkableHashMap() { - this.content = new HashMap<>(); - this.shared = false; - } - - /** - * Creates a new ForkableHashMap that contains an independent copy of the - * entries in the provided map. - *

- * This is not a deep copy; only the references to keys and values are - * copied. - * - * @param map - * The map to copy. - */ - public ForkableHashMap(Map map) { - this.content = new HashMap<>(map); - this.shared = false; - } - - /** - * Creates a new ForkableHashMap that shares the content of the given - * ForkableHashMap. It is the responsibility of the caller to ensure that - * the given ForkableHashMap has its content marked as shared. - * - * @param parent - * The ForkableHashMap to lazily copy. - */ - private ForkableHashMap(ForkableHashMap parent) { - this.content = parent.content; - this.shared = true; - } - - /** - * Creates a new ForkableHashMap that shares the content of this map. No - * actual copy operation will be performed until this map or the new map - * are modified. - * - * @return - * A new ForkableHashMap that lazily shares the content of this map. - */ - public ForkableHashMap fork() { - shared = true; - return new ForkableHashMap<>(this); - } - - /** - * Returns the map providing content for this ForkableHashMap while - * ensuring that map is safe to modify. If the map is shared with another - * ForkableHashMap, the map is first replaced with an independent copy. - * - * @return - * The map providing content for this ForkableHashMap, which shall be - * safe to modify after this function returns. - */ - private Map writableContent() { - - // Transform into independent copy if other instances of ForkableHashMap - // share the same underlying data - if (shared) { - content = new HashMap<>(content); - shared = false; - } - - return content; - - } - - @Override - public int size() { - return content.size(); - } - - @Override - public boolean isEmpty() { - return content.isEmpty(); - } - - @Override - public boolean containsKey(Object o) { - return content.containsKey(o); - } - - @Override - public boolean containsValue(Object o) { - return content.containsValue(o); - } - - @Override - public V get(Object o) { - return content.get(o); - } - - @Override - public V put(K k, V v) { - return writableContent().put(k, v); - } - - @Override - public V remove(Object o) { - return writableContent().remove(o); - } - - @Override - public void putAll(Map map) { - writableContent().putAll(map); - } - - @Override - public void clear() { - writableContent().clear(); - } - - @Override - public Set keySet() { - return Collections.unmodifiableSet(content.keySet()); - } - - @Override - public Collection values() { - return Collections.unmodifiableCollection(content.values()); - } - - @Override - public Set> entrySet() { - return Collections.unmodifiableSet(content.entrySet()); - } - - @Override - public boolean equals(Object o) { - return content.equals(o); - } - - @Override - public int hashCode() { - return content.hashCode(); - } - - @Override - public V getOrDefault(Object key, V defaultValue) { - return content.getOrDefault(key, defaultValue); - } - - @Override - public void forEach(BiConsumer action) { - content.forEach(action); - } - - @Override - public void replaceAll(BiFunction function) { - writableContent().replaceAll(function); - } - - @Override - public V putIfAbsent(K key, V value) { - return writableContent().putIfAbsent(key, value); - } - - @Override - public boolean remove(Object key, Object value) { - return writableContent().remove(key, value); - } - - @Override - public boolean replace(K key, V oldValue, V newValue) { - return writableContent().replace(key, oldValue, newValue); - } - - @Override - public V replace(K key, V value) { - return writableContent().replace(key, value); - } - - @Override - public V computeIfAbsent(K key, Function mappingFunction) { - return content.computeIfAbsent(key, mappingFunction); - } - - @Override - public V computeIfPresent(K key, BiFunction remappingFunction) { - return content.computeIfPresent(key, remappingFunction); - } - - @Override - public V compute(K key, BiFunction remappingFunction) { - return content.compute(key, remappingFunction); - } - - @Override - public V merge(K key, V value, BiFunction remappingFunction) { - return writableContent().merge(key, value, remappingFunction); - } - - @Override - public String toString() { - return content.toString(); - } - -} diff --git a/guacamole/src/main/java/org/apache/guacamole/log/GuacamoleLogbackServiceProvider.java b/guacamole/src/main/java/org/apache/guacamole/log/GuacamoleLogbackServiceProvider.java index c1414c554..4ae3e1a97 100644 --- a/guacamole/src/main/java/org/apache/guacamole/log/GuacamoleLogbackServiceProvider.java +++ b/guacamole/src/main/java/org/apache/guacamole/log/GuacamoleLogbackServiceProvider.java @@ -19,10 +19,7 @@ package org.apache.guacamole.log; -import ch.qos.logback.classic.LoggerContext; -import ch.qos.logback.classic.util.ContextInitializer; -import ch.qos.logback.core.joran.spi.JoranException; -import org.slf4j.ILoggerFactory; +import ch.qos.logback.classic.util.LogbackMDCAdapter; import org.slf4j.IMarkerFactory; import org.slf4j.helpers.BasicMarkerFactory; import org.slf4j.spi.MDCAdapter; @@ -30,7 +27,7 @@ import org.slf4j.spi.SLF4JServiceProvider; /** * SLF4JServiceProvider that overrides the default behavior of Logback's - * service provider, forcing the use of a custom MDCAdapter. + * service provider, forcing the use of a custom, context-aware logger factory. */ public class GuacamoleLogbackServiceProvider implements SLF4JServiceProvider { @@ -42,22 +39,18 @@ public class GuacamoleLogbackServiceProvider implements SLF4JServiceProvider { private static final String SLF4J_API_MAX_VERSION = "2.0.99"; /** - * The name of the LoggerContext maintained by this service provider. + * The singleton instance of the LoggerFactory maintained by this service + * provider. The custom implementation provided here uses the call stack + * and our various custom class loaders to determine the context of the + * loggers that will be created. */ - private static final String LOGGER_CONTEXT_NAME = "guacamole"; - - /** - * The singleton instance of the LoggerContext maintained by this service - * provider. The LoggerContext is actually the LoggerFactory. - */ - private LoggerContext context; + private ContextAwareLoggerFactory loggerFactory; /** * The singleton instance of the MDCAdapter implementation that should be - * used by Logback and SLF4J. The custom implementation provided here - * allows the context map to be inherited by child threads. + * used by Logback and SLF4J. */ - private final MDCAdapter mdcAdapter = new InheritableMDCAdapter(); + private final MDCAdapter mdcAdapter = new LogbackMDCAdapter(); /** * The singleton MarkerFactory instance maintained by this service @@ -67,23 +60,7 @@ public class GuacamoleLogbackServiceProvider implements SLF4JServiceProvider { @Override public void initialize() { - - context = new LoggerContext(); - context.setName(LOGGER_CONTEXT_NAME); - context.setMDCAdapter(mdcAdapter); - - // Perform basic init (prior to loading any Guacamole-specific - // configuration, which is loaded by the LogModule) - try { - new ContextInitializer(context).autoConfig(); - } - catch (JoranException | RuntimeException | Error e) { - System.err.println("Logging system could not be initialized: " + e.getMessage()); - e.printStackTrace(System.err); - } - - context.start(); - + loggerFactory = new ContextAwareLoggerFactory(mdcAdapter); } @Override @@ -92,8 +69,8 @@ public class GuacamoleLogbackServiceProvider implements SLF4JServiceProvider { } @Override - public ILoggerFactory getLoggerFactory() { - return context; + public ContextAwareLoggerFactory getLoggerFactory() { + return loggerFactory; } @Override diff --git a/guacamole/src/main/java/org/apache/guacamole/log/InheritableMDCAdapter.java b/guacamole/src/main/java/org/apache/guacamole/log/InheritableMDCAdapter.java deleted file mode 100644 index e75586b47..000000000 --- a/guacamole/src/main/java/org/apache/guacamole/log/InheritableMDCAdapter.java +++ /dev/null @@ -1,245 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you 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 org.apache.guacamole.log; - -import java.util.Deque; -import java.util.HashMap; -import java.util.LinkedList; -import java.util.Map; -import org.slf4j.spi.MDCAdapter; - -/** - * MDCAdapter implementation that allows individual string context values to be - * inherited by child threads. Deque context values (as manipulated with - * {@link #pushByKey(java.lang.String, java.lang.String)}, etc.) are not - * inherited. - */ -public class InheritableMDCAdapter implements MDCAdapter { - - // - // IMPORTANT: Thread-local values MUST be removed at web application - // shutdown or a memory leak may result. To avoid this, care must be - // taken in this implementation: - // - // * Ensure that a balanced remove() for every put() is sufficient to - // clear and remove the thread-local itself. - // - // * Calls to ThreadLocal.get() implicitly assign a value to the - // thread-local if no value was present before. This value needs to be - // removed to avoid leaking references at web application shutdown. - // - - /** - * MDC context map that is automatically inherited by child threads. - *

- * IMPORTANT: As a thread-local value, it is critical that this value be - * removed at web application shutdown or a memory leak may result. Here, - * we maintain a reference within the thread-local only as long as at least - * one value is stored. Once the map is empty, the thread-local is removed. - */ - private final InheritableThreadLocal> context = - new InheritableThreadLocal>() { - - @Override - protected ForkableHashMap initialValue() { - return new ForkableHashMap<>(); - } - - @Override - protected ForkableHashMap childValue(ForkableHashMap parentValue) { - // This function is invoked by the JVM for every thread created, - // so simply creating a new copy is expensive. We instead create a - // copy-on-write reference. - return parentValue.fork(); - } - - }; - - /** - * MDC deque map that is NOT automatically inherited by child threads. We - * choose not to inherit the deque map as that would result in pushed values - * possibly never getting popped. - *

- * NOTE: The Deques have no relation to the single values maintained using - * {@link #put(java.lang.String, java.lang.String)} and retrieved using - * {@link #get(java.lang.String)} which are intentionally stored - * separately in {@link #context}. This matches the behavior of the Logback - * implementation of MDC. - *

- * IMPORTANT: As a thread-local value, it is critical that this value be - * removed at web application shutdown or a memory leak may result. Here, - * we maintain a reference within the thread-local only as long as at least - * one non-empty deque is stored. Once no non-empty deques are present - * empty, the thread-local is removed. - */ - private final ThreadLocal>> contextDeques = - new ThreadLocal>>() { - - @Override - protected Map> initialValue() { - return new HashMap<>(); - } - - }; - - @Override - public void put(String key, String value) { - context.get().put(key, value); - } - - @Override - public String get(String key) { - - ForkableHashMap contextMap = context.get(); - - // Do not maintain a thread-local reference to an empty map - if (contextMap.isEmpty()) { - context.remove(); - return null; - } - - return contextMap.get(key); - - } - - @Override - public void remove(String key) { - - ForkableHashMap contextMap = context.get(); - contextMap.remove(key); - - // Do not maintain a thread-local reference to an empty map - if (contextMap.isEmpty()) - context.remove(); - - } - - @Override - public void clear() { - // Do not maintain a thread-local reference to an empty map - context.remove(); - } - - @Override - public Map getCopyOfContextMap() { - - ForkableHashMap contextMap = context.get(); - - // Do not maintain a thread-local reference to an empty map - instead - // repurpose the new empty map as the "copy" - if (contextMap.isEmpty()) { - context.remove(); - return contextMap; - } - - // Universally copying the context map would be expensive, as the - // getCopyOfContextMap() function is invoked by Logback for every log - // message produced. Instead, we create a copy-on-write reference to - // the context map. - return contextMap.fork(); - - } - - @Override - public void setContextMap(Map contextMap) { - - // Do not maintain a thread-local reference to an empty map - if (contextMap.isEmpty()) - context.remove(); - - else - context.set(new ForkableHashMap<>(contextMap)); - - } - - /** - * Returns the Deque of pushed values associated with the given key, as - * required for {@link #pushByKey(java.lang.String, java.lang.String)} and - * {@link #popByKey(java.lang.String)}. If no Deque yet exists for the - * given key, it is first created. - * - * @param key - * The key to retrieve the Deque for. - * - * @return - * The Deque of pushed values for the given key. - */ - private Deque getDeque(String key) { - - Deque values = contextDeques.get().get(key); - if (values == null) { - values = new LinkedList<>(); - contextDeques.get().put(key, values); - } - - return values; - - } - - @Override - public void pushByKey(String key, String value) { - getDeque(key).push(value); - } - - @Override - public String popByKey(String key) { - - Deque deque = getDeque(key); - String value = deque.pop(); - - // Do not maintain any references to empty deques (so that it's easy to - // detect when there are no items in any deques) - if (deque.isEmpty()) - clearDequeByKey(key); - - return value; - - } - - @Override - public Deque getCopyOfDequeByKey(String key) { - - Deque deque = getDeque(key); - - // Do not maintain a thread-local reference to an empty deque - instead - // repurpose the new empty deque as the "copy" - if (deque.isEmpty()) { - clearDequeByKey(key); // ... and ensure the deque map is removed if empty, too - return deque; - } - - return new LinkedList<>(deque); - - } - - @Override - public void clearDequeByKey(String key) { - - // Clear by entirely removing - Map> dequeMap = contextDeques.get(); - dequeMap.remove(key); - - // Do not maintain a thread-local reference to an empty map - if (dequeMap.isEmpty()) - contextDeques.remove(); - - } - -} diff --git a/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java b/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java index e3b7d8b23..ef07aeb84 100644 --- a/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java +++ b/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java @@ -19,10 +19,7 @@ package org.apache.guacamole.log; -import ch.qos.logback.classic.LoggerContext; -import ch.qos.logback.classic.joran.JoranConfigurator; import ch.qos.logback.core.joran.spi.JoranException; -import ch.qos.logback.core.util.StatusPrinter; import com.google.inject.AbstractModule; import java.io.File; import java.io.FileInputStream; @@ -31,8 +28,8 @@ import java.io.IOException; import java.io.InputStream; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.environment.Environment; -import org.apache.guacamole.net.auth.AuthenticationProvider; import org.apache.guacamole.properties.EnumGuacamoleProperty; +import org.slf4j.ILoggerFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,95 +48,6 @@ public class LogModule extends AbstractModule { */ private final Environment environment; - /** - * The name of the MDC key that should be used to store Guacamole-specific - * context information. If an extension logs a message, this information - * will include the identifier of the relevant authentication provider. - */ - public static final String MDC_CONTEXT_KEY = "context"; - - /** - * The name that should be considered the top-level context when populating - * the MDC entry pointed to by {@link #MDC_CONTEXT_KEY}. - */ - public static final String MDC_CONTEXT_ROOT = "guacamole"; - - /** - * Returns the context information that should be stored in MDC under - * {@link #MDC_CONTEXT_KEY} for any message logged by the given - * authentication provider. - * - * @param authProvider - * The authentication provider to provide context for. - * - * @return - * The context information that should be stored in MDC for any message - * logged by the given authentication provider. - */ - public static String getMDCContext(AuthenticationProvider authProvider) { - return LogModule.MDC_CONTEXT_ROOT + "/" + authProvider.getIdentifier(); - } - - /** - * Regular expression that matches one or more newlines, regardless of style - * (Windows, Linux, etc.), preserving only the first such newline. This - * expression is intended to reliably match newlines that may be interpreted - * by the shell/viewer, regardless of the platform producing the log - * message. - */ - private static final String NEWLINE_REGEX = - - // Locate the right edge of the first newline sequence, whether - // that is CR, LF, CRLF, or LFCR - "(?<=" - - // Do not match newline sequences that are not the first - // newline in the sequence - + "(? Date: Thu, 11 Dec 2025 22:39:46 -0800 Subject: [PATCH 06/13] GUACAMOLE-2181: Avoid NPE if servlet container is shut down before extensions are loaded. --- .../GuacamoleServletContextListener.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContextListener.java b/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContextListener.java index aef48592a..99db0b9fd 100644 --- a/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContextListener.java +++ b/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContextListener.java @@ -297,13 +297,15 @@ public class GuacamoleServletContextListener extends GuiceServletContextListener } // Inform any listeners that application shutdown has completed - try { - listenerService.handleEvent(new ApplicationShutdownEvent() { - // The application shutdown event currently has no content - }); - } - catch (GuacamoleException e) { - logger.error("An extension listening for application shutdown failed: {}", e.getMessage(), e); + if (listenerService != null) { + try { + listenerService.handleEvent(new ApplicationShutdownEvent() { + // The application shutdown event currently has no content + }); + } + catch (GuacamoleException e) { + logger.error("An extension listening for application shutdown failed: {}", e.getMessage(), e); + } } } From 1457b186ff2941e249bb9b9d15ed988e219afa87 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Thu, 11 Dec 2025 22:40:46 -0800 Subject: [PATCH 07/13] GUACAMOLE-2181: Ensure our custom logging factory is used. --- .../GuacamoleServletContainerInitializer.java | 64 +++++++++++++++++++ .../javax.servlet.ServletContainerInitializer | 1 + guacamole/src/main/webapp/WEB-INF/web.xml | 8 +++ 3 files changed, 73 insertions(+) create mode 100644 guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContainerInitializer.java create mode 100644 guacamole/src/main/resources/META-INF/services/javax.servlet.ServletContainerInitializer diff --git a/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContainerInitializer.java b/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContainerInitializer.java new file mode 100644 index 000000000..afdbb6785 --- /dev/null +++ b/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContainerInitializer.java @@ -0,0 +1,64 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.guacamole; + +import java.util.Set; +import javax.servlet.ServletContainerInitializer; +import javax.servlet.ServletContext; +import javax.servlet.ServletException; +import org.apache.guacamole.log.GuacamoleLogbackServiceProvider; +import org.slf4j.LoggerFactory; +import org.slf4j.helpers.Reporter; + +/** + * ServletContainerInitializer which forces the SLF4J service provider to be + * Guacamole's built-in logging system. This is necessary as Logback also + * includes an SLF4J service provider, and there is no guarantee which provider + * will be chosen by SLF4J in the case that multiple are present. + *

+ * SLF4J uses a "slf4j.provider" system property to allow a specific provider + * to be chosen, which this initializer sets automatically. + */ +public class GuacamoleServletContainerInitializer implements ServletContainerInitializer { + + @Override + public void onStartup(Set> c, ServletContext ctx) throws ServletException { + + // Ideally, we want to use Guacamole's built-in logging facility, but + // we shouldn't override the administrator if they have their own SLF4J + // provider that needs to be used instead (ie: for some log aggregation + // service) + if (System.getProperty(LoggerFactory.PROVIDER_PROPERTY_KEY) == null) { + + System.setProperty(LoggerFactory.PROVIDER_PROPERTY_KEY, + GuacamoleLogbackServiceProvider.class.getCanonicalName()); + + // Avoid excessive verbosity from SLF4J unless the administrator + // has explicitly chosen SLF4J's verbosity level (the SLF4J provider + // class will otherwise be logged at startup, which is an internal + // detail and not typically needed) + if (System.getProperty(Reporter.SLF4J_INTERNAL_VERBOSITY_KEY) == null) + System.setProperty(Reporter.SLF4J_INTERNAL_VERBOSITY_KEY, "WARN"); + + } + + } + +} diff --git a/guacamole/src/main/resources/META-INF/services/javax.servlet.ServletContainerInitializer b/guacamole/src/main/resources/META-INF/services/javax.servlet.ServletContainerInitializer new file mode 100644 index 000000000..11801bac0 --- /dev/null +++ b/guacamole/src/main/resources/META-INF/services/javax.servlet.ServletContainerInitializer @@ -0,0 +1 @@ +org.apache.guacamole.GuacamoleServletContainerInitializer diff --git a/guacamole/src/main/webapp/WEB-INF/web.xml b/guacamole/src/main/webapp/WEB-INF/web.xml index ba1f6115e..3b99914c1 100644 --- a/guacamole/src/main/webapp/WEB-INF/web.xml +++ b/guacamole/src/main/webapp/WEB-INF/web.xml @@ -28,6 +28,14 @@ index.html + + + logbackDisableServletContainerInitializer + true + + cacheRevalidationFilter From b5748a1219e2b2ec160b389b751870a8b5aea01d Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Thu, 11 Dec 2025 22:58:20 -0800 Subject: [PATCH 08/13] GUACAMOLE-2181: Silence noisy WebSocket error when socket is closed. If the WebSocket is closed while a message is on its way out, an IllegalStateException is thrown. --- .../websocket/GuacamoleWebSocketTunnelEndpoint.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/guacamole-common/src/main/java/org/apache/guacamole/websocket/GuacamoleWebSocketTunnelEndpoint.java b/guacamole-common/src/main/java/org/apache/guacamole/websocket/GuacamoleWebSocketTunnelEndpoint.java index 405e6ed90..901ed8348 100644 --- a/guacamole-common/src/main/java/org/apache/guacamole/websocket/GuacamoleWebSocketTunnelEndpoint.java +++ b/guacamole-common/src/main/java/org/apache/guacamole/websocket/GuacamoleWebSocketTunnelEndpoint.java @@ -149,7 +149,12 @@ public abstract class GuacamoleWebSocketTunnelEndpoint extends Endpoint { // connection has changed, and synchronization need only be performed // in context of the new remote. synchronized (remote) { - remote.sendText(instruction); + try { + remote.sendText(instruction); + } + catch (IllegalStateException e) { + throw new IOException("WebSocket message could not be sent.", e); + } } } @@ -280,7 +285,7 @@ public abstract class GuacamoleWebSocketTunnelEndpoint extends Endpoint { } catch (IOException e) { - logger.debug("I/O error prevents further reads.", e); + logger.debug("I/O error prevents further WebSocket communication.", e); closeConnection(session, GuacamoleStatus.SERVER_ERROR); } From 4098e7680189fd1407d56467177a192cc0b9d8cf Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 12 Dec 2025 10:26:28 -0800 Subject: [PATCH 09/13] GUACAMOLE-2181: Warning regarding log init failures is non-fatal, not bailing out. --- guacamole/src/main/java/org/apache/guacamole/log/LogModule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java b/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java index ef07aeb84..8410b8d83 100644 --- a/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java +++ b/guacamole/src/main/java/org/apache/guacamole/log/LogModule.java @@ -120,7 +120,7 @@ public class LogModule extends AbstractModule { try (InputStream logbackConfiguration = getLogbackConfiguration()) { - // Bail out if the system is somehow preventing Guacamole from + // Warn if the system is somehow preventing Guacamole from // using its own logging ILoggerFactory loggerFactory = LoggerFactory.getILoggerFactory(); if (!(loggerFactory instanceof ReconfigurableLoggerFactory)) { From 42f450315eb87fe346a57fdb9c461b290d55f0c0 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 12 Dec 2025 10:26:49 -0800 Subject: [PATCH 10/13] GUACAMOLE-2181: Pre-initialize logging such that format is always consistent. --- .../GuacamoleServletContainerInitializer.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContainerInitializer.java b/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContainerInitializer.java index afdbb6785..1eea13c7c 100644 --- a/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContainerInitializer.java +++ b/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContainerInitializer.java @@ -19,11 +19,17 @@ package org.apache.guacamole; +import ch.qos.logback.core.joran.spi.JoranException; +import java.io.IOException; +import java.io.InputStream; import java.util.Set; import javax.servlet.ServletContainerInitializer; import javax.servlet.ServletContext; import javax.servlet.ServletException; import org.apache.guacamole.log.GuacamoleLogbackServiceProvider; +import org.apache.guacamole.log.LogLevel; +import org.apache.guacamole.log.ReconfigurableLoggerFactory; +import org.slf4j.ILoggerFactory; import org.slf4j.LoggerFactory; import org.slf4j.helpers.Reporter; @@ -38,6 +44,12 @@ import org.slf4j.helpers.Reporter; */ public class GuacamoleServletContainerInitializer implements ServletContainerInitializer { + /** + * The log level that should be used when we have not yet loaded any + * configuration dictating otherwise. + */ + private static final LogLevel EARLY_LOG_LEVEL = LogLevel.INFO; + @Override public void onStartup(Set> c, ServletContext ctx) throws ServletException { @@ -59,6 +71,20 @@ public class GuacamoleServletContainerInitializer implements ServletContainerIni } + // Attempt early initialization of Logback (before we begin parsing + // Guacamole's configuration), such that even startup messages follow + // a consistent format + try (InputStream logbackConfiguration = EARLY_LOG_LEVEL.getLogbackConfiguration()) { + ILoggerFactory loggerFactory = LoggerFactory.getILoggerFactory(); + if (loggerFactory instanceof ReconfigurableLoggerFactory) + ((ReconfigurableLoggerFactory) loggerFactory).reconfigure(logbackConfiguration); + } + catch (JoranException | IOException e) { + System.err.println("Early initialization of Logback failed - " + + "logging format may not match until the web application " + + "finishes starting up: " + e.getMessage()); + } + } } From 5ad5a46cfe539fc038c5c87a7fb1eebb0696abdd Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 12 Dec 2025 12:37:06 -0800 Subject: [PATCH 11/13] GUACAMOLE-2181: Clarify log messages for RADIUS challenge parsing failures. --- .../auth/radius/AuthenticationProviderService.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/AuthenticationProviderService.java b/extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/AuthenticationProviderService.java index 23c72911b..c82680288 100644 --- a/extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/AuthenticationProviderService.java +++ b/extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/AuthenticationProviderService.java @@ -94,14 +94,18 @@ public class AuthenticationProviderService { // Try to get the state attribute - if it's not there, we have a problem RadiusAttribute stateAttr = challengePacket.findAttribute(Attr_State.TYPE); if (stateAttr == null) { - logger.error("Something went wrong, state attribute not present."); + logger.error("RADIUS server did not include the required \"{}\" " + + "attribute in its challenge packet - cannot continue.", + Attr_State.NAME); return null; } // We need to get the reply message so we know what to ask the user RadiusAttribute replyAttr = challengePacket.findAttribute(Attr_ReplyMessage.TYPE); if (replyAttr == null) { - logger.error("No reply message received from the server."); + logger.error("RADIUS server did not include the required \"{}\" " + + "attribute in its challenge packet - cannot continue.", + Attr_ReplyMessage.NAME); return null; } From f309d6de95d462579d0e6b8b2a52c455ee642768 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 12 Dec 2025 15:49:03 -0800 Subject: [PATCH 12/13] GUACAMOLE-2181: Disable WADL support (silence Jersey warning). We don't use WADL support and it has historically been implicitly disabled through the lack of a JAXB implementation. Jersey warns of this every time Guacamole starts up: WARN: JAXBContext implementation could not be found. WADL feature is disabled. This disables WADL explicitly. --- .../main/java/org/apache/guacamole/GuacamoleApplication.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/guacamole/src/main/java/org/apache/guacamole/GuacamoleApplication.java b/guacamole/src/main/java/org/apache/guacamole/GuacamoleApplication.java index 7f3e4603c..bdaf9213a 100644 --- a/guacamole/src/main/java/org/apache/guacamole/GuacamoleApplication.java +++ b/guacamole/src/main/java/org/apache/guacamole/GuacamoleApplication.java @@ -26,6 +26,7 @@ import javax.ws.rs.ApplicationPath; import org.glassfish.hk2.api.ServiceLocator; import org.glassfish.jersey.jackson.JacksonFeature; import org.glassfish.jersey.server.ResourceConfig; +import org.glassfish.jersey.server.ServerProperties; import org.jvnet.hk2.guice.bridge.api.GuiceBridge; import org.jvnet.hk2.guice.bridge.api.GuiceIntoHK2Bridge; import org.slf4j.bridge.SLF4JBridgeHandler; @@ -68,6 +69,10 @@ public class GuacamoleApplication extends ResourceConfig { // Automatically scan for REST resources packages("org.apache.guacamole.rest"); + // Explicitly disable WADL generation (we don't use it and Jersey warns + // of the fact that it won't generate a WADL otherwise) + property(ServerProperties.WADL_FEATURE_DISABLE, true); + // Use Jackson for JSON register(JacksonFeature.class); From acf0684915f9ad8a086b15829295042404924476 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 12 Dec 2025 16:00:05 -0800 Subject: [PATCH 13/13] GUACAMOLE-2181: Remove LDAP reference to non-existent overlay language. The LDAP extension's guac-manifest.json refers to a German translation in "de.json", but that translation file doesn't actually exist for LDAP, hence the following warning at startup: WARN: Overlay language resource "de" does not exist. This corrects that warning by removing the erroneous reference. --- .../guacamole-auth-ldap/src/main/resources/guac-manifest.json | 1 - 1 file changed, 1 deletion(-) diff --git a/extensions/guacamole-auth-ldap/src/main/resources/guac-manifest.json b/extensions/guacamole-auth-ldap/src/main/resources/guac-manifest.json index dc812d399..737d9533f 100644 --- a/extensions/guacamole-auth-ldap/src/main/resources/guac-manifest.json +++ b/extensions/guacamole-auth-ldap/src/main/resources/guac-manifest.json @@ -10,7 +10,6 @@ ], "translations" : [ - "translations/de.json", "translations/en.json", "translations/zh.json" ]