From d7d33bfbd0a1d3f41b288d1ae8bf8f6fefe35022 Mon Sep 17 00:00:00 2001 From: Francois Papon Date: Thu, 9 May 2019 01:31:40 +0400 Subject: [PATCH] =?UTF-8?q?[SHIRO-685]=C2=A0Potential=20NullPointerExcepti?= =?UTF-8?q?on=20if=20PermissionResolver=20return=20null/empty=20string?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../apache/shiro/realm/AuthorizingRealm.java | 7 ++- .../shiro/realm/AuthorizingRealmTest.java | 56 +++++++++++++++++-- 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/apache/shiro/realm/AuthorizingRealm.java b/core/src/main/java/org/apache/shiro/realm/AuthorizingRealm.java index 254472ffa3..8f69a2495a 100644 --- a/core/src/main/java/org/apache/shiro/realm/AuthorizingRealm.java +++ b/core/src/main/java/org/apache/shiro/realm/AuthorizingRealm.java @@ -26,6 +26,7 @@ import org.apache.shiro.subject.PrincipalCollection; import org.apache.shiro.util.CollectionUtils; import org.apache.shiro.util.Initializable; +import org.apache.shiro.util.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -431,8 +432,10 @@ private Collection resolvePermissions(Collection stringPerms if (resolver != null && !CollectionUtils.isEmpty(stringPerms)) { perms = new LinkedHashSet(stringPerms.size()); for (String strPermission : stringPerms) { - Permission permission = resolver.resolvePermission(strPermission); - perms.add(permission); + if (StringUtils.clean(strPermission) != null) { + Permission permission = resolver.resolvePermission(strPermission); + perms.add(permission); + } } } return perms; diff --git a/core/src/test/java/org/apache/shiro/realm/AuthorizingRealmTest.java b/core/src/test/java/org/apache/shiro/realm/AuthorizingRealmTest.java index 32c50dfbf2..c78d66919f 100644 --- a/core/src/test/java/org/apache/shiro/realm/AuthorizingRealmTest.java +++ b/core/src/test/java/org/apache/shiro/realm/AuthorizingRealmTest.java @@ -18,7 +18,18 @@ */ package org.apache.shiro.realm; -import org.apache.shiro.authc.*; +import java.security.Principal; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import org.apache.shiro.authc.AuthenticationException; +import org.apache.shiro.authc.AuthenticationInfo; +import org.apache.shiro.authc.AuthenticationToken; +import org.apache.shiro.authc.SimpleAccount; +import org.apache.shiro.authc.SimpleAuthenticationInfo; +import org.apache.shiro.authc.UsernamePasswordToken; import org.apache.shiro.authc.credential.AllowAllCredentialsMatcher; import org.apache.shiro.authz.AuthorizationInfo; import org.apache.shiro.authz.Permission; @@ -26,15 +37,18 @@ import org.apache.shiro.authz.UnauthorizedException; import org.apache.shiro.authz.permission.RolePermissionResolver; import org.apache.shiro.authz.permission.WildcardPermission; +import org.apache.shiro.authz.permission.WildcardPermissionResolver; import org.apache.shiro.subject.PrincipalCollection; import org.apache.shiro.subject.SimplePrincipalCollection; import org.junit.After; -import static org.junit.Assert.*; import org.junit.Before; import org.junit.Test; -import java.security.Principal; -import java.util.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** @@ -214,6 +228,40 @@ public Collection resolvePermissionsInRole( String roleString ) assertTrue( realm.isPermitted( pCollection, "other:bar:foo" ) ); } + @Test + public void testRealmWithEmptyOrNullPermissions() { + Principal principal = new UsernamePrincipal("rolePermResolver"); + PrincipalCollection pCollection = new SimplePrincipalCollection(principal, "testRealmWithRolePermissionResolver"); + + AuthorizingRealm realm = new AllowAllRealm(); + realm.setRolePermissionResolver( new RolePermissionResolver() + { + public Collection resolvePermissionsInRole( String roleString ) + { + Collection permissions = new HashSet(); + if( roleString.equals( ROLE )) + { + permissions.add( new WildcardPermission( ROLE + ":perm1" ) ); + permissions.add( new WildcardPermission( ROLE + ":perm2" ) ); + permissions.add( new WildcardPermission( ROLE + ": " ) ); + permissions.add( new WildcardPermission( ROLE + ":\t" ) ); + permissions.add( new WildcardPermission( "other:*:foo" ) ); + } + return permissions; + } + }); + + realm.setPermissionResolver(new WildcardPermissionResolver()); + SimpleAuthorizationInfo authorizationInfo = (SimpleAuthorizationInfo) realm.getAuthorizationInfo(pCollection); + assertNotNull(authorizationInfo); + authorizationInfo.addStringPermission(""); + authorizationInfo.addStringPermission(" "); + authorizationInfo.addStringPermission("\t"); + authorizationInfo.addStringPermission(null); + Collection permissions = realm.getPermissions(authorizationInfo); + assertEquals(permissions.size(), 4); + } + private void assertArrayEquals(boolean[] expected, boolean[] actual) { if (expected.length != actual.length) { fail("Expected array of length [" + expected.length + "] but received array of length [" + actual.length + "]");