This is an automated email from the ASF dual-hosted git repository. klund pushed a commit to branch support/1.14 in repository https://gitbox.apache.org/repos/asf/geode.git
commit 5d9e4b5b54b84c2de28b670fe6feb0b0c44443f7 Author: Kirk Lund <kl...@apache.org> AuthorDate: Thu Feb 10 10:53:21 2022 -0800 GEODE-9980: Improve error handling of serial filters (#7355) Improves the error handling of serial filter configuration and filtering. The API throws a runtime exception wrapping any thrown exceptions. When geode.enableGlobalSerialFilter is FALSE: * Startup succeeds without throwing any exceptions even if an older version of Java throws "java.lang.ClassNotFoundException sun.misc.ObjectInputFilter". When geode.enableGlobalSerialFilter is TRUE: * Startup fails by throwing an exception when configuration of serial filter fails for any reason. Renames ObjectInputFilter to avoid confusion with the Java interface of the same name. Fixes a bug found in ReflectiveFacadeGlobalSerialFilterTest which resulted in configuring a non-mocked process-wide serial filter that polluted the JVM for all later tests. Fixes other minor details in LocatorLauncher, InternalDataSerializer and various related tests. (cherry picked from commit ce57e9fd2b8b644cadc469209e12e4fbd281e0d9) (cherry picked from commit 6ecdce0d9ef9040f3bbb90c8ea4ff5eb99d712fc) --- .../filter/SerialFilterAssertions.java | 30 +++--- .../apache/geode/distributed/LocatorLauncher.java | 16 ++- .../apache/geode/distributed/ServerLauncher.java | 16 ++- .../geode/internal/InternalDataSerializer.java | 29 ++++-- .../geode/management/internal/ManagementAgent.java | 7 +- .../geode/distributed/LocatorLauncherTest.java | 2 +- ...ationWhenFilterIsAlreadySetIntegrationTest.java | 53 ++++++++++ ...enObjectInputFilterNotFoundIntegrationTest.java | 108 ++++++++++++++++++++ ...nputFilterApiSetFilterBlankIntegrationTest.java | 1 - ....java => FilterAlreadyConfiguredException.java} | 22 +++-- .../serialization/filter/FilterConfiguration.java | 2 +- .../serialization/filter/GlobalSerialFilter.java | 5 +- .../filter/GlobalSerialFilterConfiguration.java | 85 ++++++---------- .../filter/JmxSerialFilterConfiguration.java | 15 +-- ...nputFilter.java => NullStreamSerialFilter.java} | 4 +- .../filter/ObjectInputFilterInvocationHandler.java | 109 +++++++++++++++++++++ .../filter/ReflectiveFacadeGlobalSerialFilter.java | 43 ++++++-- .../ReflectiveFacadeGlobalSerialFilterFactory.java | 20 ++-- .../ReflectiveFacadeObjectInputFilterFactory.java | 61 ------------ ...ava => ReflectiveFacadeStreamSerialFilter.java} | 49 +++++++-- ...ReflectiveFacadeStreamSerialFilterFactory.java} | 35 +++---- .../filter/ReflectiveObjectInputFilterApi.java | 59 +++-------- ...ectInputFilter.java => StreamSerialFilter.java} | 7 +- ...Factory.java => StreamSerialFilterFactory.java} | 4 +- ...ertyGlobalSerialFilterConfigurationFactory.java | 24 ++++- ....java => UnableToSetSerialFilterException.java} | 22 +++-- ...anctioned-geode-serialization-serializables.txt | 2 + .../GlobalSerialFilterConfigurationTest.java | 84 ++++++++++++---- .../JmxSerialFilterConfigurationFactoryTest.java | 9 ++ .../filter/JmxSerialFilterConfigurationTest.java | 50 ++++++---- .../filter/NullObjectInputFilterTest.java | 4 +- .../ObjectInputFilterInvocationHandlerTest.java | 97 ++++++++++++++++++ ...lectiveFacadeGlobalSerialFilterFactoryTest.java | 65 ++++++------ .../ReflectiveFacadeGlobalSerialFilterTest.java | 88 ++++++++++++++--- ...flectiveFacadeObjectInputFilterFactoryTest.java | 12 +-- .../ReflectiveFacadeObjectInputFilterTest.java | 77 ++++++++++----- .../ReflectiveObjectInputFilterApiFactoryTest.java | 9 ++ .../filter/ReflectiveObjectInputFilterApiTest.java | 11 ++- .../filter/SerialFilterAssertions.java | 53 +++++----- ...GlobalSerialFilterConfigurationFactoryTest.java | 101 ++++++++++++++++--- ...rtyJmxSerialFilterConfigurationFactoryTest.java | 12 ++- 41 files changed, 1067 insertions(+), 435 deletions(-) diff --git a/geode-core/src/integrationTest/java/org/apache/geode/internal/serialization/filter/SerialFilterAssertions.java b/geode-core/src/integrationTest/java/org/apache/geode/internal/serialization/filter/SerialFilterAssertions.java index b6d552a..8432e7d 100644 --- a/geode-core/src/integrationTest/java/org/apache/geode/internal/serialization/filter/SerialFilterAssertions.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/internal/serialization/filter/SerialFilterAssertions.java @@ -30,35 +30,29 @@ public class SerialFilterAssertions { public static void assertThatSerialFilterIsNull() throws InvocationTargetException, IllegalAccessException { - boolean exists = API.getSerialFilter() != null; - assertThat(exists) - .as("ObjectInputFilter$Config.getSerialFilter() is null") - .isFalse(); + assertThat(API.getSerialFilter()) + .as("ObjectInputFilter$Config.getSerialFilter()") + .isNull(); } public static void assertThatSerialFilterIsNotNull() throws InvocationTargetException, IllegalAccessException { - boolean exists = API.getSerialFilter() != null; - assertThat(exists) - .as("ObjectInputFilter$Config.getSerialFilter() is not null") - .isTrue(); + assertThat(API.getSerialFilter()) + .as("ObjectInputFilter$Config.getSerialFilter()") + .isNotNull(); } public static void assertThatSerialFilterIsSameAs(Object objectInputFilter) throws InvocationTargetException, IllegalAccessException { - Object currentFilter = API.getSerialFilter(); - boolean sameIdentity = currentFilter == objectInputFilter; - assertThat(sameIdentity) - .as("ObjectInputFilter$Config.getSerialFilter() is same as parameter") - .isTrue(); + assertThat(API.getSerialFilter()) + .as("ObjectInputFilter$Config.getSerialFilter()") + .isSameAs(objectInputFilter); } public static void assertThatSerialFilterIsNotSameAs(Object objectInputFilter) throws InvocationTargetException, IllegalAccessException { - Object currentFilter = API.getSerialFilter(); - boolean sameIdentity = currentFilter == objectInputFilter; - assertThat(sameIdentity) - .as("ObjectInputFilter$Config.getSerialFilter() is same as parameter") - .isFalse(); + assertThat(API.getSerialFilter()) + .as("ObjectInputFilter$Config.getSerialFilter()") + .isNotSameAs(objectInputFilter); } } diff --git a/geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java b/geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java index fa9f7cc..e64d6b1 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java @@ -89,6 +89,7 @@ import org.apache.geode.internal.process.ProcessUtils; import org.apache.geode.internal.process.UnableToControlProcessException; import org.apache.geode.internal.security.SecurableCommunicationChannel; import org.apache.geode.internal.serialization.filter.SystemPropertyGlobalSerialFilterConfigurationFactory; +import org.apache.geode.internal.serialization.filter.UnableToSetSerialFilterException; import org.apache.geode.lang.AttachAPINotFoundException; import org.apache.geode.logging.internal.log4j.api.LogService; import org.apache.geode.management.internal.util.HostUtils; @@ -704,10 +705,7 @@ public class LocatorLauncher extends AbstractLauncher<String> { if (isStartable()) { INSTANCE.compareAndSet(null, this); - boolean serializationFilterConfigured = - new SystemPropertyGlobalSerialFilterConfigurationFactory() - .create(new DistributedSerializableObjectConfig(getDistributedSystemProperties())) - .configure(); + boolean serializationFilterConfigured = configureGlobalSerialFilterIfEnabled(); try { this.process = @@ -772,6 +770,16 @@ public class LocatorLauncher extends AbstractLauncher<String> { } } + private boolean configureGlobalSerialFilterIfEnabled() { + try { + return new SystemPropertyGlobalSerialFilterConfigurationFactory() + .create(new DistributedSerializableObjectConfig(getDistributedSystemProperties())) + .configure(); + } catch (UnableToSetSerialFilterException e) { + throw new RuntimeException(e); + } + } + @Override protected Properties getDistributedSystemProperties() { return super.getDistributedSystemProperties(getProperties()); diff --git a/geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java b/geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java index d1b6c97..7458206 100755 --- a/geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java @@ -96,6 +96,7 @@ import org.apache.geode.internal.process.ProcessLauncherContext; import org.apache.geode.internal.process.ProcessType; import org.apache.geode.internal.process.UnableToControlProcessException; import org.apache.geode.internal.serialization.filter.SystemPropertyGlobalSerialFilterConfigurationFactory; +import org.apache.geode.internal.serialization.filter.UnableToSetSerialFilterException; import org.apache.geode.lang.AttachAPINotFoundException; import org.apache.geode.logging.internal.executors.LoggingThread; import org.apache.geode.logging.internal.log4j.api.LogService; @@ -789,10 +790,7 @@ public class ServerLauncher extends AbstractLauncher<String> { if (isStartable()) { INSTANCE.compareAndSet(null, this); - boolean serializationFilterConfigured = - new SystemPropertyGlobalSerialFilterConfigurationFactory() - .create(new DistributedSerializableObjectConfig(getDistributedSystemProperties())) - .configure(); + boolean serializationFilterConfigured = configureGlobalSerialFilterIfEnabled(); try { process = getControllableProcess(); @@ -891,6 +889,16 @@ public class ServerLauncher extends AbstractLauncher<String> { getServiceName(), getWorkingDirectory(), getId())); } + private boolean configureGlobalSerialFilterIfEnabled() { + try { + return new SystemPropertyGlobalSerialFilterConfigurationFactory() + .create(new DistributedSerializableObjectConfig(getDistributedSystemProperties())) + .configure(); + } catch (UnableToSetSerialFilterException e) { + throw new RuntimeException(e); + } + } + Cache createCache(Properties gemfireProperties) { Iterable<ServerLauncherCacheProvider> loader = getServerLauncherCacheProviders(); for (ServerLauncherCacheProvider provider : loader) { diff --git a/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java b/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java index 064044f..ab3c53f 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java +++ b/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java @@ -69,6 +69,7 @@ import java.util.concurrent.TimeUnit; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; +import org.jetbrains.annotations.TestOnly; import org.apache.geode.CancelException; import org.apache.geode.CanonicalInstantiator; @@ -117,12 +118,13 @@ import org.apache.geode.internal.serialization.SerializationContext; import org.apache.geode.internal.serialization.SerializationVersions; import org.apache.geode.internal.serialization.StaticSerialization; import org.apache.geode.internal.serialization.VersionedDataStream; -import org.apache.geode.internal.serialization.filter.NullObjectInputFilter; -import org.apache.geode.internal.serialization.filter.ObjectInputFilter; -import org.apache.geode.internal.serialization.filter.ObjectInputFilterFactory; -import org.apache.geode.internal.serialization.filter.ReflectiveFacadeObjectInputFilterFactory; +import org.apache.geode.internal.serialization.filter.NullStreamSerialFilter; +import org.apache.geode.internal.serialization.filter.ReflectiveFacadeStreamSerialFilterFactory; import org.apache.geode.internal.serialization.filter.SanctionedSerializablesService; import org.apache.geode.internal.serialization.filter.SerializableObjectConfig; +import org.apache.geode.internal.serialization.filter.StreamSerialFilter; +import org.apache.geode.internal.serialization.filter.StreamSerialFilterFactory; +import org.apache.geode.internal.serialization.filter.UnableToSetSerialFilterException; import org.apache.geode.internal.util.concurrent.CopyOnWriteHashMap; import org.apache.geode.logging.internal.log4j.api.LogService; import org.apache.geode.pdx.NonPortableClassException; @@ -286,12 +288,12 @@ public abstract class InternalDataSerializer extends DataSerializer { "org.apache.geode.cache.query.internal.cq.ServerCQImpl"; @Immutable - private static final ObjectInputFilter defaultSerializationFilter = new NullObjectInputFilter(); + private static final StreamSerialFilter defaultSerializationFilter = new NullStreamSerialFilter(); /** * A deserialization filter for ObjectInputStreams */ @MakeNotStatic - private static ObjectInputFilter serializationFilter = defaultSerializationFilter; + private static StreamSerialFilter serializationFilter = defaultSerializationFilter; /** * support for old GemFire clients and WAN sites - needed to enable moving from GemFire to Geode */ @@ -418,14 +420,14 @@ public abstract class InternalDataSerializer extends DataSerializer { Collection<SanctionedSerializablesService> services) { logger.info("initializing InternalDataSerializer with {} services", services.size()); - ObjectInputFilterFactory objectInputFilterFactory = - new ReflectiveFacadeObjectInputFilterFactory(); + StreamSerialFilterFactory objectInputFilterFactory = + new ReflectiveFacadeStreamSerialFilterFactory(); serializationFilter = objectInputFilterFactory.create(config, loadSanctionedClassNames(services)); } - @VisibleForTesting + @TestOnly static void clearSerializationFilter() { serializationFilter = defaultSerializationFilter; } @@ -2687,7 +2689,14 @@ public abstract class InternalDataSerializer extends DataSerializer { } ObjectInput ois = new DSObjectInputStream(stream); - serializationFilter.setFilterOn((ObjectInputStream) ois); + + try { + serializationFilter.setFilterOn((ObjectInputStream) ois); + } catch (UnableToSetSerialFilterException e) { + // maintain existing behavior for validate-serializable-objects + throw new UnsupportedOperationException(e); + } + if (stream instanceof VersionedDataStream) { KnownVersion v = ((VersionedDataStream) stream).getVersion(); if (KnownVersion.CURRENT != v && v != null) { diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java b/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java index 22d50bd..85571c2 100755 --- a/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java @@ -66,6 +66,7 @@ import org.apache.geode.internal.security.SecurableCommunicationChannel; import org.apache.geode.internal.security.SecurityService; import org.apache.geode.internal.security.shiro.JMXShiroAuthenticator; import org.apache.geode.internal.serialization.filter.FilterConfiguration; +import org.apache.geode.internal.serialization.filter.UnableToSetSerialFilterException; import org.apache.geode.internal.tcp.TCPConduit; import org.apache.geode.logging.internal.log4j.api.LogService; import org.apache.geode.management.ManagementException; @@ -135,7 +136,11 @@ public class ManagementAgent { } public synchronized void startAgent() { - filterConfiguration.configure(); + try { + filterConfiguration.configure(); + } catch (UnableToSetSerialFilterException e) { + throw new RuntimeException(e); + } loadWebApplications(); if (!this.running && this.config.getJmxManagerPort() != 0) { diff --git a/geode-core/src/distributedTest/java/org/apache/geode/distributed/LocatorLauncherTest.java b/geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java similarity index 97% rename from geode-core/src/distributedTest/java/org/apache/geode/distributed/LocatorLauncherTest.java rename to geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java index d7ba3c5..5bcb6d4 100644 --- a/geode-core/src/distributedTest/java/org/apache/geode/distributed/LocatorLauncherTest.java +++ b/geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java @@ -30,7 +30,7 @@ import org.apache.geode.distributed.internal.InternalLocator; public class LocatorLauncherTest { @Test - public void canBeMocked() throws Exception { + public void canBeMocked() { LocatorLauncher mockLocatorLauncher = mock(LocatorLauncher.class); InternalLocator mockInternalLocator = mock(InternalLocator.class); diff --git a/geode-serialization/src/integrationTest/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilterConfigurationWhenFilterIsAlreadySetIntegrationTest.java b/geode-serialization/src/integrationTest/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilterConfigurationWhenFilterIsAlreadySetIntegrationTest.java new file mode 100644 index 0000000..89f3b91 --- /dev/null +++ b/geode-serialization/src/integrationTest/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilterConfigurationWhenFilterIsAlreadySetIntegrationTest.java @@ -0,0 +1,53 @@ +/* + * 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.internal.serialization.filter; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; +import static org.mockito.Mockito.mock; + +import java.lang.reflect.InvocationTargetException; + +import org.junit.Before; +import org.junit.Test; + +public class GlobalSerialFilterConfigurationWhenFilterIsAlreadySetIntegrationTest { + + @Before + public void filterIsAlreadySet() throws InvocationTargetException, IllegalAccessException { + ObjectInputFilterApiFactory factory = new ReflectiveObjectInputFilterApiFactory(); + ObjectInputFilterApi api = factory.createObjectInputFilterApi(); + Object filter = api.createFilter("*"); + api.setSerialFilter(filter); + } + + @Test + public void throwsObjectInputFilterException_whenFilterIsAlreadySet() { + FilterConfiguration configuration = + new GlobalSerialFilterConfiguration(mock(SerializableObjectConfig.class)); + + Throwable thrown = catchThrowable(() -> { + configuration.configure(); + }); + + assertThat(thrown) + .isInstanceOf(UnableToSetSerialFilterException.class) + .hasMessage( + "Unable to configure a global serialization filter because filter has already been set non-null.") + .hasCauseInstanceOf(InvocationTargetException.class) + .hasRootCauseInstanceOf(IllegalStateException.class) + .hasRootCauseMessage("Serial filter can only be set once"); + } +} diff --git a/geode-serialization/src/integrationTest/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilterConfigurationWhenObjectInputFilterNotFoundIntegrationTest.java b/geode-serialization/src/integrationTest/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilterConfigurationWhenObjectInputFilterNotFoundIntegrationTest.java new file mode 100644 index 0000000..92d9937 --- /dev/null +++ b/geode-serialization/src/integrationTest/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilterConfigurationWhenObjectInputFilterNotFoundIntegrationTest.java @@ -0,0 +1,108 @@ +/* + * 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.internal.serialization.filter; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.catchThrowable; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoInteractions; + +import org.apache.logging.log4j.Logger; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.contrib.java.lang.system.RestoreSystemProperties; + +public class GlobalSerialFilterConfigurationWhenObjectInputFilterNotFoundIntegrationTest { + + private final boolean supportsObjectInputFilter = false; + + private ReflectiveFacadeGlobalSerialFilterFactory globalSerialFilterFactory_throws; + private SerializableObjectConfig config; + private Logger logger; + + @Rule + public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties(); + + @Before + public void whenObjectInputFilter_classNotFound() { + globalSerialFilterFactory_throws = + new ReflectiveFacadeGlobalSerialFilterFactory(() -> { + throw new UnsupportedOperationException( + new ClassNotFoundException("sun.misc.ObjectInputFilter")); + }); + } + + @Before + public void setUpMocks() { + config = mock(SerializableObjectConfig.class); + logger = mock(Logger.class); + } + + @Test + public void doesNotThrow_whenEnableGlobalSerialFilterIsFalse_andObjectInputFilterClassNotFound() { + System.clearProperty("geode.enableGlobalSerialFilter"); + GlobalSerialFilterConfigurationFactory factory = + new SystemPropertyGlobalSerialFilterConfigurationFactory(); + FilterConfiguration configuration = factory.create(config); + + assertThatCode(configuration::configure) + .doesNotThrowAnyException(); + } + + @Test + public void logsNothing_whenEnableGlobalSerialFilterIsFalse_andObjectInputFilterClassNotFound() + throws UnableToSetSerialFilterException { + System.clearProperty("geode.enableGlobalSerialFilter"); + GlobalSerialFilterConfigurationFactory factory = + new SystemPropertyGlobalSerialFilterConfigurationFactory(() -> supportsObjectInputFilter); + FilterConfiguration configuration = factory.create(config); + + configuration.configure(); + + verifyNoInteractions(logger); + } + + @Test + public void doesNotConfigureOrThrow_whenEnableGlobalSerialFilterIsTrue_andObjectInputFilterClassNotFound() + throws UnableToSetSerialFilterException { + System.setProperty("geode.enableGlobalSerialFilter", "true"); + GlobalSerialFilterConfigurationFactory factory = + new SystemPropertyGlobalSerialFilterConfigurationFactory(() -> supportsObjectInputFilter); + FilterConfiguration configuration = factory.create(config); + + boolean result = configuration.configure(); + + assertThat(result).isFalse(); + } + + @Test + public void logsWarning_whenEnableGlobalSerialFilterIsTrue_andObjectInputFilterClassNotFound() { + System.setProperty("geode.enableGlobalSerialFilter", "true"); + FilterConfiguration configuration = new GlobalSerialFilterConfiguration( + config, logger, globalSerialFilterFactory_throws); + + Throwable thrown = catchThrowable(() -> { + configuration.configure(); + }); + + assertThat(thrown) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessage("java.lang.ClassNotFoundException: sun.misc.ObjectInputFilter") + .hasRootCauseInstanceOf(ClassNotFoundException.class) + .hasRootCauseMessage("sun.misc.ObjectInputFilter"); + } +} diff --git a/geode-serialization/src/integrationTest/java/org/apache/geode/internal/serialization/filter/ReflectiveObjectInputFilterApiSetFilterBlankIntegrationTest.java b/geode-serialization/src/integrationTest/java/org/apache/geode/internal/serialization/filter/ReflectiveObjectInputFilterApiSetFilterBlankIntegrationTest.java index 6155c8b..d5714f6 100644 --- a/geode-serialization/src/integrationTest/java/org/apache/geode/internal/serialization/filter/ReflectiveObjectInputFilterApiSetFilterBlankIntegrationTest.java +++ b/geode-serialization/src/integrationTest/java/org/apache/geode/internal/serialization/filter/ReflectiveObjectInputFilterApiSetFilterBlankIntegrationTest.java @@ -44,7 +44,6 @@ public class ReflectiveObjectInputFilterApiSetFilterBlankIntegrationTest { if (isJavaVersionAtLeast(JAVA_9)) { apiPackage = JAVA_IO; } - } @Test diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/NullObjectInputFilter.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/FilterAlreadyConfiguredException.java similarity index 58% copy from geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/NullObjectInputFilter.java copy to geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/FilterAlreadyConfiguredException.java index e25c073..e42990e 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/NullObjectInputFilter.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/FilterAlreadyConfiguredException.java @@ -14,15 +14,23 @@ */ package org.apache.geode.internal.serialization.filter; -import java.io.ObjectInputStream; - /** - * Implementation of {@code ObjectInputFilter} that does nothing. + * Checked exception thrown when configuring a filter fails because a non-null filter already + * exists. All uses of this exception are caught and rethrown before reaching the user. */ -public class NullObjectInputFilter implements ObjectInputFilter { +public class FilterAlreadyConfiguredException extends UnableToSetSerialFilterException { + + public FilterAlreadyConfiguredException(String message) { + super(message); + } - @Override - public void setFilterOn(ObjectInputStream objectInputStream) { - // Do nothing, this is the case where we don't filter. + public FilterAlreadyConfiguredException(String message, Throwable cause) { + super(message, cause); } + + public FilterAlreadyConfiguredException(Throwable cause) { + super(cause); + } + + private static final long serialVersionUID = -6102549374563510704L; } diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/FilterConfiguration.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/FilterConfiguration.java index 9a51a28..cf12790 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/FilterConfiguration.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/FilterConfiguration.java @@ -23,5 +23,5 @@ public interface FilterConfiguration { /** * Returns true if serialization filter was successfully configured. */ - boolean configure(); + boolean configure() throws UnableToSetSerialFilterException; } diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilter.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilter.java index 87c19bd..7858878 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilter.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilter.java @@ -22,6 +22,9 @@ interface GlobalSerialFilter { /** * Sets this serialization filter as the process-wide filter. + * + * @throws FilterAlreadyConfiguredException if a non-null serialization filter already exists + * @throws UnableToSetSerialFilterException if there's any failure setting a serialization filter */ - void setFilter(); + void setFilter() throws UnableToSetSerialFilterException; } diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilterConfiguration.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilterConfiguration.java index 7d312ac..1bfd028 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilterConfiguration.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilterConfiguration.java @@ -14,13 +14,10 @@ */ package org.apache.geode.internal.serialization.filter; -import static java.util.Objects.nonNull; -import static org.apache.commons.lang3.exception.ExceptionUtils.getRootCause; import static org.apache.geode.internal.serialization.filter.SanctionedSerializables.loadSanctionedClassNames; import static org.apache.geode.internal.serialization.filter.SanctionedSerializables.loadSanctionedSerializablesServices; import java.util.Set; -import java.util.function.Consumer; import java.util.function.Supplier; import org.apache.logging.log4j.Logger; @@ -37,7 +34,7 @@ class GlobalSerialFilterConfiguration implements FilterConfiguration { private final SerializableObjectConfig serializableObjectConfig; private final FilterPatternFactory filterPatternFactory; private final Supplier<Set<String>> sanctionedClassesSupplier; - private final Consumer<String> logger; + private final Logger logger; private final GlobalSerialFilterFactory globalSerialFilterFactory; /** @@ -45,15 +42,22 @@ class GlobalSerialFilterConfiguration implements FilterConfiguration { */ GlobalSerialFilterConfiguration(SerializableObjectConfig serializableObjectConfig) { this(serializableObjectConfig, - new DefaultFilterPatternFactory(), - () -> loadSanctionedClassNames(loadSanctionedSerializablesServices()), - LOGGER::info, (pattern, sanctionedClasses) -> new ReflectiveFacadeGlobalSerialFilterFactory() .create(pattern, sanctionedClasses)); } - GlobalSerialFilterConfiguration(SerializableObjectConfig serializableObjectConfig, - Consumer<String> logger, GlobalSerialFilterFactory globalSerialFilterFactory) { + GlobalSerialFilterConfiguration( + SerializableObjectConfig serializableObjectConfig, + GlobalSerialFilterFactory globalSerialFilterFactory) { + this(serializableObjectConfig, + LOGGER, + globalSerialFilterFactory); + } + + GlobalSerialFilterConfiguration( + SerializableObjectConfig serializableObjectConfig, + Logger logger, + GlobalSerialFilterFactory globalSerialFilterFactory) { this(serializableObjectConfig, new DefaultFilterPatternFactory(), () -> loadSanctionedClassNames(loadSanctionedSerializablesServices()), @@ -65,7 +69,7 @@ class GlobalSerialFilterConfiguration implements FilterConfiguration { SerializableObjectConfig serializableObjectConfig, FilterPatternFactory filterPatternFactory, Supplier<Set<String>> sanctionedClassesSupplier, - Consumer<String> logger, + Logger logger, GlobalSerialFilterFactory globalSerialFilterFactory) { this.serializableObjectConfig = serializableObjectConfig; this.filterPatternFactory = filterPatternFactory; @@ -75,48 +79,23 @@ class GlobalSerialFilterConfiguration implements FilterConfiguration { } @Override - public boolean configure() { - try { - // enable validate-serializable-objects - serializableObjectConfig.setValidateSerializableObjects(true); - - // create a GlobalSerialFilter - String pattern = filterPatternFactory - .create(serializableObjectConfig.getSerializableObjectFilterIfEnabled()); - Set<String> sanctionedClasses = sanctionedClassesSupplier.get(); - GlobalSerialFilter globalSerialFilter = - globalSerialFilterFactory.create(pattern, sanctionedClasses); - - // invoke setFilter on GlobalSerialFilter to set the process-wide filter - globalSerialFilter.setFilter(); - - // log statement that filter is now configured - logger.accept("Global serial filter is now configured."); - return true; - - } catch (UnsupportedOperationException e) { - if (hasRootCauseWithMessage(e, IllegalStateException.class, - "Serial filter can only be set once")) { - - // log statement that filter was already configured - logger.accept("Global serial filter is already configured."); - } - return false; - } - } - - private static boolean hasRootCauseWithMessage(Throwable throwable, - Class<? extends Throwable> causeClass, String message) { - Throwable rootCause = getRootCause(throwable); - return isInstanceOf(rootCause, causeClass) && hasMessage(rootCause, message); - } - - private static boolean isInstanceOf(Throwable throwable, Class<? extends Throwable> causeClass) { - return nonNull(throwable) && throwable.getClass().equals(causeClass); - } - - private static boolean hasMessage(Throwable throwable, String message) { - return nonNull(throwable) && throwable.getMessage().equalsIgnoreCase(message); + public boolean configure() throws UnableToSetSerialFilterException { + // enable validate-serializable-objects + serializableObjectConfig.setValidateSerializableObjects(true); + + // create a GlobalSerialFilter + String pattern = filterPatternFactory + .create(serializableObjectConfig.getSerializableObjectFilterIfEnabled()); + Set<String> sanctionedClasses = sanctionedClassesSupplier.get(); + GlobalSerialFilter globalSerialFilter = + globalSerialFilterFactory.create(pattern, sanctionedClasses); + + // invoke setFilter on GlobalSerialFilter to set the process-wide filter + globalSerialFilter.setFilter(); + + // log statement that filter is now configured + logger.info("Global serialization filter is now configured."); + return true; } /** @@ -132,7 +111,7 @@ class GlobalSerialFilterConfiguration implements FilterConfiguration { /** * Default implementation of {@code FilterPatternFactory}. */ - static class DefaultFilterPatternFactory implements FilterPatternFactory { + public static class DefaultFilterPatternFactory implements FilterPatternFactory { @Override public String create(String optionalSerializableObjectFilter) { diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/JmxSerialFilterConfiguration.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/JmxSerialFilterConfiguration.java index 21b0908..86c7191 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/JmxSerialFilterConfiguration.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/JmxSerialFilterConfiguration.java @@ -16,8 +16,6 @@ package org.apache.geode.internal.serialization.filter; import static org.apache.commons.lang3.StringUtils.isBlank; -import java.util.function.Consumer; - import org.apache.logging.log4j.Logger; import org.apache.geode.annotations.VisibleForTesting; @@ -42,17 +40,20 @@ class JmxSerialFilterConfiguration implements FilterConfiguration { private final String key; private final String value; - private final Consumer<String> logger; + private final Logger logger; /** * Constructs instance for the specified system property and filter pattern. */ JmxSerialFilterConfiguration(String property, String pattern) { - this(property, pattern, LOGGER::info); + this(property, pattern, LOGGER); } @VisibleForTesting - JmxSerialFilterConfiguration(String property, String pattern, Consumer<String> logger) { + JmxSerialFilterConfiguration( + String property, + String pattern, + Logger logger) { key = property; value = pattern; this.logger = logger; @@ -62,11 +63,11 @@ class JmxSerialFilterConfiguration implements FilterConfiguration { public boolean configure() { if (isBlank(System.getProperty(key))) { System.setProperty(key, value); - logger.accept("System property '" + key + "' is now configured with '" + value + "'."); + logger.info("System property '" + key + "' is now configured with '" + value + "'."); return true; } - logger.accept("System property '" + key + "' is already configured."); + logger.info("System property '" + key + "' is already configured."); return false; } } diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/NullObjectInputFilter.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/NullStreamSerialFilter.java similarity index 88% copy from geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/NullObjectInputFilter.java copy to geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/NullStreamSerialFilter.java index e25c073..226159b 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/NullObjectInputFilter.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/NullStreamSerialFilter.java @@ -17,9 +17,9 @@ package org.apache.geode.internal.serialization.filter; import java.io.ObjectInputStream; /** - * Implementation of {@code ObjectInputFilter} that does nothing. + * Implementation of {@code StreamSerialFilter} that does nothing. */ -public class NullObjectInputFilter implements ObjectInputFilter { +public class NullStreamSerialFilter implements StreamSerialFilter { @Override public void setFilterOn(ObjectInputStream objectInputStream) { diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ObjectInputFilterInvocationHandler.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ObjectInputFilterInvocationHandler.java new file mode 100644 index 0000000..7c084b6 --- /dev/null +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ObjectInputFilterInvocationHandler.java @@ -0,0 +1,109 @@ +/* + * 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.internal.serialization.filter; + +import static java.util.Collections.unmodifiableCollection; +import static java.util.Objects.requireNonNull; + +import java.io.InvalidClassException; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.util.Collection; + +import org.apache.logging.log4j.Logger; + +import org.apache.geode.logging.internal.log4j.api.LogService; + +class ObjectInputFilterInvocationHandler implements InvocationHandler { + + private static final Logger logger = LogService.getLogger(); + + private final Method ObjectInputFilter_checkInput; + private final Method ObjectInputFilter_FilterInfo_serialClass; + private final Object ObjectInputFilter_Status_ALLOWED; + private final Object ObjectInputFilter_Status_REJECTED; + + private final Object objectInputFilter; + private final Collection<String> sanctionedClasses; + + ObjectInputFilterInvocationHandler( + Method ObjectInputFilter_checkInput, + Method ObjectInputFilter_FilterInfo_serialClass, + Object ObjectInputFilter_Status_ALLOWED, + Object ObjectInputFilter_Status_REJECTED, + Object objectInputFilter, + Collection<String> sanctionedClasses) { + this.ObjectInputFilter_checkInput = ObjectInputFilter_checkInput; + this.ObjectInputFilter_FilterInfo_serialClass = ObjectInputFilter_FilterInfo_serialClass; + this.ObjectInputFilter_Status_ALLOWED = ObjectInputFilter_Status_ALLOWED; + this.ObjectInputFilter_Status_REJECTED = ObjectInputFilter_Status_REJECTED; + this.objectInputFilter = objectInputFilter; + this.sanctionedClasses = unmodifiableCollection(sanctionedClasses); + } + + @Override + public Object invoke(Object proxy, Method method, Object[] args) + throws IllegalAccessException, IllegalArgumentException, + java.lang.reflect.InvocationTargetException { + if (!"checkInput".equals(method.getName())) { + // delegate to the actual objectInputFilter instance for any method other than checkInput + return method.invoke(objectInputFilter, args); + } + + requireNonNull(args, "Single argument FilterInfo is null"); + + if (args.length != 1) { + throw new IllegalArgumentException("Single argument FilterInfo is required"); + } + + // fetch the class of the serialized instance + Object objectInputFilter_filterInfo = args[0]; + Class<?> serialClass = + (Class<?>) ObjectInputFilter_FilterInfo_serialClass.invoke(objectInputFilter_filterInfo); + if (serialClass == null) { // no class to check, so nothing to accept-list + return ObjectInputFilter_checkInput.invoke(objectInputFilter, objectInputFilter_filterInfo); + } + + // check sanctionedClasses to determine if the name of the class is ALLOWED + String serialClassName = serialClass.getName(); + if (serialClass.isArray()) { + serialClassName = serialClass.getComponentType().getName(); + } + if (sanctionedClasses.contains(serialClassName)) { + return ObjectInputFilter_Status_ALLOWED; + } + + // check the filter to determine if the class is ALLOWED + Object objectInputFilter_Status = + ObjectInputFilter_checkInput.invoke(objectInputFilter, objectInputFilter_filterInfo); + if (objectInputFilter_Status == ObjectInputFilter_Status_REJECTED) { + logger.fatal("Serialization filter is rejecting class {}", serialClassName, + new InvalidClassException(serialClassName)); + } + return objectInputFilter_Status; + } + + @Override + public String toString() { + return new StringBuilder(getClass().getSimpleName()) + .append("@") + .append(Integer.toHexString(hashCode())) + .append('{') + .append("objectInputFilter=").append(objectInputFilter) + .append(", sanctionedClassesCount=").append(sanctionedClasses.size()) + .append('}') + .toString(); + } +} diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilter.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilter.java index 4a1152d..164bffc 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilter.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilter.java @@ -15,7 +15,8 @@ package org.apache.geode.internal.serialization.filter; import static java.util.Collections.unmodifiableCollection; -import static org.apache.geode.internal.serialization.filter.ObjectInputFilterUtils.throwUnsupportedOperationException; +import static java.util.Objects.requireNonNull; +import static org.apache.commons.lang3.exception.ExceptionUtils.getRootCause; import java.lang.reflect.InvocationTargetException; import java.util.Collection; @@ -35,7 +36,7 @@ class ReflectiveFacadeGlobalSerialFilter implements GlobalSerialFilter { */ ReflectiveFacadeGlobalSerialFilter(ObjectInputFilterApi api, String pattern, Collection<String> sanctionedClasses) { - this.api = api; + this.api = requireNonNull(api, "ObjectInputFilterApi is required"); this.pattern = pattern; this.sanctionedClasses = unmodifiableCollection(sanctionedClasses); } @@ -44,7 +45,7 @@ class ReflectiveFacadeGlobalSerialFilter implements GlobalSerialFilter { * Invokes interface-defined operation to set this as the process-wide filter. */ @Override - public void setFilter() { + public void setFilter() throws UnableToSetSerialFilterException { try { // create the ObjectInputFilter to set as the global serial filter Object objectInputFilter = api.createObjectInputFilterProxy(pattern, sanctionedClasses); @@ -53,9 +54,7 @@ class ReflectiveFacadeGlobalSerialFilter implements GlobalSerialFilter { api.setSerialFilter(objectInputFilter); } catch (IllegalAccessException | InvocationTargetException e) { - throwUnsupportedOperationException( - "Geode was unable to configure a global serialization filter", - e); + handleExceptionThrownByApi(e); } } @@ -68,4 +67,36 @@ class ReflectiveFacadeGlobalSerialFilter implements GlobalSerialFilter { .append('}') .toString(); } + + private void handleExceptionThrownByApi(ReflectiveOperationException e) + throws UnableToSetSerialFilterException { + String className = getClassName(e); + switch (className) { + case "java.lang.IllegalAccessException": + throw new UnableToSetSerialFilterException( + "Unable to configure a global serialization filter using reflection.", + e); + case "java.lang.reflect.InvocationTargetException": + if (getRootCause(e) instanceof IllegalStateException) { + // ObjectInputFilter throws IllegalStateException + // if the filter has already been set non-null + throw new FilterAlreadyConfiguredException( + "Unable to configure a global serialization filter because filter has already been set non-null.", + e); + } + String causeClassName = e.getCause() == null ? getClassName(e) : getClassName(e.getCause()); + throw new UnableToSetSerialFilterException( + "Unable to configure a global serialization filter because invocation target threw " + + causeClassName + ".", + e); + default: + throw new UnableToSetSerialFilterException( + "Unable to configure a global serialization filter.", + e); + } + } + + private static String getClassName(Throwable throwable) { + return throwable.getClass().getName(); + } } diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilterFactory.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilterFactory.java index 022c766..3826e00 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilterFactory.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilterFactory.java @@ -14,11 +14,8 @@ */ package org.apache.geode.internal.serialization.filter; -import static java.util.Objects.requireNonNull; - import java.util.Collection; - -import org.apache.geode.annotations.VisibleForTesting; +import java.util.function.Supplier; /** * Creates an instance of {@code GlobalSerialFilter} that delegates to {@code ObjectInputFilterApi} @@ -26,19 +23,24 @@ import org.apache.geode.annotations.VisibleForTesting; */ class ReflectiveFacadeGlobalSerialFilterFactory implements GlobalSerialFilterFactory { - private final ObjectInputFilterApi api; + private final Supplier<ObjectInputFilterApi> objectInputFilterApiSupplier; ReflectiveFacadeGlobalSerialFilterFactory() { - this(new ReflectiveObjectInputFilterApiFactory().createObjectInputFilterApi()); + this(() -> new ReflectiveObjectInputFilterApiFactory().createObjectInputFilterApi()); + } + + ReflectiveFacadeGlobalSerialFilterFactory(ObjectInputFilterApi objectInputFilterApi) { + this(() -> objectInputFilterApi); } - @VisibleForTesting - ReflectiveFacadeGlobalSerialFilterFactory(ObjectInputFilterApi api) { - this.api = requireNonNull(api, "ObjectInputFilterApi is required"); + ReflectiveFacadeGlobalSerialFilterFactory( + Supplier<ObjectInputFilterApi> objectInputFilterApiSupplier) { + this.objectInputFilterApiSupplier = objectInputFilterApiSupplier; } @Override public GlobalSerialFilter create(String pattern, Collection<String> sanctionedClasses) { + ObjectInputFilterApi api = objectInputFilterApiSupplier.get(); return new ReflectiveFacadeGlobalSerialFilter(api, pattern, sanctionedClasses); } } diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeObjectInputFilterFactory.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeObjectInputFilterFactory.java deleted file mode 100644 index a38416c..0000000 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeObjectInputFilterFactory.java +++ /dev/null @@ -1,61 +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.internal.serialization.filter; - -import static java.util.Objects.requireNonNull; -import static org.apache.geode.internal.serialization.filter.ObjectInputFilterUtils.supportsObjectInputFilter; -import static org.apache.geode.internal.serialization.filter.ObjectInputFilterUtils.throwUnsupportedOperationException; - -import java.util.Set; - -/** - * Creates an instance of {@code ObjectInputFilter} that delegates to {@code ObjectInputFilterApi} - * to maintain independence from the JRE version. - */ -public class ReflectiveFacadeObjectInputFilterFactory implements ObjectInputFilterFactory { - - private static final String UNSUPPORTED_MESSAGE = - "A serialization filter has been specified but this version of Java does not support serialization filters - ObjectInputFilter is not available"; - - private final ObjectInputFilterApi api; - - public ReflectiveFacadeObjectInputFilterFactory() { - this(new ReflectiveObjectInputFilterApiFactory().createObjectInputFilterApi()); - } - - private ReflectiveFacadeObjectInputFilterFactory(ObjectInputFilterApi api) { - this.api = requireNonNull(api, "ObjectInputFilterApi is required"); - } - - @Override - public ObjectInputFilter create(SerializableObjectConfig config, Set<String> sanctionedClasses) { - if (config.getValidateSerializableObjects()) { - requireObjectInputFilter(); - - String pattern = new SanctionedSerializablesFilterPattern() - .append(config.getSerializableObjectFilter()) - .pattern(); - - return new ReflectiveFacadeObjectInputFilter(api, pattern, sanctionedClasses); - } - return new NullObjectInputFilter(); - } - - public static void requireObjectInputFilter() { - if (!supportsObjectInputFilter()) { - throwUnsupportedOperationException(UNSUPPORTED_MESSAGE); - } - } -} diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeObjectInputFilter.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeStreamSerialFilter.java similarity index 55% rename from geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeObjectInputFilter.java rename to geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeStreamSerialFilter.java index 08bae44..902cf60 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeObjectInputFilter.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeStreamSerialFilter.java @@ -15,7 +15,8 @@ package org.apache.geode.internal.serialization.filter; import static java.util.Collections.unmodifiableCollection; -import static org.apache.geode.internal.serialization.filter.ObjectInputFilterUtils.throwUnsupportedOperationException; +import static java.util.Objects.requireNonNull; +import static org.apache.commons.lang3.exception.ExceptionUtils.getRootCause; import java.io.ObjectInputStream; import java.lang.reflect.InvocationTargetException; @@ -25,7 +26,7 @@ import java.util.Collection; * Implementation of {@code ObjectInputFilter} that delegates to {@code ObjectInputFilterApi} to * maintain independence from the JRE version. */ -class ReflectiveFacadeObjectInputFilter implements ObjectInputFilter { +class ReflectiveFacadeStreamSerialFilter implements StreamSerialFilter { private final ObjectInputFilterApi api; private final String pattern; @@ -34,11 +35,11 @@ class ReflectiveFacadeObjectInputFilter implements ObjectInputFilter { /** * Constructs instance with the specified collaborators. */ - ReflectiveFacadeObjectInputFilter(ObjectInputFilterApi api, String pattern, + ReflectiveFacadeStreamSerialFilter(ObjectInputFilterApi api, String pattern, Collection<String> sanctionedClasses) { + this.api = requireNonNull(api, "ObjectInputFilterApi is required"); this.pattern = pattern; this.sanctionedClasses = unmodifiableCollection(sanctionedClasses); - this.api = api; } /** @@ -46,7 +47,8 @@ class ReflectiveFacadeObjectInputFilter implements ObjectInputFilter { * {@code ObjectInputStream}. */ @Override - public void setFilterOn(ObjectInputStream objectInputStream) { + public void setFilterOn(ObjectInputStream objectInputStream) + throws UnableToSetSerialFilterException { try { // create the ObjectInputFilter to set as the global serial filter Object objectInputFilter = api.createObjectInputFilterProxy(pattern, sanctionedClasses); @@ -55,10 +57,7 @@ class ReflectiveFacadeObjectInputFilter implements ObjectInputFilter { api.setObjectInputFilter(objectInputStream, objectInputFilter); } catch (IllegalAccessException | InvocationTargetException e) { - throwUnsupportedOperationException( - "Geode was unable to configure a serialization filter on input stream '" - + objectInputStream.hashCode() + "'", - e); + handleExceptionThrownByApi(e); } } @@ -75,4 +74,36 @@ class ReflectiveFacadeObjectInputFilter implements ObjectInputFilter { ObjectInputFilterApi getObjectInputFilterApi() { return api; } + + private void handleExceptionThrownByApi(ReflectiveOperationException e) + throws UnableToSetSerialFilterException { + String className = getClassName(e); + switch (className) { + case "java.lang.IllegalAccessException": + throw new UnableToSetSerialFilterException( + "Unable to configure an input stream serialization filter using reflection.", + e); + case "java.lang.reflect.InvocationTargetException": + if (getRootCause(e) instanceof IllegalStateException) { + // ObjectInputFilter throws IllegalStateException + // if the filter has already been set non-null + throw new FilterAlreadyConfiguredException( + "Unable to configure an input stream serialization filter because a non-null filter has already been set.", + e); + } + String causeClassName = e.getCause() == null ? getClassName(e) : getClassName(e.getCause()); + throw new UnableToSetSerialFilterException( + "Unable to configure an input stream serialization filter because invocation target threw " + + causeClassName + ".", + e); + default: + throw new UnableToSetSerialFilterException( + "Unable to configure an input stream serialization filter.", + e); + } + } + + private static String getClassName(Throwable throwable) { + return throwable.getClass().getName(); + } } diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilterFactory.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeStreamSerialFilterFactory.java similarity index 52% copy from geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilterFactory.java copy to geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeStreamSerialFilterFactory.java index 022c766..3c47998 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilterFactory.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeStreamSerialFilterFactory.java @@ -14,31 +14,26 @@ */ package org.apache.geode.internal.serialization.filter; -import static java.util.Objects.requireNonNull; - -import java.util.Collection; - -import org.apache.geode.annotations.VisibleForTesting; +import java.util.Set; /** - * Creates an instance of {@code GlobalSerialFilter} that delegates to {@code ObjectInputFilterApi} + * Creates an instance of {@code ObjectInputFilter} that delegates to {@code ObjectInputFilterApi} * to maintain independence from the JRE version. */ -class ReflectiveFacadeGlobalSerialFilterFactory implements GlobalSerialFilterFactory { - - private final ObjectInputFilterApi api; - - ReflectiveFacadeGlobalSerialFilterFactory() { - this(new ReflectiveObjectInputFilterApiFactory().createObjectInputFilterApi()); - } - - @VisibleForTesting - ReflectiveFacadeGlobalSerialFilterFactory(ObjectInputFilterApi api) { - this.api = requireNonNull(api, "ObjectInputFilterApi is required"); - } +public class ReflectiveFacadeStreamSerialFilterFactory implements StreamSerialFilterFactory { @Override - public GlobalSerialFilter create(String pattern, Collection<String> sanctionedClasses) { - return new ReflectiveFacadeGlobalSerialFilter(api, pattern, sanctionedClasses); + public StreamSerialFilter create(SerializableObjectConfig config, Set<String> sanctionedClasses) { + ObjectInputFilterApi api = + new ReflectiveObjectInputFilterApiFactory().createObjectInputFilterApi(); + + if (config.getValidateSerializableObjects()) { + String pattern = new SanctionedSerializablesFilterPattern() + .append(config.getSerializableObjectFilter()) + .pattern(); + + return new ReflectiveFacadeStreamSerialFilter(api, pattern, sanctionedClasses); + } + return new NullStreamSerialFilter(); } } diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveObjectInputFilterApi.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveObjectInputFilterApi.java index 10c3aad..7b2e40a 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveObjectInputFilterApi.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ReflectiveObjectInputFilterApi.java @@ -14,7 +14,6 @@ */ package org.apache.geode.internal.serialization.filter; -import java.io.InvalidClassException; import java.io.ObjectInputStream; import java.lang.reflect.InvocationHandler; import java.lang.reflect.InvocationTargetException; @@ -22,10 +21,7 @@ import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.util.Collection; -import org.apache.logging.log4j.Logger; - import org.apache.geode.annotations.VisibleForTesting; -import org.apache.geode.logging.internal.log4j.api.LogService; /** * Implementation of {@code ObjectInputFilterApi} that uses reflection and a dynamic proxy to wrap @@ -33,19 +29,17 @@ import org.apache.geode.logging.internal.log4j.api.LogService; */ public class ReflectiveObjectInputFilterApi implements ObjectInputFilterApi { - private static final Logger logger = LogService.getLogger(); - protected final ApiPackage apiPackage; // api.package.ObjectInputFilter protected final Class<?> ObjectInputFilter; - private final Method ObjectInputFilter_checkInput; - private final Object ObjectInputFilter_Status_ALLOWED; - private final Object ObjectInputFilter_Status_REJECTED; + protected final Method ObjectInputFilter_checkInput; + protected final Object ObjectInputFilter_Status_ALLOWED; + protected final Object ObjectInputFilter_Status_REJECTED; // api.package.ObjectInputFilter$Config - private final Class<?> ObjectInputFilter_Config; - private final Method ObjectInputFilter_Config_createFilter; + protected final Class<?> ObjectInputFilter_Config; + protected final Method ObjectInputFilter_Config_createFilter; private final Method ObjectInputFilter_Config_getObjectInputFilter; private final Method ObjectInputFilter_Config_setObjectInputFilter; private final Method ObjectInputFilter_Config_getSerialFilter; @@ -53,7 +47,7 @@ public class ReflectiveObjectInputFilterApi implements ObjectInputFilterApi { // api.package.ObjectInputFilter$FilterInfo private final Class<?> ObjectInputFilter_FilterInfo; - private final Method ObjectInputFilter_FilterInfo_serialClass; + protected final Method ObjectInputFilter_FilterInfo_serialClass; /** * Use reflection to look up the classes and methods for the API. @@ -144,43 +138,18 @@ public class ReflectiveObjectInputFilterApi implements ObjectInputFilterApi { * effectively override the call to widen the filter to include the User's addition to the set * of sanctioned serializables. */ - InvocationHandler invocationHandler = (proxy, method, args) -> { - if (!"checkInput".equals(method.getName())) { - throw new UnsupportedOperationException( - "ObjectInputFilter." + method.getName() + " is not implemented"); - } - - // fetch the class of the serialized instance - Object objectInputFilter_filterInfo = args[0]; - Class<?> serialClass = - (Class<?>) ObjectInputFilter_FilterInfo_serialClass.invoke(objectInputFilter_filterInfo); - if (serialClass == null) { // no class to check, so nothing to accept-list - return ObjectInputFilter_checkInput.invoke(objectInputFilter, objectInputFilter_filterInfo); - } - - // check sanctionedClasses to determine if the name of the class is ALLOWED - String serialClassName = serialClass.getName(); - if (serialClass.isArray()) { - serialClassName = serialClass.getComponentType().getName(); - } - if (sanctionedClasses.contains(serialClassName)) { - return ObjectInputFilter_Status_ALLOWED; - } - - // check the filter to determine if the class is ALLOWED - Object objectInputFilter_Status = - ObjectInputFilter_checkInput.invoke(objectInputFilter, objectInputFilter_filterInfo); - if (objectInputFilter_Status == ObjectInputFilter_Status_REJECTED) { - logger.fatal("Serialization filter is rejecting class {}", serialClassName, - new InvalidClassException(serialClassName)); - } - return objectInputFilter_Status; - }; + InvocationHandler invocationHandler = new ObjectInputFilterInvocationHandler( + ObjectInputFilter_checkInput, + ObjectInputFilter_FilterInfo_serialClass, + ObjectInputFilter_Status_ALLOWED, + ObjectInputFilter_Status_REJECTED, + objectInputFilter, + sanctionedClasses); // wrap the filter within a proxy to inject the above invocation handler return Proxy.newProxyInstance( ObjectInputFilter.getClassLoader(), - new Class[] {ObjectInputFilter}, + new Class<?>[] {ObjectInputFilter}, invocationHandler); } diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ObjectInputFilter.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/StreamSerialFilter.java similarity index 76% rename from geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ObjectInputFilter.java rename to geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/StreamSerialFilter.java index 091f8f6..1d182e4 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ObjectInputFilter.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/StreamSerialFilter.java @@ -20,10 +20,13 @@ import java.io.ObjectInputStream; * Defines operation to set this serialization filter on an {@code ObjectInputStream}. */ @FunctionalInterface -public interface ObjectInputFilter { +public interface StreamSerialFilter { /** * Sets this serialization filter on the specified {@code ObjectInputStream}. + * + * @throws FilterAlreadyConfiguredException if a non-null serialization filter already exists + * @throws UnableToSetSerialFilterException if there's any failure setting a serialization filter */ - void setFilterOn(ObjectInputStream objectInputStream); + void setFilterOn(ObjectInputStream objectInputStream) throws UnableToSetSerialFilterException; } diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ObjectInputFilterFactory.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/StreamSerialFilterFactory.java similarity index 88% rename from geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ObjectInputFilterFactory.java rename to geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/StreamSerialFilterFactory.java index c5fe4bf..c5a3113 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/ObjectInputFilterFactory.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/StreamSerialFilterFactory.java @@ -17,8 +17,8 @@ package org.apache.geode.internal.serialization.filter; import java.util.Set; @FunctionalInterface -public interface ObjectInputFilterFactory { +public interface StreamSerialFilterFactory { - ObjectInputFilter create(SerializableObjectConfig serializableObjectConfig, + StreamSerialFilter create(SerializableObjectConfig serializableObjectConfig, Set<String> sanctionedClasses); } diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/SystemPropertyGlobalSerialFilterConfigurationFactory.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/SystemPropertyGlobalSerialFilterConfigurationFactory.java index 4c592d3..b78cbfd 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/SystemPropertyGlobalSerialFilterConfigurationFactory.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/SystemPropertyGlobalSerialFilterConfigurationFactory.java @@ -16,6 +16,8 @@ package org.apache.geode.internal.serialization.filter; import static org.apache.commons.lang3.StringUtils.isBlank; +import java.util.function.BooleanSupplier; + import org.apache.geode.internal.lang.SystemProperty; /** @@ -30,9 +32,15 @@ public class SystemPropertyGlobalSerialFilterConfigurationFactory public SystemPropertyGlobalSerialFilterConfigurationFactory() { // enable GlobalSerialFilter only under these conditions: - // (1) jdk.serialFilter must be blank - // (2) geode.enableGlobalSerialFilter must be set "true" - this(isBlank(System.getProperty("jdk.serialFilter")) && + // (1) JRE supports ObjectInputFilter in either sun.misc. or java.io. package + // (2) jdk.serialFilter must be blank + // (3) geode.enableGlobalSerialFilter must be set "true" + this(ObjectInputFilterUtils::supportsObjectInputFilter); + } + + SystemPropertyGlobalSerialFilterConfigurationFactory(BooleanSupplier supportsObjectInputFilter) { + this(supportsObjectInputFilter.getAsBoolean() && + isBlank(System.getProperty("jdk.serialFilter")) && SystemProperty .getProductBooleanProperty("enableGlobalSerialFilter") .orElse(false)); @@ -47,6 +55,14 @@ public class SystemPropertyGlobalSerialFilterConfigurationFactory if (enabled) { return new GlobalSerialFilterConfiguration(serializableObjectConfig); } - return () -> false; + return new NullFilterConfiguration(); + } + + private static class NullFilterConfiguration implements FilterConfiguration { + + @Override + public boolean configure() { + return false; + } } } diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/NullObjectInputFilter.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/UnableToSetSerialFilterException.java similarity index 60% rename from geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/NullObjectInputFilter.java rename to geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/UnableToSetSerialFilterException.java index e25c073..a8683c8 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/NullObjectInputFilter.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/filter/UnableToSetSerialFilterException.java @@ -14,15 +14,23 @@ */ package org.apache.geode.internal.serialization.filter; -import java.io.ObjectInputStream; - /** - * Implementation of {@code ObjectInputFilter} that does nothing. + * Checked exception thrown when there's a failure using Java's ObjectInputFilter. All uses of this + * exception are caught and rethrown before reaching the user. */ -public class NullObjectInputFilter implements ObjectInputFilter { +public class UnableToSetSerialFilterException extends Exception { + + public UnableToSetSerialFilterException(String message) { + super(message); + } - @Override - public void setFilterOn(ObjectInputStream objectInputStream) { - // Do nothing, this is the case where we don't filter. + public UnableToSetSerialFilterException(String message, Throwable cause) { + super(message, cause); } + + public UnableToSetSerialFilterException(Throwable cause) { + super(cause); + } + + private static final long serialVersionUID = 3406028558181224120L; } diff --git a/geode-serialization/src/main/resources/org/apache/geode/internal/serialization/sanctioned-geode-serialization-serializables.txt b/geode-serialization/src/main/resources/org/apache/geode/internal/serialization/sanctioned-geode-serialization-serializables.txt index 3133970..29fa124 100644 --- a/geode-serialization/src/main/resources/org/apache/geode/internal/serialization/sanctioned-geode-serialization-serializables.txt +++ b/geode-serialization/src/main/resources/org/apache/geode/internal/serialization/sanctioned-geode-serialization-serializables.txt @@ -2,3 +2,5 @@ org/apache/geode/internal/serialization/DSCODE,false,value:byte org/apache/geode/internal/serialization/DSFIDNotFoundException,true,130596009484324655,dsfid:int,versionOrdinal:short org/apache/geode/internal/serialization/UnsupportedSerializationVersionException,true,3572445857994216007 org/apache/geode/internal/serialization/filter/ApiPackage,false,prefix:java/lang/String +org/apache/geode/internal/serialization/filter/FilterAlreadyConfiguredException,true,-6102549374563510704 +org/apache/geode/internal/serialization/filter/UnableToSetSerialFilterException,true,3406028558181224120 diff --git a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilterConfigurationTest.java b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilterConfigurationTest.java index e31126b..2f02ed5 100644 --- a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilterConfigurationTest.java +++ b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/GlobalSerialFilterConfigurationTest.java @@ -14,60 +14,102 @@ */ package org.apache.geode.internal.serialization.filter; +import static org.apache.geode.internal.serialization.filter.SerialFilterAssertions.assertThatSerialFilterIsNull; import static org.apache.geode.util.internal.UncheckedUtils.uncheckedCast; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; -import java.util.function.Consumer; +import java.lang.reflect.InvocationTargetException; +import org.apache.logging.log4j.Logger; +import org.junit.After; import org.junit.Before; import org.junit.Test; public class GlobalSerialFilterConfigurationTest { - private SerializableObjectConfig config; + private SerializableObjectConfig serializableObjectConfig; private GlobalSerialFilter globalSerialFilter; - private Consumer<String> logger; + private Logger logger; @Before public void setUp() { - config = mock(SerializableObjectConfig.class); + serializableObjectConfig = mock(SerializableObjectConfig.class); globalSerialFilter = mock(GlobalSerialFilter.class); - logger = uncheckedCast(mock(Consumer.class)); + logger = uncheckedCast(mock(Logger.class)); + } + + @After + public void serialFilterIsNull() throws InvocationTargetException, IllegalAccessException { + assertThatSerialFilterIsNull(); } @Test - public void configureLogs_whenUnsupportedOperationExceptionIsThrown_withCause() { - doThrow(new UnsupportedOperationException( + public void logsInfo_whenOperationIsSuccessful() throws UnableToSetSerialFilterException { + FilterConfiguration filterConfiguration = new GlobalSerialFilterConfiguration( + serializableObjectConfig, + logger, + (pattern, sanctionedClasses) -> globalSerialFilter); + + filterConfiguration.configure(); + + verify(logger).info("Global serialization filter is now configured."); + verify(logger, never()).warn(any(Object.class)); + verify(logger, never()).error(any(Object.class)); + } + + @Test + public void rethrowsWhenIllegalStateExceptionIsThrownByApi() + throws UnableToSetSerialFilterException { + doThrow(new UnableToSetSerialFilterException( new IllegalStateException("Serial filter can only be set once"))) .when(globalSerialFilter).setFilter(); FilterConfiguration filterConfiguration = new GlobalSerialFilterConfiguration( - config, logger, (pattern, sanctionedClasses) -> globalSerialFilter); + serializableObjectConfig, + (pattern, sanctionedClasses) -> globalSerialFilter); - filterConfiguration.configure(); + Throwable thrown = catchThrowable(() -> { + filterConfiguration.configure(); + }); - verify(logger).accept("Global serial filter is already configured."); + assertThat(thrown) + .isInstanceOf(UnableToSetSerialFilterException.class) + .hasMessage("java.lang.IllegalStateException: Serial filter can only be set once") + .hasRootCauseInstanceOf(IllegalStateException.class) + .hasRootCauseMessage("Serial filter can only be set once"); } @Test - public void configureDoesNotLog_whenUnsupportedOperationExceptionIsThrown_withoutCause() { - doThrow(new UnsupportedOperationException("testing with no root cause")) - .when(globalSerialFilter).setFilter(); + public void rethrowsWhenClassNotFoundExceptionIsThrownByApi() + throws UnableToSetSerialFilterException { + doThrow(new UnableToSetSerialFilterException( + new ClassNotFoundException("sun.misc.ObjectInputFilter"))) + .when(globalSerialFilter).setFilter(); FilterConfiguration filterConfiguration = new GlobalSerialFilterConfiguration( - config, logger, (pattern, sanctionedClasses) -> globalSerialFilter); + serializableObjectConfig, + (pattern, sanctionedClasses) -> globalSerialFilter); - filterConfiguration.configure(); + Throwable thrown = catchThrowable(() -> { + filterConfiguration.configure(); + }); - verifyNoInteractions(logger); + assertThat(thrown) + .isInstanceOf(UnableToSetSerialFilterException.class) + .hasMessage("java.lang.ClassNotFoundException: sun.misc.ObjectInputFilter") + .hasRootCauseInstanceOf(ClassNotFoundException.class) + .hasRootCauseMessage("sun.misc.ObjectInputFilter"); } @Test - public void configureSetsValidateSerializableObjects() { - SerializableObjectConfig serializableObjectConfig = mock(SerializableObjectConfig.class); - FilterConfiguration filterConfiguration = - new GlobalSerialFilterConfiguration(serializableObjectConfig); + public void setsValidateSerializableObjects() throws UnableToSetSerialFilterException { + FilterConfiguration filterConfiguration = new GlobalSerialFilterConfiguration( + serializableObjectConfig, + (pattern, sanctionedClasses) -> globalSerialFilter); filterConfiguration.configure(); diff --git a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/JmxSerialFilterConfigurationFactoryTest.java b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/JmxSerialFilterConfigurationFactoryTest.java index e7e72f6..53aa5b7 100644 --- a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/JmxSerialFilterConfigurationFactoryTest.java +++ b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/JmxSerialFilterConfigurationFactoryTest.java @@ -18,9 +18,13 @@ import static org.apache.commons.lang3.JavaVersion.JAVA_1_8; import static org.apache.commons.lang3.JavaVersion.JAVA_9; import static org.apache.commons.lang3.SystemUtils.isJavaVersionAtLeast; import static org.apache.commons.lang3.SystemUtils.isJavaVersionAtMost; +import static org.apache.geode.internal.serialization.filter.SerialFilterAssertions.assertThatSerialFilterIsNull; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assumptions.assumeThat; +import java.lang.reflect.InvocationTargetException; + +import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.junit.contrib.java.lang.system.RestoreSystemProperties; @@ -30,6 +34,11 @@ public class JmxSerialFilterConfigurationFactoryTest { @Rule public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties(); + @After + public void serialFilterIsNull() throws InvocationTargetException, IllegalAccessException { + assertThatSerialFilterIsNull(); + } + @Test public void createsConditionalJmxSerialFilterConfiguration_onJava9orGreater() { assumeThat(isJavaVersionAtLeast(JAVA_9)).isTrue(); diff --git a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/JmxSerialFilterConfigurationTest.java b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/JmxSerialFilterConfigurationTest.java index ad05630..75f1efd 100644 --- a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/JmxSerialFilterConfigurationTest.java +++ b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/JmxSerialFilterConfigurationTest.java @@ -14,13 +14,16 @@ */ package org.apache.geode.internal.serialization.filter; +import static org.apache.geode.internal.serialization.filter.SerialFilterAssertions.assertThatSerialFilterIsNull; import static org.apache.geode.util.internal.UncheckedUtils.uncheckedCast; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -import java.util.function.Consumer; +import java.lang.reflect.InvocationTargetException; +import org.apache.logging.log4j.Logger; +import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -31,7 +34,7 @@ public class JmxSerialFilterConfigurationTest { private static final String SYSTEM_PROPERTY = "system.property.name"; private String pattern; - private Consumer<String> loggerConsumer; + private Logger logger; @Rule public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties(); @@ -39,7 +42,12 @@ public class JmxSerialFilterConfigurationTest { @Before public void setUp() { pattern = "the-filter-pattern"; - loggerConsumer = uncheckedCast(mock(Consumer.class)); + logger = uncheckedCast(mock(Logger.class)); + } + + @After + public void serialFilterIsNull() throws InvocationTargetException, IllegalAccessException { + assertThatSerialFilterIsNull(); } @Test @@ -50,7 +58,7 @@ public class JmxSerialFilterConfigurationTest { } @Test - public void setsPropertyValue() { + public void setsPropertyValue() throws UnableToSetSerialFilterException { FilterConfiguration filterConfiguration = new JmxSerialFilterConfiguration(SYSTEM_PROPERTY, pattern); @@ -62,7 +70,7 @@ public class JmxSerialFilterConfigurationTest { } @Test - public void setsPropertyValue_ifExistingValueIsNull() { + public void setsPropertyValue_ifExistingValueIsNull() throws UnableToSetSerialFilterException { System.clearProperty(SYSTEM_PROPERTY); FilterConfiguration filterConfiguration = new JmxSerialFilterConfiguration(SYSTEM_PROPERTY, pattern); @@ -75,7 +83,7 @@ public class JmxSerialFilterConfigurationTest { } @Test - public void setsPropertyValue_ifExistingValueIsEmpty() { + public void setsPropertyValue_ifExistingValueIsEmpty() throws UnableToSetSerialFilterException { System.setProperty(SYSTEM_PROPERTY, ""); FilterConfiguration filterConfiguration = new JmxSerialFilterConfiguration(SYSTEM_PROPERTY, pattern); @@ -88,7 +96,7 @@ public class JmxSerialFilterConfigurationTest { } @Test - public void setsPropertyValue_ifExistingValueIsBlank() { + public void setsPropertyValue_ifExistingValueIsBlank() throws UnableToSetSerialFilterException { System.setProperty(SYSTEM_PROPERTY, " "); FilterConfiguration filterConfiguration = new JmxSerialFilterConfiguration(SYSTEM_PROPERTY, pattern); @@ -101,33 +109,34 @@ public class JmxSerialFilterConfigurationTest { } @Test - public void logsNowConfigured_ifExistingValueIsEmpty() { + public void logsSuccess_ifExistingValueIsEmpty() throws UnableToSetSerialFilterException { System.setProperty(SYSTEM_PROPERTY, ""); FilterConfiguration filterConfiguration = - new JmxSerialFilterConfiguration(SYSTEM_PROPERTY, pattern, loggerConsumer); + new JmxSerialFilterConfiguration(SYSTEM_PROPERTY, pattern, logger); filterConfiguration.configure(); - verify(loggerConsumer) - .accept("System property '" + SYSTEM_PROPERTY + "' is now configured with '" + + verify(logger) + .info("System property '" + SYSTEM_PROPERTY + "' is now configured with '" + pattern + "'."); } @Test - public void logsNowConfigured_ifExistingValueIsBlank() { + public void logsSuccess_ifExistingValueIsBlank() throws UnableToSetSerialFilterException { System.setProperty(SYSTEM_PROPERTY, " "); FilterConfiguration filterConfiguration = - new JmxSerialFilterConfiguration(SYSTEM_PROPERTY, pattern, loggerConsumer); + new JmxSerialFilterConfiguration(SYSTEM_PROPERTY, pattern, logger); filterConfiguration.configure(); - verify(loggerConsumer) - .accept("System property '" + SYSTEM_PROPERTY + "' is now configured with '" + + verify(logger) + .info("System property '" + SYSTEM_PROPERTY + "' is now configured with '" + pattern + "'."); } @Test - public void doesNotSetPropertyValue_ifExistingValueIsNotEmpty() { + public void doesNotSetPropertyValue_ifExistingValueIsNotEmpty() + throws UnableToSetSerialFilterException { String existingValue = "existing-value-of-property"; System.setProperty(SYSTEM_PROPERTY, existingValue); FilterConfiguration filterConfiguration = @@ -141,15 +150,16 @@ public class JmxSerialFilterConfigurationTest { } @Test - public void logsAlreadyConfiguredMessage_ifExistingPropertyValueIsNotEmpty() { + public void logsWarning_ifExistingPropertyValueIsNotEmpty() + throws UnableToSetSerialFilterException { String existingValue = "existing-value-of-property"; System.setProperty(SYSTEM_PROPERTY, existingValue); FilterConfiguration filterConfiguration = - new JmxSerialFilterConfiguration(SYSTEM_PROPERTY, pattern, loggerConsumer); + new JmxSerialFilterConfiguration(SYSTEM_PROPERTY, pattern, logger); filterConfiguration.configure(); - verify(loggerConsumer) - .accept("System property '" + SYSTEM_PROPERTY + "' is already configured."); + verify(logger) + .info("System property '" + SYSTEM_PROPERTY + "' is already configured."); } } diff --git a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/NullObjectInputFilterTest.java b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/NullObjectInputFilterTest.java index 2ab18a9..557dbdf 100644 --- a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/NullObjectInputFilterTest.java +++ b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/NullObjectInputFilterTest.java @@ -24,9 +24,9 @@ import org.junit.Test; public class NullObjectInputFilterTest { @Test - public void doesNothing() { + public void doesNothing() throws UnableToSetSerialFilterException { ObjectInputStream objectInputStream = mock(ObjectInputStream.class); - ObjectInputFilter filter = new NullObjectInputFilter(); + StreamSerialFilter filter = new NullStreamSerialFilter(); filter.setFilterOn(objectInputStream); diff --git a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ObjectInputFilterInvocationHandlerTest.java b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ObjectInputFilterInvocationHandlerTest.java new file mode 100644 index 0000000..6248ea3 --- /dev/null +++ b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ObjectInputFilterInvocationHandlerTest.java @@ -0,0 +1,97 @@ +/* + * 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.internal.serialization.filter; + +import static java.util.Collections.emptySet; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; + +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; + +import org.junit.Before; +import org.junit.Test; + +public class ObjectInputFilterInvocationHandlerTest { + + private static final String PATTERN = "*"; + + private Class<?> ObjectInputFilter; + private Method ObjectInputFilter_checkInput; + private Method ObjectInputFilter_FilterInfo_serialClass; + private Object ObjectInputFilter_Status_ALLOWED; + private Object ObjectInputFilter_Status_REJECTED; + + private Object objectInputFilter; + + @Before + public void setUp() + throws InvocationTargetException, IllegalAccessException, NoSuchMethodException { + ReflectiveObjectInputFilterApi api = + (ReflectiveObjectInputFilterApi) new ReflectiveObjectInputFilterApiFactory() + .createObjectInputFilterApi(); + + ObjectInputFilter = api.ObjectInputFilter; + ObjectInputFilter_checkInput = api.ObjectInputFilter_checkInput; + Class<?> objectInputFilter_Config = api.ObjectInputFilter_Config; + Method objectInputFilter_Config_createFilter = api.ObjectInputFilter_Config_createFilter; + ObjectInputFilter_Status_ALLOWED = api.ObjectInputFilter_Status_ALLOWED; + ObjectInputFilter_Status_REJECTED = api.ObjectInputFilter_Status_REJECTED; + ObjectInputFilter_FilterInfo_serialClass = api.ObjectInputFilter_FilterInfo_serialClass; + + objectInputFilter = objectInputFilter_Config_createFilter + .invoke(objectInputFilter_Config, PATTERN); + } + + @Test + public void sanctionedClassesIsRequired() { + Throwable thrown = catchThrowable(() -> { + new ObjectInputFilterInvocationHandler( + ObjectInputFilter_checkInput, + ObjectInputFilter_FilterInfo_serialClass, + ObjectInputFilter_Status_ALLOWED, + ObjectInputFilter_Status_REJECTED, + objectInputFilter, + null); + }); + + assertThat(thrown).isInstanceOf(NullPointerException.class); + } + + @Test + public void toStringIsInvokedOnProxiedInstance() { + InvocationHandler invocationHandler = new ObjectInputFilterInvocationHandler( + ObjectInputFilter_checkInput, + ObjectInputFilter_FilterInfo_serialClass, + ObjectInputFilter_Status_ALLOWED, + ObjectInputFilter_Status_REJECTED, + objectInputFilter, + emptySet()); + Object proxy = objectInputFilterProxy(invocationHandler); + + String result = proxy.toString(); + + assertThat(result).isEqualTo(PATTERN); + } + + private Object objectInputFilterProxy(InvocationHandler invocationHandler) { + return Proxy.newProxyInstance( + getClass().getClassLoader(), + new Class[] {ObjectInputFilter}, + invocationHandler); + } +} diff --git a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilterFactoryTest.java b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilterFactoryTest.java index 34c83c4..844d0da 100644 --- a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilterFactoryTest.java +++ b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilterFactoryTest.java @@ -15,57 +15,62 @@ package org.apache.geode.internal.serialization.filter; import static java.util.Collections.emptySet; +import static org.apache.geode.internal.serialization.filter.SerialFilterAssertions.assertThatSerialFilterIsNull; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; +import static org.assertj.core.api.Assertions.catchThrowable; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; import java.lang.reflect.InvocationTargetException; +import java.util.function.Supplier; +import org.junit.After; +import org.junit.Rule; import org.junit.Test; +import org.junit.contrib.java.lang.system.RestoreSystemProperties; public class ReflectiveFacadeGlobalSerialFilterFactoryTest { - @Test - public void constructsDelegatingGlobalSerialFilter() { - ObjectInputFilterApi api = mock(ObjectInputFilterApi.class); - GlobalSerialFilterFactory factory = new ReflectiveFacadeGlobalSerialFilterFactory(api); + @Rule + public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties(); - GlobalSerialFilter filter = factory.create("pattern", emptySet()); - - assertThat(filter).isInstanceOf(ReflectiveFacadeGlobalSerialFilter.class); + @After + public void serialFilterIsNull() throws InvocationTargetException, IllegalAccessException { + assertThatSerialFilterIsNull(); } + /** + * Creates an instance of ReflectiveFacadeGlobalSerialFilter. + */ @Test - public void delegatesToObjectInputFilterApiToCreateObjectInputFilter() - throws InvocationTargetException, IllegalAccessException { + public void createsReflectiveFacadeGlobalSerialFilter() { ObjectInputFilterApi api = mock(ObjectInputFilterApi.class); GlobalSerialFilterFactory factory = new ReflectiveFacadeGlobalSerialFilterFactory(api); - GlobalSerialFilter filter = factory.create("pattern", emptySet()); - Object objectInputFilter = mock(Object.class); - when(api.createObjectInputFilterProxy(any(), any())) - .thenReturn(objectInputFilter); - - filter.setFilter(); + GlobalSerialFilter filter = factory.create("pattern", emptySet()); - verify(api).createObjectInputFilterProxy(any(), any()); + assertThat(filter).isInstanceOf(ReflectiveFacadeGlobalSerialFilter.class); } + /** + * Throws ClassNotFoundException nested inside an UnsupportedOperationException when the trying \ + * to load the JDK ObjectInputFilter via reflection throws ClassNotFoundException. + */ @Test - public void delegatesToObjectInputFilterApiToSetSerialFilter() - throws InvocationTargetException, IllegalAccessException { - ObjectInputFilterApi api = mock(ObjectInputFilterApi.class); - GlobalSerialFilterFactory factory = new ReflectiveFacadeGlobalSerialFilterFactory(api); - GlobalSerialFilter filter = factory.create("pattern", emptySet()); - Object objectInputFilter = mock(Object.class); - - when(api.createObjectInputFilterProxy(any(), any())) - .thenReturn(objectInputFilter); + public void throws_whenObjectInputFilterClassNotFound() { + Supplier<ObjectInputFilterApi> objectInputFilterApiSupplier = () -> { + throw new UnsupportedOperationException("ObjectInputFilter is not available.", + new ClassNotFoundException("sun.misc.ObjectInputFilter")); + }; - filter.setFilter(); + Throwable thrown = catchThrowable(() -> { + new ReflectiveFacadeGlobalSerialFilterFactory(objectInputFilterApiSupplier) + .create("pattern", emptySet()); + }); - verify(api).setSerialFilter(objectInputFilter); + assertThat(thrown) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessage("ObjectInputFilter is not available.") + .hasRootCauseInstanceOf(ClassNotFoundException.class) + .hasRootCauseMessage("sun.misc.ObjectInputFilter"); } } diff --git a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilterTest.java b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilterTest.java index 5966d1b..1635635 100644 --- a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilterTest.java +++ b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeGlobalSerialFilterTest.java @@ -15,20 +15,25 @@ package org.apache.geode.internal.serialization.filter; import static java.util.Arrays.asList; +import static java.util.Collections.emptySet; import static java.util.Collections.singleton; +import static org.apache.geode.internal.serialization.filter.SerialFilterAssertions.assertThatSerialFilterIsNull; import static org.apache.geode.util.internal.UncheckedUtils.uncheckedCast; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.catchThrowable; import static org.mockito.ArgumentCaptor.forClass; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyCollection; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import java.lang.reflect.InvocationTargetException; import java.util.Collection; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -36,19 +41,28 @@ import org.mockito.ArgumentCaptor; public class ReflectiveFacadeGlobalSerialFilterTest { private ObjectInputFilterApi api; + private Object objectInputFilter; @Before public void setUp() { api = mock(ObjectInputFilterApi.class); + objectInputFilter = new Object(); + } + + @After + public void serialFilterIsNull() throws InvocationTargetException, IllegalAccessException { + assertThatSerialFilterIsNull(); } @Test public void createsObjectInputFilterProxy() - throws InvocationTargetException, IllegalAccessException { + throws InvocationTargetException, IllegalAccessException, UnableToSetSerialFilterException { String pattern = "the-pattern"; Collection<String> sanctionedClasses = asList("class-name-one", "class-name-two"); GlobalSerialFilter globalSerialFilter = new ReflectiveFacadeGlobalSerialFilter(api, pattern, sanctionedClasses); + when(api.createObjectInputFilterProxy(eq("the-pattern"), anyCollection())) + .thenReturn(objectInputFilter); globalSerialFilter.setFilter(); @@ -58,9 +72,12 @@ public class ReflectiveFacadeGlobalSerialFilterTest { } @Test - public void setsSerialFilter() throws InvocationTargetException, IllegalAccessException { + public void setsSerialFilter() + throws InvocationTargetException, IllegalAccessException, UnableToSetSerialFilterException { GlobalSerialFilter globalSerialFilter = new ReflectiveFacadeGlobalSerialFilter(api, "the-pattern", singleton("class-name")); + when(api.createObjectInputFilterProxy(eq("the-pattern"), anyCollection())) + .thenReturn(objectInputFilter); globalSerialFilter.setFilter(); @@ -68,37 +85,84 @@ public class ReflectiveFacadeGlobalSerialFilterTest { } @Test - public void propagatesIllegalAccessExceptionInUnsupportedOperationException() + public void propagatesIllegalAccessExceptionInObjectInputFilterException() throws InvocationTargetException, IllegalAccessException { - IllegalAccessException exception = new IllegalAccessException("testing"); - doThrow(exception).when(api).setSerialFilter(any()); GlobalSerialFilter globalSerialFilter = new ReflectiveFacadeGlobalSerialFilter(api, "the-pattern", singleton("class-name")); + IllegalAccessException illegalAccessException = new IllegalAccessException("testing"); + when(api.createObjectInputFilterProxy(eq("the-pattern"), anyCollection())) + .thenReturn(objectInputFilter); + doThrow(illegalAccessException) + .when(api).setSerialFilter(any()); Throwable thrown = catchThrowable(() -> { globalSerialFilter.setFilter(); }); assertThat(thrown) - .isInstanceOf(UnsupportedOperationException.class) - .hasRootCause(exception); + .isInstanceOf(UnableToSetSerialFilterException.class) + .hasMessage("Unable to configure a global serialization filter using reflection.") + .hasRootCause(illegalAccessException) + .hasRootCauseMessage("testing"); } @Test - public void propagatesInvocationTargetExceptionInUnsupportedOperationException() + public void propagatesInvocationTargetExceptionInObjectInputFilterException() throws InvocationTargetException, IllegalAccessException { - InvocationTargetException exception = + GlobalSerialFilter globalSerialFilter = + new ReflectiveFacadeGlobalSerialFilter(api, "the-pattern", singleton("class-name")); + InvocationTargetException invocationTargetException = new InvocationTargetException(new Exception("testing"), "testing"); - doThrow(exception).when(api).setSerialFilter(any()); + doThrow(invocationTargetException).when(api).setSerialFilter(any()); + + Throwable thrown = catchThrowable(() -> { + globalSerialFilter.setFilter(); + }); + + assertThat(thrown) + .isInstanceOf(UnableToSetSerialFilterException.class) + .hasMessage( + "Unable to configure a global serialization filter because invocation target threw " + + Exception.class.getName() + ".") + .hasCause(invocationTargetException) + .hasRootCauseInstanceOf(Exception.class) + .hasRootCauseMessage("testing"); + } + + /** + * The ObjectInputFilter API throws IllegalStateException nested within InvocationTargetException + * if a non-null filter already exists. + */ + @Test + public void propagatesNestedIllegalStateExceptionInObjectInputFilterException() + throws InvocationTargetException, IllegalAccessException { GlobalSerialFilter globalSerialFilter = new ReflectiveFacadeGlobalSerialFilter(api, "the-pattern", singleton("class-name")); + InvocationTargetException invocationTargetException = + new InvocationTargetException(new IllegalStateException("testing"), "testing"); + doThrow(invocationTargetException).when(api).setSerialFilter(any()); Throwable thrown = catchThrowable(() -> { globalSerialFilter.setFilter(); }); assertThat(thrown) - .isInstanceOf(UnsupportedOperationException.class) - .hasCause(exception); + .isInstanceOf(FilterAlreadyConfiguredException.class) + .hasMessage( + "Unable to configure a global serialization filter because filter has already been set non-null.") + .hasCauseInstanceOf(InvocationTargetException.class) + .hasRootCauseInstanceOf(IllegalStateException.class) + .hasRootCauseMessage("testing"); + } + + @Test + public void requiresObjectInputFilterApi() { + Throwable thrown = catchThrowable(() -> { + new ReflectiveFacadeGlobalSerialFilter(null, "pattern", emptySet()); + }); + + assertThat(thrown) + .isInstanceOf(NullPointerException.class) + .hasMessage("ObjectInputFilterApi is required"); } } diff --git a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeObjectInputFilterFactoryTest.java b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeObjectInputFilterFactoryTest.java index e35b5bd..30c947b 100644 --- a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeObjectInputFilterFactoryTest.java +++ b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeObjectInputFilterFactoryTest.java @@ -48,10 +48,10 @@ public class ReflectiveFacadeObjectInputFilterFactoryTest { assumeThat(isJavaVersionAtLeast(JAVA_9)).isTrue(); // arrange - ObjectInputFilterFactory factory = new ReflectiveFacadeObjectInputFilterFactory(); + StreamSerialFilterFactory factory = new ReflectiveFacadeStreamSerialFilterFactory(); // act - ObjectInputFilter objectInputFilter = factory.create(config, SANCTIONED_CLASSES); + StreamSerialFilter objectInputFilter = factory.create(config, SANCTIONED_CLASSES); // assert assertThat(getApiPackage(getObjectInputFilterApi(objectInputFilter))).isEqualTo(JAVA_IO); @@ -62,17 +62,17 @@ public class ReflectiveFacadeObjectInputFilterFactoryTest { assumeThat(isJavaVersionAtMost(JAVA_1_8)).isTrue(); // arrange - ObjectInputFilterFactory factory = new ReflectiveFacadeObjectInputFilterFactory(); + StreamSerialFilterFactory factory = new ReflectiveFacadeStreamSerialFilterFactory(); // act - ObjectInputFilter objectInputFilter = factory.create(config, SANCTIONED_CLASSES); + StreamSerialFilter objectInputFilter = factory.create(config, SANCTIONED_CLASSES); // assert assertThat(getApiPackage(getObjectInputFilterApi(objectInputFilter))).isEqualTo(SUN_MISC); } - private static ObjectInputFilterApi getObjectInputFilterApi(ObjectInputFilter result) { - ReflectiveFacadeObjectInputFilter impl = (ReflectiveFacadeObjectInputFilter) result; + private static ObjectInputFilterApi getObjectInputFilterApi(StreamSerialFilter result) { + ReflectiveFacadeStreamSerialFilter impl = (ReflectiveFacadeStreamSerialFilter) result; ObjectInputFilterApi objectInputFilterApi = impl.getObjectInputFilterApi(); assertThat(objectInputFilterApi).isInstanceOf(ReflectiveObjectInputFilterApi.class); return objectInputFilterApi; diff --git a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeObjectInputFilterTest.java b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeObjectInputFilterTest.java index cca697f..3976129 100644 --- a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeObjectInputFilterTest.java +++ b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveFacadeObjectInputFilterTest.java @@ -50,11 +50,11 @@ public class ReflectiveFacadeObjectInputFilterTest { @Test public void createsObjectInputFilterProxy() - throws InvocationTargetException, IllegalAccessException { + throws InvocationTargetException, IllegalAccessException, UnableToSetSerialFilterException { String pattern = "the-pattern"; Collection<String> sanctionedClasses = asList("class-name-one", "class-name-two"); - ObjectInputFilter objectInputFilter = - new ReflectiveFacadeObjectInputFilter(api, pattern, sanctionedClasses); + StreamSerialFilter objectInputFilter = + new ReflectiveFacadeStreamSerialFilter(api, pattern, sanctionedClasses); objectInputFilter.setFilterOn(objectInputStream); @@ -64,9 +64,10 @@ public class ReflectiveFacadeObjectInputFilterTest { } @Test - public void setsSerialFilter() throws InvocationTargetException, IllegalAccessException { - ObjectInputFilter objectInputFilter = - new ReflectiveFacadeObjectInputFilter(api, "the-pattern", singleton("class-name")); + public void setsSerialFilter() + throws InvocationTargetException, IllegalAccessException, UnableToSetSerialFilterException { + StreamSerialFilter objectInputFilter = + new ReflectiveFacadeStreamSerialFilter(api, "the-pattern", singleton("class-name")); objectInputFilter.setFilterOn(objectInputStream); @@ -76,43 +77,75 @@ public class ReflectiveFacadeObjectInputFilterTest { @Test public void propagatesIllegalAccessExceptionInUnsupportedOperationException() throws InvocationTargetException, IllegalAccessException { - IllegalAccessException exception = new IllegalAccessException("testing"); - doThrow(exception).when(api).setObjectInputFilter(same(objectInputStream), any()); - ObjectInputFilter objectInputFilter = - new ReflectiveFacadeObjectInputFilter(api, "the-pattern", singleton("class-name")); + IllegalAccessException illegalAccessException = new IllegalAccessException("testing"); + doThrow(illegalAccessException).when(api).setObjectInputFilter(same(objectInputStream), any()); + StreamSerialFilter objectInputFilter = + new ReflectiveFacadeStreamSerialFilter(api, "the-pattern", singleton("class-name")); Throwable thrown = catchThrowable(() -> { objectInputFilter.setFilterOn(objectInputStream); }); assertThat(thrown) - .isInstanceOf(UnsupportedOperationException.class) - .hasRootCause(exception); + .isInstanceOf(UnableToSetSerialFilterException.class) + .hasMessage("Unable to configure an input stream serialization filter using reflection.") + .hasRootCause(illegalAccessException); } @Test public void propagatesInvocationTargetExceptionInUnsupportedOperationException() throws InvocationTargetException, IllegalAccessException { - InvocationTargetException exception = + InvocationTargetException invocationTargetException = new InvocationTargetException(new Exception("testing"), "testing"); - doThrow(exception).when(api).setObjectInputFilter(same(objectInputStream), any()); - ObjectInputFilter objectInputFilter = - new ReflectiveFacadeObjectInputFilter(api, "the-pattern", singleton("class-name")); + doThrow(invocationTargetException).when(api).setObjectInputFilter(same(objectInputStream), + any()); + StreamSerialFilter objectInputFilter = + new ReflectiveFacadeStreamSerialFilter(api, "the-pattern", singleton("class-name")); Throwable thrown = catchThrowable(() -> { objectInputFilter.setFilterOn(objectInputStream); }); assertThat(thrown) - .isInstanceOf(UnsupportedOperationException.class) - .hasCause(exception); + .isInstanceOf(UnableToSetSerialFilterException.class) + .hasMessage( + "Unable to configure an input stream serialization filter because invocation target threw " + + Exception.class.getName() + ".") + .hasCause(invocationTargetException); } + /** + * The ObjectInputFilter API throws IllegalStateException nested within InvocationTargetException + * if a non-null filter already exists. + */ @Test - public void delegatesToObjectInputFilterApiToCreateObjectInputFilter() + public void propagatesNestedIllegalStateExceptionInObjectInputFilterException() throws InvocationTargetException, IllegalAccessException { + StreamSerialFilter objectInputFilter = + new ReflectiveFacadeStreamSerialFilter(api, "the-pattern", singleton("class-name")); + InvocationTargetException invocationTargetException = + new InvocationTargetException(new IllegalStateException("testing"), "testing"); + doThrow(invocationTargetException) + .when(api).setObjectInputFilter(same(objectInputStream), any()); + + Throwable thrown = catchThrowable(() -> { + objectInputFilter.setFilterOn(objectInputStream); + }); + + assertThat(thrown) + .isInstanceOf(FilterAlreadyConfiguredException.class) + .hasMessage( + "Unable to configure an input stream serialization filter because a non-null filter has already been set.") + .hasCauseInstanceOf(InvocationTargetException.class) + .hasRootCauseInstanceOf(IllegalStateException.class) + .hasRootCauseMessage("testing"); + } + + @Test + public void delegatesToObjectInputFilterApiToCreateObjectInputFilter() + throws InvocationTargetException, IllegalAccessException, UnableToSetSerialFilterException { ObjectInputFilterApi api = mock(ObjectInputFilterApi.class); - ObjectInputFilter filter = new ReflectiveFacadeObjectInputFilter(api, "pattern", emptySet()); + StreamSerialFilter filter = new ReflectiveFacadeStreamSerialFilter(api, "pattern", emptySet()); Object objectInputFilter = mock(Object.class); ObjectInputStream objectInputStream = mock(ObjectInputStream.class); @@ -126,9 +159,9 @@ public class ReflectiveFacadeObjectInputFilterTest { @Test public void delegatesToObjectInputFilterApiToSetFilterOnObjectInputStream() - throws InvocationTargetException, IllegalAccessException { + throws InvocationTargetException, IllegalAccessException, UnableToSetSerialFilterException { ObjectInputFilterApi api = mock(ObjectInputFilterApi.class); - ObjectInputFilter filter = new ReflectiveFacadeObjectInputFilter(api, "pattern", emptySet()); + StreamSerialFilter filter = new ReflectiveFacadeStreamSerialFilter(api, "pattern", emptySet()); Object objectInputFilter = mock(Object.class); ObjectInputStream objectInputStream = mock(ObjectInputStream.class); diff --git a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveObjectInputFilterApiFactoryTest.java b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveObjectInputFilterApiFactoryTest.java index b3ac61f..a1eb877 100644 --- a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveObjectInputFilterApiFactoryTest.java +++ b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveObjectInputFilterApiFactoryTest.java @@ -18,13 +18,22 @@ import static org.apache.commons.lang3.JavaVersion.JAVA_1_8; import static org.apache.commons.lang3.JavaVersion.JAVA_9; import static org.apache.commons.lang3.SystemUtils.isJavaVersionAtLeast; import static org.apache.commons.lang3.SystemUtils.isJavaVersionAtMost; +import static org.apache.geode.internal.serialization.filter.SerialFilterAssertions.assertThatSerialFilterIsNull; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assumptions.assumeThat; +import java.lang.reflect.InvocationTargetException; + +import org.junit.After; import org.junit.Test; public class ReflectiveObjectInputFilterApiFactoryTest { + @After + public void serialFilterIsNull() throws InvocationTargetException, IllegalAccessException { + assertThatSerialFilterIsNull(); + } + @Test public void createsInstanceOfReflectionObjectInputFilterApi() { ObjectInputFilterApiFactory factory = new ReflectiveObjectInputFilterApiFactory(); diff --git a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveObjectInputFilterApiTest.java b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveObjectInputFilterApiTest.java index 8cf1f8c..b456eb3 100644 --- a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveObjectInputFilterApiTest.java +++ b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/ReflectiveObjectInputFilterApiTest.java @@ -22,6 +22,7 @@ import static org.apache.commons.lang3.JavaVersion.JAVA_1_8; import static org.apache.commons.lang3.JavaVersion.JAVA_9; import static org.apache.commons.lang3.SerializationUtils.serialize; import static org.apache.commons.lang3.SystemUtils.isJavaVersionAtLeast; +import static org.apache.geode.internal.serialization.filter.SerialFilterAssertions.assertThatSerialFilterIsNull; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.catchThrowable; @@ -32,6 +33,7 @@ import java.io.ObjectInputStream; import java.io.Serializable; import java.lang.reflect.InvocationTargetException; +import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -48,6 +50,11 @@ public class ReflectiveObjectInputFilterApiTest { } } + @After + public void serialFilterIsNull() throws InvocationTargetException, IllegalAccessException { + assertThatSerialFilterIsNull(); + } + @Test public void createFilterGivenValidPatternReturnsNewFilter() throws IllegalAccessException, InvocationTargetException { @@ -89,9 +96,7 @@ public class ReflectiveObjectInputFilterApiTest { Object filter = api.getSerialFilter(); - assertThat(filter) - .as("ObjectInputFilter$Config.getSerialFilter()") - .isNull(); + assertThat(filter).isNull(); } @Test diff --git a/geode-core/src/integrationTest/java/org/apache/geode/internal/serialization/filter/SerialFilterAssertions.java b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/SerialFilterAssertions.java similarity index 52% copy from geode-core/src/integrationTest/java/org/apache/geode/internal/serialization/filter/SerialFilterAssertions.java copy to geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/SerialFilterAssertions.java index b6d552a..760e391 100644 --- a/geode-core/src/integrationTest/java/org/apache/geode/internal/serialization/filter/SerialFilterAssertions.java +++ b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/SerialFilterAssertions.java @@ -14,12 +14,14 @@ */ package org.apache.geode.internal.serialization.filter; +import static java.lang.System.lineSeparator; +import static org.apache.commons.lang3.exception.ExceptionUtils.getStackTrace; import static org.assertj.core.api.Assertions.assertThat; import java.lang.reflect.InvocationTargetException; -@SuppressWarnings({"unused", "WeakerAccess"}) -public class SerialFilterAssertions { +@SuppressWarnings("unused") +class SerialFilterAssertions { private static final ObjectInputFilterApi API = new ReflectiveObjectInputFilterApiFactory() .createObjectInputFilterApi(); @@ -28,37 +30,40 @@ public class SerialFilterAssertions { // do not instantiate } - public static void assertThatSerialFilterIsNull() + static void assertThatSerialFilterIsNull() throws InvocationTargetException, IllegalAccessException { - boolean exists = API.getSerialFilter() != null; - assertThat(exists) - .as("ObjectInputFilter$Config.getSerialFilter() is null") - .isFalse(); + assertThat(API.getSerialFilter()) + .as("ObjectInputFilter$Config.getSerialFilter()") + .isNull(); } - public static void assertThatSerialFilterIsNotNull() + static void assertThatSerialFilterIsNotNull() throws InvocationTargetException, IllegalAccessException { - boolean exists = API.getSerialFilter() != null; - assertThat(exists) - .as("ObjectInputFilter$Config.getSerialFilter() is not null") - .isTrue(); + assertThat(API.getSerialFilter()) + .as("ObjectInputFilter$Config.getSerialFilter()") + .isNotNull(); } - public static void assertThatSerialFilterIsSameAs(Object objectInputFilter) + static void assertThatSerialFilterIsSameAs(Object objectInputFilter) throws InvocationTargetException, IllegalAccessException { - Object currentFilter = API.getSerialFilter(); - boolean sameIdentity = currentFilter == objectInputFilter; - assertThat(sameIdentity) - .as("ObjectInputFilter$Config.getSerialFilter() is same as parameter") - .isTrue(); + assertThat(API.getSerialFilter()) + .as("ObjectInputFilter$Config.getSerialFilter()") + .isSameAs(objectInputFilter); } - public static void assertThatSerialFilterIsNotSameAs(Object objectInputFilter) + static void assertThatSerialFilterIsNotSameAs(Object objectInputFilter) throws InvocationTargetException, IllegalAccessException { - Object currentFilter = API.getSerialFilter(); - boolean sameIdentity = currentFilter == objectInputFilter; - assertThat(sameIdentity) - .as("ObjectInputFilter$Config.getSerialFilter() is same as parameter") - .isFalse(); + assertThat(API.getSerialFilter()) + .as("ObjectInputFilter$Config.getSerialFilter()") + .isNotSameAs(objectInputFilter); + } + + static String failMessageWithStackTraces(String message, Iterable<Throwable> stackTraces) { + StringBuilder formattedStackTraces = new StringBuilder(message); + for (Throwable stackTrace : stackTraces) { + formattedStackTraces.append(lineSeparator()); + formattedStackTraces.append(getStackTrace(stackTrace)); + } + return formattedStackTraces.toString(); } } diff --git a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/SystemPropertyGlobalSerialFilterConfigurationFactoryTest.java b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/SystemPropertyGlobalSerialFilterConfigurationFactoryTest.java index 45070a6..0d01e7a 100644 --- a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/SystemPropertyGlobalSerialFilterConfigurationFactoryTest.java +++ b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/SystemPropertyGlobalSerialFilterConfigurationFactoryTest.java @@ -14,62 +14,131 @@ */ package org.apache.geode.internal.serialization.filter; +import static org.apache.geode.internal.serialization.filter.SerialFilterAssertions.assertThatSerialFilterIsNull; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.mockito.Mockito.mock; +import java.lang.reflect.InvocationTargetException; + +import org.junit.After; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.contrib.java.lang.system.RestoreSystemProperties; public class SystemPropertyGlobalSerialFilterConfigurationFactoryTest { + private SerializableObjectConfig config; + @Rule public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties(); + @Before + public void setUp() { + config = mock(SerializableObjectConfig.class); + } + + @After + public void serialFilterIsNull() throws InvocationTargetException, IllegalAccessException { + assertThatSerialFilterIsNull(); + } + @Test - public void createsConditionalGlobalSerialFilterConfiguration_whenEnableGlobalSerialFilter_isSet() { - System.setProperty("geode.enableGlobalSerialFilter", "true"); + public void createsNoOp_whenEnableGlobalSerialFilterIsFalse() + throws UnableToSetSerialFilterException { + System.clearProperty("geode.enableGlobalSerialFilter"); GlobalSerialFilterConfigurationFactory factory = new SystemPropertyGlobalSerialFilterConfigurationFactory(); - FilterConfiguration filterConfiguration = factory.create(mock(SerializableObjectConfig.class)); + FilterConfiguration configuration = factory.create(config); - assertThat(filterConfiguration).isInstanceOf(GlobalSerialFilterConfiguration.class); + boolean result = configuration.configure(); + + assertThat(result).isFalse(); } @Test - public void createsNoOp_whenEnableGlobalSerialFilter_isNotSet() { + public void createsEnabledGlobalSerialFilterConfiguration_whenEnableGlobalSerialFilterIsTrue() { + System.setProperty("geode.enableGlobalSerialFilter", "true"); GlobalSerialFilterConfigurationFactory factory = new SystemPropertyGlobalSerialFilterConfigurationFactory(); - FilterConfiguration filterConfiguration = factory.create(mock(SerializableObjectConfig.class)); + FilterConfiguration configuration = factory.create(config); - assertThat(filterConfiguration) - .isNotInstanceOf(GlobalSerialFilterConfiguration.class); + // don't actually invoke configure because this is a unit test + assertThat(configuration).isInstanceOf(GlobalSerialFilterConfiguration.class); } @Test - public void createsNoOp_whenJdkSerialFilter_isSet() { + public void createsNoOp_whenJdkSerialFilterExists() throws UnableToSetSerialFilterException { System.setProperty("jdk.serialFilter", "*"); GlobalSerialFilterConfigurationFactory factory = new SystemPropertyGlobalSerialFilterConfigurationFactory(); - FilterConfiguration filterConfiguration = factory.create(mock(SerializableObjectConfig.class)); + FilterConfiguration configuration = factory.create(config); - assertThat(filterConfiguration) - .isNotInstanceOf(GlobalSerialFilterConfiguration.class); + boolean result = configuration.configure(); + + assertThat(result).isFalse(); } @Test - public void createsNoOp_whenJdkSerialFilter_andEnableGlobalSerialFilter_areBothSet() { + public void createsNoOp_whenJdkSerialFilterExists_andEnableGlobalSerialFilterIsTrue() + throws UnableToSetSerialFilterException { System.setProperty("jdk.serialFilter", "*"); System.setProperty("geode.enableGlobalSerialFilter", "true"); GlobalSerialFilterConfigurationFactory factory = new SystemPropertyGlobalSerialFilterConfigurationFactory(); - FilterConfiguration filterConfiguration = factory.create(mock(SerializableObjectConfig.class)); + FilterConfiguration configuration = factory.create(config); + + boolean result = configuration.configure(); + + assertThat(result).isFalse(); + } + + @Test + public void createsNoOp_whenEnableGlobalSerialFilterIsFalse_andJreDoesNotSupportObjectInputFilter() + throws UnableToSetSerialFilterException { + boolean supportsObjectInputFilter = false; + GlobalSerialFilterConfigurationFactory configurationFactory = + new SystemPropertyGlobalSerialFilterConfigurationFactory(() -> supportsObjectInputFilter); + + FilterConfiguration configuration = configurationFactory.create(config); + + boolean result = configuration.configure(); + + assertThat(result).isFalse(); + } + + @Test + public void createsNoOp_whenEnableGlobalSerialFilterIsTrue_andJreDoesNotSupportObjectInputFilter() + throws UnableToSetSerialFilterException { + System.setProperty("geode.enableGlobalSerialFilter", "true"); + boolean supportsObjectInputFilter = false; + GlobalSerialFilterConfigurationFactory configurationFactory = + new SystemPropertyGlobalSerialFilterConfigurationFactory(() -> supportsObjectInputFilter); + + FilterConfiguration configuration = configurationFactory.create(config); + + boolean result = configuration.configure(); + + assertThat(result).isFalse(); + } + + @Test + public void doesNotThrow_whenEnableGlobalSerialFilterIsFalse_andJreDoesNotSupportObjectInputFilter() { + System.clearProperty("geode.enableGlobalSerialFilter"); + GlobalSerialFilterConfigurationFactory factory = + new SystemPropertyGlobalSerialFilterConfigurationFactory(); + + assertThatCode(() -> { + + FilterConfiguration configuration = factory.create(config); + + configuration.configure(); - assertThat(filterConfiguration) - .isNotInstanceOf(GlobalSerialFilterConfiguration.class); + }).doesNotThrowAnyException(); } } diff --git a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/SystemPropertyJmxSerialFilterConfigurationFactoryTest.java b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/SystemPropertyJmxSerialFilterConfigurationFactoryTest.java index d95ef9c..76a9a51 100644 --- a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/SystemPropertyJmxSerialFilterConfigurationFactoryTest.java +++ b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/filter/SystemPropertyJmxSerialFilterConfigurationFactoryTest.java @@ -46,7 +46,8 @@ public class SystemPropertyJmxSerialFilterConfigurationFactoryTest { } @Test - public void returnsEnabledConfiguration_whenFilteringIsEnabled() { + public void returnsEnabledConfiguration_whenFilteringIsEnabled() + throws UnableToSetSerialFilterException { JmxSerialFilterConfigurationFactory factory = new SystemPropertyJmxSerialFilterConfigurationFactory(true, PATTERN); @@ -56,7 +57,8 @@ public class SystemPropertyJmxSerialFilterConfigurationFactoryTest { } @Test - public void returnsDisabledConfiguration_whenFilteringIsDisabled() { + public void returnsDisabledConfiguration_whenFilteringIsDisabled() + throws UnableToSetSerialFilterException { JmxSerialFilterConfigurationFactory factory = new SystemPropertyJmxSerialFilterConfigurationFactory(false, PATTERN); @@ -66,7 +68,8 @@ public class SystemPropertyJmxSerialFilterConfigurationFactoryTest { } @Test - public void enabledConfiguration_setsJmxSerialFilterProperty() { + public void enabledConfiguration_setsJmxSerialFilterProperty() + throws UnableToSetSerialFilterException { JmxSerialFilterConfigurationFactory factory = new SystemPropertyJmxSerialFilterConfigurationFactory(true, PATTERN); FilterConfiguration filterConfiguration = factory.create(); @@ -77,7 +80,8 @@ public class SystemPropertyJmxSerialFilterConfigurationFactoryTest { } @Test - public void disabledConfiguration_doesNotSetAnySystemProperties() { + public void disabledConfiguration_doesNotSetAnySystemProperties() + throws UnableToSetSerialFilterException { Map<?, ?> propertiesBefore = System.getProperties(); JmxSerialFilterConfigurationFactory factory = new SystemPropertyJmxSerialFilterConfigurationFactory(true, PATTERN);