Skip to content

Commit 60a723b

Browse files
committed
fix(redis): harden generated command arguments
1 parent 310ec6a commit 60a723b

4 files changed

Lines changed: 32 additions & 11 deletions

File tree

‎chat2db-community-server/chat2db-community-plugins/chat2db-community-redis/src/main/java/ai/chat2db/plugin/redis/RedisSqlBuilder.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import java.util.List;
4040
import java.util.Map;
4141
import java.util.Objects;
42+
import java.util.UUID;
4243

4344
import static ai.chat2db.plugin.redis.util.RedisValueUtils.getRedisValue;
4445

@@ -303,7 +304,7 @@ private void appendListOperation(StringBuilder script, String keyName, String op
303304
// Positional delete so a later duplicate occurrence removes exactly the
304305
// edited row: LSET idx <tombstone> then LREM 1 <tombstone>. Value-based
305306
// LREM would always hit the FIRST duplicate instead.
306-
String tombstone = "__chat2db_deleted__" + index;
307+
String tombstone = "__chat2db_deleted__" + UUID.randomUUID();
307308
script.append("LSET ").append(getRedisValue(keyName))
308309
.append(RedisConstants.COMMAND_ARGUMENT_SEPARATOR).append(index)
309310
.append(RedisConstants.COMMAND_ARGUMENT_SEPARATOR).append(getRedisValue(tombstone))
@@ -345,7 +346,8 @@ private void appendStringOperation(StringBuilder script, String keyName, String
345346

346347
private void appendZAddCommand(StringBuilder script, String keyName, String score, String value) {
347348
script.append(RedisConstants.COMMAND_ZSET_ADD_PREFIX).append(getRedisValue(keyName))
348-
.append(RedisConstants.COMMAND_ARGUMENT_SEPARATOR).append(score)
349+
.append(RedisConstants.COMMAND_ARGUMENT_SEPARATOR)
350+
.append(getRedisValue(StringUtils.defaultString(score)))
349351
.append(RedisConstants.COMMAND_ARGUMENT_SEPARATOR)
350352
.append(getRedisValue(StringUtils.defaultString(value)))
351353
.append(SQLConstants.LINE_SEPARATOR);

‎chat2db-community-server/chat2db-community-plugins/chat2db-community-redis/src/main/java/ai/chat2db/plugin/redis/util/RedisValueUtils.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,21 @@
33
public class RedisValueUtils {
44

55
public static String getRedisValue(String value) {
6-
if(value == null) {
6+
if (value == null) {
77
return null;
88
}
9-
if(value.contains("\\")) {
9+
if (value.indexOf('\0') >= 0 || value.indexOf('\r') >= 0 || value.indexOf('\n') >= 0) {
10+
throw new IllegalArgumentException("Redis JDBC command arguments cannot contain NUL, CR, or LF");
11+
}
12+
if (value.contains("\\")) {
1013
value = value.replace("\\", "\\\\");
1114
}
12-
if(value.contains("'")) {
15+
if (value.contains("'")) {
1316
value = value.replace("'", "\\'");
1417
}
15-
if(value.contains("\"")) {
18+
if (value.contains("\"")) {
1619
value = value.replace("\"", "\\\"");
1720
}
18-
return "'"+value+"'";
21+
return "'" + value + "'";
1922
}
2023
}

‎chat2db-community-server/chat2db-community-plugins/chat2db-community-redis/src/test/java/ai/chat2db/plugin/redis/RedisSqlBuilderTest.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ void buildByQueryResultGeneratesZSetMutationCommands() {
9696
operation("DELETE", List.of("2", "w", "2.0"), null)),
9797
new HashMap<>());
9898

99-
assertEquals("MULTI \n" + "ZADD 'k' 1.5 'v'\n" + "ZREM 'k' 'w'\n" + "EXEC",
99+
assertEquals("MULTI \n" + "ZADD 'k' '1.5' 'v'\n" + "ZREM 'k' 'w'\n" + "EXEC",
100100
RedisSqlBuilder.getInstance().buildByQueryResult(queryResult));
101101
}
102102

@@ -112,9 +112,17 @@ void buildByQueryResultGeneratesListMutationCommands() {
112112

113113
// DELETE on row 1 is positional now (tombstone via LSET+LREM) so duplicate
114114
// values cannot remove the wrong occurrence.
115-
assertEquals("MULTI \n" + "LSET 'k' 1 'b'\n" + "RPUSH 'k' 'c'\n"
116-
+ "LSET 'k' 0 '__chat2db_deleted__0'\n" + "LREM 'k' 1 '__chat2db_deleted__0'\n" + "EXEC",
117-
RedisSqlBuilder.getInstance().buildByQueryResult(queryResult));
115+
String script = RedisSqlBuilder.getInstance().buildByQueryResult(queryResult);
116+
String[] lines = script.split("\\n");
117+
118+
assertEquals("MULTI ", lines[0]);
119+
assertEquals("LSET 'k' 1 'b'", lines[1]);
120+
assertEquals("RPUSH 'k' 'c'", lines[2]);
121+
String tombstone = lines[3].substring(lines[3].lastIndexOf(' ') + 1);
122+
assertTrue(tombstone.matches("'__chat2db_deleted__[0-9a-f-]+'"));
123+
assertEquals("LSET 'k' 0 " + tombstone, lines[3]);
124+
assertEquals("LREM 'k' 1 " + tombstone, lines[4]);
125+
assertEquals("EXEC", lines[5]);
118126
}
119127

120128
@Test

‎chat2db-community-server/chat2db-community-plugins/chat2db-community-redis/src/test/java/ai/chat2db/plugin/redis/util/RedisValueUtilsTest.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import static ai.chat2db.plugin.redis.util.RedisValueUtils.getRedisValue;
66
import static org.junit.jupiter.api.Assertions.assertEquals;
77
import static org.junit.jupiter.api.Assertions.assertNull;
8+
import static org.junit.jupiter.api.Assertions.assertThrows;
89

910
class RedisValueUtilsTest {
1011

@@ -28,4 +29,11 @@ void escapesSingleQuote() {
2829
void returnsNullForNullInput() {
2930
assertNull(getRedisValue(null));
3031
}
32+
33+
@Test
34+
void rejectsCharactersThatRedisJdbcSplitsBeforeQuoteParsing() {
35+
assertThrows(IllegalArgumentException.class, () -> getRedisValue("first\nDEL other"));
36+
assertThrows(IllegalArgumentException.class, () -> getRedisValue("first\rDEL other"));
37+
assertThrows(IllegalArgumentException.class, () -> getRedisValue("first\0second"));
38+
}
3139
}

0 commit comments

Comments
 (0)