CNDB-15280: Remove user data from AbstractReadQuery.toCQLString

src/java/org/apache/cassandra/db/AbstractReadQuery.java

@@ -19,6 +19,8 @@
 
 import java.util.Set;
 
+import com.google.common.annotations.VisibleForTesting;
+
 import org.apache.cassandra.cql3.ColumnIdentifier;
 import org.apache.cassandra.cql3.CqlBuilder;
 import org.apache.cassandra.cql3.statements.SelectOptions;
@@ -62,7 +64,7 @@ public TableMetadata metadata()
     // Monitorable interface
     public String name()
     {
-        return toCQLString();
+        return toRedactedCQLString();
     }
 
     @Override
@@ -96,23 +98,53 @@ public ColumnFilter columnFilter()
     }
 
     /**
-     * Recreate the CQL string corresponding to this query.
+     * Recreates the CQL string corresponding to this query, representing any specific values with '?',
+     * to prevent leaking sensitive data.
+     * @see #toCQLString(boolean)
+     */
+    public String toRedactedCQLString()
+    {
+        return toCQLString(true);
+    }
+
+    /**
+     * Recreates the CQL string corresponding to this query, printing specific values without any redaction.
+     * This might leak sensitive data if the query string ends up in logs or any other unprotected place, so this only
+     * should be used for debugging purposes or to present the query string to the same end user that created the query.
+     * @see #toCQLString(boolean)
+     */
+    public String toUnredactedCQLString()
+    {
+        return toCQLString(false);
+    }
+
+    /**
+     * Recreates the CQL string corresponding to this query.
+     * </p>
+     * If the {@code redact} parameter is set to {@code true}, the query string will be redacted, replacing any specific
+     * column values with '?'. If set to {@code false}, the query string will not be redacted, and it might expose the
+     * queried column values which might contain sensitive data. The latter will be problematic if the query string ends
+     * up in logs or any other unprotected place. Therefore, non-redaction should only be used for debugging purposes or
+     * to present the query string to the same end user that created the query.
      * <p>
      * Note that in general the returned string will not be exactly the original user string, first
-     * because there isn't always a single syntax for a given query,  but also because we don't have
+     * because there isn't always a single syntax for a given query, but also because we don't have
      * all the information needed (we know the non-PK columns queried but not the PK ones as internally
-     * we query them all). So this shouldn't be relied too strongly, but this should be good enough for
-     * debugging purpose which is what this is for.
+     * we query them all). So this shouldn't be relied upon too strongly, but this should be good enough for
+     * debugging purposes which is what this is for.
+     *
+     * @param redact whether to redact the queried column values.
      */
-    public String toCQLString()
+    @VisibleForTesting
+    protected String toCQLString(boolean redact)
     {
         CqlBuilder builder = new CqlBuilder();
-        builder.append("SELECT ").append(columnFilter().toCQLString());
+        builder.append("SELECT ").append(columnFilter().toCQLString(redact));
         builder.append(" FROM ").append(ColumnIdentifier.maybeQuote(metadata().keyspace))
                .append('.')
                .append(ColumnIdentifier.maybeQuote(metadata().name));
 
-        appendCQLWhereClause(builder);
+        appendCQLWhereClause(builder, redact);
 
         if (limits() != DataLimits.NONE)
             builder.append(' ').append(limits());
@@ -132,5 +164,5 @@ public String toCQLString()
         return builder.toString();
     }
 
-    protected abstract void appendCQLWhereClause(CqlBuilder builder);
+    protected abstract void appendCQLWhereClause(CqlBuilder builder, boolean redact);
 }

src/java/org/apache/cassandra/db/Clustering.java

@@ -49,7 +49,7 @@ public default Clustering<?> clone(ByteBufferCloner cloner)
         ByteBuffer[] newValues = new ByteBuffer[size()];
         for (int i = 0; i < size(); i++)
         {
-            ByteBuffer val = accessor().toBuffer(get(i));
+            ByteBuffer val = bufferAt(i);
             newValues[i] = val == null ? null : cloner.clone(val);
         }
         return new BufferClustering(newValues);
@@ -78,13 +78,14 @@ public default String toString(TableMetadata metadata)
         return sb.toString();
     }
 
-    public default String toCQLString(TableMetadata metadata)
+    default String toCQLString(TableMetadata metadata, boolean redact)
     {
         StringBuilder sb = new StringBuilder();
         for (int i = 0; i < size(); i++)
         {
             ColumnMetadata c = metadata.clusteringColumns().get(i);
-            sb.append(i == 0 ? "" : ", ").append(c.type.getString(get(i), accessor()));
+            ByteBuffer value = bufferAt(i);
+            sb.append(i == 0 ? "" : ", ").append(c.type.toCQLString(value, redact));
         }
         return sb.toString();
     }

src/java/org/apache/cassandra/db/ClusteringPrefix.java

@@ -358,11 +358,11 @@ default int dataSize()
     default ByteBuffer serializeAsPartitionKey()
     {
         if (size() == 1)
-            return accessor().toBuffer(get(0));
+            return bufferAt(0);
 
         ByteBuffer[] values = new ByteBuffer[size()];
         for (int i = 0; i < size(); i++)
-            values[i] = accessor().toBuffer(get(i));
+            values[i] = bufferAt(i);
         return CompositeType.build(ByteBufferAccessor.instance, values);
     }
 
@@ -752,4 +752,4 @@ public static boolean equals(ClusteringPrefix<?> prefix, Object o)
         return equals(prefix, (ClusteringPrefix<?>) o);
     }
 
-}
+}

src/java/org/apache/cassandra/db/DataRange.java

@@ -19,7 +19,6 @@
 import java.io.IOException;
 import java.nio.ByteBuffer;
 
-import org.apache.cassandra.cql3.CqlBuilder;
 import org.apache.cassandra.schema.ColumnMetadata;
 import org.apache.cassandra.schema.TableMetadata;
 import org.apache.cassandra.db.filter.*;
@@ -272,65 +271,59 @@ public String toString(TableMetadata metadata)
         return String.format("range=%s pfilter=%s", keyRange.getString(metadata.partitionKeyType), clusteringIndexFilter.toString(metadata));
     }
 
-    public String toCQLString(TableMetadata metadata)
+    public String toCQLString(TableMetadata metadata, boolean redact)
     {
         if (isUnrestricted())
             return "";
 
-        CqlBuilder builder = new CqlBuilder();
-
-        boolean needAnd = false;
-
         if (isSinglePartition())
         {
             /*
              * Single partition queries using an index are internally mapped to range commands where the start and end
              * key are the same. If that is the case, we want to print the query as an equality on the partition key
              * rather than a token range, as if it was a partition query, for better readability.
              */
-            builder.append(((DecoratedKey) startKey()).toCQLString(metadata));
-            needAnd = true;
+            return ((DecoratedKey) startKey()).toCQLString(metadata, redact);
         }
         else
         {
+            StringBuilder builder = new StringBuilder();
             if (!startKey().isMinimum())
             {
-                appendClause(startKey(), builder, metadata, true, keyRange.isStartInclusive());
-                needAnd = true;
+                appendCQLClause(startKey(), builder, metadata, true, keyRange.isStartInclusive(), redact);
             }
             if (!stopKey().isMinimum())
             {
-                if (needAnd)
+                if (builder.length() > 0)
                     builder.append(" AND ");
-                appendClause(stopKey(), builder, metadata, false, keyRange.isEndInclusive());
-                needAnd = true;
+                appendCQLClause(stopKey(), builder, metadata, false, keyRange.isEndInclusive(), redact);
             }
+            return builder.toString();
         }
-
-        String filterString = clusteringIndexFilter.toCQLString(metadata);
-        if (!filterString.isEmpty())
-            builder.append(needAnd ? " AND " : "").append(filterString);
-
-        return builder.toString();
     }
 
-    private void appendClause(PartitionPosition pos, CqlBuilder builder, TableMetadata metadata, boolean isStart, boolean isInclusive)
+    private void appendCQLClause(PartitionPosition pos,
+                                 StringBuilder builder,
+                                 TableMetadata metadata,
+                                 boolean isStart,
+                                 boolean isInclusive,
+                                 boolean redact)
     {
         builder.append("token(");
         builder.append(ColumnMetadata.toCQLString(metadata.partitionKeyColumns()));
         builder.append(") ");
         if (pos instanceof DecoratedKey)
         {
-            builder.append(getOperator(isStart, isInclusive)).append(" ");
+            builder.append(getOperator(isStart, isInclusive)).append(' ');
             builder.append("token(");
-            appendKeyString(builder, metadata.partitionKeyType, ((DecoratedKey)pos).getKey());
-            builder.append(")");
+            appendKeyString(builder, metadata.partitionKeyType, ((DecoratedKey)pos).getKey(), redact);
+            builder.append(')');
         }
         else
         {
             Token.KeyBound keyBound = (Token.KeyBound) pos;
-            builder.append(getOperator(isStart, isStart == keyBound.isMinimumBound)).append(" ");
-            builder.append(keyBound.getToken());
+            builder.append(getOperator(isStart, isStart == keyBound.isMinimumBound)).append(' ');
+            builder.append(redact ? "?" : keyBound.getToken());
         }
     }
 
@@ -343,18 +336,18 @@ private static String getOperator(boolean isStart, boolean isInclusive)
 
     // TODO: this is reused in SinglePartitionReadCommand but this should not really be here. Ideally
     // we need a more "native" handling of composite partition keys.
-    public static void appendKeyString(CqlBuilder builder, AbstractType<?> type, ByteBuffer key)
+    public static void appendKeyString(StringBuilder builder, AbstractType<?> type, ByteBuffer key, boolean redact)
     {
         if (type instanceof CompositeType)
         {
             CompositeType ct = (CompositeType)type;
             ByteBuffer[] values = ct.split(key);
             for (int i = 0; i < ct.subTypes().size(); i++)
-                builder.append(i == 0 ? "" : ", ").append(ct.subTypes().get(i).getString(values[i]));
+                builder.append(i == 0 ? "" : ", ").append(ct.subTypes().get(i).toCQLString(values[i], redact));
         }
         else
         {
-            builder.append(type.getString(key));
+            builder.append(type.toCQLString(key, redact));
         }
     }
 

src/java/org/apache/cassandra/db/DecoratedKey.java

@@ -170,26 +170,29 @@ public String toString()
      * Generate CQL representation of this partition key for the given table.
      * For single-column keys: "k = 0"
      * For multi-column keys: "k1 = 1 AND k2 = 2"
+     *
+     * @param metadata the table metadata
+     * @param redact whether to redact the key value, as in "k1 = ? AND k2 = ?".
      */
-    public String toCQLString(TableMetadata metadata)
+    public String toCQLString(TableMetadata metadata, boolean redact)
     {
         List<ColumnMetadata> columns = metadata.partitionKeyColumns();
 
         if (columns.size() == 1)
-            return toCQLString(columns.get(0), getKey());
+            return toCQLString(columns.get(0), getKey(), redact);
 
         ByteBuffer[] values = ((CompositeType) metadata.partitionKeyType).split(getKey());
         StringJoiner joiner = new StringJoiner(" AND ");
 
         for (int i = 0; i < columns.size(); i++)
-            joiner.add(toCQLString(columns.get(i), values[i]));
+            joiner.add(toCQLString(columns.get(i), values[i], redact));
 
         return joiner.toString();
     }
 
-    private static String toCQLString(ColumnMetadata metadata, ByteBuffer key)
+    private static String toCQLString(ColumnMetadata metadata, ByteBuffer key, boolean redact)
     {
-        return String.format("%s = %s", metadata.name.toCQLString(), metadata.type.getString(key));
+        return String.format("%s = %s", metadata.name.toCQLString(), metadata.type.toCQLString(key, redact));
     }
 
     public Token getToken()

src/java/org/apache/cassandra/db/MultiPartitionReadQuery.java

@@ -0,0 +1,81 @@
+/*
+ * Copyright DataStax, Inc.
+ *
+ * Licensed 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.db;
+
+import java.util.List;
+
+import org.apache.cassandra.cql3.CqlBuilder;
+import org.apache.cassandra.schema.TableMetadata;
+
+/**
+ * A {@code ReadQuery} for multiple partitions, restricted by one of more data ranges.
+ */
+public interface MultiPartitionReadQuery extends ReadQuery
+{
+    List<DataRange> ranges();
+
+    default void appendCQLWhereClause(CqlBuilder builder, boolean redact)
+    {
+        // Append the data ranges.
+        TableMetadata metadata = metadata();
+        boolean hasRanges = appendRanges(builder, redact);
+
+        // Append the clustering index filter and the row filter.
+        String filter = ranges().get(0).clusteringIndexFilter.toCQLString(metadata, rowFilter() , redact);
+        if (!filter.isEmpty())
+        {
+            if (filter.startsWith("ORDER BY"))
+                builder.append(" ");
+            else if (hasRanges)
+                builder.append(" AND ");
+            else
+                builder.append(" WHERE ");
+            builder.append(filter);
+        }
+    }
+
+    private boolean appendRanges(CqlBuilder builder, boolean redact)
+    {
+        List<DataRange> ranges = ranges();
+        if (ranges.size() == 1)
+        {
+            DataRange range = ranges.get(0);
+            if (range.isUnrestricted())
+                return false;
+
+            String rangeString = range.toCQLString(metadata(), redact);
+            if (!rangeString.isEmpty())
+            {
+                builder.append(" WHERE ").append(rangeString);
+                return true;
+            }
+        }
+        else
+        {
+            builder.append(" WHERE ").append('(');
+            for (int i = 0; i < ranges.size(); i++)
+            {
+                if (i > 0)
+                    builder.append(" OR ");
+                builder.append(ranges.get(i).toCQLString(metadata(), redact));
+            }
+            builder.append(')');
+            return true;
+        }
+        return false;
+    }
+}