From 22737dce7e5021e1a6682bc9313fbae3a14b4f77 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Wed, 3 Jan 2018 10:11:04 -0600 Subject: [PATCH] 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 --- .../crypto/password/DelegatingPasswordEncoder.java | 9 +++------ .../password/DelegatingPasswordEncoderTests.java | 10 +++------- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/crypto/src/main/java/org/springframework/security/crypto/password/DelegatingPasswordEncoder.java b/crypto/src/main/java/org/springframework/security/crypto/password/DelegatingPasswordEncoder.java index 4e4a0f842e..21542f6ddc 100644 --- a/crypto/src/main/java/org/springframework/security/crypto/password/DelegatingPasswordEncoder.java +++ b/crypto/src/main/java/org/springframework/security/crypto/password/DelegatingPasswordEncoder.java @@ -16,6 +16,7 @@ package org.springframework.security.crypto.password; +import java.util.HashMap; import java.util.Map; /** @@ -155,7 +156,7 @@ public class DelegatingPasswordEncoder implements PasswordEncoder { } this.idForEncode = 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; } 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) { return this.defaultPasswordEncoderForMatches .matches(rawPassword, prefixEncodedPassword); diff --git a/crypto/src/test/java/org/springframework/security/crypto/password/DelegatingPasswordEncoderTests.java b/crypto/src/test/java/org/springframework/security/crypto/password/DelegatingPasswordEncoderTests.java index 211ff32be8..d5deea8004 100644 --- a/crypto/src/test/java/org/springframework/security/crypto/password/DelegatingPasswordEncoderTests.java +++ b/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 java.util.HashMap; +import java.util.Hashtable; import java.util.Map; import static org.assertj.core.api.Assertions.assertThat; @@ -47,9 +48,6 @@ public class DelegatingPasswordEncoderTests { @Mock private PasswordEncoder invalidId; - @Mock - private Map throwingDelegates; - private String bcryptId = "bcrypt"; private String rawPassword = "password"; @@ -173,11 +171,9 @@ public class DelegatingPasswordEncoderTests { @Test 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)) .isInstanceOf(IllegalArgumentException.class)