From ba434b8231360347984c9a9906e5469767bfbcec Mon Sep 17 00:00:00 2001 From: nvharikrishna Date: Wed, 14 Dec 2022 00:25:54 +0530 Subject: [PATCH] Fix NPE in fqltool dump on null value If fullquerylog is enabled and a prepared query executed with null value binded, then fqltool dump is running into NPE. Checking value for null before wrapping it. patch by N V Harikrishna; reviewed by Stefan Miklosovic and Brandon Williams for CASSANDRA-18113 --- CHANGES.txt | 1 + build.xml | 2 +- .../cassandra/fqltool/commands/Dump.java | 22 +++-- .../cassandra/fqltool/commands/DumpTest.java | 93 +++++++++++++++++++ 4 files changed, 111 insertions(+), 7 deletions(-) create mode 100644 tools/fqltool/test/unit/org/apache/cassandra/fqltool/commands/DumpTest.java diff --git a/CHANGES.txt b/CHANGES.txt index 4e8cf2d0f7..5fa7bc481d 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0.8 + * Fix NPE in fqltool dump on null value (CASSANDRA-18113) * Improve unit tests performance (CASSANDRA-17427) * Connect to listen address when own broadcast address is requested (CASSANDRA-18200) * Add safeguard so cleanup fails when node has pending ranges (CASSANDRA-16418) diff --git a/build.xml b/build.xml index dff8ce96b3..ec95b372e8 100644 --- a/build.xml +++ b/build.xml @@ -1074,7 +1074,7 @@ - + diff --git a/tools/fqltool/src/org/apache/cassandra/fqltool/commands/Dump.java b/tools/fqltool/src/org/apache/cassandra/fqltool/commands/Dump.java index f26f0f92ad..8c05db1ed7 100644 --- a/tools/fqltool/src/org/apache/cassandra/fqltool/commands/Dump.java +++ b/tools/fqltool/src/org/apache/cassandra/fqltool/commands/Dump.java @@ -26,6 +26,8 @@ import java.util.Collections; import java.util.List; import java.util.stream.Collectors; +import com.google.common.annotations.VisibleForTesting; + import io.airlift.airline.Arguments; import io.airlift.airline.Command; import io.airlift.airline.Option; @@ -164,7 +166,8 @@ public class Dump implements Runnable } } - private static void dumpQuery(QueryOptions options, WireIn wireIn, StringBuilder sb) + @VisibleForTesting + static void dumpQuery(QueryOptions options, WireIn wireIn, StringBuilder sb) { sb.append("Query: ") .append(wireIn.read(FullQueryLogger.QUERY).text()) @@ -219,12 +222,19 @@ public class Dump implements Runnable boolean first = true; for (ByteBuffer value : values) { - Bytes bytes = Bytes.wrapForRead(value); - long maxLength2 = Math.min(1024, bytes.readLimit() - bytes.readPosition()); - toHexString(bytes, bytes.readPosition(), maxLength2, sb); - if (maxLength2 < bytes.readLimit() - bytes.readPosition()) + if (null == value) { - sb.append("... truncated").append(System.lineSeparator()); + sb.append("null").append(System.lineSeparator()); + } + else + { + Bytes bytes = Bytes.wrapForRead(value); + long maxLength2 = Math.min(1024, bytes.readLimit() - bytes.readPosition()); + toHexString(bytes, bytes.readPosition(), maxLength2, sb); + if (maxLength2 < bytes.readLimit() - bytes.readPosition()) + { + sb.append("... truncated").append(System.lineSeparator()); + } } if (first) diff --git a/tools/fqltool/test/unit/org/apache/cassandra/fqltool/commands/DumpTest.java b/tools/fqltool/test/unit/org/apache/cassandra/fqltool/commands/DumpTest.java new file mode 100644 index 0000000000..77d7563787 --- /dev/null +++ b/tools/fqltool/test/unit/org/apache/cassandra/fqltool/commands/DumpTest.java @@ -0,0 +1,93 @@ +/* + * 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.fqltool.commands; + +import java.nio.ByteBuffer; +import java.util.Arrays; +import java.util.List; +import java.util.Objects; + +import org.junit.Assert; +import org.junit.Test; + +import net.openhft.chronicle.wire.ValueIn; +import net.openhft.chronicle.wire.WireIn; +import org.apache.cassandra.cql3.QueryOptions; +import org.apache.cassandra.db.ConsistencyLevel; +import org.apache.cassandra.fql.FullQueryLogger; +import org.apache.cassandra.transport.ProtocolVersion; +import org.mockito.Mockito; + +public class DumpTest +{ + @Test + public void testDumpQueryNullValues() + { + String keyspace = "ks1"; + + List values = Arrays.asList(ByteBuffer.wrap(new byte[]{ 1 }), null); + + QueryOptions queryOptions = QueryOptions.create( + ConsistencyLevel.LOCAL_QUORUM, + values, + true, + 1, + null, + null, + ProtocolVersion.CURRENT, + keyspace + ); + + ValueIn mockValueIn = Mockito.mock(ValueIn.class); + Mockito.when(mockValueIn.text()).thenReturn("INSERT INTO ks1.t1 (k, v) VALUES (?,?)"); + + WireIn mockWireIn = Mockito.mock(WireIn.class); + Mockito.when(mockWireIn.read(FullQueryLogger.QUERY)).thenReturn(mockValueIn); + + StringBuilder sb = new StringBuilder(); + Dump.dumpQuery(queryOptions, mockWireIn, sb); + + String[] lines = sb.toString().split(System.lineSeparator()); + boolean valuesStarted = false; + int count = 0; + int nullcount = 0; + for (String line : lines) + { + if (line.startsWith("Values:")) + { + valuesStarted = true; + continue; + } + if (valuesStarted) + { + if ("-----".equals(line)) + { + continue; + } + if (null == values.get(count++)) + { + nullcount++; + Assert.assertEquals("null", line); + } + } + } + + Assert.assertEquals(values.stream().filter(Objects::isNull).count(), nullcount); + } +}