Fix crash when calling internal container command without arguments (#14690)

Addresses crash and clarifies errors around container commands.

- Update server.c to handle container commands with no subcommand: emit
"missing subcommand. Try HELP."; keep "unknown subcommand" for invalid
subcommands; for unknown commands, include args preview only when
present
- Add a test module command subcommands.internal_container with a
subcommand for validation
- Add unit test asserting missing subcommand error when calling the
internal container command without arguments
This commit is contained in:
Stav-Levi 2026-01-21 08:38:04 +02:00 committed by GitHub
parent 5c5c7c5a2c
commit 25f780b662
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 31 additions and 12 deletions

View file

@ -4146,18 +4146,26 @@ int commandCheckExistence(client *c, sds *err) {
sds cmd = sdsnew((char *)c->argv[0]->ptr);
sdstoupper(cmd);
*err = sdsnew(NULL);
*err = sdscatprintf(*err, "unknown subcommand '%.128s'. Try %s HELP.",
(char *)c->argv[1]->ptr, cmd);
if (c->argc < 2) {
*err = sdscatprintf(*err, "missing subcommand. Try %s HELP.", cmd);
} else {
*err = sdscatprintf(*err, "unknown subcommand '%.128s'. Try %s HELP.",
(char *)c->argv[1]->ptr, cmd);
}
sdsfree(cmd);
} else {
sds args = sdsempty();
int i;
for (i=1; i < c->argc && sdslen(args) < 128; i++)
args = sdscatprintf(args, "'%.*s' ", 128-(int)sdslen(args), (char*)c->argv[i]->ptr);
*err = sdsnew(NULL);
*err = sdscatprintf(*err, "unknown command '%.128s', with args beginning with: %s",
(char*)c->argv[0]->ptr, args);
sdsfree(args);
*err = sdscatprintf(*err, "unknown command '%.128s'", (char *)c->argv[0]->ptr);
if (c->argc >= 2) {
sds args = sdsempty();
for (int i = 1; i < c->argc && sdslen(args) < 128; i++)
args = sdscatprintf(args, "'%.*s' ", 128 - (int)sdslen(args), (char *)c->argv[i]->ptr);
*err = sdscatprintf(*err, ", with args beginning with: %s", args);
sdsfree(args);
}
}
/* Make sure there are no newlines in the string, otherwise invalid protocol
* is emitted (The args come from the user, they may contain any character). */

View file

@ -895,7 +895,7 @@ int TestBasics(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
if (!TestAssertStringReply(ctx,RedisModule_CallReplyArrayElement(reply, 1),"1234",4)) goto fail;
T("foo", "E");
if (!TestAssertErrorReply(ctx,reply,"ERR unknown command 'foo', with args beginning with: ",53)) goto fail;
if (!TestAssertErrorReply(ctx,reply,"ERR unknown command 'foo'",25)) goto fail;
T("set", "Ec", "x");
if (!TestAssertErrorReply(ctx,reply,"ERR wrong number of arguments for 'set' command",47)) goto fail;

View file

@ -108,5 +108,12 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
/* Trying to create a sub-subcommand fails */
RedisModule_Assert(RedisModule_CreateSubcommand(subcmd,"get",NULL,"",0,0,0) == REDISMODULE_ERR);
/* Internal container command for testing */
if (RedisModule_CreateCommand(ctx,"subcommands.internal_container",NULL,"internal",0,0,0) == REDISMODULE_ERR)
return REDISMODULE_ERR;
RedisModuleCommand *internal_parent = RedisModule_GetCommand(ctx,"subcommands.internal_container");
if (RedisModule_CreateSubcommand(internal_parent,"test",cmd_set,"internal",0,0,0) == REDISMODULE_ERR)
return REDISMODULE_ERR;
return REDISMODULE_OK;
}

View file

@ -229,7 +229,7 @@ foreach call_type {nested normal} {
r config resetstat
# simple module command that replies with string error
assert_error "ERR unknown command 'hgetalllll', with args beginning with:" {r do_rm_call hgetalllll}
assert_error "ERR unknown command 'hgetalllll'" {r do_rm_call hgetalllll}
assert_equal [errorrstat ERR r] {count=1}
# simple module command that replies with string error

View file

@ -6,7 +6,7 @@ start_cluster 1 0 [list config_lines $modules] {
set r [srv 0 client]
test {Internal command without internal connection fails as an unknown command} {
assert_error {*unknown command*with args beginning with:*} {r internalauth.internalcommand}
assert_error {*unknown command*} {r internalauth.internalcommand}
}
test {Wrong internalsecret fails authentication} {

View file

@ -51,6 +51,10 @@ start_server {tags {"modules external:skip"}} {
assert_equal [lsearch $commands "set"] -1
}
test "Internal container command without subcommand returns missing subcommand error" {
assert_error {*missing subcommand*} {r subcommands.internal_container}
}
test "Unload the module - subcommands" {
assert_equal {OK} [r module unload subcommands]
}