Skip to content

Commit d460915

Browse files
committed
MessageHeaderAccessor handle self-copy correctly
1. Revert changes in setHeader from 5.2.9 that caused regression on self-copy. 2. Update copyHeaders to ensure it still gets a copy of native headers. 3. Exit if source and target are the same instance, as an optimization. Closes gh-26155
1 parent 42216b7 commit d460915

File tree

3 files changed

+62
-34
lines changed

3 files changed

+62
-34
lines changed

spring-messaging/src/main/java/org/springframework/messaging/support/MessageHeaderAccessor.java

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -378,27 +378,29 @@ private List<String> getMatchingHeaderNames(String pattern, @Nullable Map<String
378378
* {@link #copyHeadersIfAbsent(Map)} to avoid overwriting values.
379379
*/
380380
public void copyHeaders(@Nullable Map<String, ?> headersToCopy) {
381-
if (headersToCopy != null) {
382-
headersToCopy.forEach((key, value) -> {
383-
if (!isReadOnly(key)) {
384-
setHeader(key, value);
385-
}
386-
});
381+
if (headersToCopy == null || this.headers == headersToCopy) {
382+
return;
387383
}
384+
headersToCopy.forEach((key, value) -> {
385+
if (!isReadOnly(key)) {
386+
setHeader(key, value);
387+
}
388+
});
388389
}
389390

390391
/**
391392
* Copy the name-value pairs from the provided Map.
392393
* <p>This operation will <em>not</em> overwrite any existing values.
393394
*/
394395
public void copyHeadersIfAbsent(@Nullable Map<String, ?> headersToCopy) {
395-
if (headersToCopy != null) {
396-
headersToCopy.forEach((key, value) -> {
397-
if (!isReadOnly(key)) {
398-
setHeaderIfAbsent(key, value);
399-
}
400-
});
396+
if (headersToCopy == null || this.headers == headersToCopy) {
397+
return;
401398
}
399+
headersToCopy.forEach((key, value) -> {
400+
if (!isReadOnly(key)) {
401+
setHeaderIfAbsent(key, value);
402+
}
403+
});
402404
}
403405

404406
protected boolean isReadOnly(String headerName) {

spring-messaging/src/main/java/org/springframework/messaging/support/NativeMessageHeaderAccessor.java

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ protected NativeMessageHeaderAccessor(@Nullable Message<?> message) {
7575
@SuppressWarnings("unchecked")
7676
Map<String, List<String>> map = (Map<String, List<String>>) getHeader(NATIVE_HEADERS);
7777
if (map != null) {
78+
// setHeader checks for equality but we need copy of native headers
79+
setHeader(NATIVE_HEADERS, null);
7880
setHeader(NATIVE_HEADERS, new LinkedMultiValueMap<>(map));
7981
}
8082
}
@@ -103,38 +105,28 @@ public void setImmutable() {
103105
if (isMutable()) {
104106
Map<String, List<String>> map = getNativeHeaders();
105107
if (map != null) {
108+
// setHeader checks for equality but we need immutable wrapper
109+
setHeader(NATIVE_HEADERS, null);
106110
setHeader(NATIVE_HEADERS, Collections.unmodifiableMap(map));
107111
}
108112
super.setImmutable();
109113
}
110114
}
111115

112116
@Override
113-
public void setHeader(String name, @Nullable Object value) {
114-
if (name.equalsIgnoreCase(NATIVE_HEADERS)) {
115-
// Force removal since setHeader checks for equality
116-
super.setHeader(NATIVE_HEADERS, null);
117+
public void copyHeaders(@Nullable Map<String, ?> headersToCopy) {
118+
if (headersToCopy == null) {
119+
return;
117120
}
118-
super.setHeader(name, value);
119-
}
120121

121-
@Override
122-
@SuppressWarnings("unchecked")
123-
public void copyHeaders(@Nullable Map<String, ?> headersToCopy) {
124-
if (headersToCopy != null) {
125-
Map<String, List<String>> nativeHeaders = getNativeHeaders();
126-
Map<String, List<String>> map = (Map<String, List<String>>) headersToCopy.get(NATIVE_HEADERS);
127-
if (map != null) {
128-
if (nativeHeaders != null) {
129-
nativeHeaders.putAll(map);
130-
}
131-
else {
132-
nativeHeaders = new LinkedMultiValueMap<>(map);
133-
}
134-
}
135-
super.copyHeaders(headersToCopy);
136-
setHeader(NATIVE_HEADERS, nativeHeaders);
122+
@SuppressWarnings("unchecked")
123+
Map<String, List<String>> map = (Map<String, List<String>>) headersToCopy.get(NATIVE_HEADERS);
124+
if (map != null && map != getNativeHeaders()) {
125+
map.forEach(this::setNativeHeaderValues);
137126
}
127+
128+
// setHeader checks for equality, native headers should be equal by now
129+
super.copyHeaders(headersToCopy);
138130
}
139131

140132
/**
@@ -201,6 +193,30 @@ public void setNativeHeader(String name, @Nullable String value) {
201193
}
202194
}
203195

196+
/**
197+
* Variant of {@link #addNativeHeader(String, String)} for all values.
198+
* @since 5.2.12
199+
*/
200+
public void setNativeHeaderValues(String name, @Nullable List<String> values) {
201+
Assert.state(isMutable(), "Already immutable");
202+
Map<String, List<String>> map = getNativeHeaders();
203+
if (values == null) {
204+
if (map != null && map.get(name) != null) {
205+
setModified(true);
206+
map.remove(name);
207+
}
208+
return;
209+
}
210+
if (map == null) {
211+
map = new LinkedMultiValueMap<>(3);
212+
setHeader(NATIVE_HEADERS, map);
213+
}
214+
if (!ObjectUtils.nullSafeEquals(values, getHeader(name))) {
215+
setModified(true);
216+
map.put(name, new ArrayList<>(values));
217+
}
218+
}
219+
204220
/**
205221
* Add the specified native header value to existing values.
206222
* <p>In order for this to work, the accessor must be {@link #isMutable()

spring-messaging/src/test/java/org/springframework/messaging/support/NativeMessageHeaderAccessorTests.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,4 +241,14 @@ void copyImmutableToMutable() {
241241
assertThat(((NativeMessageHeaderAccessor) accessor).getNativeHeader("foo")).containsExactly("bar", "baz");
242242
}
243243

244+
@Test // gh-26155
245+
void copySelf() {
246+
NativeMessageHeaderAccessor accessor = new NativeMessageHeaderAccessor();
247+
accessor.addNativeHeader("foo", "bar");
248+
accessor.setHeader("otherHeader", "otherHeaderValue");
249+
accessor.setLeaveMutable(true);
250+
251+
// Does not fail with ConcurrentModificationException
252+
accessor.copyHeaders(accessor.getMessageHeaders());
253+
}
244254
}

0 commit comments

Comments
 (0)