CLIENT-5184 Reject negative selectByPath / modifyByPath flags instead of masking them into a valid operation

client/src/com/aerospike/client/cdt/CdtOperation.java

@@ -75,7 +75,8 @@ public static Operation selectByPath(String binName, int flags, CTX... ctx) {
         if (binName == null || binName.isEmpty() || binName.length() > Bin.MAX_BIN_NAME_LENGTH) {
             throw new AerospikeException(ResultCode.PARAMETER_ERROR, "binName cannot be null, empty, or exceed " + Bin.MAX_BIN_NAME_LENGTH + " characters");
         }
-        if (ctx == null || ctx.length == 0) { 
+        validateFlags(flags, "select");
+        if (ctx == null || ctx.length == 0) {
             packedBytes = Pack.pack(CDT.Type.SELECT.value, flags);
         } else {
             packedBytes = packCdtSelect(CDT.Type.SELECT, flags, ctx);
@@ -130,15 +131,32 @@ public static Operation modifyByPath(String binName, int flags, Expression modif
         if (binName == null || binName.isEmpty() || binName.length() > Bin.MAX_BIN_NAME_LENGTH) {
             throw new AerospikeException(ResultCode.PARAMETER_ERROR, "binName cannot be null, empty, or exceed " + Bin.MAX_BIN_NAME_LENGTH + " characters");
         }
-        if (ctx == null || ctx.length == 0) { 
+        validateFlags(flags, "modify");
+        if (ctx == null || ctx.length == 0) {
             packedBytes = Pack.pack(CDT.Type.SELECT.value, flags, modifyExp);
         } else {
             packedBytes = packCdtModify(CDT.Type.SELECT, flags, modifyExp, ctx);
         }
 
         return new Operation(Operation.Type.CDT_MODIFY, binName, Value.get(packedBytes, ParticleType.BLOB));
     }
-
+
+	/**
+	 * Reject invalid path flags instead of masking them into a valid operation.
+	 * <p>
+	 * Legal select flags are the exposed {@link SelectFlags} constants (0..3 and 0x10)
+	 * and legal modify flags are the {@link ModifyFlags} constants (0 and 0x10); a
+	 * negative value, or one with bit 2 (the internal apply bit) set, is never valid.
+	 * Without this check the flag is rewritten before packing (select cleared bit 2,
+	 * modify sets it) and a negative value silently aliases onto a valid server return
+	 * type instead of erroring (CLIENT-5184).
+	 */
+	private static void validateFlags(int flags, String name) {
+	    if (flags < 0 || (flags & 4) != 0) {
+	        throw new AerospikeException(ResultCode.PARAMETER_ERROR, "invalid " + name + " flag " + flags);
+	    }
+	}
+
 	private static byte[] packCdtSelect(CDT.Type typeSelect, int flags, CTX... ctx) {
         Packer packer = new Packer();
 
@@ -155,9 +173,9 @@ private static byte[] packCdtSelect(CDT.Type typeSelect, int flags, CTX... ctx)
                     packer.packByteArray(c.exp.getBytes(), 0, c.exp.getBytes().length);
             }
 
-	        // Ensure the apply flag is cleared, since no expression is provided.
-	        // This avoids problems if the caller accidentally sets bit 2 in the flags field.
-            packer.packInt(flags & ~4);
+            // Pack the validated select flag. selectByPath rejects any value with bit 2
+            // (the internal apply bit) set, so no mask is needed here (CLIENT-5184).
+            packer.packInt(flags);
 
             if (i == 0) {
                 packer.createBuffer();

client/src/com/aerospike/client/exp/CdtExp.java

@@ -16,6 +16,8 @@
 */ 
 package com.aerospike.client.exp;
 
+import com.aerospike.client.AerospikeException;
+import com.aerospike.client.ResultCode;
 import com.aerospike.client.Value;
 import com.aerospike.client.cdt.CTX;
 import com.aerospike.client.cdt.ModifyFlags;
@@ -89,6 +91,7 @@ enum Type {
 	 * @return				an {@link Exp} that evaluates to the type specified by {@code returnType}
 	 */
     public static Exp selectByPath(Exp.Type returnType, int flags, Exp bin, CTX... ctx) {
+        validateFlags(flags, "select");
         byte[] bytes = packCdtSelect(Type.SELECT, flags, ctx);
 
         return new Exp.Module(bin, bytes, returnType.code, MODULE);
@@ -141,11 +144,27 @@ public static Exp selectByPath(Exp.Type returnType, int flags, Exp bin, CTX... c
 	 * @return				an {@link Exp} containing the entire modified CDT structure
 	 */
     public static Exp modifyByPath(Exp.Type returnType, int modifyFlag, Exp modifyExp, Exp bin, CTX... ctx) {
+        validateFlags(modifyFlag, "modify");
         byte[] bytes = packCdtModify(Type.SELECT, modifyFlag, modifyExp, ctx);
 
         return new Exp.Module(bin, bytes, returnType.code, MODULE | MODIFY);
     }
 
+	/**
+	 * Reject invalid path flags instead of masking them into a valid operation.
+	 * <p>
+	 * Legal select flags are the exposed {@link SelectFlags} constants (0..3 and 0x10)
+	 * and legal modify flags are the {@link ModifyFlags} constants (0 and 0x10); a
+	 * negative value, or one with bit 2 (the internal apply bit) set, is never valid.
+	 * Without this check a negative value silently aliases onto a valid server return
+	 * type instead of erroring (CLIENT-5184).
+	 */
+	private static void validateFlags(int flags, String name) {
+	    if (flags < 0 || (flags & 4) != 0) {
+	        throw new AerospikeException(ResultCode.PARAMETER_ERROR, "invalid " + name + " flag " + flags);
+	    }
+	}
+
 	private static byte[] packCdtModify(Type type, int modifyFlag, Exp modifyExp, CTX... ctx) {
         Packer packer = new Packer();
 

test/src/com/aerospike/test/sync/basic/TestCdtOperate.java

@@ -30,6 +30,7 @@
 import org.junit.Assume;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.junit.function.ThrowingRunnable;
 
 import com.aerospike.client.AerospikeException;
 import com.aerospike.client.Bin;
@@ -3789,4 +3790,77 @@ public void testMapValuesListSize() {
         assertTrue("Map values list size should be 3", result.getBoolean("sizeCheck"));
     }
 
+    // ---- CLIENT-5184: reject invalid selectByPath / modifyByPath flags ----
+    //
+    // A negative flag, or one with bit 2 (the internal apply bit) set, is
+    // unconditionally invalid for either operation. The client must reject it with
+    // PARAMETER_ERROR rather than mask it (select cleared bit 2, modify set it) into
+    // a request the server happily executes -- for a negative input the rewritten low
+    // nibble aliases onto a valid server return type, so the misuse silently succeeds.
+    // The client validates eagerly while building the operation/expression, so the
+    // rejection surfaces at build time and needs no server round-trip.
+
+    private static void assertParameterError(ThrowingRunnable build) {
+        AerospikeException ae = assertThrows(AerospikeException.class, build);
+        assertEquals(ResultCode.PARAMETER_ERROR, ae.getResultCode());
+    }
+
+    // Legal SelectFlags values are the exposed constants 0..3 and 0x10 (and ORs). The
+    // negatives are the ticket's repro table; 4..7 cover the bit-2-set positives that
+    // the old "flags & ~4" mask would have silently cleared.
+    private static final int[] INVALID_SELECT_FLAGS = {-1, -2, -3, -9, -12, -16, 4, 5, 6, 7};
+
+    // Legal ModifyFlags values are DEFAULT (0) and NO_FAIL (0x10). Bit 2 is the
+    // internal apply bit and must never be caller-supplied.
+    private static final int[] INVALID_MODIFY_FLAGS = {-1, -2, -12, -16, 4, 5, 6, 7};
+
+    @Test
+    public void testSelectByPathRejectsInvalidFlags() {
+        for (int flag : INVALID_SELECT_FLAGS) {
+            final int f = flag;
+            assertParameterError(() -> CdtOperation.selectByPath(BIN_NAME, f, CTX.allChildren()));
+        }
+    }
+
+    @Test
+    public void testModifyByPathRejectsInvalidFlags() {
+        Expression modifyExp = Exp.build(Exp.add(Exp.intLoopVar(LoopVarPart.VALUE), Exp.val(5)));
+        for (int flag : INVALID_MODIFY_FLAGS) {
+            final int f = flag;
+            assertParameterError(() -> CdtOperation.modifyByPath(BIN_NAME, f, modifyExp, CTX.allChildren()));
+        }
+    }
+
+    @Test
+    public void testCdtExpSelectByPathRejectsInvalidFlags() {
+        for (int flag : INVALID_SELECT_FLAGS) {
+            final int f = flag;
+            assertParameterError(() -> CdtExp.selectByPath(Exp.Type.LIST, f, Exp.mapBin(BIN_NAME), CTX.allChildren()));
+        }
+    }
+
+    @Test
+    public void testCdtExpModifyByPathRejectsInvalidFlags() {
+        Exp modifyExp = Exp.add(Exp.intLoopVar(LoopVarPart.VALUE), Exp.val(5));
+        for (int flag : INVALID_MODIFY_FLAGS) {
+            final int f = flag;
+            assertParameterError(() -> CdtExp.modifyByPath(Exp.Type.MAP, f, modifyExp, Exp.mapBin(BIN_NAME), CTX.allChildren()));
+        }
+    }
+
+    // Regression: valid flags must still build. NO_FAIL (0x10) sets a high bit but not
+    // bit 2, so the new guard must let it through (end-to-end execution of DEFAULT and
+    // NO_FAIL flags is exercised by the tests above, e.g. testSelectNoFailFlag).
+    @Test
+    public void testValidPathFlagsStillAccepted() {
+        Expression modExp = Exp.build(Exp.add(Exp.intLoopVar(LoopVarPart.VALUE), Exp.val(5)));
+        Exp modExp2 = Exp.add(Exp.intLoopVar(LoopVarPart.VALUE), Exp.val(5));
+
+        // No exception expected.
+        CdtOperation.selectByPath(BIN_NAME, SelectFlags.NO_FAIL, CTX.allChildren());
+        CdtOperation.modifyByPath(BIN_NAME, ModifyFlags.NO_FAIL, modExp, CTX.allChildren());
+        CdtExp.selectByPath(Exp.Type.LIST, SelectFlags.NO_FAIL, Exp.mapBin(BIN_NAME), CTX.allChildren());
+        CdtExp.modifyByPath(Exp.Type.MAP, ModifyFlags.NO_FAIL, modExp2, Exp.mapBin(BIN_NAME), CTX.allChildren());
+    }
+
 }