Ensure Session#getAttributeNames implementations return a copy

Currently, Session#getAttributeNames implementations, by delegating to MapSession, all return a session attribute map's key set. This causes ConcurrentModificationException when an attempt to modify session attributes is made while iterating over the returned attribute names.

Closes gh-1129
This commit is contained in:
Vedran Pavic
2018-07-17 15:05:03 +02:00
parent d0887fe40d
commit b778d97dc7
6 changed files with 70 additions and 4 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2016 the original author or authors.
* Copyright 2014-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@ package org.springframework.session;
import java.io.Serializable;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
@@ -141,7 +142,7 @@ public final class MapSession implements ExpiringSession, Serializable {
}
public Set<String> getAttributeNames() {
return this.sessionAttrs.keySet();
return new HashSet<String>(this.sessionAttrs.keySet());
}
public void setAttribute(String attributeName, Object attributeValue) {

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2016 the original author or authors.
* Copyright 2014-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -64,4 +64,18 @@ public class MapSessionRepositoryTests {
assertThat(session.getMaxInactiveIntervalInSeconds())
.isEqualTo(expectedMaxInterval);
}
@Test // gh-1129
public void getAttributeNamesAndRemove() {
ExpiringSession session = this.repository.createSession();
session.setAttribute("attribute1", "value1");
session.setAttribute("attribute2", "value2");
for (String attributeName : session.getAttributeNames()) {
session.removeAttribute(attributeName);
}
assertThat(session.getAttributeNames()).isEmpty();
}
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2016 the original author or authors.
* Copyright 2014-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -85,6 +85,18 @@ public class MapSessionTests {
assertThat(this.session.isExpired(now)).isTrue();
}
@Test // gh-1129
public void getAttributeNamesAndRemove() {
this.session.setAttribute("attribute1", "value1");
this.session.setAttribute("attribute2", "value2");
for (String attributeName : this.session.getAttributeNames()) {
this.session.removeAttribute(attributeName);
}
assertThat(this.session.getAttributeNames()).isEmpty();
}
static class CustomSession implements ExpiringSession {
public long getCreationTime() {

View File

@@ -805,6 +805,19 @@ public class RedisOperationsSessionRepositoryTests {
this.redisRepository.setRedisFlushMode(null);
}
@Test // gh-1129
public void getAttributeNamesAndRemove() {
RedisSession session = this.redisRepository.new RedisSession(this.cached);
session.setAttribute("attribute1", "value1");
session.setAttribute("attribute2", "value2");
for (String attributeName : session.getAttributeNames()) {
session.removeAttribute(attributeName);
}
assertThat(session.getAttributeNames()).isEmpty();
}
private String getKey(String id) {
return "spring:session:sessions:" + id;
}

View File

@@ -339,4 +339,17 @@ public class HazelcastSessionRepositoryTests {
verify(this.sessions, times(1)).values(isA(EqualPredicate.class));
}
@Test // gh-1129
public void getAttributeNamesAndRemove() {
HazelcastSession session = this.repository.createSession();
session.setAttribute("attribute1", "value1");
session.setAttribute("attribute2", "value2");
for (String attributeName : session.getAttributeNames()) {
session.removeAttribute(attributeName);
}
assertThat(session.getAttributeNames()).isEmpty();
}
}

View File

@@ -525,6 +525,19 @@ public class JdbcOperationsSessionRepositoryTests {
verify(this.jdbcOperations, times(1)).update(startsWith("DELETE"), anyLong());
}
@Test // gh-1129
public void getAttributeNamesAndRemove() {
JdbcOperationsSessionRepository.JdbcSession session = this.repository.createSession();
session.setAttribute("attribute1", "value1");
session.setAttribute("attribute2", "value2");
for (String attributeName : session.getAttributeNames()) {
session.removeAttribute(attributeName);
}
assertThat(session.getAttributeNames()).isEmpty();
}
private void assertPropagationRequiresNew() {
ArgumentCaptor<TransactionDefinition> argument =
ArgumentCaptor.forClass(TransactionDefinition.class);