L1nq0 commented on code in PR #9075: URL: https://github.com/apache/storm/pull/9075#discussion_r3946584865
########## storm-client/test/jvm/org/apache/storm/serialization/SerializableSerializerFilterTest.java: ########## @@ -0,0 +1,187 @@ +/** + * 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.storm.serialization; + +import java.io.InvalidClassException; +import java.io.ObjectInputFilter; +import java.util.ArrayDeque; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.PriorityQueue; +import org.apache.commons.collections.functors.SimulatedGadget; +import org.apache.storm.Config; +import org.apache.storm.serialization.types.ListDelegateSerializer; +import org.apache.storm.utils.Utils; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertIterableEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Tests for the JEP-290 serial filter ({@link Config#TOPOLOGY_FALL_BACK_ON_JAVA_SERIALIZATION_FILTER}) protecting the + * java-serialization fallback bridge. Every allow and deny case exercises an actual round-trip through the bridge, not the + * filter API in isolation. All round-trips go through KryoValuesSerializer and KryoValuesDeserializer end to end. + */ +public class SerializableSerializerFilterTest { + + /** The maxbytes limit set in conf/defaults.yaml. */ + private static final long DEFAULT_MAX_BYTES = 10485760L; + + /** + * Minimal conf that routes unregistered classes through the java-serialization fallback bridge. {@code filterSpec == null} + * means the filter key is absent from the conf entirely (the pre-existing behavior). + */ + private Map<String, Object> bridgeConf(String filterSpec) { + Map<String, Object> conf = new Config(); + conf.put(Config.TOPOLOGY_KRYO_FACTORY, DefaultKryoFactory.class.getName()); + conf.put(Config.TOPOLOGY_TUPLE_SERIALIZER, ListDelegateSerializer.class.getName()); + conf.put(Config.TOPOLOGY_SKIP_MISSING_KRYO_REGISTRATIONS, false); + conf.put(Config.TOPOLOGY_FALL_BACK_ON_JAVA_SERIALIZATION, true); + if (filterSpec != null) { + conf.put(Config.TOPOLOGY_FALL_BACK_ON_JAVA_SERIALIZATION_FILTER, filterSpec); + } + return conf; + } + + /** conf assembled exactly like a worker's would be: defaults.yaml + topology-level overrides. */ + private Map<String, Object> defaultsBridgeConf() { + Map<String, Object> conf = new Config(); + conf.putAll(Utils.readDefaultConfig()); + conf.put(Config.TOPOLOGY_FALL_BACK_ON_JAVA_SERIALIZATION, true); + return conf; + } + + private Object roundTrip(Map<String, Object> conf, Object value) { + KryoValuesSerializer serializer = new KryoValuesSerializer(conf); + KryoValuesDeserializer deserializer = new KryoValuesDeserializer(conf); + return deserializer.deserialize(serializer.serialize(Collections.singletonList(value))).get(0); + } + + /** Serializes {@code value} and asserts that reading it back fails with a JEP-290 rejection in the cause chain. */ + private void assertRejectedOnRead(Map<String, Object> conf, Object value) { + KryoValuesSerializer serializer = new KryoValuesSerializer(conf); + KryoValuesDeserializer deserializer = new KryoValuesDeserializer(conf); + // Writing is plain java serialization (filters apply to deserialization only), so this must succeed. + byte[] bytes = serializer.serialize(Collections.singletonList(value)); + RuntimeException ex = assertThrows(RuntimeException.class, () -> deserializer.deserialize(bytes)); + assertTrue(hasCause(ex, InvalidClassException.class), + "expected the JEP-290 filter rejection in the cause chain, got: " + ex); + } + + private static boolean hasCause(Throwable throwable, Class<? extends Throwable> type) { Review Comment: Replaced with Utils.exceptionCauseIsInstanceOf exactly as in your snippet, landed in 54e4926. While reworking the file I also dropped the fixture classes under third-party package names (the split-package hazard rzo1 had flagged), so the wildcard-depth tests now round-trip JDK classes that really go through the bridge; the fact that !java.util.** rejects a class !java.util.* would allow also shows the value took that path. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
