This is an automated email from the ASF dual-hosted git repository. brandonwilliams pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/cassandra.git
commit 2958c558707e3e2252bdd38d61981ce7eb138717 Author: Adam Holmberg <[email protected]> AuthorDate: Tue Sep 15 15:55:01 2020 -0500 Fix null type returned in StreamMessage lookup Patch by Adam Holmberg, reviewed by brandonwilliams for CASSANDRA-16131 --- CHANGES.txt | 1 + .../streaming/messages/StreamMessage.java | 24 +++----- .../streaming/messages/StreamMessageTest.java | 69 ++++++++++++++++++++++ 3 files changed, 79 insertions(+), 15 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 90f5995..4916faa 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0-beta3 + * Prevent NPE in StreamMessage in type lookup (CASSANDRA-16131) * Avoid invalid state transition exception during incremental repair (CASSANDRA-16067) * Allow zero padding in timestamp serialization (CASSANDRA-16105) * Add byte array backed cells (CASSANDRA-15393) diff --git a/src/java/org/apache/cassandra/streaming/messages/StreamMessage.java b/src/java/org/apache/cassandra/streaming/messages/StreamMessage.java index e2f08fd..e3f805e 100644 --- a/src/java/org/apache/cassandra/streaming/messages/StreamMessage.java +++ b/src/java/org/apache/cassandra/streaming/messages/StreamMessage.java @@ -18,6 +18,8 @@ package org.apache.cassandra.streaming.messages; import java.io.IOException; +import java.util.HashMap; +import java.util.Map; import io.netty.channel.Channel; @@ -72,33 +74,25 @@ public abstract class StreamMessage PREPARE_ACK (9, 5, PrepareAckMessage.serializer ), STREAM_INIT (10, 5, StreamInitMessage.serializer ); - private static final Type[] idToTypeMap; + private static final Map<Integer, Type> idToTypeMap; static { - Type[] values = values(); - - int max = Integer.MIN_VALUE; - for (Type t : values) - max = max(t.id, max); - - Type[] idMap = new Type[max + 1]; - for (Type t : values) + idToTypeMap = new HashMap<>(); + for (Type t : values()) { - if (idMap[t.id] != null) + if (idToTypeMap.put(t.id, t) != null) throw new RuntimeException("Two StreamMessage Types map to the same id: " + t.id); - idMap[t.id] = t; } - - idToTypeMap = idMap; } public static Type lookupById(int id) { - if (id < 0 || id >= idToTypeMap.length) + Type t = idToTypeMap.get(id); + if (t == null) throw new IllegalArgumentException("Invalid type id: " + id); - return idToTypeMap[id]; + return t; } public final int id; diff --git a/test/unit/org/apache/cassandra/streaming/messages/StreamMessageTest.java b/test/unit/org/apache/cassandra/streaming/messages/StreamMessageTest.java new file mode 100644 index 0000000..f50965e --- /dev/null +++ b/test/unit/org/apache/cassandra/streaming/messages/StreamMessageTest.java @@ -0,0 +1,69 @@ +/* + * 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.cassandra.streaming.messages; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +import org.junit.Test; + +import static org.junit.Assert.assertThat; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.junit.Assert.fail; + +public class StreamMessageTest +{ + @Test + public void testTypeLookup() // CASSANDRA-15965 + { + List<Integer> ids = Arrays.stream(StreamMessage.Type.values()) + .mapToInt(t -> t.id) + .boxed() + .collect(Collectors.toList()); + + for (int i: ids) + { + assertThat(StreamMessage.Type.lookupById(i), instanceOf(StreamMessage.Type.class)); + } + + int max = Collections.max(ids); + int min = Collections.min(ids); + int[] badTypes = {0, + -1, + min - 1, // right now this is redundant to zero + max + 1, + Integer.MAX_VALUE, + Integer.MIN_VALUE}; + + for (int t: badTypes) + { + try + { + StreamMessage.Type.lookupById(t); + fail("IllegalArgumentException was not thrown for " + t); + } + catch(IllegalArgumentException iae) + { + } + } + + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
