Repository: ambari Updated Branches: refs/heads/branch-2.5 4c64ffc44 -> 76a9658a0
AMBARI-19662. Unknown attributes should not be allowed in quick link filter definitions (Balazs Bence Sari via magyari_sandor) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/76a9658a Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/76a9658a Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/76a9658a Branch: refs/heads/branch-2.5 Commit: 76a9658a08683645733637f64dff42f71352eb23 Parents: 4c64ffc Author: Balazs Bence Sari <bs...@hortonworks.com> Authored: Mon Jan 23 11:54:37 2017 +0100 Committer: Sandor Magyari <smagy...@hortonworks.com> Committed: Mon Jan 23 12:56:33 2017 +0100 ---------------------------------------------------------------------- .../QuickLinksProfileBuilder.java | 26 +++++++++++++++----- .../QuickLinksProfileParser.java | 21 +++++++++++++--- .../QuickLinksProfileBuilderTest.java | 13 ++++++++-- .../QuickLinksProfileParserTest.java | 11 +++++++-- .../inconsistent_quicklinks_profile_3.json | 9 +++++++ 5 files changed, 67 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/76a9658a/ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileBuilder.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileBuilder.java b/ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileBuilder.java index fca1155..627b1bc 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileBuilder.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileBuilder.java @@ -26,10 +26,14 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Set; import javax.annotation.Nullable; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; /** * Class to create a {@link QuickLinksProfile} based on data received in a request @@ -39,6 +43,8 @@ public class QuickLinksProfileBuilder { public static final String NAME = "name"; public static final String COMPONENTS = "components"; public static final String FILTERS = "filters"; + public static final Set<String> ALLOWED_FILTER_ATTRIBUTES = + ImmutableSet.of(VISIBLE, LINK_NAME, LINK_ATTRIBUTE); /** * @@ -102,16 +108,24 @@ public class QuickLinksProfileBuilder { } List<Filter> filters = new ArrayList<>(); for (Map<String, String> filterAsMap: (Collection<Map<String, String>>)filtersRaw) { + Set<String> invalidAttributes = Sets.difference(filterAsMap.keySet(), ALLOWED_FILTER_ATTRIBUTES); + + Preconditions.checkArgument(invalidAttributes.isEmpty(), + "%s%s", + QuickLinksFilterDeserializer.PARSE_ERROR_MESSAGE_INVALID_JSON_TAG, + invalidAttributes); + String linkName = filterAsMap.get(LINK_NAME); String attributeName = filterAsMap.get(LINK_ATTRIBUTE); boolean visible = Boolean.parseBoolean(filterAsMap.get(VISIBLE)); - if (null != linkName && null != attributeName) { - throw new IllegalArgumentException( - String.format("%s link_name: %s, link_attribute: %s", - QuickLinksFilterDeserializer.PARSE_ERROR_MESSAGE, linkName, attributeName)); - } - else if (null != linkName) { + Preconditions.checkArgument(null == linkName || null == attributeName, + "%s link_name: %s, link_attribute: %s", + QuickLinksFilterDeserializer.PARSE_ERROR_MESSAGE_AMBIGUOUS_FILTER, + linkName, + attributeName); + + if (null != linkName) { filters.add(Filter.linkNameFilter(linkName, visible)); } else if (null != attributeName) { http://git-wip-us.apache.org/repos/asf/ambari/blob/76a9658a/ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParser.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParser.java b/ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParser.java index 150b7d4..1891061 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParser.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParser.java @@ -20,6 +20,8 @@ package org.apache.ambari.server.state.quicklinksprofile; import java.io.IOException; import java.net.URL; +import java.util.ArrayList; +import java.util.List; import org.codehaus.jackson.JsonParseException; import org.codehaus.jackson.JsonParser; @@ -65,9 +67,12 @@ public class QuickLinksProfileParser { * Custom deserializer is needed to handle filter polymorphism. */ class QuickLinksFilterDeserializer extends StdDeserializer<Filter> { - static final String PARSE_ERROR_MESSAGE = + static final String PARSE_ERROR_MESSAGE_AMBIGUOUS_FILTER = "A filter is not allowed to declare both link_name and link_attribute at the same time."; + static final String PARSE_ERROR_MESSAGE_INVALID_JSON_TAG = + "Invalid attribute(s) in filter declaration: "; + QuickLinksFilterDeserializer() { super(Filter.class); } @@ -88,22 +93,32 @@ class QuickLinksFilterDeserializer extends StdDeserializer<Filter> { ObjectMapper mapper = (ObjectMapper) parser.getCodec(); ObjectNode root = (ObjectNode) mapper.readTree(parser); Class<? extends Filter> filterClass = null; + List<String> invalidAttributes = new ArrayList<>(); for (String fieldName: ImmutableList.copyOf(root.getFieldNames())) { switch(fieldName) { case LinkAttributeFilter.LINK_ATTRIBUTE: if (null != filterClass) { - throw new JsonParseException(PARSE_ERROR_MESSAGE, parser.getCurrentLocation()); + throw new JsonParseException(PARSE_ERROR_MESSAGE_AMBIGUOUS_FILTER, parser.getCurrentLocation()); } filterClass = LinkAttributeFilter.class; break; case LinkNameFilter.LINK_NAME: if (null != filterClass) { - throw new JsonParseException(PARSE_ERROR_MESSAGE, parser.getCurrentLocation()); + throw new JsonParseException(PARSE_ERROR_MESSAGE_AMBIGUOUS_FILTER, parser.getCurrentLocation()); } filterClass = LinkNameFilter.class; break; + case Filter.VISIBLE: + // silently ignore here, will be parsed later in mapper.readValue + break; + default: + invalidAttributes.add(fieldName); } } + if (!invalidAttributes.isEmpty()) { + throw new JsonParseException(PARSE_ERROR_MESSAGE_INVALID_JSON_TAG + invalidAttributes, + parser.getCurrentLocation()); + } if (null == filterClass) { filterClass = AcceptAllFilter.class; } http://git-wip-us.apache.org/repos/asf/ambari/blob/76a9658a/ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileBuilderTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileBuilderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileBuilderTest.java index 1cc3fd3..49244d4 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileBuilderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileBuilderTest.java @@ -33,6 +33,7 @@ import javax.annotation.Nullable; import org.junit.Test; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Sets; public class QuickLinksProfileBuilderTest { @@ -130,14 +131,22 @@ public class QuickLinksProfileBuilderTest { } @Test(expected = QuickLinksProfileEvaluationException.class) - public void testBuildProfileInvalidProfileDefiniton() throws Exception { + public void testBuildProfileInvalidProfileDefiniton_contradictingFilters() throws Exception { // Contradicting rules in the profile Set<Map<String, String>> filters = newHashSet( filter(null, "sso", true), filter(null, "sso", false) ); - String profileJson = new QuickLinksProfileBuilder().buildQuickLinksProfile(filters, null); + new QuickLinksProfileBuilder().buildQuickLinksProfile(filters, null); + } + + @Test(expected = QuickLinksProfileEvaluationException.class) + public void testBuildProfileInvalidProfileDefiniton_invalidAttribute() throws Exception { + Map<String, String> badFilter = ImmutableMap.of("visible", "true", "linkkk_atirbuteee", "sso"); + Set<Map<String, String>> filters = newHashSet(badFilter); + + new QuickLinksProfileBuilder().buildQuickLinksProfile(filters, null); } /** http://git-wip-us.apache.org/repos/asf/ambari/blob/76a9658a/ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParserTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParserTest.java b/ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParserTest.java index eb459bb..702c498 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParserTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParserTest.java @@ -67,10 +67,17 @@ public class QuickLinksProfileParserTest { } @Test(expected = JsonParseException.class) - public void testParseInconsistentProfile() throws Exception { + public void testParseInconsistentProfile_ambigousFilterDefinition() throws Exception { String profileName = "inconsistent_quicklinks_profile.json"; QuickLinksProfileParser parser = new QuickLinksProfileParser(); - QuickLinksProfile profile = parser.parse(Resources.getResource(profileName)); + parser.parse(Resources.getResource(profileName)); + } + + @Test(expected = JsonParseException.class) + public void testParseInconsistentProfile_misspelledFilerDefinition() throws Exception { + String profileName = "inconsistent_quicklinks_profile_3.json"; + QuickLinksProfileParser parser = new QuickLinksProfileParser(); + parser.parse(Resources.getResource(profileName)); } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/ambari/blob/76a9658a/ambari-server/src/test/resources/inconsistent_quicklinks_profile_3.json ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/resources/inconsistent_quicklinks_profile_3.json b/ambari-server/src/test/resources/inconsistent_quicklinks_profile_3.json new file mode 100644 index 0000000..c349bb2 --- /dev/null +++ b/ambari-server/src/test/resources/inconsistent_quicklinks_profile_3.json @@ -0,0 +1,9 @@ +{ + "filters": [ + { + "linkkkk_attirubutee": "sso", + "visible": true + } + ], + "services": [] +} \ No newline at end of file
