fix(boot): handle Spring Session non-public request wrappers#731
Open
vlsi wants to merge 6 commits into
Open
Conversation
Spring Session wraps every incoming request in SessionRepositoryFilter$SessionRepositoryRequestWrapper, a private final inner class. Class.getMethod returns a Method whose declaringClass is that wrapper, and Method.invoke performs a language access check on the declaring class modifiers -- even when the method itself is public -- producing IllegalAccessException. The same reflection pattern lives in three sibling adapters (ServletRequestAdapter, HttpSessionAdapter, CookieAdapter), so they get the same treatment. CookieAdapter additionally switches from getDeclaredMethod to getMethod to look up inherited public accessors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…legalAccessException
The agent previously had no Spring Boot test coverage -- Maven samples
under backend/examples/ are deployment demos, not test fixtures. This
adds a new Gradle module under sample-apps/ that boots a tiny Spring
Boot 3 + Spring Session application and exercises
HttpServletRequestAdapter against the real
SessionRepositoryRequestWrapper. Reverting the previous commit makes
this test reproduce the production stacktrace verbatim.
Infrastructure changes that come with this:
- build-logic.java-17-library convention bumps toolchain and bytecode
target to 17 (Spring Boot 3 requires Java 17)
- sample-apps/ joins :plugins as a directory excluded from the global
buildscript constraints block (no parent build.gradle.kts)
- kotlin("plugin.spring") added to pluginManagement so @configuration
classes can be subclassed by Spring's CGLIB proxies
- renovate.json pins this module to Spring Boot 3.x -- a major bump
triggers a new it-spring-boot-4 sibling instead
- gradle-dependency-submit excludes :sample-apps:.* alongside
:plugins:.* so sample apps don't generate security alerts for the
agent project itself
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
testImplementation extends implementation, so the platform is inherited automatically -- no need to bind it to a local val or repeat it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…trumentation on Undertow The previous iteration of this module reproduced the contract by calling HttpServletRequestAdapter directly from the controller. That worked but left the agent itself untested -- the bytecode-injected reflection path in production was never actually exercised. This rewires the test to follow the exact production stacktrace: - Switch the embedded server to Undertow (spring-boot-starter-undertow, excluding spring-boot-starter-tomcat). Tomcat's Tomcat10HTTPEnhancer only sees org.apache.catalina.connector.Request at the valve level -- never the Spring Session wrapper, which lives inside the filter chain. Undertow23HTTPEnhancer instead targets io.undertow.servlet.handlers.FilterHandler$FilterChainImpl.doFilter with execute-before='fillNcUser$profiler(p1)' -- fired once per filter step, so it picks up the wrapper that SessionRepositoryFilter just installed. - Drop the manual HttpServletRequestAdapter call from the controller. The /probe endpoint is now a plain Spring MVC handler. - Attach the agent as a real -javaagent on the Gradle test JVM. The installer zip from :installer is extracted into build/profiler-home and the path is wired through jvmArgumentProviders (same pattern as installer-zip-test). projects.boot moves to testCompileOnly: at runtime boot.jar is loaded by the bootstrap classloader via the agent jar. - The test swaps ProfilerData.pluginLogger with a capturing delegate before the request and asserts no IllegalAccessException was swallowed. The contract is: TomcatHTTPEnhancer/Undertow23HTTPEnhancer fillNcUser$profiler must not raise IllegalAccessException when HttpServletLogUtils.fillNcUser constructs HttpServletRequestAdapter against a non-public servlet wrapper. Reverting setAccessible(true) in HttpServletRequestAdapter makes the test fail with three captured IllegalAccessExceptions whose messages match the production stacktrace verbatim. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…the Maven demo
Reuse backend/examples/spring-boot-3-undertow as the sample-app: same
source code that ships as a deployable demo doubles as the integration
test fixture. The Gradle Exec task invokes mvn package on the demo;
Testcontainers builds a throwaway eclipse-temurin:17-jre image bundling
the demo jar plus the extracted profiler installer, and starts the
container with -javaagent attached via JAVA_TOOL_OPTIONS. The test hits
/health and verifies that the agent's Undertow23 enhancer does not
swallow an IllegalAccessException while constructing
HttpServletRequestAdapter against SessionRepositoryRequestWrapper.
Companion changes:
- backend/examples/spring-boot-3-undertow:
- Bump spring-boot-starter-parent 3.1.2 -> 3.5.0 so Spring Session's
SessionRepositoryFilter is observed in the chain (in 3.1 the
@EnableSpringHttpSession bean was not auto-registered as a servlet
filter; bumping fixed that path).
- Add spring-session-core dependency and SessionConfig with an
explicit FilterRegistrationBean so the wrapper materialises
deterministically regardless of which Spring Boot autoconfig
bridges Spring Session in any given minor.
- boot/.../Profiler.java: add ECHO_PLUGIN_EXCEPTION_TO_STDERR_PROPERTY
("com.netcracker.profiler.agent.echoPluginExceptionToStderr"), a
gated, default-off fail-loud diagnostic that mirrors every swallowed
plugin exception to System.err. The slf4j/Logback binding for
ProfilerPluginLoggerImpl is unreliable in containers where the agent
loads Logback early (bootstrap classloader) and Spring Boot
reconfigures Logback later (app classloader) — the two LoggerContexts
are not guaranteed to share appender routing. This property gives the
integration test (and field operators) a classloader-independent
channel; isolating the agent's Logback context is left to a follow-up
task.
- sample-apps/it-spring-boot-3:
- Drop the previous Kotlin TestApp source set; the demo provides the
application now.
- Replace ProcessBuilder forking with Testcontainers
(org.testcontainers:testcontainers-bom:1.21.0). Force docker-java's
advertised API version to 1.43 to satisfy modern Docker daemons
(OrbStack / Docker Desktop >= 25 reject the bundled 1.32 default).
- The container's JAVA_TOOL_OPTIONS sets
-Dcom.netcracker.profiler.agent.echoPluginExceptionToStderr=true so
the test reads container stderr via withLogConsumer for the
assertion.
Reverting setAccessible(true) on getSession in HttpServletRequestAdapter
makes the new test fail with three captured IllegalAccessException
entries, matching the production stacktrace.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…er-latest The previous Testcontainers iteration built a throwaway eclipse-temurin image and copied the unpacked profiler installer into it. That worked but duplicated work the :installer:buildBaseImage task already does and diverged from the production runtime layout. Switch to FROM qubership/qubership-core-base-image:profiler-latest — the same image shipped to customers — and just COPY the demo jar on top. The base image already has /app/diag wired up; setting NC_DIAGNOSTIC_MODE=prod makes its entrypoint set JAVA_TOOL_OPTIONS=-javaagent:/app/diag/lib/agent.jar ... before exec'ing the user CMD, so the agent attaches transparently. Companion changes: - Drop the local extractInstaller Sync task (the installer is now baked into the base image, not copied per test). - Add a task dependency on :installer:buildBaseImage so the local image is rebuilt with the current agent jar when this test runs. - Replace test.profilerHomeDir system property with test.baseImageTag. - The fail-loud System.err diagnostic (-Dcom.netcracker.profiler.agent.echoPluginExceptionToStderr=true) is now passed via the CMD line (still picked up by the JVM together with the entrypoint-injected JAVA_TOOL_OPTIONS). Reverting setAccessible(true) on getSession again makes the test fail with 12 captured lines (Profiler plugin exception + first stack frame for each of the 3 instrumented filter steps). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
IllegalAccessExceptioninHttpServletRequestAdapter(and 3 sibling adapters) against Spring Session'sSessionRepositoryFilter$SessionRepositoryRequestWrapper— aprivate finalinner class whose declaring-class modifiers failMethod.invokeaccess checks even though the methods themselves are public.sample-apps/it-spring-boot-3— a new Gradle module that boots a tiny Spring Boot 3 + Spring Session app and reproduces the production stacktrace verbatim. Reverting the boot/ change makes the test fail with the sameIllegalAccessException at HttpServletRequestAdapter.java:30.build-logic.java-17-libraryconvention;:sample-apps:*joins:plugins:*in the dependency-submission exclude list; renovate pins this module to Spring Boot 3.x (a major bump triggers a newit-spring-boot-4sibling).Test plan
./gradlew --quiet :sample-apps:it-spring-boot-3:test— fails before the fix, passes after./gradlew --quiet :boot:test— passes (no regressions)🤖 Generated with Claude Code