Re: [Tarantool-patches] [PATCH 1/3] access: fix invalid error type for not found user

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the review!

On 05/11/2019 21:15, Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/11/05 17:57]:
>> +++ b/src/box/user.cc
>> @@ -521,8 +521,9 @@ user_find_by_name(const char *name, uint32_t len)
>>  	if (schema_find_id(BOX_USER_ID, 2, name, len, &uid) != 0)
>>  		return NULL;
>>  	struct user *user = user_by_id(uid);
>> -	if (user == NULL || user->def->type != SC_USER) {
>> -		diag_set(ClientError, ER_NO_SUCH_USER, tt_cstr(name, len));
>> +	if (user == NULL || user->def->type != SC_USER || uid == BOX_ID_NIL) {
>> +		diag_set(ClientError, ER_NO_SUCH_USER,
>> +			 tt_cstr(name, MIN(BOX_INVALID_NAME_MAX, len)));
>>  		return NULL;
> 
> I would not call user_by_id with BOX_ID_NIL, this is not an error
> but it takes an effort to ponder on and realize it's OK to do so.
> 
> Otherwise LGTM.
> 
> 

Hm, you are right. I should not search by an invalid ID. Fixed
on the branch.

============================================================================

diff --git a/src/box/user.cc b/src/box/user.cc
index 03b4b2e3b..e2fd32740 100644
--- a/src/box/user.cc
+++ b/src/box/user.cc
@@ -520,12 +520,14 @@ user_find_by_name(const char *name, uint32_t len)
 	uint32_t uid;
 	if (schema_find_id(BOX_USER_ID, 2, name, len, &uid) != 0)
 		return NULL;
-	struct user *user = user_by_id(uid);
-	if (user == NULL || user->def->type != SC_USER) {
-		diag_set(ClientError, ER_NO_SUCH_USER, tt_cstr(name, len));
-		return NULL;
+	if (uid != BOX_ID_NIL) {
+		struct user *user = user_by_id(uid);
+		if (user != NULL && user->def->type == SC_USER)
+			return user;
 	}
-	return user;
+	diag_set(ClientError, ER_NO_SUCH_USER,
+		 tt_cstr(name, MIN(BOX_INVALID_NAME_MAX, len)));
+	return NULL;
 }

============================================================================