This is an automated email from the ASF dual-hosted git repository. klund pushed a commit to branch support/1.13 in repository https://gitbox.apache.org/repos/asf/geode.git
commit f284c109d8f808ad4033a8f405af104683b9b0de Author: Kirk Lund <kl...@apache.org> AuthorDate: Tue Feb 8 12:04:44 2022 -0800 GEODE-9980: Improve error handling of serial filters (#7299) 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) --- .../filter/LocatorLauncherWithJmxManager.java | 2 +- .../filter/SerialFilterAssertions.java | 30 +++--- .../filter/ServerLauncherWithJmxManager.java | 2 +- .../apache/geode/distributed/LocatorLauncher.java | 16 ++- .../apache/geode/distributed/ServerLauncher.java | 16 ++- .../geode/internal/InternalDataSerializer.java | 27 +++-- .../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 ++- 43 files changed, 1067 insertions(+), 437 deletions(-) diff --git a/geode-core/src/integrationTest/java/org/apache/geode/internal/serialization/filter/LocatorLauncherWithJmxManager.java b/geode-core/src/integrationTest/java/org/apache/geode/internal/serialization/filter/LocatorLauncherWithJmxManager.java index 8bd96f5..b18865c 100644 --- a/geode-core/src/integrationTest/java/org/apache/geode/internal/serialization/filter/LocatorLauncherWithJmxManager.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/internal/serialization/filter/LocatorLauncherWithJmxManager.java @@ -30,7 +30,7 @@ import org.apache.geode.management.ManagementService; import org.apache.geode.management.internal.SystemManagementService; import org.apache.geode.test.junit.rules.CloseableReference; -abstract class LocatorLauncherWithJmxManager { +public abstract class LocatorLauncherWithJmxManager { static final String NAME = "locator"; static final String JDK_PROPERTY = "jdk.serialFilter"; 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/integrationTest/java/org/apache/geode/internal/serialization/filter/ServerLauncherWithJmxManager.java b/geode-core/src/integrationTest/java/org/apache/geode/internal/serialization/filter/ServerLauncherWithJmxManager.java index eba157c..a47c83a 100644 --- a/geode-core/src/integrationTest/java/org/apache/geode/internal/serialization/filter/ServerLauncherWithJmxManager.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/internal/serialization/filter/ServerLauncherWithJmxManager.java @@ -29,7 +29,7 @@ import org.apache.geode.management.ManagementService; import org.apache.geode.management.internal.SystemManagementService; import org.apache.geode.test.junit.rules.CloseableReference; -abstract class ServerLauncherWithJmxManager { +public abstract class ServerLauncherWithJmxManager { static final String NAME = "server"; static final String JDK_PROPERTY = "jdk.serialFilter"; 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 056dc09..56b4d83 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 @@ -117,12 +117,13 @@ import org.apache.geode.internal.serialization.SerializationVersions; import org.apache.geode.internal.serialization.StaticSerialization; import org.apache.geode.internal.serialization.Version; 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 +287,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 +419,13 @@ 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 static void clearSerializationFilter() { serializationFilter = defaultSerializationFilter; } @@ -2682,7 +2682,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) { Version v = ((VersionedDataStream) stream).getVersion(); if (Version.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);