Skip to content

Commit

Permalink
RANGER-4956: fixed dedup tags to address potential infinite loop
Browse files Browse the repository at this point in the history
  • Loading branch information
mneethiraj committed Oct 17, 2024
1 parent 8d89c91 commit 16c94a9
Show file tree
Hide file tree
Showing 2 changed files with 160 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,8 @@ public int dedupTags() {
MutablePair<Long, Long> cachedTag = cachedTags.get(tag);

if (cachedTag == null) {
cachedTags.put(tag, new MutablePair<>(tagId, 0L)); // reference count will be incremented later
} else {
cachedTags.put(tag, new MutablePair<>(tagId, 1L));
} else if (!tagId.equals(cachedTag.left)) {
if (tagId < cachedTag.left) {
replacedIds.put(cachedTag.left, tagId);
tagIdsToRemove.add(cachedTag.left);
Expand All @@ -270,6 +270,7 @@ public int dedupTags() {
}
}
}

for (Long tagIdToRemove : tagIdsToRemove) {
tags.remove(tagIdToRemove);
}
Expand All @@ -278,22 +279,28 @@ public int dedupTags() {

for (Map.Entry<Long, List<Long>> resourceEntry : resourceToTagIds.entrySet()) {
for (ListIterator<Long> listIter = resourceEntry.getValue().listIterator(); listIter.hasNext(); ) {
Long tagId = listIter.next();
final Long tagId = listIter.next();
Long mappedTagId = null;

for (Long replacerTagId = replacedIds.get(tagId); replacerTagId != null; replacerTagId = replacedIds.get(mappedTagId)) {
mappedTagId = replacerTagId;
}

for (Long replacerTagId = replacedIds.get(tagId); replacerTagId != null; replacerTagId = replacedIds.get(tagId)) {
tagId = replacerTagId;
if (mappedTagId == null) {
continue;
}

listIter.set(tagId);
listIter.set(mappedTagId);

RangerTag tag = tags.get(tagId);
if (tag != null) { // This should always be true
RangerTag tag = tags.get(mappedTagId);

if (tag != null) { // This should always be true
MutablePair<Long, Long> cachedTag = cachedTags.get(tag);

if (cachedTag != null) { // This should always be true
cachedTag.right++;
}
}

}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.ranger.plugin.util;

import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyResource;
import org.apache.ranger.plugin.model.RangerServiceResource;
import org.apache.ranger.plugin.model.RangerTag;
import org.junit.Test;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import static org.junit.Assert.assertEquals;

public class TestServiceTags {
private static final RangerServiceResource[] RESOURCES = {
new RangerServiceResource("dev_hive", Collections.singletonMap("database", new RangerPolicyResource("db1"))),
new RangerServiceResource("dev_hive", Collections.singletonMap("database", new RangerPolicyResource("db2"))),
new RangerServiceResource("dev_hive", Collections.singletonMap("database", new RangerPolicyResource("db3"))),
new RangerServiceResource("dev_hive", Collections.singletonMap("database", new RangerPolicyResource("db4"))),
new RangerServiceResource("dev_hive", Collections.singletonMap("database", new RangerPolicyResource("db5"))),
new RangerServiceResource("dev_hive", Collections.singletonMap("database", new RangerPolicyResource("db6"))),
new RangerServiceResource("dev_hive", Collections.singletonMap("database", new RangerPolicyResource("db7"))),
new RangerServiceResource("dev_hive", Collections.singletonMap("database", new RangerPolicyResource("db8"))),
new RangerServiceResource("dev_hive", Collections.singletonMap("database", new RangerPolicyResource("db9"))),
new RangerServiceResource("dev_hive", Collections.singletonMap("database", new RangerPolicyResource("db10")))
};

@Test
public void testDedupEmpty() {
ServiceTags svcTags = new ServiceTags();

assertEquals(0, svcTags.dedupTags());
}

@Test
public void testDedup_NoDupTagsNoAttr() {
RangerTag[] tags = {
new RangerTag("PII", Collections.emptyMap()),
new RangerTag("PHI", Collections.emptyMap()),
};

ServiceTags svcTags = createServiceTags(tags, RESOURCES);

assertEquals(0, svcTags.dedupTags());
}

@Test
public void testDedup_NoDupTagsWithAttr() {
RangerTag[] tags = {
new RangerTag("PII", Collections.emptyMap()),
new RangerTag("PII", Collections.singletonMap("type", "email")),
};

ServiceTags svcTags = createServiceTags(tags, RESOURCES);

assertEquals(0, svcTags.dedupTags());
}

@Test
public void testDedup_DupTagsNoAttr() {
RangerTag[] tags = {
new RangerTag("PII", Collections.emptyMap()),
new RangerTag("PII", Collections.emptyMap()),
};

ServiceTags svcTags = createServiceTags(tags, RESOURCES);

assertEquals(1, svcTags.dedupTags());
}

@Test
public void testDedup_DupTagsWithAttr() {
RangerTag[] tags = {
new RangerTag("PII", Collections.singletonMap("type", "email")),
new RangerTag("PII", Collections.singletonMap("type", "email")),
};

ServiceTags svcTags = createServiceTags(tags, RESOURCES);

assertEquals(1, svcTags.dedupTags());
}

@Test
public void testDedup_DupTagsWithAttr_MultipleCalls() {
RangerTag[] tags = {
new RangerTag("PII", Collections.singletonMap("type", "email")),
new RangerTag("PII", Collections.singletonMap("type", "email")),
new RangerTag("PCI", Collections.emptyMap()),
new RangerTag("PCI", Collections.emptyMap())
};

ServiceTags svcTags = createServiceTags(tags, RESOURCES);

assertEquals(2, svcTags.dedupTags());
assertEquals(0, svcTags.dedupTags());
}


private ServiceTags createServiceTags(RangerTag[] tags, RangerServiceResource[] resources) {
ServiceTags ret = new ServiceTags();

// add tags
for (int i = 0; i < tags.length; i++) {
RangerTag tag = tags[i];

tag.setId(i + 1L);
ret.getTags().put(tag.getId(), tag);
}

// add resources
for (int i = 0; i < resources.length; i++) {
RangerServiceResource resource = resources[i];

resource.setId(i + 1L);
ret.getServiceResources().add(resource);
}

// set resourceToTagIds
ret.getServiceResources().forEach(resource -> {
List<Long> tagIds = new ArrayList<>(ret.getTags().keySet()); // add all available tags

ret.getResourceToTagIds().put(resource.getId(), tagIds);
});

return ret;
}
}

0 comments on commit 16c94a9

Please sign in to comment.