From eaa5c010283fee4247b296cfe2d6eb36662a9ab6 Mon Sep 17 00:00:00 2001 From: Jaroslaw Grabowski Date: Thu, 3 Oct 2024 14:49:18 +0200 Subject: [PATCH 1/2] CNDB-11118 return null serializer if response verb is null response verb is null for deprecated REQUEST_RSP and INTERNAL_RSP. info.responseVerb.serializer() caused NullPointerExceptions. The null serializer is later gracefully handled in deserializePayloadPre40. --- .../cassandra/net/RequestCallbacks.java | 3 +- .../cassandra/net/RequestCallbacksTest.java | 62 +++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 test/unit/org/apache/cassandra/net/RequestCallbacksTest.java diff --git a/src/java/org/apache/cassandra/net/RequestCallbacks.java b/src/java/org/apache/cassandra/net/RequestCallbacks.java index d2f91016f01a..14b0eeedc6c4 100644 --- a/src/java/org/apache/cassandra/net/RequestCallbacks.java +++ b/src/java/org/apache/cassandra/net/RequestCallbacks.java @@ -130,10 +130,11 @@ public void addWithExpiration(AbstractWriteResponseHandler cb, assert previous == null : format("Callback already exists for id %d/%s! (%s)", message.id(), to.endpoint(), previous); } + @Nullable IVersionedAsymmetricSerializer responseSerializer(long id, InetAddressAndPort peer) { CallbackInfo info = get(id, peer); - return info == null ? null : info.responseVerb.serializer(); + return info == null || info.responseVerb == null ? null : info.responseVerb.serializer(); } @VisibleForTesting diff --git a/test/unit/org/apache/cassandra/net/RequestCallbacksTest.java b/test/unit/org/apache/cassandra/net/RequestCallbacksTest.java new file mode 100644 index 000000000000..07d7dc7cb48b --- /dev/null +++ b/test/unit/org/apache/cassandra/net/RequestCallbacksTest.java @@ -0,0 +1,62 @@ +/* + * 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.net; + +import org.junit.BeforeClass; +import org.junit.Test; + +import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.io.IVersionedAsymmetricSerializer; +import org.apache.cassandra.locator.InetAddressAndPort; + +import static org.junit.Assert.assertNull; +import static org.mockito.Mockito.mock; + +public class RequestCallbacksTest +{ + @BeforeClass + public static void init() + { + DatabaseDescriptor.daemonInitialization(); + } + + @Test + public void testInternalResponseNullSerializer() throws Exception + { + depractedResponsesShouldReturnNullSerializer(Verb.INTERNAL_RSP); + } + + @Test + public void testRequestResponseNullSerializer() throws Exception + { + depractedResponsesShouldReturnNullSerializer(Verb.REQUEST_RSP); + } + + public void depractedResponsesShouldReturnNullSerializer(Verb verb) throws Exception + { + MessagingService messagingService = mock(MessagingService.class); + RequestCallbacks requestCallbacks = new RequestCallbacks(messagingService); + Message msg = Message.remoteResponse(InetAddressAndPort.getByName("127.0.0.1"), verb, null); + requestCallbacks.addWithExpiration(mock(RequestCallback.class), msg, msg.from()); + + IVersionedAsymmetricSerializer serializer = requestCallbacks.responseSerializer(msg.id(), msg.from()); + + assertNull(serializer); + } +} \ No newline at end of file From 35eb670a1be7ceee573a06ed77f250c79deea4d8 Mon Sep 17 00:00:00 2001 From: Jaroslaw Grabowski Date: Fri, 18 Oct 2024 10:31:14 +0200 Subject: [PATCH 2/2] CNDB-11118 review remarks --- src/java/org/apache/cassandra/net/RequestCallbacks.java | 5 +++++ .../unit/org/apache/cassandra/net/RequestCallbacksTest.java | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/java/org/apache/cassandra/net/RequestCallbacks.java b/src/java/org/apache/cassandra/net/RequestCallbacks.java index 14b0eeedc6c4..ee6b6c73aa03 100644 --- a/src/java/org/apache/cassandra/net/RequestCallbacks.java +++ b/src/java/org/apache/cassandra/net/RequestCallbacks.java @@ -134,6 +134,11 @@ public void addWithExpiration(AbstractWriteResponseHandler cb, IVersionedAsymmetricSerializer responseSerializer(long id, InetAddressAndPort peer) { CallbackInfo info = get(id, peer); + /** + * For legacy {@link Verb#REQUEST_RSP} and {@link Verb#INTERNAL_RSP}, the response verb is null, + * so we can't use its serializer. That's ok these Verbs don't convey any payload, so they don't need a + * serializer. + */ return info == null || info.responseVerb == null ? null : info.responseVerb.serializer(); } diff --git a/test/unit/org/apache/cassandra/net/RequestCallbacksTest.java b/test/unit/org/apache/cassandra/net/RequestCallbacksTest.java index 07d7dc7cb48b..55c1e62421bf 100644 --- a/test/unit/org/apache/cassandra/net/RequestCallbacksTest.java +++ b/test/unit/org/apache/cassandra/net/RequestCallbacksTest.java @@ -39,16 +39,16 @@ public static void init() @Test public void testInternalResponseNullSerializer() throws Exception { - depractedResponsesShouldReturnNullSerializer(Verb.INTERNAL_RSP); + deprecatedResponsesShouldReturnNullSerializer(Verb.INTERNAL_RSP); } @Test public void testRequestResponseNullSerializer() throws Exception { - depractedResponsesShouldReturnNullSerializer(Verb.REQUEST_RSP); + deprecatedResponsesShouldReturnNullSerializer(Verb.REQUEST_RSP); } - public void depractedResponsesShouldReturnNullSerializer(Verb verb) throws Exception + public void deprecatedResponsesShouldReturnNullSerializer(Verb verb) throws Exception { MessagingService messagingService = mock(MessagingService.class); RequestCallbacks requestCallbacks = new RequestCallbacks(messagingService);