From 1f646713fe0ffc3254bec29db350f726d6f26151 Mon Sep 17 00:00:00 2001 From: kailixu Date: Thu, 13 Jun 2024 16:36:18 +0800 Subject: [PATCH 1/3] fix: check range option of alter user --- source/libs/parser/src/parTranslater.c | 16 ++++++++-------- tests/script/tsim/user/basic.sim | 4 ++++ tests/system-test/0-others/user_control.py | 2 +- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/source/libs/parser/src/parTranslater.c b/source/libs/parser/src/parTranslater.c index 3dbba397ba..5f55841a55 100644 --- a/source/libs/parser/src/parTranslater.c +++ b/source/libs/parser/src/parTranslater.c @@ -6605,8 +6605,8 @@ static int32_t buildCreateDbReq(STranslateContext* pCxt, SCreateDatabaseStmt* pS } static int32_t checkRangeOption(STranslateContext* pCxt, int32_t code, const char* pName, int64_t val, int64_t minVal, - int64_t maxVal) { - if (val >= 0 && (val < minVal || val > maxVal)) { + int64_t maxVal, bool skipMinus) { + if (skipMinus ? ((val >= 0) && (val < minVal || val > maxVal)) : (val < minVal || val > maxVal)) { return generateSyntaxErrMsgExt(&pCxt->msgBuf, code, "Invalid option %s: %" PRId64 ", valid range: [%" PRId64 ", %" PRId64 "]", pName, val, minVal, maxVal); @@ -6616,12 +6616,12 @@ static int32_t checkRangeOption(STranslateContext* pCxt, int32_t code, const cha static int32_t checkDbRangeOption(STranslateContext* pCxt, const char* pName, int32_t val, int32_t minVal, int32_t maxVal) { - return checkRangeOption(pCxt, TSDB_CODE_PAR_INVALID_DB_OPTION, pName, val, minVal, maxVal); + return checkRangeOption(pCxt, TSDB_CODE_PAR_INVALID_DB_OPTION, pName, val, minVal, maxVal, true); } static int32_t checkTableRangeOption(STranslateContext* pCxt, const char* pName, int64_t val, int64_t minVal, int64_t maxVal) { - return checkRangeOption(pCxt, TSDB_CODE_PAR_INVALID_TABLE_OPTION, pName, val, minVal, maxVal); + return checkRangeOption(pCxt, TSDB_CODE_PAR_INVALID_TABLE_OPTION, pName, val, minVal, maxVal, true); } static int32_t checkDbS3KeepLocalOption(STranslateContext* pCxt, SDatabaseOptions* pOptions) { @@ -8485,7 +8485,7 @@ static int32_t translateUseDatabase(STranslateContext* pCxt, SUseDatabaseStmt* p static int32_t translateCreateUser(STranslateContext* pCxt, SCreateUserStmt* pStmt) { int32_t code = 0; SCreateUserReq createReq = {0}; - if ((code = checkRangeOption(pCxt, TSDB_CODE_INVALID_OPTION, "sysinfo", pStmt->sysinfo, 0, 1))) { + if ((code = checkRangeOption(pCxt, TSDB_CODE_INVALID_OPTION, "sysinfo", pStmt->sysinfo, 0, 1, false))) { return code; } strcpy(createReq.user, pStmt->userName); @@ -8509,13 +8509,13 @@ static int32_t checkAlterUser(STranslateContext* pCxt, SAlterUserStmt* pStmt) { int32_t code = 0; switch (pStmt->alterType) { case TSDB_ALTER_USER_ENABLE: - code = checkRangeOption(pCxt, TSDB_CODE_INVALID_OPTION, "enable", pStmt->enable, 0, 1); + code = checkRangeOption(pCxt, TSDB_CODE_INVALID_OPTION, "enable", pStmt->enable, 0, 1, false); break; case TSDB_ALTER_USER_SYSINFO: - code = checkRangeOption(pCxt, TSDB_CODE_INVALID_OPTION, "sysinfo", pStmt->sysinfo, 0, 1); + code = checkRangeOption(pCxt, TSDB_CODE_INVALID_OPTION, "sysinfo", pStmt->sysinfo, 0, 1, false); break; case TSDB_ALTER_USER_CREATEDB: - code = checkRangeOption(pCxt, TSDB_CODE_INVALID_OPTION, "createdb", pStmt->createdb, 0, 1); + code = checkRangeOption(pCxt, TSDB_CODE_INVALID_OPTION, "createdb", pStmt->createdb, 0, 1, false); break; } return code; diff --git a/tests/script/tsim/user/basic.sim b/tests/script/tsim/user/basic.sim index 353e1d080a..0e1fbb5b40 100644 --- a/tests/script/tsim/user/basic.sim +++ b/tests/script/tsim/user/basic.sim @@ -217,14 +217,18 @@ endi sql_error CREATE USER u100 PASS 'taosdata' SYSINFO -1; sql_error CREATE USER u101 PASS 'taosdata' SYSINFO 2; sql_error CREATE USER u102 PASS 'taosdata' SYSINFO 20000; +sql_error CREATE USER u103 PASS 'taosdata' SYSINFO 1000; sql_error ALTER USER u1 enable -1 sql_error ALTER USER u1 enable 2 +sql_error ALTER USER u1 enable 1000 sql_error ALTER USER u1 enable 10000 sql_error ALTER USER u1 sysinfo -1 sql_error ALTER USER u1 sysinfo 2 +sql_error ALTER USER u1 sysinfo 1000 sql_error ALTER USER u1 sysinfo -20000 sql_error ALTER USER u1 createdb -1 sql_error ALTER USER u1 createdb 3 +sql_error ALTER USER u1 createdb 1000 sql_error ALTER USER u1 createdb 100000 system sh/exec.sh -n dnode1 -s stop -x SIGINT \ No newline at end of file diff --git a/tests/system-test/0-others/user_control.py b/tests/system-test/0-others/user_control.py index c29170e112..c4d24582e4 100644 --- a/tests/system-test/0-others/user_control.py +++ b/tests/system-test/0-others/user_control.py @@ -514,7 +514,7 @@ class TDTestCase: def test_alter_user(self): options = ["enable", "sysinfo", "createdb"] - optionErrVals = [-10000, -128, -1, 2, 127, 10000] + optionErrVals = [-10000, -128, -1, 2, 127, 1000, 10000] for optionErrVal in optionErrVals: tdSql.error("create user user_alter pass 'taosdata' sysinfo %d" % optionErrVal) tdSql.execute("create user user_alter pass 'taosdata'") From eb72cec8d21176f7bdedae3d16bd09e9bcb8e32e Mon Sep 17 00:00:00 2001 From: kailixu Date: Fri, 14 Jun 2024 08:10:07 +0800 Subject: [PATCH 2/3] chore: naming optimization --- source/libs/parser/src/parTranslater.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/libs/parser/src/parTranslater.c b/source/libs/parser/src/parTranslater.c index 5f55841a55..335324c86f 100644 --- a/source/libs/parser/src/parTranslater.c +++ b/source/libs/parser/src/parTranslater.c @@ -6605,8 +6605,8 @@ static int32_t buildCreateDbReq(STranslateContext* pCxt, SCreateDatabaseStmt* pS } static int32_t checkRangeOption(STranslateContext* pCxt, int32_t code, const char* pName, int64_t val, int64_t minVal, - int64_t maxVal, bool skipMinus) { - if (skipMinus ? ((val >= 0) && (val < minVal || val > maxVal)) : (val < minVal || val > maxVal)) { + int64_t maxVal, bool skipUndef) { + if (skipUndef ? ((val >= 0) && (val < minVal || val > maxVal)) : (val < minVal || val > maxVal)) { return generateSyntaxErrMsgExt(&pCxt->msgBuf, code, "Invalid option %s: %" PRId64 ", valid range: [%" PRId64 ", %" PRId64 "]", pName, val, minVal, maxVal); From 0ef8d3510a4e79bda648c6e2b7aae8b30d1a7cb9 Mon Sep 17 00:00:00 2001 From: kailixu Date: Fri, 14 Jun 2024 15:46:11 +0800 Subject: [PATCH 3/3] fix: use unsigned type for union bit operation --- include/common/tmsg.h | 6 +++--- source/common/src/tmsg.c | 4 ++-- source/dnode/mnode/impl/inc/mndDef.h | 6 +++--- source/dnode/mnode/impl/src/mndUser.c | 4 ++-- source/dnode/mnode/sdb/inc/sdb.h | 4 ++++ source/dnode/mnode/sdb/src/sdbRaw.c | 30 +++++++++++++++++++++++++++ 6 files changed, 44 insertions(+), 10 deletions(-) diff --git a/include/common/tmsg.h b/include/common/tmsg.h index fa89ca917a..9301b31f95 100644 --- a/include/common/tmsg.h +++ b/include/common/tmsg.h @@ -1054,10 +1054,10 @@ typedef struct { int8_t enable; int8_t isView; union { - int8_t flag; + uint8_t flag; struct { - int8_t createdb : 1; - int8_t reserve : 7; + uint8_t createdb : 1; + uint8_t reserve : 7; }; }; char user[TSDB_USER_LEN]; diff --git a/source/common/src/tmsg.c b/source/common/src/tmsg.c index d465e24d5b..1cbd646413 100644 --- a/source/common/src/tmsg.c +++ b/source/common/src/tmsg.c @@ -1813,7 +1813,7 @@ int32_t tSerializeSAlterUserReq(void *buf, int32_t bufLen, SAlterUserReq *pReq) } if (tEncodeI64(&encoder, pReq->privileges) < 0) return -1; ENCODESQL(); - if (tEncodeI8(&encoder, pReq->flag) < 0) return -1; + if (tEncodeU8(&encoder, pReq->flag) < 0) return -1; tEndEncode(&encoder); int32_t tlen = encoder.pos; @@ -1854,7 +1854,7 @@ int32_t tDeserializeSAlterUserReq(void *buf, int32_t bufLen, SAlterUserReq *pReq if (tDecodeI64(&decoder, &pReq->privileges) < 0) return -1; DECODESQL(); if (!tDecodeIsEnd(&decoder)) { - if (tDecodeI8(&decoder, &pReq->flag) < 0) return -1; + if (tDecodeU8(&decoder, &pReq->flag) < 0) return -1; } tEndDecode(&decoder); diff --git a/source/dnode/mnode/impl/inc/mndDef.h b/source/dnode/mnode/impl/inc/mndDef.h index b6fc24a910..46e606d3ea 100644 --- a/source/dnode/mnode/impl/inc/mndDef.h +++ b/source/dnode/mnode/impl/inc/mndDef.h @@ -332,10 +332,10 @@ typedef struct { int8_t sysInfo; int8_t enable; union { - int8_t flag; + uint8_t flag; struct { - int8_t createdb : 1; - int8_t reserve : 7; + uint8_t createdb : 1; + uint8_t reserve : 7; }; }; int32_t acctId; diff --git a/source/dnode/mnode/impl/src/mndUser.c b/source/dnode/mnode/impl/src/mndUser.c index 9a85d405ca..7cde8c5508 100644 --- a/source/dnode/mnode/impl/src/mndUser.c +++ b/source/dnode/mnode/impl/src/mndUser.c @@ -818,7 +818,7 @@ SSdbRaw *mndUserActionEncode(SUserObj *pUser) { SDB_SET_INT8(pRaw, dataPos, pUser->superUser, _OVER) SDB_SET_INT8(pRaw, dataPos, pUser->sysInfo, _OVER) SDB_SET_INT8(pRaw, dataPos, pUser->enable, _OVER) - SDB_SET_INT8(pRaw, dataPos, pUser->flag, _OVER) + SDB_SET_UINT8(pRaw, dataPos, pUser->flag, _OVER) SDB_SET_INT32(pRaw, dataPos, pUser->authVersion, _OVER) SDB_SET_INT32(pRaw, dataPos, pUser->passVersion, _OVER) SDB_SET_INT32(pRaw, dataPos, numOfReadDbs, _OVER) @@ -1002,7 +1002,7 @@ static SSdbRow *mndUserActionDecode(SSdbRaw *pRaw) { SDB_GET_INT8(pRaw, dataPos, &pUser->superUser, _OVER) SDB_GET_INT8(pRaw, dataPos, &pUser->sysInfo, _OVER) SDB_GET_INT8(pRaw, dataPos, &pUser->enable, _OVER) - SDB_GET_INT8(pRaw, dataPos, &pUser->flag, _OVER) + SDB_GET_UINT8(pRaw, dataPos, &pUser->flag, _OVER) if (pUser->superUser) pUser->createdb = 1; SDB_GET_INT32(pRaw, dataPos, &pUser->authVersion, _OVER) if (sver >= 4) { diff --git a/source/dnode/mnode/sdb/inc/sdb.h b/source/dnode/mnode/sdb/inc/sdb.h index 0b2de2b151..fc9b89a141 100644 --- a/source/dnode/mnode/sdb/inc/sdb.h +++ b/source/dnode/mnode/sdb/inc/sdb.h @@ -57,6 +57,7 @@ extern "C" { #define SDB_GET_INT32(pData, dataPos, val, pos) SDB_GET_VAL(pData, dataPos, val, pos, sdbGetRawInt32, int32_t) #define SDB_GET_INT16(pData, dataPos, val, pos) SDB_GET_VAL(pData, dataPos, val, pos, sdbGetRawInt16, int16_t) #define SDB_GET_INT8(pData, dataPos, val, pos) SDB_GET_VAL(pData, dataPos, val, pos, sdbGetRawInt8, int8_t) +#define SDB_GET_UINT8(pData, dataPos, val, pos) SDB_GET_VAL(pData, dataPos, val, pos, sdbGetRawUInt8, uint8_t) #define SDB_GET_RESERVE(pRaw, dataPos, valLen, pos) \ { \ @@ -76,6 +77,7 @@ extern "C" { #define SDB_SET_INT32(pRaw, dataPos, val, pos) SDB_SET_VAL(pRaw, dataPos, val, pos, sdbSetRawInt32, int32_t) #define SDB_SET_INT16(pRaw, dataPos, val, pos) SDB_SET_VAL(pRaw, dataPos, val, pos, sdbSetRawInt16, int16_t) #define SDB_SET_INT8(pRaw, dataPos, val, pos) SDB_SET_VAL(pRaw, dataPos, val, pos, sdbSetRawInt8, int8_t) +#define SDB_SET_UINT8(pRaw, dataPos, val, pos) SDB_SET_VAL(pRaw, dataPos, val, pos, sdbSetRawUInt8, uint8_t) #define SDB_SET_BINARY(pRaw, dataPos, val, valLen, pos) \ { \ @@ -388,6 +390,7 @@ void sdbGetCommitInfo(SSdb *pSdb, int64_t *index, int64_t *term, int64_t *config SSdbRaw *sdbAllocRaw(ESdbType type, int8_t sver, int32_t dataLen); void sdbFreeRaw(SSdbRaw *pRaw); int32_t sdbSetRawInt8(SSdbRaw *pRaw, int32_t dataPos, int8_t val); +int32_t sdbSetRawUInt8(SSdbRaw *pRaw, int32_t dataPos, uint8_t val); int32_t sdbSetRawInt16(SSdbRaw *pRaw, int32_t dataPos, int16_t val); int32_t sdbSetRawInt32(SSdbRaw *pRaw, int32_t dataPos, int32_t val); int32_t sdbSetRawInt64(SSdbRaw *pRaw, int32_t dataPos, int64_t val); @@ -395,6 +398,7 @@ int32_t sdbSetRawBinary(SSdbRaw *pRaw, int32_t dataPos, const char *pVal, int32 int32_t sdbSetRawDataLen(SSdbRaw *pRaw, int32_t dataLen); int32_t sdbSetRawStatus(SSdbRaw *pRaw, ESdbStatus status); int32_t sdbGetRawInt8(SSdbRaw *pRaw, int32_t dataPos, int8_t *val); +int32_t sdbGetRawUInt8(SSdbRaw *pRaw, int32_t dataPos, uint8_t *val); int32_t sdbGetRawInt16(SSdbRaw *pRaw, int32_t dataPos, int16_t *val); int32_t sdbGetRawInt32(SSdbRaw *pRaw, int32_t dataPos, int32_t *val); int32_t sdbGetRawInt64(SSdbRaw *pRaw, int32_t dataPos, int64_t *val); diff --git a/source/dnode/mnode/sdb/src/sdbRaw.c b/source/dnode/mnode/sdb/src/sdbRaw.c index 244e50b52e..4f68139155 100644 --- a/source/dnode/mnode/sdb/src/sdbRaw.c +++ b/source/dnode/mnode/sdb/src/sdbRaw.c @@ -67,6 +67,21 @@ int32_t sdbSetRawInt8(SSdbRaw *pRaw, int32_t dataPos, int8_t val) { return 0; } +int32_t sdbSetRawUInt8(SSdbRaw *pRaw, int32_t dataPos, uint8_t val) { + if (pRaw == NULL) { + terrno = TSDB_CODE_INVALID_PTR; + return -1; + } + + if (dataPos + sizeof(uint8_t) > pRaw->dataLen) { + terrno = TSDB_CODE_SDB_INVALID_DATA_LEN; + return -1; + } + + *(uint8_t *)(pRaw->pData + dataPos) = val; + return 0; +} + int32_t sdbSetRawInt32(SSdbRaw *pRaw, int32_t dataPos, int32_t val) { if (pRaw == NULL) { terrno = TSDB_CODE_INVALID_PTR; @@ -174,6 +189,21 @@ int32_t sdbGetRawInt8(SSdbRaw *pRaw, int32_t dataPos, int8_t *val) { return 0; } +int32_t sdbGetRawUInt8(SSdbRaw *pRaw, int32_t dataPos, uint8_t *val) { + if (pRaw == NULL) { + terrno = TSDB_CODE_INVALID_PTR; + return -1; + } + + if (dataPos + sizeof(uint8_t) > pRaw->dataLen) { + terrno = TSDB_CODE_SDB_INVALID_DATA_LEN; + return -1; + } + + *val = *(uint8_t *)(pRaw->pData + dataPos); + return 0; +} + int32_t sdbGetRawInt32(SSdbRaw *pRaw, int32_t dataPos, int32_t *val) { if (pRaw == NULL) { terrno = TSDB_CODE_INVALID_PTR;