Skip to content

Commit

Permalink
Use binary representation in assert crash log, cleanup in crash log
Browse files Browse the repository at this point in the history
Change assert crash log to also use binary representation like 5bdd72b.
And do not print the password in assert crash log like 56eef6f.

In addition, for 5bdd72b, we will print '"argv"',
because originally the code would print a '', and sdscatrepr will add an extra "",
so now removing the extra '' here.

Extract the getArgvReprString method and clean up the code a bit.

Examples:
```
debug assert "\x00abc"

before:
client->argv[0] = "debug" (refcount: 1)
client->argv[1] = "assert" (refcount: 1)
client->argv[2] = "" (refcount: 1)

after:
client->argv[0] = "debug" (refcount: 1)
client->argv[1] = "assert" (refcount: 1)
client->argv[2] = "\x00abc" (refcount: 1)

debug panic "\x00abc"

before:
argc: '3'
argv[0]: '"debug"'
argv[1]: '"panic"'
argv[2]: '"\x00abc"'

after:
argc: 3
argv[0]: "debug"
argv[1]: "panic"
argv[2]: "\x00abc"
```

Signed-off-by: Binbin <[email protected]>
  • Loading branch information
enjoy-binbin committed Dec 9, 2024
1 parent b09db3e commit 8e642ac
Showing 1 changed file with 18 additions and 19 deletions.
37 changes: 18 additions & 19 deletions src/debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,14 @@ __attribute__((noinline, weak)) void _serverAssert(const char *estr, const char
bugReportEnd(0, 0);
}

/* Returns the argv argument in binary representation, limited to length 128. */
sds getArgvReprString(robj *argv) {
robj *decoded = getDecodedObject(argv);
sds repr = sdscatrepr(sdsempty(), decoded->ptr, min(sdslen(decoded->ptr), 128));
decrRefCount(decoded);
return repr;
}

/* Checks if the argument at the given index should be redacted from logs. */
int shouldRedactArg(const client *c, int idx) {
serverAssert(idx < c->argc);
Expand All @@ -1073,16 +1081,12 @@ void _serverAssertPrintClientInfo(const client *c) {
serverLog(LL_WARNING, "client->argv[%d]: %zu bytes", j, sdslen((sds)c->argv[j]->ptr));
continue;
}
char buf[128];
char *arg;

if (c->argv[j]->type == OBJ_STRING && sdsEncodedObject(c->argv[j])) {
arg = (char *)c->argv[j]->ptr;
} else {
snprintf(buf, sizeof(buf), "Object type: %u, encoding: %u", c->argv[j]->type, c->argv[j]->encoding);
arg = buf;
sds repr = getArgvReprString(c->argv[j]);
serverLog(LL_WARNING, "client->argv[%d] = %s (refcount: %d)", j, repr, c->argv[j]->refcount);
sdsfree(repr);
if (!strcasecmp(c->argv[j]->ptr, "auth") || !strcasecmp(c->argv[j]->ptr, "auth2")) {
break;
}
serverLog(LL_WARNING, "client->argv[%d] = \"%s\" (refcount: %d)", j, arg, c->argv[j]->refcount);
}
}

Expand Down Expand Up @@ -1890,23 +1894,18 @@ void logCurrentClient(client *cc, const char *title) {
client = catClientInfoString(sdsempty(), cc, server.hide_user_data_from_log);
serverLog(LL_WARNING | LL_RAW, "%s\n", client);
sdsfree(client);
serverLog(LL_WARNING | LL_RAW, "argc: '%d'\n", cc->argc);
serverLog(LL_WARNING | LL_RAW, "argc: %d\n", cc->argc);
for (j = 0; j < cc->argc; j++) {
if (shouldRedactArg(cc, j)) {
serverLog(LL_WARNING | LL_RAW, "argv[%d]: %zu bytes\n", j, sdslen((sds)cc->argv[j]->ptr));
continue;
}
robj *decoded;
decoded = getDecodedObject(cc->argv[j]);
sds repr = sdscatrepr(sdsempty(), decoded->ptr, min(sdslen(decoded->ptr), 128));
serverLog(LL_WARNING | LL_RAW, "argv[%d]: '%s'\n", j, (char *)repr);
if (!strcasecmp(decoded->ptr, "auth") || !strcasecmp(decoded->ptr, "auth2")) {
sdsfree(repr);
decrRefCount(decoded);
sds repr = getArgvReprString(cc->argv[j]);
serverLog(LL_WARNING | LL_RAW, "argv[%d]: %s\n", j, repr);
sdsfree(repr);
if (!strcasecmp(cc->argv[j]->ptr, "auth") || !strcasecmp(cc->argv[j]->ptr, "auth2")) {
break;
}
sdsfree(repr);
decrRefCount(decoded);
}
/* Check if the first argument, usually a key, is found inside the
* selected DB, and if so print info about the associated object. */
Expand Down

0 comments on commit 8e642ac

Please sign in to comment.