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

@@ -78,14 +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);
             ByteBuffer value = bufferAt(i);
-            sb.append(i == 0 ? "" : ", ").append(c.type.toCQLString(value));
+            sb.append(i == 0 ? "" : ", ").append(c.type.toCQLString(value, redact));
         }
         return sb.toString();
     }

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

@@ -271,7 +271,7 @@ 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 "";
@@ -283,20 +283,20 @@ public String toCQLString(TableMetadata metadata)
              * 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.
              */
-            return ((DecoratedKey) startKey()).toCQLString(metadata);
+            return ((DecoratedKey) startKey()).toCQLString(metadata, redact);
         }
         else
         {
             StringBuilder builder = new StringBuilder();
             if (!startKey().isMinimum())
             {
-                appendCQLClause(startKey(), builder, metadata, true, keyRange.isStartInclusive());
+                appendCQLClause(startKey(), builder, metadata, true, keyRange.isStartInclusive(), redact);
             }
             if (!stopKey().isMinimum())
             {
                 if (builder.length() > 0)
                     builder.append(" AND ");
-                appendCQLClause(stopKey(), builder, metadata, false, keyRange.isEndInclusive());
+                appendCQLClause(stopKey(), builder, metadata, false, keyRange.isEndInclusive(), redact);
             }
             return builder.toString();
         }
@@ -306,7 +306,8 @@ private void appendCQLClause(PartitionPosition pos,
                                  StringBuilder builder,
                                  TableMetadata metadata,
                                  boolean isStart,
-                                 boolean isInclusive)
+                                 boolean isInclusive,
+                                 boolean redact)
     {
         builder.append("token(");
         builder.append(ColumnMetadata.toCQLString(metadata.partitionKeyColumns()));
@@ -315,14 +316,14 @@ private void appendCQLClause(PartitionPosition pos,
         {
             builder.append(getOperator(isStart, isInclusive)).append(' ');
             builder.append("token(");
-            appendKeyString(builder, metadata.partitionKeyType, ((DecoratedKey)pos).getKey());
+            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());
         }
     }
 
@@ -335,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(StringBuilder 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).toCQLString(values[i]));
+                builder.append(i == 0 ? "" : ", ").append(ct.subTypes().get(i).toCQLString(values[i], redact));
         }
         else
         {
-            builder.append(type.toCQLString(key));
+            builder.append(type.toCQLString(key, redact));
         }
     }
 

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

@@ -172,26 +172,27 @@ public String toString()
      * 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.toCQLString(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

@@ -28,14 +28,14 @@ public interface MultiPartitionReadQuery extends ReadQuery
 {
     List<DataRange> ranges();
 
-    default void appendCQLWhereClause(CqlBuilder builder)
+    default void appendCQLWhereClause(CqlBuilder builder, boolean redact)
     {
         // Append the data ranges.
         TableMetadata metadata = metadata();
-        boolean hasRanges = appendRanges(builder);
+        boolean hasRanges = appendRanges(builder, redact);
 
         // Append the clustering index filter and the row filter.
-        String filter = ranges().get(0).clusteringIndexFilter.toCQLString(metadata, rowFilter());
+        String filter = ranges().get(0).clusteringIndexFilter.toCQLString(metadata, rowFilter() , redact);
         if (!filter.isEmpty())
         {
             if (filter.startsWith("ORDER BY"))
@@ -48,7 +48,7 @@ else if (hasRanges)
         }
     }
 
-    private boolean appendRanges(CqlBuilder builder)
+    private boolean appendRanges(CqlBuilder builder, boolean redact)
     {
         List<DataRange> ranges = ranges();
         if (ranges.size() == 1)
@@ -57,7 +57,7 @@ private boolean appendRanges(CqlBuilder builder)
             if (range.isUnrestricted())
                 return false;
 
-            String rangeString = range.toCQLString(metadata());
+            String rangeString = range.toCQLString(metadata(), redact);
             if (!rangeString.isEmpty())
             {
                 builder.append(" WHERE ").append(rangeString);
@@ -71,7 +71,7 @@ private boolean appendRanges(CqlBuilder builder)
             {
                 if (i > 0)
                     builder.append(" OR ");
-                builder.append(ranges.get(i).toCQLString(metadata()));
+                builder.append(ranges.get(i).toCQLString(metadata(), redact));
             }
             builder.append(')');
             return true;

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

@@ -321,9 +321,9 @@ public Verb verb()
     }
 
     @Override
-    public void appendCQLWhereClause(CqlBuilder builder)
+    public void appendCQLWhereClause(CqlBuilder builder, boolean redact)
     {
-        MultiPartitionReadQuery.super.appendCQLWhereClause(builder);
+        MultiPartitionReadQuery.super.appendCQLWhereClause(builder, redact);
     }
 
     @Override

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

@@ -394,9 +394,9 @@ public Verb verb()
     }
 
     @Override
-    public void appendCQLWhereClause(CqlBuilder builder)
+    public void appendCQLWhereClause(CqlBuilder builder, boolean redact)
     {
-        PartitionRangeReadQuery.super.appendCQLWhereClause(builder);
+        PartitionRangeReadQuery.super.appendCQLWhereClause(builder, redact);
     }
 
     @Override

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

@@ -607,7 +607,7 @@ private Threshold.GuardedCounter createTombstoneCounter()
                 Threshold guardrail = shouldRespectTombstoneThresholds()
                                                 ? Guardrails.scannedTombstones
                                                 : DefaultGuardrail.DefaultThreshold.NEVER_TRIGGERED;
-                return guardrail.newCounter(ReadCommand.this::toCQLString, true, null);
+                return guardrail.newCounter(ReadCommand.this::toRedactedCQLString, false, null);
             }
 
             private MetricRecording()
@@ -671,7 +671,7 @@ private void countTombstone(ClusteringPrefix<?> clustering)
                 {
                     metric.tombstoneFailures.inc();
                     throw new TombstoneOverwhelmingException(tombstones.get(),
-                                                             ReadCommand.this.toCQLString(),
+                                                             ReadCommand.this.toRedactedCQLString(),
                                                              ReadCommand.this.metadata(),
                                                              currentKey,
                                                              clustering);

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

@@ -112,7 +112,7 @@ int oldestUnrepairedTombstone()
     {
         return oldestUnrepairedTombstone;
     }
-    
+
     void updateMinOldestUnrepairedTombstone(int candidate)
     {
         oldestUnrepairedTombstone = Math.min(oldestUnrepairedTombstone, candidate);
@@ -254,15 +254,15 @@ public boolean isRepairedDataDigestConclusive()
     {
         return repairedDataInfo.isConclusive();
     }
-    
+
     public RepairedDataInfo getRepairedDataInfo()
     {
         return repairedDataInfo;
     }
 
     private void addSample()
     {
-        String cql = command.toCQLString();
+        String cql = command.toRedactedCQLString();
         int timeMicros = (int) Math.min(TimeUnit.NANOSECONDS.toMicros(clock.now() - createdAtNanos), Integer.MAX_VALUE);
         ColumnFamilyStore cfs = ColumnFamilyStore.getIfExists(baseMetadata.id);
         if (cfs != null)

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

@@ -1152,9 +1152,9 @@ public Verb verb()
     }
 
     @Override
-    public void appendCQLWhereClause(CqlBuilder builder)
+    public void appendCQLWhereClause(CqlBuilder builder, boolean redact)
     {
-        SinglePartitionReadQuery.super.appendCQLWhereClause(builder);
+        SinglePartitionReadQuery.super.appendCQLWhereClause(builder, redact);
     }
 
     protected void serializeSelection(DataOutputPlus out, int version) throws IOException

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

@@ -159,16 +159,16 @@ default boolean selectsClustering(DecoratedKey key, Clustering<?> clustering)
         return rowFilter().clusteringKeyRestrictionsAreSatisfiedBy(clustering);
     }
 
-    default void appendCQLWhereClause(CqlBuilder builder)
+    default void appendCQLWhereClause(CqlBuilder builder, boolean redact)
     {
         builder.append(" WHERE ");
 
         // Append the partition key restrictions.
         TableMetadata metadata = metadata();
-        builder.append(partitionKey().toCQLString(metadata));
+        builder.append(partitionKey().toCQLString(metadata, redact));
 
         // Append the clustering index filter and the row filter.
-        String filter = clusteringIndexFilter().toCQLString(metadata(), rowFilter());
+        String filter = clusteringIndexFilter().toCQLString(metadata(), rowFilter(), redact);
         if (!filter.isEmpty())
         {
             builder.append(filter.startsWith("ORDER BY") ? " " : " AND ")