Skip to content

Commit 2c9ed46

Browse files
rstoyanchevrstoyanchev
committed
Improve RequestAttributesThreadLocalAccessor
Ensure access to request attributes after initial REQUEST dispatch is done, and the RequestAttributes markedCompleted. Closes gh-32296
1 parent 3ada9a0 commit 2c9ed46

File tree

2 files changed

+143
-1
lines changed

2 files changed

+143
-1
lines changed

‎spring-web/src/main/java/org/springframework/web/context/request/RequestAttributesThreadLocalAccessor.java

Lines changed: 89 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,12 @@
1616

1717
package org.springframework.web.context.request;
1818

19+
import java.util.Enumeration;
20+
import java.util.HashMap;
21+
import java.util.Map;
22+
1923
import io.micrometer.context.ThreadLocalAccessor;
24+
import jakarta.servlet.http.HttpServletRequest;
2025

2126
import org.springframework.lang.Nullable;
2227

@@ -26,6 +31,7 @@
2631
* {@link RequestAttributes} propagation.
2732
*
2833
* @author Tadaya Tsuyukubo
34+
* @author Rossen Stoyanchev
2935
* @since 6.2
3036
*/
3137
public class RequestAttributesThreadLocalAccessor implements ThreadLocalAccessor<RequestAttributes> {
@@ -44,7 +50,11 @@ public Object key() {
4450
@Override
4551
@Nullable
4652
public RequestAttributes getValue() {
47-
return RequestContextHolder.getRequestAttributes();
53+
RequestAttributes request = RequestContextHolder.getRequestAttributes();
54+
if (request instanceof ServletRequestAttributes sra && !(sra instanceof SnapshotServletRequestAttributes)) {
55+
request = new SnapshotServletRequestAttributes(sra);
56+
}
57+
return request;
4858
}
4959

5060
@Override
@@ -57,4 +67,82 @@ public void setValue() {
5767
RequestContextHolder.resetRequestAttributes();
5868
}
5969

70+
71+
/**
72+
* ServletRequestAttributes that takes another instance, and makes a copy of the
73+
* request attributes at present to provides extended read access during async
74+
* handling when the DispatcherServlet has exited from the initial REQUEST dispatch
75+
* and marked the request {@link ServletRequestAttributes#requestCompleted()}.
76+
* <p>Note that beyond access to request attributes, here is no attempt to support
77+
* setting or removing request attributes, nor to access session attributes after
78+
* the initial REQUEST dispatch has exited.
79+
*/
80+
private static final class SnapshotServletRequestAttributes extends ServletRequestAttributes {
81+
82+
private final ServletRequestAttributes delegate;
83+
84+
private final Map<String, Object> attributeMap;
85+
86+
public SnapshotServletRequestAttributes(ServletRequestAttributes requestAttributes) {
87+
super(requestAttributes.getRequest(), requestAttributes.getResponse());
88+
this.delegate = requestAttributes;
89+
this.attributeMap = getAttributes(requestAttributes.getRequest());
90+
}
91+
92+
private static Map<String, Object> getAttributes(HttpServletRequest request) {
93+
Map<String, Object> map = new HashMap<>();
94+
Enumeration<String> names = request.getAttributeNames();
95+
while (names.hasMoreElements()) {
96+
String name = names.nextElement();
97+
map.put(name, request.getAttribute(name));
98+
}
99+
return map;
100+
}
101+
102+
// Delegate methods that check isRequestActive()
103+
104+
@Nullable
105+
@Override
106+
public Object getAttribute(String name, int scope) {
107+
if (scope == RequestAttributes.SCOPE_REQUEST && !this.delegate.isRequestActive()) {
108+
return this.attributeMap.get(name);
109+
}
110+
try {
111+
return this.delegate.getAttribute(name, scope);
112+
}
113+
catch (IllegalStateException ex) {
114+
if (scope == RequestAttributes.SCOPE_REQUEST) {
115+
return this.attributeMap.get(name);
116+
}
117+
throw ex;
118+
}
119+
}
120+
121+
@Override
122+
public String[] getAttributeNames(int scope) {
123+
if (scope == RequestAttributes.SCOPE_REQUEST && !this.delegate.isRequestActive()) {
124+
return this.attributeMap.keySet().toArray(new String[0]);
125+
}
126+
try {
127+
return this.delegate.getAttributeNames(scope);
128+
}
129+
catch (IllegalStateException ex) {
130+
if (scope == RequestAttributes.SCOPE_REQUEST) {
131+
return this.attributeMap.keySet().toArray(new String[0]);
132+
}
133+
throw ex;
134+
}
135+
}
136+
137+
@Override
138+
public void setAttribute(String name, Object value, int scope) {
139+
this.delegate.setAttribute(name, value, scope);
140+
}
141+
142+
@Override
143+
public void removeAttribute(String name, int scope) {
144+
this.delegate.removeAttribute(name, scope);
145+
}
146+
}
147+
60148
}

‎spring-web/src/test/java/org/springframework/web/context/request/RequestAttributesThreadLocalAccessorTests.java

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,18 @@
2323
import io.micrometer.context.ContextSnapshot;
2424
import io.micrometer.context.ContextSnapshot.Scope;
2525
import io.micrometer.context.ContextSnapshotFactory;
26+
import org.junit.jupiter.api.Test;
2627
import org.junit.jupiter.params.ParameterizedTest;
2728
import org.junit.jupiter.params.provider.Arguments;
2829
import org.junit.jupiter.params.provider.MethodSource;
2930

31+
import org.springframework.web.testfixture.servlet.MockHttpServletRequest;
32+
import org.springframework.web.testfixture.servlet.MockHttpServletResponse;
33+
3034
import static org.assertj.core.api.Assertions.assertThat;
35+
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
3136
import static org.mockito.Mockito.mock;
37+
import static org.springframework.web.context.request.RequestAttributes.SCOPE_REQUEST;
3238

3339
/**
3440
* Tests for {@link RequestAttributesThreadLocalAccessor}.
@@ -73,6 +79,54 @@ void propagation(RequestAttributes previousRequest, RequestAttributes currentReq
7379
assertThat(requestAfterScope).hasValueSatisfying(value -> assertThat(value).isSameAs(previousRequest));
7480
}
7581

82+
@Test
83+
void accessAfterRequestMarkedCompleted() {
84+
MockHttpServletRequest servletRequest = new MockHttpServletRequest();
85+
servletRequest.setAttribute("k1", "v1");
86+
servletRequest.setAttribute("k2", "v2");
87+
88+
ServletRequestAttributes attributes = new ServletRequestAttributes(servletRequest, new MockHttpServletResponse());
89+
ContextSnapshot snapshot = getSnapshotFor(attributes);
90+
attributes.requestCompleted(); // REQUEST dispatch ends, async handling continues
91+
92+
try (Scope scope = snapshot.setThreadLocals()) {
93+
RequestAttributes current = RequestContextHolder.getRequestAttributes();
94+
assertThat(current).isNotNull();
95+
assertThat(current.getAttributeNames(SCOPE_REQUEST)).containsExactly("k1", "k2");
96+
assertThat(current.getAttribute("k1", SCOPE_REQUEST)).isEqualTo("v1");
97+
assertThat(current.getAttribute("k2", SCOPE_REQUEST)).isEqualTo("v2");
98+
assertThatIllegalStateException().isThrownBy(() -> current.setAttribute("k3", "v3", SCOPE_REQUEST));
99+
}
100+
}
101+
102+
@Test
103+
void accessBeforeRequestMarkedCompleted() {
104+
105+
MockHttpServletRequest servletRequest = new MockHttpServletRequest();
106+
ServletRequestAttributes previous = new ServletRequestAttributes(servletRequest, new MockHttpServletResponse());
107+
108+
ContextSnapshot snapshot = getSnapshotFor(previous);
109+
110+
RequestContextHolder.setRequestAttributes(previous);
111+
try {
112+
try (Scope scope = snapshot.setThreadLocals()) {
113+
RequestAttributes attributes = RequestContextHolder.getRequestAttributes();
114+
assertThat(attributes).isNotNull();
115+
attributes.setAttribute("k1", "v1", SCOPE_REQUEST);
116+
}
117+
RequestAttributes attributes = RequestContextHolder.getRequestAttributes();
118+
assertThat(attributes).isNotNull();
119+
attributes.setAttribute("k2", "v2", SCOPE_REQUEST);
120+
}
121+
finally {
122+
RequestContextHolder.resetRequestAttributes();
123+
}
124+
125+
assertThat(previous.getAttributeNames(SCOPE_REQUEST)).containsExactly("k1", "k2");
126+
assertThat(previous.getAttribute("k1", SCOPE_REQUEST)).isEqualTo("v1");
127+
assertThat(previous.getAttribute("k2", SCOPE_REQUEST)).isEqualTo("v2");
128+
}
129+
76130
private ContextSnapshot getSnapshotFor(RequestAttributes request) {
77131
RequestContextHolder.setRequestAttributes(request);
78132
try {

0 commit comments

Comments
 (0)