Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,16 @@ public LoggerContext getContext(
final boolean currentContext,
final List<URI> configLocations,
final String name) {
final LoggerContext ctx =
selector.getContext(fqcn, loader, currentContext, null /*this probably needs to change*/);
if (externalContext != null && ctx.getExternalContext() == null) {
ctx.setExternalContext(externalContext);
final LoggerContext ctx;
if (externalContext instanceof Map.Entry) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ramanathan1504, would you mind pointing me the code calling this method with an externalContext of type Map.Entry, please?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vy

The externalContext is passed as a Map.Entry<String, Object> from the log4j-web module.
In Log4jWebInitializerImpl.java (lines 177 and 183), it calls:
WebLoggerContextUtils.createExternalEntry(this.servletContext)

This returns the Map.Entry which is then passed through Configurator.initialize(...) down to the Log4jContextFactory.

I used instanceof Map.Entry in the Factory to ensure that when composite configurations are used, the externalContext is correctly unwrapped and set so that WebLookup can find the ServletContext. However, I'm open to moving this logic to Configurator or elsewhere if you feel the Factory should remain agnostic of this wrapper.

@SuppressWarnings("unchecked")
final Map.Entry<String, Object> entry = (Map.Entry<String, Object>) externalContext;
ctx = selector.getContext(fqcn, loader, entry, currentContext, null);
} else {
ctx = selector.getContext(fqcn, loader, currentContext, null);
if (externalContext != null && ctx.getExternalContext() == null) {
ctx.setExternalContext(externalContext);
}
}
if (name != null) {
ctx.setName(name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,26 @@
*/
package org.apache.logging.log4j.web;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import jakarta.servlet.ServletContext;
import org.apache.logging.log4j.core.LoggerContext;
import org.apache.logging.log4j.core.config.Configuration;
import org.apache.logging.log4j.core.impl.ContextAnchor;
import org.apache.logging.log4j.core.lookup.StrSubstitutor;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;

public class WebLookupTest {

@AfterEach
void tearDown() {
ContextAnchor.THREAD_CONTEXT.remove();
}

// TODO: re-enable when https://github.com/spring-projects/spring-framework/issues/25354 is fixed

// @Test
Expand Down Expand Up @@ -94,5 +112,63 @@ public class WebLookupTest {
// initializer.stop();
// ContextAnchor.THREAD_CONTEXT.remove();
// }
/**
* Regression test for GitHub issue #2351:
* "Missing servlet context in web lookup when using composite configuration".
*
* When log4jConfiguration contains a comma-separated list of config files,
* the resulting composite LoggerContext must still expose the ServletContext
* via WebLoggerContextUtils.getServletContext() so that ${web:*} lookups resolve.
*/
@Test
void testCompositeConfigurationServletContextName() throws Exception {
ContextAnchor.THREAD_CONTEXT.remove();

final String expectedServletContextName = "CompositeTest";

// Use Mockito to create a minimal ServletContext (no Spring dependency needed)
final ServletContext servletContext = mock(ServletContext.class);
when(servletContext.getServletContextName()).thenReturn(expectedServletContextName);
when(servletContext.getContextPath()).thenReturn("/composite-test");
// Composite configuration: two comma-separated config files
when(servletContext.getInitParameter(Log4jWebSupport.LOG4J_CONFIG_LOCATION))
.thenReturn("log4j2-combined.xml,log4j2-override.xml");
// Let the initializer resolve each file via the servlet context resource lookup
when(servletContext.getResource("log4j2-combined.xml"))
.thenReturn(getClass().getResource("/log4j2-combined.xml"));
when(servletContext.getResource("log4j2-override.xml"))
.thenReturn(getClass().getResource("/log4j2-override.xml"));

final Log4jWebLifeCycle initializer = WebLoggerContextUtils.getWebLifeCycle(servletContext);
try {
initializer.start();
initializer.setLoggerContext();

final LoggerContext ctx = ContextAnchor.THREAD_CONTEXT.get();
assertNotNull(ctx, "No LoggerContext");

// The servlet context MUST be reachable via the web lookup for composite config.
// Before the fix this returns null, breaking all ${web:*} lookups.
assertNotNull(
WebLoggerContextUtils.getServletContext(),
"ServletContext is null in composite configuration - "
+ "${web:*} lookups will not resolve (issue #2351)");

final Configuration config = ctx.getConfiguration();
assertNotNull(config, "No Configuration");

final StrSubstitutor substitutor = config.getStrSubstitutor();
assertNotNull(substitutor, "No StrSubstitutor");

// Core assertion: ${web:servletContextName} must resolve to the actual name
final String value = substitutor.replace("${web:servletContextName}");
assertEquals(
expectedServletContextName,
value,
"${web:servletContextName} did not resolve in composite configuration (issue #2351)");
} finally {
initializer.stop();
ContextAnchor.THREAD_CONTEXT.remove();
}
}
}