Time parsing 32bit corner-case

[matteo-cristino]

Since time is an integer, it could take values from -2147483648 to 2147483647, but if I try to do

TIME.new(-2147483648)

the following error is returned

Could not read unix timestamp -2.147484e+09

While this error is not returned in the case I use

TIME.new("-2147483648")

This seems due to some conversion from number to string done internally in lua since, as stated in the documentation, both number and strings are seen as string from lua_isstring function (making also the lua_isnumber branching useless)

Zenroom/src/zen_time.c

Lines 74 to 121 in ca02f6c

	ztime_t* time_arg(lua_State *L, int n) {
	Z(L);
	ztime_t *result = (ztime_t*)malloc(sizeof(ztime_t));
	if(result == NULL) {
	return NULL;
	}
	void *ud = luaL_testudata(L, n, "zenroom.time");
	if(ud) {
	*result = *(ztime_t*)ud;
	goto end;
	}
	if(lua_isstring(L, 1)) {
	const char* arg = lua_tostring(L, 1);
	char *pEnd;
	long l_result = strtol(arg, &pEnd, 10);
	if(*pEnd) {
	free(result);
	lerror(L, "Could not read unix timestamp %s", arg);
	return NULL;
	} else if (l_result < INT_MIN || l_result > INT_MAX) {
	free(result);
	lerror(L, "Could not read unix timestamp %s out of range", arg);
	return NULL;
	}
	*result = (ztime_t)l_result;
	goto end;
	}
	// number argument, import
	if(lua_isnumber(L, 1)) {
	lua_Number number = lua_tonumber(L, 1);
	*result = (int)number;
	goto end;
	}
	octet *o = o_arg(L, n);
	if(o) {
	if(o->len != sizeof(ztime_t)) {
	free(result);
	zerror(L, "Wrong size timestamp %s", __func__);
	return NULL;
	}
	memcpy(result, o->val, sizeof(ztime_t));
	o_free(L, o);
	goto end;
	}
	end:
	if(result) Z->memcount_times++;
	return result;
	}

and thus they both pass thorugh the lua_tostring function. I tried also to move the branch with lua_isnumber before the lua_isstring one, but it seemed that after a certain number, lua was not able to read them correctly.

Thus at the moment, if we use numbers in input we are only loosing the time -2147483648 and using strings in input we should not lose anything. Anyway interesting to notice and maybe look further into it in case any other problems with numbers came up.

[jaromil]

Is this recently fenced off by fixing the js 32bit time conversion?