From 1ba85d002a824a12b0107bdd2b493a3a0516cec9 Mon Sep 17 00:00:00 2001 From: Binbin Date: Tue, 10 Dec 2024 00:37:04 +0800 Subject: [PATCH] Use binary representation in assert crash log, cleanup in crash log (#1410) Change assert crash log to also use binary representation like 5bdd72bea77d4bb237441c9a671e80edcdc998ad. And do not print the password in assert crash log like 56eef6fb5ab7a755485c19f358761954ca459472. In addition, for 5bdd72bea77d4bb237441c9a671e80edcdc998ad, 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 --- src/debug.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/debug.c b/src/debug.c index 38b66dacb..7407af351 100644 --- a/src/debug.c +++ b/src/debug.c @@ -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); @@ -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); } } @@ -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. */