Remove incorrect usages of Arrays.binarySearch (elastic/x-pack-elasticsearch#954)

This is a follow-on to elastic/x-pack-elasticsearch#939, which removes the use of Arrays.binarySearch in the FieldPermissions
class. This change removes other incorrect uses in the rest of the x-pack code and replaces them
with a stream based implementation.

Original commit: elastic/x-pack-elasticsearch@ccca7e9bad
This commit is contained in:
Jay Modi 2017-04-04 17:48:58 +01:00 committed by GitHub
parent c9834bc826
commit 1e42473f77
7 changed files with 23 additions and 11 deletions

View File

@ -57,7 +57,7 @@ public class HtmlSanitizer {
static PolicyFactory createCommonPolicy(String[] allow, String[] disallow) {
HtmlPolicyBuilder policyBuilder = new HtmlPolicyBuilder();
if (Arrays.binarySearch(allow, "_all") > -1) {
if (Arrays.stream(allow).anyMatch("_all"::equals)) {
return policyBuilder
.allowElements(TABLE_TAGS)
.allowAttributes("span").onElements("col")

View File

@ -242,7 +242,7 @@ public class AuthorizationService extends AbstractComponent {
&& indicesAccessControl.getIndexPermissions(SecurityLifecycleService.SECURITY_INDEX_NAME).isGranted()
&& XPackUser.is(authentication.getRunAsUser()) == false
&& MONITOR_INDEX_PREDICATE.test(action) == false
&& Arrays.binarySearch(authentication.getRunAsUser().roles(), ReservedRolesStore.SUPERUSER_ROLE.name()) < 0) {
&& isSuperuser(authentication.getRunAsUser()) == false) {
// only the XPackUser is allowed to work with this index, but we should allow indices monitoring actions through for debugging
// purposes. These monitor requests also sometimes resolve indices concretely and then requests them
logger.debug("user [{}] attempted to directly perform [{}] against the security index [{}]",
@ -437,6 +437,11 @@ public class AuthorizationService extends AbstractComponent {
return authorizationError("action [{}] is unauthorized for user [{}]", action, user.principal());
}
static boolean isSuperuser(User user) {
return Arrays.stream(user.roles())
.anyMatch(ReservedRolesStore.SUPERUSER_ROLE.name()::equals);
}
public static void addSettings(List<Setting<?>> settings) {
settings.add(ANONYMOUS_AUTHORIZATION_EXCEPTION_SETTING);
}

View File

@ -20,6 +20,8 @@ import java.util.List;
import java.util.Map;
import java.util.function.Predicate;
import static org.elasticsearch.xpack.security.authz.AuthorizationService.isSuperuser;
/**
* Abstraction used to make sure that we lazily load authorized indices only when requested and only maximum once per request. Also
* makes sure that authorized indices don't get updated throughout the same request for the same user.
@ -57,7 +59,7 @@ class AuthorizedIndices {
}
}
if (XPackUser.is(user) == false && Arrays.binarySearch(user.roles(), ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR.getName()) < 0) {
if (isSuperuser(user) == false) {
// we should filter out the .security index from wildcards
indicesAndAliases.remove(SecurityLifecycleService.SECURITY_INDEX_NAME);
}

View File

@ -151,8 +151,7 @@ public class SecurityIpFilterRule implements IpFilterRule {
static IpFilterRule getRule(boolean isAllowRule, String value) {
IpFilterRuleType filterRuleType = isAllowRule ? IpFilterRuleType.ACCEPT : IpFilterRuleType.REJECT;
String[] values = value.split(",");
int allRuleIndex = Arrays.binarySearch(values, 0, values.length, "_all");
if (allRuleIndex >= 0) {
if (Arrays.stream(values).anyMatch("_all"::equals)) {
// all rule was found. It should be the only rule!
if (values.length != 1) {
throw new IllegalArgumentException("rules that specify _all may not have other values!");

View File

@ -79,7 +79,7 @@ public class ClearRealmsCacheTests extends SecurityIntegTestCase {
@Override
public void assertEviction(User prevUser, User newUser) {
if (Arrays.binarySearch(evicted_usernames, prevUser.principal()) >= 0) {
if (Arrays.stream(evicted_usernames).anyMatch(prevUser.principal()::equals)) {
assertThat(prevUser, not(sameInstance(newUser)));
} else {
assertThat(prevUser, sameInstance(newUser));
@ -115,7 +115,7 @@ public class ClearRealmsCacheTests extends SecurityIntegTestCase {
@Override
public void assertEviction(User prevUser, User newUser) {
if (Arrays.binarySearch(evicted_usernames, prevUser.principal()) >= 0) {
if (Arrays.stream(evicted_usernames).anyMatch(prevUser.principal()::equals)) {
assertThat(prevUser, not(sameInstance(newUser)));
} else {
assertThat(prevUser, sameInstance(newUser));

View File

@ -134,7 +134,10 @@ public class ValidationTests extends ESTestCase {
char first;
while (true) {
first = randomUnicodeOfLength(1).charAt(0);
if (Arrays.binarySearch(allowedFirstChars, first) < 0) {
final char finalChar = first;
if (new String(allowedFirstChars).chars()
.mapToObj(c -> (char) c)
.anyMatch(c -> c.equals(finalChar)) == false) {
break;
}
}
@ -152,7 +155,10 @@ public class ValidationTests extends ESTestCase {
char c;
while (true) {
c = randomUnicodeOfLength(1).charAt(0);
if (Arrays.binarySearch(allowedSubsequent, c) < 0) {
final char finalChar = c;
if (new String(allowedSubsequent).chars()
.mapToObj(c1 -> (char) c1)
.anyMatch(c1 -> c1.equals(finalChar)) == false) {
break;
}
}

View File

@ -440,11 +440,11 @@ public class CertificateToolTests extends ESTestCase {
for (GeneralName generalName : subjAltNames.getNames()) {
if (generalName.getTagNo() == GeneralName.dNSName) {
String dns = ((ASN1String)generalName.getName()).getString();
assertThat(Collections.binarySearch(certInfo.dnsNames, dns), greaterThanOrEqualTo(0));
assertTrue(certInfo.dnsNames.stream().anyMatch(dns::equals));
} else if (generalName.getTagNo() == GeneralName.iPAddress) {
byte[] ipBytes = DEROctetString.getInstance(generalName.getName()).getOctets();
String ip = NetworkAddress.format(InetAddress.getByAddress(ipBytes));
assertThat(Collections.binarySearch(certInfo.ipAddresses, ip), greaterThanOrEqualTo(0));
assertTrue(certInfo.ipAddresses.stream().anyMatch(ip::equals));
} else {
fail("unknown general name with tag " + generalName.getTagNo());
}