Tarantool development patches archive
 help / color / mirror / Atom feed
From: Kirill Yukhin <kyukhin@tarantool.org>
To: "n.pettik" <korablev@tarantool.org>
Cc: tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH] sql: do not allow oversized integer literals
Date: Wed, 18 Jul 2018 10:57:14 +0300	[thread overview]
Message-ID: <20180718075714.4dl3hlfuuoqimv5p@tarantool.org> (raw)
In-Reply-To: <9995AAE4-3420-4685-9AC9-2E2854D4E905@tarantool.org>

Hello Nikita,
Thanks for review, my answers are inlined. Updated patch in the bottom.

On 18 июл 04:10, n.pettik wrote:
> 
> > Before the patch, big integer constant were silently
> 
> ‘was’ or ‘constants’ — choose one.
Fixed.

> > converted to floating point values. Fix that by issuing
> > error message if value doesn't fit in int64_t range.
> > 
> > Also, refactor surrounding code as per Tarantool's code style.
> 
> Well, it was quite complicated to find your initial fix through
> code style corrections. Next time lets separate somehow
> such patches or, at least left notes what (and/or where) exactly
> has been fixed.
Ok.

> > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> > index b1650cf..43d71eb 100644
> > --- a/src/box/sql/expr.c
> > +++ b/src/box/sql/expr.c
> > @@ -1087,9 +1087,9 @@ sqlite3ExprAssignVarNumber(Parse * pParse, Expr * pExpr, u32 n)
> > 			 * "nnn" to an integer and use it as the
> > 			 * variable number
> > 			 */
> > -			i64 i;
> > +			int64_t i;
> > 			int bOk =
> > -			    0 == sqlite3Atoi64(&z[1], &i, n - 1);
> > +			    0 == sql_atoi64(&z[1], &i, n - 1); 
> You can fit it into one line:
> int bOk = 0 == sql_atoi64(&z[1], &i, n - 1);
Fixed. Also, replaced w/ bool here.

> > static void
> > -codeInteger(Parse * pParse, Expr * pExpr, int negFlag, int iMem)
> > +sql_expr_code_int(struct Parse *parse, struct Expr *expr, bool neg_flag,
> > +		  int mem)
> 
> I wouldn’t use ’sql_’ prefix for static functions. Btw, I still have doubts
> concerning usage of prefixes in SQL - lets document it somewhere.
Well, news to me. Ok, prefix removed.

> Also, according to codestyle bool vars are used with ‘is_’ or ‘if_’ prefixes.
Fixed.

> > {
> > -	Vdbe *v = pParse->pVdbe;
> > -	if (pExpr->flags & EP_IntValue) {
> > -		int i = pExpr->u.iValue;
> > +	struct Vdbe *v = parse->pVdbe;
> > +	if (expr->flags & EP_IntValue) {
> > +		int i = expr->u.iValue;
> > 		assert(i >= 0);
> > -		if (negFlag)
> > +		if (neg_flag)
> > 			i = -i;
> > -		sqlite3VdbeAddOp2(v, OP_Integer, i, iMem);
> > +		sqlite3VdbeAddOp2(v, OP_Integer, i, mem);
> > 	} else {
> > 		int c;
> > +		int64_t value;
> > +		const char *z = expr->u.zToken;
> > +		assert(z != NULL);
> > +		c = sql_dec_or_hex_to_i64(z, &value);
> 
> Don’t delay initialisation until first usage:
> int c = sql_dec_or_hex_to_i64(z, &value);
Fixed.

> > +		if (c == 1 || (c == 2 && !neg_flag)
> > +		    || (neg_flag && value == SMALLEST_INT64)) {
> > +                        if (sqlite3_strnicmp(z, "0x", 2) == 0) {
> > +                                sqlite3ErrorMsg(parse,
> > +                                                "hex literal too big: %s%s",
> > +                                                neg_flag ? "-" : "", z); 
> I see here broken indentation.
Fixed.

> > diff --git a/src/box/sql/main.c b/src/box/sql/main.c
> > index 00dc7a6..cbca90a 100644
> > --- a/src/box/sql/main.c
> > +++ b/src/box/sql/main.c
> > @@ -2468,10 +2468,9 @@ sqlite3_uri_int64(const char *zFilename,	/* Filename as passed to xOpen */
> > 		  sqlite3_int64 bDflt)	/* return if parameter is missing */
> > {
> > 	const char *z = sqlite3_uri_parameter(zFilename, zParam);
> > -	sqlite3_int64 v;
> > -	if (z && sqlite3DecOrHexToI64(z, &v) == SQLITE_OK) {
> > +	int64_t v;
> > +	if (z != NULL && sql_dec_or_hex_to_i64(z, &v) == SQLITE_OK) 
> Why do you use here SQLITE_OK? Lets just use comparison with zero.
> (AFAIR we decided to avoid using this macros.)
Ok.

> > diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
> > +int
> > +sql_atoi64(const char *z, int64_t *val, int length);
> > +
> > +/**
> > + * Transform a UTF-8 integer literal, in either decimal or hexadecimal,
> > + * into a 64-bit signed integer.  This routine accepts hexadecimal literals,
> Fit comments into 66 chars.
Done.

> > diff --git a/src/box/sql/util.c b/src/box/sql/util.c
> > index e4c2c5d..d6e750c 100644
> > --- a/src/box/sql/util.c
> > +++ b/src/box/sql/util.c
> > + *
> > + * @param z String being parsed.
> > + * @param[out] val Output integer value.
> > + * @param length String length in bytes.
> > + * @retval
> > + *     0    Successful transformation.  Fits in a 64-bit signed integer.
> > + *     1    Integer too large for a 64-bit signed integer or is malformed
> > + *     2    Special case of 9223372036854775808
> >  */
> You can remove comment from definition: it repeats one from declaration.
Done.

> > int
> > -sqlite3Atoi64(const char *zNum, i64 * pNum, int length)
> > +sql_atoi64(const char *z, int64_t *val, int length)
> > {
> > 	int incr = 1; // UTF-8
> Remove this comment pls: it is not informative and violates codestyle.
Done.

> > -	testcase(i == 18);
> > -	testcase(i == 19);
> > -	testcase(i == 20);
> > -	if (&zNum[i] < zEnd	/* Extra bytes at the end */
> > -	    || (i == 0 && zStart == zNum)	/* No digits */
> > +	if (&z[i] < zEnd	/* Extra bytes at the end */
> Remove comments (or put them above code to be explained);
> Put binary operator to line above:
> 
> if (A &&
>     B...
Done both.

> > -	{
> > -		return sqlite3Atoi64(z, pOut, sqlite3Strlen30(z));
> > +	} else {
> > +		return sql_atoi64(z, val, sqlite3Strlen30(z));
> > 	}
> You can remove ‘else’ branch: ‘if' branch ends with return
> (It is not vital, but code will look clearer, at least for me).
Done.

> > diff --git a/test/sql/gh-2347-max-int-literals.test.lua b/test/sql/gh-2347-max-int-literals.test.lua
> > new file mode 100644
> > index 0000000..4b1ef0d
> > --- /dev/null
> > +++ b/test/sql/gh-2347-max-int-literals.test.lua
> 
> Lets avoid creating separate test-file for such insignificant issue.
> You can put it for instance to sql/mist.test.lua or to one of sql-tap tests.
I think, that this minor change affects visible behaviour. So, let's keep
tests for such changes in separate files.

> Also, it seems that you forgot to commit .result file.
Added.

--
Regards, Kirill Yukhin

commit 5ff577f62222bdc3ebd227161774669a9d1b6396
Author: Kirill Yukhin <kyukhin@tarantool.org>
Date:   Tue Jul 17 08:54:24 2018 +0300

    sql: do not allow oversized integer literals
    
    Before the patch, big integer constants were silently
    converted to floating point values. Fix that by issuing
    error message if value doesn't fit in int64_t range.
    
    Also, refactor surrounding code as per Tarantool's code style.
    
    Closes #2347

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index b1650cf..f662a99 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -1087,15 +1087,14 @@ sqlite3ExprAssignVarNumber(Parse * pParse, Expr * pExpr, u32 n)
 			 * "nnn" to an integer and use it as the
 			 * variable number
 			 */
-			i64 i;
-			int bOk =
-			    0 == sqlite3Atoi64(&z[1], &i, n - 1);
+			int64_t i;
+			bool is_ok = 0 == sql_atoi64(&z[1], &i, n - 1);
 			x = (ynVar) i;
 			testcase(i == 0);
 			testcase(i == 1);
 			testcase(i == SQL_BIND_PARAMETER_MAX - 1);
 			testcase(i == SQL_BIND_PARAMETER_MAX);
-			if (bOk == 0 || i < 1 || i > SQL_BIND_PARAMETER_MAX) {
+			if (!is_ok || i < 1 || i > SQL_BIND_PARAMETER_MAX) {
 				sqlite3ErrorMsg(pParse,
 						"variable number must be between $1 and $%d",
 						SQL_BIND_PARAMETER_MAX);
@@ -3259,51 +3258,48 @@ codeReal(Vdbe * v, const char *z, int negateFlag, int iMem)
 }
 #endif
 
-/*
+/**
  * Generate an instruction that will put the integer describe by
  * text z[0..n-1] into register iMem.
  *
- * Expr.u.zToken is always UTF8 and zero-terminated.
+ * @param parse Parsing context.
+ * @param expr Expression being parsed. Expr.u.zToken is always
+ *             UTF8 and zero-terminated.
+ * @param neg_flag True if value is negative.
+ * @param mem Register to store parsed integer
  */
 static void
-codeInteger(Parse * pParse, Expr * pExpr, int negFlag, int iMem)
+expr_code_int(struct Parse *parse, struct Expr *expr, bool is_neg,
+	      int mem)
 {
-	Vdbe *v = pParse->pVdbe;
-	if (pExpr->flags & EP_IntValue) {
-		int i = pExpr->u.iValue;
+	struct Vdbe *v = parse->pVdbe;
+	if (expr->flags & EP_IntValue) {
+		int i = expr->u.iValue;
 		assert(i >= 0);
-		if (negFlag)
+		if (is_neg)
 			i = -i;
-		sqlite3VdbeAddOp2(v, OP_Integer, i, iMem);
+		sqlite3VdbeAddOp2(v, OP_Integer, i, mem);
 	} else {
-		int c;
-		i64 value;
-		const char *z = pExpr->u.zToken;
-		assert(z != 0);
-		c = sqlite3DecOrHexToI64(z, &value);
-		if (c == 1 || (c == 2 && !negFlag)
-		    || (negFlag && value == SMALLEST_INT64)) {
-#ifdef SQLITE_OMIT_FLOATING_POINT
-			sqlite3ErrorMsg(pParse, "oversized integer: %s%s",
-					negFlag ? "-" : "", z);
-#else
-#ifndef SQLITE_OMIT_HEX_INTEGER
+		int64_t value;
+		const char *z = expr->u.zToken;
+		assert(z != NULL);
+		int c = sql_dec_or_hex_to_i64(z, &value);
+		if (c == 1 || (c == 2 && !is_neg)
+		    || (is_neg && value == SMALLEST_INT64)) {
 			if (sqlite3_strnicmp(z, "0x", 2) == 0) {
-				sqlite3ErrorMsg(pParse,
+				sqlite3ErrorMsg(parse,
 						"hex literal too big: %s%s",
-						negFlag ? "-" : "", z);
-			} else
-#endif
-			{
-				codeReal(v, z, negFlag, iMem);
+						is_neg ? "-" : "", z);
+			} else {
+				sqlite3ErrorMsg(parse,
+						"oversized integer: %s%s",
+						is_neg ? "-" : "", z);
 			}
-#endif
 		} else {
-			if (negFlag) {
+			if (is_neg)
 				value = c == 2 ? SMALLEST_INT64 : -value;
-			}
-			sqlite3VdbeAddOp4Dup8(v, OP_Int64, 0, iMem, 0,
-					      (u8 *) & value, P4_INT64);
+			sqlite3VdbeAddOp4Dup8(v, OP_Int64, 0, mem, 0,
+					      (u8 *)&value, P4_INT64);
 		}
 	}
 }
@@ -3715,7 +3711,7 @@ sqlite3ExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 							target, pExpr->op2);
 		}
 	case TK_INTEGER:{
-			codeInteger(pParse, pExpr, 0, target);
+			expr_code_int(pParse, pExpr, false, target);
 			return target;
 		}
 #ifndef SQLITE_OMIT_FLOATING_POINT
@@ -3875,7 +3871,7 @@ sqlite3ExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 			Expr *pLeft = pExpr->pLeft;
 			assert(pLeft);
 			if (pLeft->op == TK_INTEGER) {
-				codeInteger(pParse, pLeft, 1, target);
+				expr_code_int(pParse, pLeft, true, target);
 				return target;
 #ifndef SQLITE_OMIT_FLOATING_POINT
 			} else if (pLeft->op == TK_FLOAT) {
diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index 00dc7a6..85bc7e9 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -2468,10 +2468,9 @@ sqlite3_uri_int64(const char *zFilename,	/* Filename as passed to xOpen */
 		  sqlite3_int64 bDflt)	/* return if parameter is missing */
 {
 	const char *z = sqlite3_uri_parameter(zFilename, zParam);
-	sqlite3_int64 v;
-	if (z && sqlite3DecOrHexToI64(z, &v) == SQLITE_OK) {
+	int64_t v;
+	if (z != NULL && sql_dec_or_hex_to_i64(z, &v) == 0)
 		bDflt = v;
-	}
 	return bDflt;
 }
 
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 18bf949..4a972a1 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -4354,8 +4354,53 @@ char
 sqlite3TableColumnAffinity(struct space_def *def, int idx);
 
 char sqlite3ExprAffinity(Expr * pExpr);
-int sqlite3Atoi64(const char *, i64 *, int);
-int sqlite3DecOrHexToI64(const char *, i64 *);
+
+
+/**
+ * Convert z to a 64-bit signed integer.  z must be decimal. This
+ * routine does *not* accept hexadecimal notation.
+ *
+ * If the z value is representable as a 64-bit twos-complement
+ * integer, then write that value into *val and return 0.
+ *
+ * If z is exactly 9223372036854775808, return 2.  This special
+ * case is broken out because while 9223372036854775808 cannot be a
+ * signed 64-bit integer, its negative -9223372036854775808 can be.
+ *
+ * If z is too big for a 64-bit integer and is not
+ * 9223372036854775808  or if z contains any non-numeric text,
+ * then return 1.
+ *
+ * length is the number of bytes in the string (bytes, not characters).
+ * The string is not necessarily zero-terminated.  The encoding is
+ * given by enc.
+ *
+ * @param z String being parsed.
+ * @param[out] val Output integer value.
+ * @param length String length in bytes.
+ * @retval
+ *     0    Successful transformation.  Fits in a 64-bit signed integer.
+ *     1    Integer too large for a 64-bit signed integer or is malformed
+ *     2    Special case of 9223372036854775808
+ */
+int
+sql_atoi64(const char *z, int64_t *val, int length);
+
+/**
+ * Transform a UTF-8 integer literal, in either decimal or
+ * hexadecimal, into a 64-bit signed integer.  This routine
+ * accepts hexadecimal literals, whereas sql_atoi64() does not.
+ *
+ * @param z Literal being parsed.
+ * @param[out] val Parsed value.
+ * @retval
+ *     0    Successful transformation.  Fits in a 64-bit signed integer.
+ *     1    Integer too large for a 64-bit signed integer or is malformed
+ *     2    Special case of 9223372036854775808
+ */
+int
+sql_dec_or_hex_to_i64(const char *z, int64_t *val);
+
 void sqlite3ErrorWithMsg(sqlite3 *, int, const char *, ...);
 void sqlite3Error(sqlite3 *, int);
 void sqlite3SystemError(sqlite3 *, int);
diff --git a/src/box/sql/util.c b/src/box/sql/util.c
index e4c2c5d..a7406c0 100644
--- a/src/box/sql/util.c
+++ b/src/box/sql/util.c
@@ -621,70 +621,46 @@ compare2pow63(const char *zNum, int incr)
 	return c;
 }
 
-/*
- * Convert zNum to a 64-bit signed integer.  zNum must be decimal. This
- * routine does *not* accept hexadecimal notation.
- *
- * If the zNum value is representable as a 64-bit twos-complement
- * integer, then write that value into *pNum and return 0.
- *
- * If zNum is exactly 9223372036854775808, return 2.  This special
- * case is broken out because while 9223372036854775808 cannot be a
- * signed 64-bit integer, its negative -9223372036854775808 can be.
- *
- * If zNum is too big for a 64-bit integer and is not
- * 9223372036854775808  or if zNum contains any non-numeric text,
- * then return 1.
- *
- * length is the number of bytes in the string (bytes, not characters).
- * The string is not necessarily zero-terminated.  The encoding is
- * given by enc.
- */
 int
-sqlite3Atoi64(const char *zNum, i64 * pNum, int length)
+sql_atoi64(const char *z, int64_t *val, int length)
 {
-	int incr = 1; // UTF-8
+	int incr = 1;
 	u64 u = 0;
 	int neg = 0;		/* assume positive */
 	int i;
 	int c = 0;
 	int nonNum = 0;		/* True if input contains UTF16 with high byte non-zero */
 	const char *zStart;
-	const char *zEnd = zNum + length;
+	const char *zEnd = z + length;
 	incr = 1;
-	while (zNum < zEnd && sqlite3Isspace(*zNum))
-		zNum += incr;
-	if (zNum < zEnd) {
-		if (*zNum == '-') {
+	while (z < zEnd && sqlite3Isspace(*z))
+		z += incr;
+	if (z < zEnd) {
+		if (*z == '-') {
 			neg = 1;
-			zNum += incr;
-		} else if (*zNum == '+') {
-			zNum += incr;
+			z += incr;
+		} else if (*z == '+') {
+			z += incr;
 		}
 	}
-	zStart = zNum;
-	while (zNum < zEnd && zNum[0] == '0') {
-		zNum += incr;
-	}			/* Skip leading zeros. */
-	for (i = 0; &zNum[i] < zEnd && (c = zNum[i]) >= '0' && c <= '9';
+	zStart = z;
+	/* Skip leading zeros. */
+	while (z < zEnd && z[0] == '0') {
+		z += incr;
+	}
+	for (i = 0; &z[i] < zEnd && (c = z[i]) >= '0' && c <= '9';
 	     i += incr) {
 		u = u * 10 + c - '0';
 	}
 	if (u > LARGEST_INT64) {
-		*pNum = neg ? SMALLEST_INT64 : LARGEST_INT64;
+		*val = neg ? SMALLEST_INT64 : LARGEST_INT64;
 	} else if (neg) {
-		*pNum = -(i64) u;
+		*val = -(i64) u;
 	} else {
-		*pNum = (i64) u;
+		*val = (i64) u;
 	}
-	testcase(i == 18);
-	testcase(i == 19);
-	testcase(i == 20);
-	if (&zNum[i] < zEnd	/* Extra bytes at the end */
-	    || (i == 0 && zStart == zNum)	/* No digits */
-	    ||i > 19 * incr	/* Too many digits */
-	    || nonNum		/* UTF16 with high-order bytes non-zero */
-	    ) {
+	if (&z[i] < zEnd || (i == 0 && zStart == z) || i > 19 * incr ||
+	    nonNum) {
 		/* zNum is empty or contains non-numeric text or is longer
 		 * than 19 digits (thus guaranteeing that it is too large)
 		 */
@@ -695,7 +671,7 @@ sqlite3Atoi64(const char *zNum, i64 * pNum, int length)
 		return 0;
 	} else {
 		/* zNum is a 19-digit numbers.  Compare it against 9223372036854775808. */
-		c = compare2pow63(zNum, incr);
+		c = compare2pow63(z, incr);
 		if (c < 0) {
 			/* zNum is less than 9223372036854775808 so it fits */
 			assert(u <= LARGEST_INT64);
@@ -713,37 +689,19 @@ sqlite3Atoi64(const char *zNum, i64 * pNum, int length)
 	}
 }
 
-/*
- * Transform a UTF-8 integer literal, in either decimal or hexadecimal,
- * into a 64-bit signed integer.  This routine accepts hexadecimal literals,
- * whereas sqlite3Atoi64() does not.
- *
- * Returns:
- *
- *     0    Successful transformation.  Fits in a 64-bit signed integer.
- *     1    Integer too large for a 64-bit signed integer or is malformed
- *     2    Special case of 9223372036854775808
- */
 int
-sqlite3DecOrHexToI64(const char *z, i64 * pOut)
+sql_dec_or_hex_to_i64(const char *z, int64_t *val)
 {
-#ifndef SQLITE_OMIT_HEX_INTEGER
-	if (z[0] == '0' && (z[1] == 'x' || z[1] == 'X')
-	    ) {
-		u64 u = 0;
+	if (z[0] == '0' && (z[1] == 'x' || z[1] == 'X')) {
+		uint64_t u = 0;
 		int i, k;
-		for (i = 2; z[i] == '0'; i++) {
-		}
-		for (k = i; sqlite3Isxdigit(z[k]); k++) {
+		for (i = 2; z[i] == '0'; i++);
+		for (k = i; sqlite3Isxdigit(z[k]); k++)
 			u = u * 16 + sqlite3HexToInt(z[k]);
-		}
-		memcpy(pOut, &u, 8);
+		memcpy(val, &u, 8);
 		return (z[k] == 0 && k - i <= 16) ? 0 : 1;
-	} else
-#endif				/* SQLITE_OMIT_HEX_INTEGER */
-	{
-		return sqlite3Atoi64(z, pOut, sqlite3Strlen30(z));
 	}
+	return sql_atoi64(z, val, sqlite3Strlen30(z));
 }
 
 /*
@@ -768,7 +726,6 @@ sqlite3GetInt32(const char *zNum, int *pValue)
 	} else if (zNum[0] == '+') {
 		zNum++;
 	}
-#ifndef SQLITE_OMIT_HEX_INTEGER
 	else if (zNum[0] == '0' && (zNum[1] == 'x' || zNum[1] == 'X')
 		 && sqlite3Isxdigit(zNum[2])
 	    ) {
@@ -786,7 +743,6 @@ sqlite3GetInt32(const char *zNum, int *pValue)
 			return 0;
 		}
 	}
-#endif
 	while (zNum[0] == '0')
 		zNum++;
 	for (i = 0; i < 11 && (c = zNum[i] - '0') >= 0 && c <= 9; i++) {
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index f50e389..195638e 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -291,7 +291,7 @@ applyNumericAffinity(Mem *pRec, int bTryForInt)
 	i64 iValue;
 	assert((pRec->flags & (MEM_Str|MEM_Int|MEM_Real))==MEM_Str);
 	if (sqlite3AtoF(pRec->z, &rValue, pRec->n)==0) return;
-	if (0==sqlite3Atoi64(pRec->z, &iValue, pRec->n)) {
+	if (0 == sql_atoi64(pRec->z, (int64_t *)&iValue, pRec->n)) {
 		pRec->u.i = iValue;
 		pRec->flags |= MEM_Int;
 	} else {
@@ -389,12 +389,10 @@ static u16 SQLITE_NOINLINE computeNumericType(Mem *pMem)
 {
 	assert((pMem->flags & (MEM_Int|MEM_Real))==0);
 	assert((pMem->flags & (MEM_Str|MEM_Blob))!=0);
-	if (sqlite3AtoF(pMem->z, &pMem->u.r, pMem->n)==0) {
+	if (sqlite3AtoF(pMem->z, &pMem->u.r, pMem->n)==0)
 		return 0;
-	}
-	if (sqlite3Atoi64(pMem->z, &pMem->u.i, pMem->n)==SQLITE_OK) {
+	if (sql_atoi64(pMem->z, (int64_t *)&pMem->u.i, pMem->n)==SQLITE_OK)
 		return MEM_Int;
-	}
 	return MEM_Real;
 }
 
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index 2ce9074..5d92a27 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -459,9 +459,9 @@ sqlite3VdbeIntValue(Mem * pMem)
 	} else if (flags & MEM_Real) {
 		return doubleToInt64(pMem->u.r);
 	} else if (flags & (MEM_Str | MEM_Blob)) {
-		i64 value = 0;
+		int64_t value = 0;
 		assert(pMem->z || pMem->n == 0);
-		sqlite3Atoi64(pMem->z, &value, pMem->n);
+		sql_atoi64(pMem->z, &value, pMem->n);
 		return value;
 	} else {
 		return 0;
@@ -562,7 +562,7 @@ sqlite3VdbeMemNumerify(Mem * pMem)
 {
 	if ((pMem->flags & (MEM_Int | MEM_Real | MEM_Null)) == 0) {
 		assert((pMem->flags & (MEM_Blob | MEM_Str)) != 0);
-		if (0 == sqlite3Atoi64(pMem->z, &pMem->u.i, pMem->n)) {
+		if (0 == sql_atoi64(pMem->z, (int64_t *)&pMem->u.i, pMem->n)) {
 			MemSetTypeFlag(pMem, MEM_Int);
 		} else {
 			pMem->u.r = sqlite3VdbeRealValue(pMem);
diff --git a/test/sql-tap/default.test.lua b/test/sql-tap/default.test.lua
index 9d59767..2f08a51 100755
--- a/test/sql-tap/default.test.lua
+++ b/test/sql-tap/default.test.lua
@@ -146,14 +146,13 @@ test:do_execsql_test(
 	d INT DEFAULT -2147483647,
 	e INT DEFAULT -2147483648,
 	f INT DEFAULT (-9223372036854775808),
-	g INT DEFAULT 9223372036854775808,
 	h INT DEFAULT (-(-9223372036854775807))
 	);
 	INSERT INTO t300 DEFAULT VALUES;
-	SELECT a, b, c, d, e, f, cast(g as text), h FROM t300;
+	SELECT a, b, c, d, e, f, h FROM t300;
 	]], {
 	-- <default-3.3>
-	2147483647, 2147483648, 9223372036854775807LL, -2147483647, -2147483648, -9223372036854775808LL, "9.22337203685478e+18", 9223372036854775807LL
+	2147483647, 2147483648, 9223372036854775807LL, -2147483647, -2147483648, -9223372036854775808LL, 9223372036854775807LL
 	-- </default-3.3>
 })
 
diff --git a/test/sql/gh-2347-max-int-literals.result b/test/sql/gh-2347-max-int-literals.result
new file mode 100644
index 0000000..c289a80
--- /dev/null
+++ b/test/sql/gh-2347-max-int-literals.result
@@ -0,0 +1,28 @@
+test_run = require('test_run').new()
+---
+...
+engine = test_run:get_cfg('engine')
+---
+...
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+---
+...
+box.cfg{}
+---
+...
+box.sql.execute("select (9223372036854775807)")
+---
+- - [9223372036854775807]
+...
+box.sql.execute("select (-9223372036854775808)")
+---
+- - [-9223372036854775808]
+...
+box.sql.execute("select (9223372036854775808)")
+---
+- error: 'oversized integer: 9223372036854775808'
+...
+box.sql.execute("select (-9223372036854775809)")
+---
+- error: 'oversized integer: -9223372036854775809'
+...
diff --git a/test/sql/gh-2347-max-int-literals.test.lua b/test/sql/gh-2347-max-int-literals.test.lua
new file mode 100644
index 0000000..4b1ef0d
--- /dev/null
+++ b/test/sql/gh-2347-max-int-literals.test.lua
@@ -0,0 +1,11 @@
+test_run = require('test_run').new()
+engine = test_run:get_cfg('engine')
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+
+box.cfg{}
+
+box.sql.execute("select (9223372036854775807)")
+box.sql.execute("select (-9223372036854775808)")
+
+box.sql.execute("select (9223372036854775808)")
+box.sql.execute("select (-9223372036854775809)")

  reply	other threads:[~2018-07-18  7:57 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-17  6:08 [tarantool-patches] " Kirill Yukhin
2018-07-18  1:10 ` [tarantool-patches] " n.pettik
2018-07-18  7:57   ` Kirill Yukhin [this message]
2018-07-18 17:56     ` n.pettik
2018-07-19  8:53       ` Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180718075714.4dl3hlfuuoqimv5p@tarantool.org \
    --to=kyukhin@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH] sql: do not allow oversized integer literals' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox