diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java index 58f4aaf41ad2..01c8d01ebd69 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java @@ -14,7 +14,9 @@ */ package org.apache.geode.management.internal.cli.commands; -import static org.apache.geode.management.internal.ManagementHelper.isClassNameValid; + + +import static org.apache.geode.management.configuration.ClassName.isClassNameValid; import java.util.ArrayList; import java.util.Arrays; diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/converters/ClassNameConverterTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/converters/ClassNameConverterTest.java index b765ca1de97a..cb8d3115daf1 100644 --- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/converters/ClassNameConverterTest.java +++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/converters/ClassNameConverterTest.java @@ -49,7 +49,7 @@ public void convertClassAndEmptyProp() { @Test public void convertWithOnlyDelimiter() { - assertThat(converter.convertFromText("{", ClassName.class, "")).isEqualTo(ClassName.EMPTY); + assertThat(converter.convertFromText("{}", ClassName.class, "")).isEqualTo(ClassName.EMPTY); } @Test diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/domain/ClassNameTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/domain/ClassNameTest.java index 4ff13a6ddd63..c7b8217f5504 100644 --- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/domain/ClassNameTest.java +++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/domain/ClassNameTest.java @@ -15,18 +15,22 @@ package org.apache.geode.management.internal.cli.domain; +import static org.apache.geode.management.configuration.ClassName.isClassNameValid; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.util.Properties; +import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.Test; import org.apache.geode.management.configuration.ClassName; import org.apache.geode.management.internal.configuration.domain.DeclarableTypeInstantiator; +import org.apache.geode.util.internal.GeodeJsonMapper; public class ClassNameTest { + private ObjectMapper mapper = GeodeJsonMapper.getMapper(); @Test public void constructWithoutProps() { @@ -114,4 +118,83 @@ public void getInstanceWithProps() { MyCacheWriter obj = DeclarableTypeInstantiator.newInstance(cacheWriter, null); assertThat(obj.getProperties()).containsEntry("k", "v").containsOnlyKeys("k"); } + + @Test + public void jsonMapper() throws Exception { + Properties properties = new Properties(); + properties.put("key1", "value1"); + ClassName className = new ClassName("abc", properties); + ClassName className1 = new ClassName("abc", mapper.writeValueAsString(properties)); + assertThat(className).isEqualToComparingFieldByField(className1); + String json = mapper.writeValueAsString(className); + String json1 = mapper.writeValueAsString(className1); + assertThat(json).isEqualTo(json1); + + ClassName className2 = mapper.readValue(json, ClassName.class); + assertThat(className2).isEqualToComparingFieldByField(className); + } + + @Test + public void deserializeIllegalClassName() throws Exception { + assertThatThrownBy(() -> new ClassName("a b")).isInstanceOf(IllegalArgumentException.class); + String json = "{\"className\":\"a b\"}"; + assertThatThrownBy(() -> mapper.readValue(json, ClassName.class)) + .hasCauseInstanceOf(IllegalArgumentException.class); + } + + @Test + public void isValidClassNameGivenEmptyStringReturnsFalse() { + assertThat(isClassNameValid("")).isFalse(); + } + + @Test + public void isValidClassNameGivenDashReturnsFalse() { + assertThat(isClassNameValid("-")).isFalse(); + } + + @Test + public void isValidClassNameGivenSpaceReturnsFalse() { + assertThat(isClassNameValid(" ")).isFalse(); + } + + @Test + public void isValidClassNameGivenCommaReturnsFalse() { + assertThat(isClassNameValid(",")).isFalse(); + } + + @Test + public void isValidClassNameGivenLeadingDotReturnsFalse() { + assertThat(isClassNameValid(".a")).isFalse(); + } + + @Test + public void isValidClassNameGivenTrailingDotReturnsFalse() { + assertThat(isClassNameValid("a.")).isFalse(); + } + + @Test + public void isValidClassNameGivenTwoDotsReturnsFalse() { + assertThat(isClassNameValid("a..a")).isFalse(); + } + + @Test + public void isValidClassNameGivenNameThatStartsWithDigitReturnsFalse() { + assertThat(isClassNameValid("9a")).isFalse(); + } + + @Test + public void isValidClassNameGivenNameReturnsTrue() { + assertThat(isClassNameValid("a9")).isTrue(); + } + + @Test + public void isValidClassNameGivenDotDelimitedNamesReturnsTrue() { + assertThat(isClassNameValid("$a1._b2.c3$")).isTrue(); + } + + @Test + public void isValidClassNameGivenMiddleNameThatStartsWithDigitReturnsFalse() { + assertThat(isClassNameValid("a1.2b.c3")).isFalse(); + } + } diff --git a/geode-management/src/main/java/org/apache/geode/management/configuration/ClassName.java b/geode-management/src/main/java/org/apache/geode/management/configuration/ClassName.java index 791adbd2faba..720e3c9bbd7f 100644 --- a/geode-management/src/main/java/org/apache/geode/management/configuration/ClassName.java +++ b/geode-management/src/main/java/org/apache/geode/management/configuration/ClassName.java @@ -21,10 +21,11 @@ import java.util.Properties; import java.util.regex.Pattern; +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.commons.lang3.StringUtils; -import org.apache.geode.management.internal.ManagementHelper; import org.apache.geode.util.internal.GeodeJsonMapper; /** @@ -35,8 +36,8 @@ public class ClassName implements Serializable { private static final long serialVersionUID = 1L; - private String className = ""; - private Properties initProperties = new Properties(); + private String className; + private Properties initProperties; private static ObjectMapper mapper = GeodeJsonMapper.getMapper(); /** @@ -45,11 +46,6 @@ public class ClassName implements Serializable { */ public static final ClassName EMPTY = new ClassName(""); - /** - * Default constructor used for serialization. - */ - public ClassName() {} - /** * Object to be instantiated using the empty param constructor of the className * @@ -60,6 +56,8 @@ public ClassName(String className) { } /** + * this is a convenient way to create a ClassName object using json represented properties + * * @param className this class needs to have a no-arg constructor * @param jsonInitProperties a json representation of the initialization properties * that will be passed to {@link org.apache.geode.cache.Declarable#initialize}. @@ -67,15 +65,12 @@ public ClassName(String className) { * @throws IllegalArgumentException if jsonInitProperties is invalid JSON */ public ClassName(String className, String jsonInitProperties) { - if (StringUtils.isBlank(className)) { - return; - } - if (!ManagementHelper.isClassNameValid(className)) { - throw new IllegalArgumentException("Invalid className"); - } - this.className = className; + this(className, createProperties(jsonInitProperties)); + } + + private static Properties createProperties(String jsonInitProperties) { try { - initProperties = mapper.readValue(jsonInitProperties, Properties.class); + return mapper.readValue(jsonInitProperties, Properties.class); } catch (IOException e) { throw new IllegalArgumentException("Invalid JSON: " + jsonInitProperties, e); } @@ -86,7 +81,18 @@ public ClassName(String className, String jsonInitProperties) { * @param properties the initialization properties * that will be passed to {@link org.apache.geode.cache.Declarable#initialize}. */ - public ClassName(String className, Properties properties) { + @JsonCreator + public ClassName(@JsonProperty("className") String className, + @JsonProperty("initProperties") Properties properties) { + if (StringUtils.isBlank(className)) { + this.className = ""; + this.initProperties = new Properties(); + return; + } + // validate the className + if (!isClassNameValid(className)) { + throw new IllegalArgumentException("Invalid className"); + } this.className = className; this.initProperties = properties; } @@ -105,14 +111,6 @@ public Properties getInitProperties() { return initProperties; } - private static boolean isClassNameValid(String fqcn) { - if (StringUtils.isBlank(fqcn)) { - return false; - } - String regex = "([\\p{L}_$][\\p{L}\\p{N}_$]*\\.)*[\\p{L}_$][\\p{L}\\p{N}_$]*"; - return Pattern.matches(regex, fqcn); - } - @Override public int hashCode() { return Objects.hash(className, initProperties); @@ -134,4 +132,12 @@ public boolean equals(Object o) { && this.getInitProperties().equals(that.getInitProperties()); } + public static boolean isClassNameValid(String className) { + if (StringUtils.isBlank(className)) { + return false; + } + String regex = "([\\p{L}_$][\\p{L}\\p{N}_$]*\\.)*[\\p{L}_$][\\p{L}\\p{N}_$]*"; + return Pattern.matches(regex, className); + } + } diff --git a/geode-management/src/main/java/org/apache/geode/management/internal/ManagementHelper.java b/geode-management/src/main/java/org/apache/geode/management/internal/ManagementHelper.java deleted file mode 100644 index 47c76507e31c..000000000000 --- a/geode-management/src/main/java/org/apache/geode/management/internal/ManagementHelper.java +++ /dev/null @@ -1,37 +0,0 @@ - -/* - * 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.geode.management.internal; - -import java.util.regex.Pattern; - -/** - * Contains static helper methods for management code. - */ -public class ManagementHelper { - private ManagementHelper() { - // no instances of this class allowed - } - - /** - * @return true if the given className is valid - */ - public static boolean isClassNameValid(String className) { - String regex = "([\\p{L}_$][\\p{L}\\p{N}_$]*\\.)*[\\p{L}_$][\\p{L}\\p{N}_$]*"; - return Pattern.matches(regex, className); - } - -} diff --git a/geode-management/src/test/java/org/apache/geode/management/internal/ManagementHelperTest.java b/geode-management/src/test/java/org/apache/geode/management/internal/ManagementHelperTest.java deleted file mode 100644 index cadb9355e128..000000000000 --- a/geode-management/src/test/java/org/apache/geode/management/internal/ManagementHelperTest.java +++ /dev/null @@ -1,76 +0,0 @@ -/* - * 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.geode.management.internal; - -import static org.assertj.core.api.Assertions.assertThat; - -import org.junit.Test; - -public class ManagementHelperTest { - @Test - public void isValidClassNameGivenEmptyStringReturnsFalse() { - assertThat(ManagementHelper.isClassNameValid("")).isFalse(); - } - - @Test - public void isValidClassNameGivenDashReturnsFalse() { - assertThat(ManagementHelper.isClassNameValid("-")).isFalse(); - } - - @Test - public void isValidClassNameGivenSpaceReturnsFalse() { - assertThat(ManagementHelper.isClassNameValid(" ")).isFalse(); - } - - @Test - public void isValidClassNameGivenCommaReturnsFalse() { - assertThat(ManagementHelper.isClassNameValid(",")).isFalse(); - } - - @Test - public void isValidClassNameGivenLeadingDotReturnsFalse() { - assertThat(ManagementHelper.isClassNameValid(".a")).isFalse(); - } - - @Test - public void isValidClassNameGivenTrailingDotReturnsFalse() { - assertThat(ManagementHelper.isClassNameValid("a.")).isFalse(); - } - - @Test - public void isValidClassNameGivenTwoDotsReturnsFalse() { - assertThat(ManagementHelper.isClassNameValid("a..a")).isFalse(); - } - - @Test - public void isValidClassNameGivenNameThatStartsWithDigitReturnsFalse() { - assertThat(ManagementHelper.isClassNameValid("9a")).isFalse(); - } - - @Test - public void isValidClassNameGivenNameReturnsTrue() { - assertThat(ManagementHelper.isClassNameValid("a9")).isTrue(); - } - - @Test - public void isValidClassNameGivenDotDelimitedNamesReturnsTrue() { - assertThat(ManagementHelper.isClassNameValid("$a1._b2.c3$")).isTrue(); - } - - @Test - public void isValidClassNameGivenMiddleNameThatStartsWithDigitReturnsFalse() { - assertThat(ManagementHelper.isClassNameValid("a1.2b.c3")).isFalse(); - } -} diff --git a/geode-web-management/src/integrationTest/java/org/apache/geode/management/internal/rest/PdxManagementTest.java b/geode-web-management/src/integrationTest/java/org/apache/geode/management/internal/rest/PdxManagementTest.java index 6ab1faf3a49e..4a2ea12820e0 100644 --- a/geode-web-management/src/integrationTest/java/org/apache/geode/management/internal/rest/PdxManagementTest.java +++ b/geode-web-management/src/integrationTest/java/org/apache/geode/management/internal/rest/PdxManagementTest.java @@ -85,4 +85,16 @@ public void success() throws Exception { } } + + @Test + public void invalidClassName() throws Exception { + String json = "{\"readSerialized\":true,\"pdxSerializer\":{\"className\":\"class name\"}}"; + + context.perform(post("/experimental/configurations/pdx") + .with(httpBasic("clusterManage", "clusterManage")) + .content(json)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.statusCode", is("ILLEGAL_ARGUMENT"))) + .andExpect(jsonPath("$.statusMessage", containsString("Invalid className"))); + } }