Fix traceId discrepancy in case error in servlet web

[nkonev]

Fixes https://github.com/spring-projects/spring-security/issues/12610

---

As I see from my research, the bug happens only in the real servlet environment, supposedly because of HttpServletResponse.sendError(), I didn't manage to reproduce it via mockMvc. Tomcat creates a new HttpServletRequest in case error, which is related to "/error" path, which will be served by ErrorController. Fortunately, request attributes are copied from the original request to the "/error"-related one, so we can put the ObservationView into the original request in order to extract this ObservationView from "/error"-related request and create a new parent observation with a given parent ObservationView. The last ObservationView propagates TraceContext to the new Observation so we have the same traceId.

---

Reproducer is here https://github.com/nkonev/trace-discrepancy-reproducer

curl -i 'http://localhost:8060/internal/profile/auth' -X GET -H 'Accept: text/plain, */*'

To check that the bug is fixed, you can run publishMavenJavaPublicationToMavenLocal in spring-security:web

./gradlew :spring-security-web:publishMavenJavaPublicationToMavenLocal --offline

or via IDE

then switch to the SNAPSHOT dependency https://github.com/nkonev/trace-discrepancy-reproducer/commit/131a271130726f8332db19d1e286f1debae18ff2

Before:

After:

---

UPD: I also checked this PR with jetty and undertow, it works fine.

[nkonev]

PTAL @jzheaux

[nkonev]

@jzheaux When you have a chance, I’d appreciate your review or thoughts.

[nkonev]

@marcusdacoregio @jzheaux please take a look when you have a time

[nkonev]

Thank you for review @jzheaux !

That may mean dispatching to ERROR manually in the test.

Can you please explain it a bit more ? I still don't understand for what I need to write an assertion ?

May be somewhere there is a similar test which I can copy and modify for this particular issue ?

Did you mean to use MockMvc ?

---

In the time when I wrote this PR I considered two options - to make the test in itest/web/src/integration-test/java/org/springframework/security/integration folder or in web/src/test/java/org/springframework/security/web/server/ObservationWebFilterChainDecoratorTests.java file. I didn't manage to decide which is better for this particular bug.

I considered to make the following test: Create request 1 then somehow fail it then manually copy attributes to the request 2 (/error-related) -- to simulate servlet container then assert the tracing contexts has the similar traceId.

I thought that approach like this is too complex, too artificial and it seems I would tested the mock's behavior.

Do you mean to make the test like this ? Or maybe something different ?

[nkonev]

Thank you for review @jzheaux !

UPD: I managed to make the test in web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java looking similar to other tests in this file.

[nkonev]

Hi, just noticed that this PR was moved to the next milestone.

Just want to ensure that all is OK and it won't be abandoned.

@jzheaux @rwinch

P. S. sorry for bothering if my concerns are wrong

[rwinch]

It's still planned. It's just that our releases are time bound and we didn't have time to review and merge so we pushed it back

[nkonev]

@jzheaux Let's do another attempt to merge this PR =)

I'm ready to answer questions and to make necessary edits if need.

[jzheaux]

Thanks for your patience, @nkonev. This is now merged into 6.4.x, 6.5.x, and main.