Skip to content

fix(common): strip leading whitespace in ClassPreLoader #2560

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
phipag wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix(common): strip leading whitespace in ClassPreLoader #2560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a58b45d
fix(common): strip leading whitespace in ClassPreLoader
phipag Jul 3, 2026
6f64a17
fix(common): reject malformed classesloaded.txt entries and fix traci…
phipag Jul 3, 2026
1db2df5
fix(common): simplify class name pattern to avoid backtracking
phipag Jul 3, 2026
8cf0c98
refactor(common): drop class name pattern, rely on Class.forName
phipag Jul 3, 2026
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
20 changes: 15 additions & 5 deletions ...ommon/src/main/java/software/amazon/lambda/powertools/common/internal/ClassPreLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,17 @@
import java.net.URLConnection;
import java.util.Enumeration;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Used to preload classes to support automatic priming for SnapStart
*/
public final class ClassPreLoader {
public static final String CLASSES_FILE = "classesloaded.txt";

private static final Logger LOG = LoggerFactory.getLogger(ClassPreLoader.class);

private ClassPreLoader() {
// Hide default constructor
}
Expand All @@ -43,7 +48,7 @@
URL url = files.nextElement();
URLConnection conn = url.openConnection();
conn.setUseCaches(false);
InputStream is = conn.getInputStream();

Check failure on line 51 in powertools-common/src/main/java/software/amazon/lambda/powertools/common/internal/ClassPreLoader.java

View workflow job for this annotation

GitHub Actions / pmd_analyse

Ensure that resources like this InputStream object are closed after use 

Ensure that resources (like `java.sql.Connection`, `java.sql.Statement`, and `java.sql.ResultSet` objects
and any subtype of `java.lang.AutoCloseable`) are always closed after use.
Failing to do so might result in resource leaks.

Note: It suffices to configure the super type, e.g. `java.lang.AutoCloseable`, so that this rule automatically triggers
on any subtype (e.g. `java.io.FileInputStream`). Additionally specifying `java.sql.Connection` helps in detecting
the types, if the type resolution / auxclasspath is not correctly setup.

Note: Since PMD 6.16.0 the default value for the property `types` contains `java.lang.AutoCloseable` and detects
now cases where the standard `java.io.*Stream` classes are involved. In order to restore the old behaviour,
just remove "AutoCloseable" from the types.

The property `allowedResourceMethodPatterns` can be used to specify method invocation patterns that return
resources which are managed externally and don't need to be closed by the caller. This is useful for
servlet-related streams like `HttpServletRequest.getReader()` or `HttpServletResponse.getWriter()`,
which are managed by the servlet container. The patterns use InvocationMatcher syntax
(e.g., `javax.servlet.ServletRequest#getReader()`).

CloseResource (Priority: 1, Ruleset: Error Prone)
https://docs.pmd-code.org/snapshot/pmd_rules_java_errorprone.html#closeresource
preloadClassesFromStream(is);
}
} catch (IOException ignored) {
Expand All @@ -57,6 +62,7 @@
* @param is
*/
private static void preloadClassesFromStream(InputStream is) {
int loaded = 0;
try (is;
InputStreamReader isr = new InputStreamReader(is, StandardCharsets.UTF_8);
BufferedReader reader = new BufferedReader(isr)) {
Expand All @@ -66,26 +72,30 @@
if (idx != -1) {
line = line.substring(0, idx);
}
final String className = line.stripTrailing();
if (!className.isBlank()) {
loadClassIfFound(className);
final String className = line.strip();
if (!className.isBlank() && loadClassIfFound(className)) {
loaded++;
}
}
} catch (Exception ignored) {
// No action is required if preloading fails for any reason
}
LOG.debug("SnapStart priming: preloaded {} class(es) from {}", loaded, CLASSES_FILE);
}

/**
* Initializes the class with given name if found, ignores otherwise
*
* @param className
* @param className the binary name of the class to load
* @return true if the class was found and loaded, false otherwise
*/
private static void loadClassIfFound(String className) {
private static boolean loadClassIfFound(String className) {
try {
Class.forName(className, true, ClassPreLoader.class.getClassLoader());
return true;
} catch (ClassNotFoundException e) {
// No action is required if the class with given name cannot be found
return false;
}
}
}
25 changes: 23 additions & 2 deletions ...n/src/test/java/software/amazon/lambda/powertools/common/internal/ClassPreLoaderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
class ClassPreLoaderTest {

// Making this volatile so the Thread Context doesn't need any special handling
static volatile boolean dummyClassLoaded = false;

Check failure on line 10 in powertools-common/src/test/java/software/amazon/lambda/powertools/common/internal/ClassPreLoaderTest.java

View workflow job for this annotation

GitHub Actions / pmd_analyse

Use of modifier volatile is not recommended. 

Use of the keyword 'volatile' is generally used to fine tune a Java application, and therefore, requires
a good expertise of the Java Memory Model. Moreover, its range of action is somewhat misknown. Therefore,
the volatile keyword should not be used for maintenance purpose and portability.

AvoidUsingVolatile (Priority: 1, Ruleset: Multithreading)
https://docs.pmd-code.org/snapshot/pmd_rules_java_multithreading.html#avoidusingvolatile
static volatile boolean leadingSpaceDummyClassLoaded = false;

Check failure on line 11 in powertools-common/src/test/java/software/amazon/lambda/powertools/common/internal/ClassPreLoaderTest.java

View workflow job for this annotation

GitHub Actions / pmd_analyse

Use of modifier volatile is not recommended. 

Use of the keyword 'volatile' is generally used to fine tune a Java application, and therefore, requires
a good expertise of the Java Memory Model. Moreover, its range of action is somewhat misknown. Therefore,
the volatile keyword should not be used for maintenance purpose and portability.

AvoidUsingVolatile (Priority: 1, Ruleset: Multithreading)
https://docs.pmd-code.org/snapshot/pmd_rules_java_multithreading.html#avoidusingvolatile

/**
* Dummy class to be loaded by ClassPreLoader in test.
Expand All @@ -20,15 +21,35 @@
dummyClassLoaded = true;
}
}

/**
* Dummy class whose name is referenced with a leading space in
* <i>powertools-common/src/test/resources/classesloaded.txt</i>.
* Some modules ship a classesloaded.txt with a leading space on every line, so the loader must
* strip leading whitespace before calling Class.forName.
*/
static class LeadingSpaceDummyClass {
static {
leadingSpaceDummyClassLoaded = true;
}
}

@Test
void preloadClasses_shouldIgnoreInvalidClassesAndLoadValidClasses() {

dummyClassLoaded = false;
// powertools-common/src/test/resources/classesloaded.txt has a class that does not exist
leadingSpaceDummyClassLoaded = false;
// A class only runs its static initializer the first time it is loaded, so this test calls
// preloadClasses once and asserts on all classes listed in classesloaded.txt together.
// powertools-common/src/test/resources/classesloaded.txt has a class that does not exist.
// Verify that the missing class did not throw any exception
assertDoesNotThrow(ClassPreLoader::preloadClasses);

// When the classloaded.txt is a mixed bag of valid and invalid classes, Valid class must load
// When the classloaded.txt is a mixed bag of valid and invalid classes, valid classes must load
assertTrue(dummyClassLoaded, "DummyClass should be loaded");
// classesloaded.txt references LeadingSpaceDummyClass with a leading space. The loader must
// strip it before Class.forName, otherwise the class is never loaded.
assertTrue(leadingSpaceDummyClassLoaded,
"LeadingSpaceDummyClass should be loaded despite the leading space");
}
}
3 changes: 2 additions & 1 deletion powertools-common/src/test/resources/classesloaded.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
software.amazon.lambda.powertools.common.internal.NonExistingClass
software.amazon.lambda.powertools.common.internal.ClassPreLoaderTest$DummyClass
software.amazon.lambda.powertools.common.internal.ClassPreLoaderTest$DummyClass
software.amazon.lambda.powertools.common.internal.ClassPreLoaderTest$LeadingSpaceDummyClass
6 changes: 5 additions & 1 deletion ...lambda/powertools/tracing/handlers/PowerTracerToolEnabledForResponseWithCustomMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@
import software.amazon.lambda.powertools.tracing.TracingUtils;

public class PowerTracerToolEnabledForResponseWithCustomMapper implements RequestHandler<Object, Object> {
static {
public PowerTracerToolEnabledForResponseWithCustomMapper() {
// Set the custom mapper in the constructor rather than a static initializer. A static
// initializer only runs the first time this class is loaded, which makes the resulting
// global state depend on when the class happens to be loaded (for example by SnapStart
// class priming). Setting it in the constructor makes each instantiation deterministic.
ObjectMapper objectMapper = new ObjectMapper();
SimpleModule simpleModule = new SimpleModule();
simpleModule.addSerializer(ChildClass.class, new ChildSerializer());
Expand Down
5 changes: 5 additions & 0 deletions ...test/java/software/amazon/lambda/powertools/tracing/internal/LambdaTracingAspectTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import software.amazon.lambda.powertools.tracing.handlers.PowerTracerToolEnabledExplicitlyForResponseAndError;
import software.amazon.lambda.powertools.tracing.handlers.PowerTracerToolEnabledForError;
import software.amazon.lambda.powertools.tracing.handlers.PowerTracerToolEnabledForResponse;
import software.amazon.lambda.powertools.tracing.TracingUtils;
import software.amazon.lambda.powertools.tracing.handlers.PowerTracerToolEnabledForResponseWithCustomMapper;
import software.amazon.lambda.powertools.tracing.handlers.PowerTracerToolEnabledForStream;
import software.amazon.lambda.powertools.tracing.handlers.PowerTracerToolEnabledForStreamWithNoMetaData;
Expand Down Expand Up @@ -70,6 +71,10 @@ void setUp() throws IllegalAccessException {
@AfterEach
void tearDown() {
AWSXRay.endSegment();
// Some tests (e.g. the custom mapper handler) set the static TracingUtils.objectMapper.
// Reset it so it does not leak into later tests running in the same JVM fork and change how
// responses and errors are serialized.
TracingUtils.defaultObjectMapper(null);
}

@Test
Expand Down
Loading