Skip to content

Commit fe83085

Browse files
vyvy
authored
Merge branch '2.x' into feature/2.x/enumerating-and-exposing-named-patters
2 parents 4ce71a1 + 8ec5703 commit fe83085

File tree

24 files changed

+426
-50
lines changed

24 files changed

+426
-50
lines changed

‎log4j-api/src/main/resources/META-INF/native-image/org.apache.logging.log4j/log4j-api/resource-config.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@
66
}
77
]
88
}
9-
}
9+
}

‎log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/processor/GraalVmProcessorTest.java‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,18 @@ class GraalVmProcessorTest {
103103
private static final Object FAKE_CONSTRAINT_VALIDATOR = onlyNoArgsConstructor(FAKE_CONSTRAINT_VALIDATOR_NAME);
104104
private static final String FAKE_PLUGIN_VISITOR_NAME = "example.FakeAnnotations$FakePluginVisitor";
105105
private static final Object FAKE_PLUGIN_VISITOR = onlyNoArgsConstructor(FAKE_PLUGIN_VISITOR_NAME);
106+
private static final String FAKE_CONVERTER_NAME = "example.FakeConverter";
107+
private static final Object FAKE_CONVERTER = asMap(
108+
"name",
109+
FAKE_CONVERTER_NAME,
110+
"methods",
111+
singletonList(asMap(
112+
"name",
113+
"newInstance",
114+
"parameterTypes",
115+
asList("org.apache.logging.log4j.core.config.Configuration", "java.lang.String[]"))),
116+
"fields",
117+
emptyList());
106118

107119
private static final String GROUP_ID = "groupId";
108120
private static final String ARTIFACT_ID = "artifactId";
@@ -155,7 +167,8 @@ static Stream<Arguments> containsSpecificEntries() {
155167
Arguments.of(FAKE_PLUGIN_BUILDER_NAME, FAKE_PLUGIN_BUILDER),
156168
Arguments.of(FAKE_PLUGIN_NESTED_NAME, FAKE_PLUGIN_NESTED),
157169
Arguments.of(FAKE_CONSTRAINT_VALIDATOR_NAME, FAKE_CONSTRAINT_VALIDATOR),
158-
Arguments.of(FAKE_PLUGIN_VISITOR_NAME, FAKE_PLUGIN_VISITOR));
170+
Arguments.of(FAKE_PLUGIN_VISITOR_NAME, FAKE_PLUGIN_VISITOR),
171+
Arguments.of(FAKE_CONVERTER_NAME, FAKE_CONVERTER));
159172
}
160173

161174
@ParameterizedTest
@@ -168,7 +181,9 @@ void containsSpecificEntries(String className, Object expectedJson) throws IOExc
168181
assertThatJson(reachabilityMetadata)
169182
.inPath(String.format("$[?(@.name == '%s')]", className))
170183
.isArray()
171-
.contains(json(expectedJson));
184+
.hasSize(1)
185+
.first()
186+
.isEqualTo(json(expectedJson));
172187
}
173188

174189
static Stream<Arguments> reachabilityMetadataPath() {
@@ -214,7 +229,7 @@ void whenNoGroupIdAndArtifactId_thenWarningIsPrinted(@TempDir(cleanup = CleanupM
214229
}
215230
// The generated folder name should be deterministic and based solely on the descriptor content.
216231
// If the descriptor changes, this test and the expected folder name must be updated accordingly.
217-
assertThat(reachabilityMetadataFolders).hasSize(1).containsExactly(path.resolve("62162090"));
232+
assertThat(reachabilityMetadataFolders).hasSize(1).containsExactly(path.resolve("72c240aa"));
218233
assertThat(reachabilityMetadataFolders.get(0).resolve("reflect-config.json"))
219234
.as("Reachability metadata file")
220235
.exists();
@@ -250,7 +265,6 @@ private static List<String> generateDescriptor(
250265
}
251266

252267
// Compile the sources
253-
final Path descriptorFilePath = outputDir.resolve("plugins.xml");
254268
final DiagnosticCollector<JavaFileObject> diagnosticCollector = new DiagnosticCollector<>();
255269
final JavaCompiler.CompilationTask task =
256270
compiler.getTask(null, fileManager, diagnosticCollector, options, null, sources);
@@ -260,6 +274,8 @@ private static List<String> generateDescriptor(
260274
return diagnosticCollector.getDiagnostics().stream()
261275
.filter(d -> d.getKind() != Diagnostic.Kind.NOTE)
262276
.map(d -> d.getMessage(Locale.ROOT))
277+
// This message appears when the test runs on JDK 8
278+
.filter(m -> !"unknown enum constant java.lang.annotation.ElementType.MODULE".equals(m))
263279
.collect(Collectors.toList());
264280
}
265281
}

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@
1616
*/
1717
package org.apache.logging.log4j.core.util.internal.instant;
1818

19+
import static java.util.concurrent.Executors.newSingleThreadExecutor;
1920
import static org.assertj.core.api.Assertions.assertThat;
2021
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2122
import static org.mockito.ArgumentMatchers.any;
2223
import static org.mockito.ArgumentMatchers.eq;
2324
import static org.mockito.Mockito.doAnswer;
2425
import static org.mockito.Mockito.mock;
26+
import static org.mockito.Mockito.times;
2527
import static org.mockito.Mockito.verify;
2628
import static org.mockito.Mockito.verifyNoMoreInteractions;
2729
import static org.mockito.Mockito.when;
@@ -30,13 +32,16 @@
3032
import java.util.Locale;
3133
import java.util.Random;
3234
import java.util.TimeZone;
35+
import java.util.concurrent.ExecutionException;
36+
import java.util.concurrent.ExecutorService;
3337
import java.util.function.Function;
3438
import org.apache.logging.log4j.core.time.Instant;
3539
import org.apache.logging.log4j.core.time.MutableInstant;
3640
import org.junit.jupiter.api.Test;
3741
import org.junit.jupiter.params.ParameterizedTest;
3842
import org.junit.jupiter.params.provider.MethodSource;
3943
import org.junit.jupiter.params.provider.ValueSource;
44+
import org.junitpioneer.jupiter.Issue;
4045

4146
class InstantPatternThreadLocalCachedFormatterTest {
4247

@@ -289,4 +294,53 @@ private static MutableInstant createInstant(final long epochMillis, final int ep
289294
instant.initFromEpochMilli(epochMillis, epochMillisNanos);
290295
return instant;
291296
}
297+
298+
@Test
299+
@Issue("https://github.com/apache/logging-log4j2/issues/3792")
300+
void should_be_thread_safe() throws Exception {
301+
// Instead of randomly testing the thread safety, we test that the current implementation does not
302+
// cache results across threads.
303+
//
304+
// Modify this test if the implementation changes in the future.
305+
final InstantPatternFormatter patternFormatter = mock(InstantPatternFormatter.class);
306+
when(patternFormatter.getPrecision()).thenReturn(ChronoUnit.MILLIS);
307+
308+
final Instant instant = INSTANT0;
309+
final String output = "thread-output";
310+
doAnswer(invocation -> {
311+
StringBuilder buffer = invocation.getArgument(0);
312+
buffer.append(output)
313+
.append('-')
314+
.append(Thread.currentThread().getName());
315+
return null;
316+
})
317+
.when(patternFormatter)
318+
.formatTo(any(StringBuilder.class), eq(instant));
319+
320+
final InstantFormatter cachedFormatter =
321+
InstantPatternThreadLocalCachedFormatter.ofMilliPrecision(patternFormatter);
322+
323+
final int threadCount = 2;
324+
for (int i = 0; i < threadCount; i++) {
325+
formatOnNewThread(cachedFormatter, instant, output);
326+
}
327+
verify(patternFormatter, times(threadCount)).formatTo(any(StringBuilder.class), eq(instant));
328+
}
329+
330+
private static void formatOnNewThread(
331+
final InstantFormatter formatter, final Instant instant, final String expectedOutput)
332+
throws ExecutionException, InterruptedException {
333+
ExecutorService executor = newSingleThreadExecutor();
334+
try {
335+
executor.submit(() -> {
336+
String formatted = formatter.format(instant);
337+
assertThat(formatted)
338+
.isEqualTo(expectedOutput + "-"
339+
+ Thread.currentThread().getName());
340+
})
341+
.get();
342+
} finally {
343+
executor.shutdown();
344+
}
345+
}
292346
}

‎log4j-core-test/src/test/resources/GraalVmProcessorTest/java/FakeConverter.java‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package example;
18+
19+
import org.apache.logging.log4j.core.LogEvent;
20+
import org.apache.logging.log4j.core.config.Configuration;
21+
import org.apache.logging.log4j.core.config.plugins.Plugin;
22+
import org.apache.logging.log4j.core.pattern.ConverterKeys;
23+
import org.apache.logging.log4j.core.pattern.LogEventPatternConverter;
24+
import org.apache.logging.log4j.core.pattern.PatternConverter;
25+
import org.jspecify.annotations.NullMarked;
26+
import org.jspecify.annotations.Nullable;
27+
28+
@NullMarked
29+
@Plugin(name = "FakePatternConverter", category = PatternConverter.CATEGORY)
30+
@ConverterKeys({"f", "fake"})
31+
public final class FakeConverter extends LogEventPatternConverter {
32+
33+
private FakeConverter(@Nullable final Configuration config, @Nullable final String[] options) {
34+
super("Fake", "fake");
35+
}
36+
37+
public static FakeConverter newInstance(
38+
@Nullable final Configuration config, @Nullable final String[] options) {
39+
return new FakeConverter(config, options);
40+
}
41+
42+
@Override
43+
public void format(LogEvent event, StringBuilder toAppendTo) {
44+
toAppendTo.append("FakeConverter: ").append(event.getMessage().getFormattedMessage());
45+
}
46+
}

‎log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/processor/GraalVmProcessor.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,9 @@ public String visitArray(final ArrayType t, @Nullable Void unused) {
300300

301301
@Override
302302
public @Nullable String visitDeclared(final DeclaredType t, final Void unused) {
303-
return processingEnv.getTypeUtils().erasure(t).toString();
303+
return safeCast(t.asElement(), TypeElement.class)
304+
.getQualifiedName()
305+
.toString();
304306
}
305307
},
306308
null);

‎log4j-core/src/main/java/org/apache/logging/log4j/core/net/UrlConnectionFactory.java‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.apache.logging.log4j.core.net.ssl.SslConfiguration;
3434
import org.apache.logging.log4j.core.net.ssl.SslConfigurationFactory;
3535
import org.apache.logging.log4j.core.util.AuthorizationProvider;
36+
import org.apache.logging.log4j.core.util.internal.SystemUtils;
3637
import org.apache.logging.log4j.util.PropertiesUtil;
3738
import org.apache.logging.log4j.util.Strings;
3839

@@ -51,7 +52,24 @@ public class UrlConnectionFactory {
5152
private static final String HTTP = "http";
5253
private static final String HTTPS = "https";
5354
private static final String JAR = "jar";
54-
private static final String DEFAULT_ALLOWED_PROTOCOLS = "https, file, jar";
55+
/**
56+
* Default list of protocols that are allowed to be used for configuration files and other trusted resources.
57+
* <p>
58+
* By default, we trust the following protocols:
59+
* <dl>
60+
* <dt>file</dt>
61+
* <dd>Local files</dd>
62+
* <dt>https</dt>
63+
* <dd>Resources retrieved through TLS to guarantee their integrity</dd>
64+
* <dt>jar</dt>
65+
* <dd>Resources retrieved from JAR files</dd>
66+
* <dt>resource</dt>
67+
* <dd>Resources embedded in a GraalVM native image</dd>
68+
* </dl>
69+
*/
70+
private static final String DEFAULT_ALLOWED_PROTOCOLS =
71+
SystemUtils.isGraalVm() ? "file, https, jar, resource" : "file, https, jar";
72+
5573
private static final String NO_PROTOCOLS = "_none";
5674
public static final String ALLOWED_PROTOCOLS = "log4j2.Configuration.allowedProtocols";
5775

‎log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/SystemUtils.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,21 @@ public static boolean isOsAndroid() {
3636
return getJavaVendor().contains("Android");
3737
}
3838

39+
/**
40+
* Checks if the current runtime is GraalVM.
41+
* <p>
42+
* See <a href="https://www.graalvm.org/sdk/javadoc/org/graalvm/nativeimage/ImageInfo.html#PROPERTY_IMAGE_CODE_KEY">ImageInfo.PROPERTY_IMAGE_CODE_KEY</a>.
43+
* </p>
44+
* @return true if the current runtime is GraalVM, false otherwise.
45+
*/
46+
public static boolean isGraalVm() {
47+
try {
48+
return System.getProperty("org.graalvm.nativeimage.imagecode") != null;
49+
} catch (final SecurityException e) {
50+
LOGGER.debug("Unable to determine if the current runtime is GraalVM.", e);
51+
return false;
52+
}
53+
}
54+
3955
private SystemUtils() {}
4056
}

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

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ final class InstantPatternThreadLocalCachedFormatter implements InstantPatternFo
3838
private final ThreadLocal<Object[]> epochInstantAndBufferRef =
3939
ThreadLocal.withInitial(InstantPatternThreadLocalCachedFormatter::createEpochInstantAndBuffer);
4040

41-
private Object[] lastEpochInstantAndBuffer = createEpochInstantAndBuffer();
42-
4341
private static Object[] createEpochInstantAndBuffer() {
4442
return new Object[] {-1L, new StringBuilder()};
4543
}
@@ -89,32 +87,21 @@ public ChronoUnit getPrecision() {
8987
public void formatTo(final StringBuilder buffer, final Instant instant) {
9088
requireNonNull(buffer, "buffer");
9189
requireNonNull(instant, "instant");
92-
final Object[] prevEpochInstantAndBuffer = lastEpochInstantAndBuffer;
93-
final long prevEpochInstant = (long) prevEpochInstantAndBuffer[0];
94-
final StringBuilder prevBuffer = (StringBuilder) prevEpochInstantAndBuffer[1];
90+
final Object[] epochInstantAndBuffer = epochInstantAndBufferRef.get();
91+
final long prevEpochInstant = (long) epochInstantAndBuffer[0];
92+
final StringBuilder localBuffer = (StringBuilder) epochInstantAndBuffer[1];
9593
final long nextEpochInstant = epochInstantExtractor.apply(instant);
96-
if (prevEpochInstant == nextEpochInstant) {
97-
buffer.append(prevBuffer);
98-
} else {
99-
100-
// We could have used `StringBuilders.trimToMaxSize()` on `prevBuffer`.
94+
if (prevEpochInstant != nextEpochInstant) {
95+
// We could have used `StringBuilders.trimToMaxSize()` on `localBuffer`.
10196
// That is, we wouldn't want exploded `StringBuilder`s in hundreds of `ThreadLocal`s.
10297
// Though we are formatting instants and always expect to produce strings of more or less the same length.
10398
// Hence, no need for truncation.
10499

105-
// Populate a new cache entry
106-
final Object[] nextEpochInstantAndBuffer = epochInstantAndBufferRef.get();
107-
nextEpochInstantAndBuffer[0] = nextEpochInstant;
108-
final StringBuilder nextBuffer = (StringBuilder) nextEpochInstantAndBuffer[1];
109-
nextBuffer.setLength(0);
110-
formatter.formatTo(nextBuffer, instant);
111-
112-
// Update the effective cache entry
113-
lastEpochInstantAndBuffer = nextEpochInstantAndBuffer;
114-
115-
// Help out the request
116-
buffer.append(nextBuffer);
100+
epochInstantAndBuffer[0] = nextEpochInstant;
101+
localBuffer.setLength(0);
102+
formatter.formatTo(localBuffer, instant);
117103
}
104+
buffer.append(localBuffer);
118105
}
119106

120107
@Override

0 commit comments

Comments
 (0)