From 2b66793535eff78fba14fee0a75d0091c5636f56 Mon Sep 17 00:00:00 2001 From: "Michael J. Simons" Date: Sat, 30 Dec 2017 13:56:36 +0100 Subject: [PATCH] Catch possible NullPointerException Some maps may throw a NullPointerException when get is called with null. This commit catches the exceptions and just leaves the delegate null. Fixes gh-4936 --- .../password/DelegatingPasswordEncoder.java | 7 ++++++- .../DelegatingPasswordEncoderTests.java | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) 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 b805029cb0..4e4a0f842e 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 @@ -116,6 +116,7 @@ import java.util.Map; * @see org.springframework.security.crypto.factory.PasswordEncoderFactories * * @author Rob Winch + * @author Michael Simons * @since 5.0 */ public class DelegatingPasswordEncoder implements PasswordEncoder { @@ -190,7 +191,11 @@ public class DelegatingPasswordEncoder implements PasswordEncoder { return true; } String id = extractId(prefixEncodedPassword); - PasswordEncoder delegate = this.idToPasswordEncoder.get(id); + PasswordEncoder delegate = null; + try { + delegate = this.idToPasswordEncoder.get(id); + } catch(NullPointerException e) { + } 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 d03120b4b4..211ff32be8 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 @@ -33,6 +33,7 @@ import static org.mockito.Mockito.when; /** * @author Rob Winch + * @author Michael Simons * @since 5.0 */ @RunWith(MockitoJUnitRunner.class) @@ -46,6 +47,9 @@ public class DelegatingPasswordEncoderTests { @Mock private PasswordEncoder invalidId; + @Mock + private Map throwingDelegates; + private String bcryptId = "bcrypt"; private String rawPassword = "password"; @@ -167,6 +171,21 @@ public class DelegatingPasswordEncoderTests { verifyZeroInteractions(this.bcrypt, this.noop); } + @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); + + DelegatingPasswordEncoder passwordEncoder = new DelegatingPasswordEncoder(this.bcryptId, throwingDelegates); + + assertThatThrownBy(() -> passwordEncoder.matches(this.rawPassword, this.rawPassword)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("There is no PasswordEncoder mapped for the id \"null\""); + + verifyZeroInteractions(this.bcrypt, this.noop); + } + @Test public void matchesWhenNullIdThenDelegatesToInvalidId() { this.delegates.put(null, this.invalidId);