浏览代码

Polish

DelegatingPasswordEncoder copies the provided Map. This ensures that
references to the Map do not update the state of DelegatingPasswordEncoder
and NullPointerException is avoided for implementations that do not allow
a null key.

Issue: gh-4936
Rob Winch 7 年之前
父节点
当前提交
22737dce7e

+ 3 - 6
crypto/src/main/java/org/springframework/security/crypto/password/DelegatingPasswordEncoder.java

@@ -16,6 +16,7 @@
 
 
 package org.springframework.security.crypto.password;
 package org.springframework.security.crypto.password;
 
 
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Map;
 
 
 /**
 /**
@@ -155,7 +156,7 @@ public class DelegatingPasswordEncoder implements PasswordEncoder {
 		}
 		}
 		this.idForEncode = idForEncode;
 		this.idForEncode = idForEncode;
 		this.passwordEncoderForEncode = idToPasswordEncoder.get(idForEncode);
 		this.passwordEncoderForEncode = idToPasswordEncoder.get(idForEncode);
-		this.idToPasswordEncoder = idToPasswordEncoder;
+		this.idToPasswordEncoder = new HashMap<>(idToPasswordEncoder);
 	}
 	}
 
 
 	/**
 	/**
@@ -191,11 +192,7 @@ public class DelegatingPasswordEncoder implements PasswordEncoder {
 			return true;
 			return true;
 		}
 		}
 		String id = extractId(prefixEncodedPassword);
 		String id = extractId(prefixEncodedPassword);
-		PasswordEncoder delegate = null;
-		try {
-			delegate = this.idToPasswordEncoder.get(id);
-		} catch(NullPointerException e) {
-		}
+		PasswordEncoder delegate = this.idToPasswordEncoder.get(id);
 		if(delegate == null) {
 		if(delegate == null) {
 			return this.defaultPasswordEncoderForMatches
 			return this.defaultPasswordEncoderForMatches
 				.matches(rawPassword, prefixEncodedPassword);
 				.matches(rawPassword, prefixEncodedPassword);

+ 3 - 7
crypto/src/test/java/org/springframework/security/crypto/password/DelegatingPasswordEncoderTests.java

@@ -23,6 +23,7 @@ import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.mockito.junit.MockitoJUnitRunner;
 
 
 import java.util.HashMap;
 import java.util.HashMap;
+import java.util.Hashtable;
 import java.util.Map;
 import java.util.Map;
 
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -47,9 +48,6 @@ public class DelegatingPasswordEncoderTests {
 	@Mock
 	@Mock
 	private PasswordEncoder invalidId;
 	private PasswordEncoder invalidId;
 
 
-	@Mock
-	private Map<String, PasswordEncoder> throwingDelegates;
-
 	private String bcryptId = "bcrypt";
 	private String bcryptId = "bcrypt";
 
 
 	private String rawPassword = "password";
 	private String rawPassword = "password";
@@ -173,11 +171,9 @@ public class DelegatingPasswordEncoderTests {
 
 
 	@Test
 	@Test
 	public void matchesWhenIdIsNullThenFalse() {
 	public void matchesWhenIdIsNullThenFalse() {
-		when(this.throwingDelegates.containsKey(this.bcryptId)).thenReturn(true);
-		when(this.throwingDelegates.get(this.bcryptId)).thenReturn(this.bcrypt);
-		when(this.throwingDelegates.get(null)).thenThrow(NullPointerException.class);
+		this.delegates = new Hashtable<>(this.delegates);
 
 
-		DelegatingPasswordEncoder passwordEncoder = new DelegatingPasswordEncoder(this.bcryptId, throwingDelegates);
+		DelegatingPasswordEncoder passwordEncoder = new DelegatingPasswordEncoder(this.bcryptId, this.delegates);
 
 
 		assertThatThrownBy(() -> passwordEncoder.matches(this.rawPassword, this.rawPassword))
 		assertThatThrownBy(() -> passwordEncoder.matches(this.rawPassword, this.rawPassword))
 			.isInstanceOf(IllegalArgumentException.class)
 			.isInstanceOf(IllegalArgumentException.class)