Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit] Limit exponent range in number parsing.
@ 2023-11-21  8:52 Sergey Kaplun via Tarantool-patches
  2023-11-21 23:14 ` Maxim Kokryashkin via Tarantool-patches
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2023-11-21  8:52 UTC (permalink / raw)
  To: Maxim Kokryashkin, Sergey Bronnikov; +Cc: tarantool-patches

From: Mike Pall <mike>

Reported by XmiliaH.

(cherry-picked from commit e56048753634c32ea6eeedf74cef6f9cfea5f4ed)

When parsing exponent powers greater than (1 << 16) * 10 == (65536 *
10), the exponent values are cut without handling any values greater.
This patch fixes the behaviour, but restricts the power maximum value by
`STRSCAN_MAXEXP` (1 << 20).

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#9145
---

Branch:
https://github.com/tarantool/luajit/tree/skaplun/lj-788-limit-exponents-range
Tarantool PR: https://github.com/tarantool/tarantool/pull/9386
Related issues:
* https://github.com/LuaJIT/LuaJIT/issues/788
* https://github.com/tarantool/tarantool/issues/9145


 src/lj_strscan.c                              |  5 +++-
 .../lj-788-limit-exponents-range.test.lua     | 29 +++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 test/tarantool-tests/lj-788-limit-exponents-range.test.lua

diff --git a/src/lj_strscan.c b/src/lj_strscan.c
index ae8945e1..129010fd 100644
--- a/src/lj_strscan.c
+++ b/src/lj_strscan.c
@@ -63,6 +63,7 @@
 #define STRSCAN_MAXDIG	800		/* 772 + extra are sufficient. */
 #define STRSCAN_DDIG	(STRSCAN_DIG/2)
 #define STRSCAN_DMASK	(STRSCAN_DDIG-1)
+#define STRSCAN_MAXEXP	(1 << 20)
 
 /* Helpers for circular buffer. */
 #define DNEXT(a)	(((a)+1) & STRSCAN_DMASK)
@@ -449,6 +450,7 @@ StrScanFmt lj_strscan_scan(const uint8_t *p, MSize len, TValue *o,
       if (dig) {
 	ex = (int32_t)(dp-(p-1)); dp = p-1;
 	while (ex < 0 && *dp-- == '0') ex++, dig--;  /* Skip trailing zeros. */
+	if (ex <= -STRSCAN_MAXEXP) return STRSCAN_ERROR;
 	if (base == 16) ex *= 4;
       }
     }
@@ -462,7 +464,8 @@ StrScanFmt lj_strscan_scan(const uint8_t *p, MSize len, TValue *o,
       if (!lj_char_isdigit(*p)) return STRSCAN_ERROR;
       xx = (*p++ & 15);
       while (lj_char_isdigit(*p)) {
-	if (xx < 65536) xx = xx * 10 + (*p & 15);
+	xx = xx * 10 + (*p & 15);
+	if (xx >= STRSCAN_MAXEXP) return STRSCAN_ERROR;
 	p++;
       }
       ex += negx ? -(int32_t)xx : (int32_t)xx;
diff --git a/test/tarantool-tests/lj-788-limit-exponents-range.test.lua b/test/tarantool-tests/lj-788-limit-exponents-range.test.lua
new file mode 100644
index 00000000..8ab31600
--- /dev/null
+++ b/test/tarantool-tests/lj-788-limit-exponents-range.test.lua
@@ -0,0 +1,29 @@
+local tap = require('tap')
+
+-- Test file to demonstrate incorrect behaviour of exponent number
+-- form parsing.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/788.
+local test = tap.test('lj-788-limit-exponents-range')
+test:plan(2)
+
+-- Before the patch, the powers greater than (1 << 16) * 10
+-- (655360) were parsed incorrectly. After the patch, powers
+-- greater than 1 << 20 (1048576 `STRSCAN_MAXEXP`) are considered
+-- invalid. See <src/lj_strscan.c> for details.
+-- Choose the first value between these values and the second
+-- value bigger than `STRSCAN_MAXEXP` to check parsing correctness
+-- for the first one, and `STRSCAN_ERROR` for the second case.
+local PARSABLE_EXP_POWER  = 1000000
+local TOO_LARGE_EXP_POWER = 1050000
+
+local function form_exp_string(n)
+  return '0.' .. string.rep('0', n - 1) .. '1e' .. tostring(n)
+end
+
+test:is(tonumber(form_exp_string(PARSABLE_EXP_POWER)), 1,
+        'correct parsing of large exponent')
+
+test:is(tonumber(form_exp_string(TOO_LARGE_EXP_POWER)), nil,
+        'too big exponent power is not parsed')
+
+test:done(true)
-- 
2.42.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Tarantool-patches]  [PATCH luajit] Limit exponent range in number parsing.
  2023-11-21  8:52 [Tarantool-patches] [PATCH luajit] Limit exponent range in number parsing Sergey Kaplun via Tarantool-patches
@ 2023-11-21 23:14 ` Maxim Kokryashkin via Tarantool-patches
  2024-01-23 21:14 ` Sergey Kaplun via Tarantool-patches
  2024-02-15 13:50 ` Igor Munkin via Tarantool-patches
  2 siblings, 0 replies; 7+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2023-11-21 23:14 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

[-- Attachment #1: Type: text/plain, Size: 3781 bytes --]


Hi, Sergey!
Thanks for the patch!
LGTM
 
--
Best regards,
Maxim Kokryashkin
 
  
>Вторник, 21 ноября 2023, 11:57 +03:00 от Sergey Kaplun <skaplun@tarantool.org>:
> 
>From: Mike Pall <mike>
>
>Reported by XmiliaH.
>
>(cherry-picked from commit e56048753634c32ea6eeedf74cef6f9cfea5f4ed)
>
>When parsing exponent powers greater than (1 << 16) * 10 == (65536 *
>10), the exponent values are cut without handling any values greater.
>This patch fixes the behaviour, but restricts the power maximum value by
>`STRSCAN_MAXEXP` (1 << 20).
>
>Sergey Kaplun:
>* added the description and the test for the problem
>
>Part of tarantool/tarantool#9145
>---
>
>Branch:
>https://github.com/tarantool/luajit/tree/skaplun/lj-788-limit-exponents-range
>Tarantool PR:  https://github.com/tarantool/tarantool/pull/9386
>Related issues:
>*  https://github.com/LuaJIT/LuaJIT/issues/788
>*  https://github.com/tarantool/tarantool/issues/9145
>
>
> src/lj_strscan.c | 5 +++-
> .../lj-788-limit-exponents-range.test.lua | 29 +++++++++++++++++++
> 2 files changed, 33 insertions(+), 1 deletion(-)
> create mode 100644 test/tarantool-tests/lj-788-limit-exponents-range.test.lua
>
>diff --git a/src/lj_strscan.c b/src/lj_strscan.c
>index ae8945e1..129010fd 100644
>--- a/src/lj_strscan.c
>+++ b/src/lj_strscan.c
>@@ -63,6 +63,7 @@
> #define STRSCAN_MAXDIG 800 /* 772 + extra are sufficient. */
> #define STRSCAN_DDIG (STRSCAN_DIG/2)
> #define STRSCAN_DMASK (STRSCAN_DDIG-1)
>+#define STRSCAN_MAXEXP (1 << 20)
> 
> /* Helpers for circular buffer. */
> #define DNEXT(a) (((a)+1) & STRSCAN_DMASK)
>@@ -449,6 +450,7 @@ StrScanFmt lj_strscan_scan(const uint8_t *p, MSize len, TValue *o,
>       if (dig) {
>  ex = (int32_t)(dp-(p-1)); dp = p-1;
>  while (ex < 0 && *dp-- == '0') ex++, dig--; /* Skip trailing zeros. */
>+ if (ex <= -STRSCAN_MAXEXP) return STRSCAN_ERROR;
>  if (base == 16) ex *= 4;
>       }
>     }
>@@ -462,7 +464,8 @@ StrScanFmt lj_strscan_scan(const uint8_t *p, MSize len, TValue *o,
>       if (!lj_char_isdigit(*p)) return STRSCAN_ERROR;
>       xx = (*p++ & 15);
>       while (lj_char_isdigit(*p)) {
>- if (xx < 65536) xx = xx * 10 + (*p & 15);
>+ xx = xx * 10 + (*p & 15);
>+ if (xx >= STRSCAN_MAXEXP) return STRSCAN_ERROR;
>  p++;
>       }
>       ex += negx ? -(int32_t)xx : (int32_t)xx;
>diff --git a/test/tarantool-tests/lj-788-limit-exponents-range.test.lua b/test/tarantool-tests/lj-788-limit-exponents-range.test.lua
>new file mode 100644
>index 00000000..8ab31600
>--- /dev/null
>+++ b/test/tarantool-tests/lj-788-limit-exponents-range.test.lua
>@@ -0,0 +1,29 @@
>+local tap = require('tap')
>+
>+-- Test file to demonstrate incorrect behaviour of exponent number
>+-- form parsing.
>+-- See also:  https://github.com/LuaJIT/LuaJIT/issues/788 .
>+local test = tap.test('lj-788-limit-exponents-range')
>+test:plan(2)
>+
>+-- Before the patch, the powers greater than (1 << 16) * 10
>+-- (655360) were parsed incorrectly. After the patch, powers
>+-- greater than 1 << 20 (1048576 `STRSCAN_MAXEXP`) are considered
>+-- invalid. See <src/lj_strscan.c> for details.
>+-- Choose the first value between these values and the second
>+-- value bigger than `STRSCAN_MAXEXP` to check parsing correctness
>+-- for the first one, and `STRSCAN_ERROR` for the second case.
>+local PARSABLE_EXP_POWER = 1000000
>+local TOO_LARGE_EXP_POWER = 1050000
>+
>+local function form_exp_string(n)
>+ return '0.' .. string.rep('0', n - 1) .. '1e' .. tostring(n)
>+end
>+
>+test:is(tonumber(form_exp_string(PARSABLE_EXP_POWER)), 1,
>+ 'correct parsing of large exponent')
>+
>+test:is(tonumber(form_exp_string(TOO_LARGE_EXP_POWER)), nil,
>+ 'too big exponent power is not parsed')
>+
>+test:done(true)
>--
>2.42.1
 

[-- Attachment #2: Type: text/html, Size: 5183 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Limit exponent range in number parsing.
  2023-11-21  8:52 [Tarantool-patches] [PATCH luajit] Limit exponent range in number parsing Sergey Kaplun via Tarantool-patches
  2023-11-21 23:14 ` Maxim Kokryashkin via Tarantool-patches
@ 2024-01-23 21:14 ` Sergey Kaplun via Tarantool-patches
  2024-01-25  7:47   ` Sergey Bronnikov via Tarantool-patches
  2024-02-15 13:50 ` Igor Munkin via Tarantool-patches
  2 siblings, 1 reply; 7+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-01-23 21:14 UTC (permalink / raw)
  To: Maxim Kokryashkin, Sergey Bronnikov; +Cc: tarantool-patches


From: Mike Pall <mike>

Reported by XmiliaH.

(cherry-picked from commit e56048753634c32ea6eeedf74cef6f9cfea5f4ed)

When parsing exponent powers greater than (1 << 16) * 10 == (65536 *
10), the exponent values are cut without handling any values greater.
This patch fixes the behaviour, but restricts the power maximum value by
`STRSCAN_MAXEXP` (1 << 20).

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#9145
---

Branch:
https://github.com/tarantool/luajit/tree/skaplun/lj-788-limit-exponents-range
Tarantool PR: https://github.com/tarantool/tarantool/pull/9386
Related issues:
* https://github.com/LuaJIT/LuaJIT/issues/788
* https://github.com/tarantool/tarantool/issues/9145


 src/lj_strscan.c                              |  5 +++-
 .../lj-788-limit-exponents-range.test.lua     | 29 +++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 test/tarantool-tests/lj-788-limit-exponents-range.test.lua

diff --git a/src/lj_strscan.c b/src/lj_strscan.c
index ae8945e1..129010fd 100644
--- a/src/lj_strscan.c
+++ b/src/lj_strscan.c
@@ -63,6 +63,7 @@
 #define STRSCAN_MAXDIG	800		/* 772 + extra are sufficient. */
 #define STRSCAN_DDIG	(STRSCAN_DIG/2)
 #define STRSCAN_DMASK	(STRSCAN_DDIG-1)
+#define STRSCAN_MAXEXP	(1 << 20)
 
 /* Helpers for circular buffer. */
 #define DNEXT(a)	(((a)+1) & STRSCAN_DMASK)
@@ -449,6 +450,7 @@ StrScanFmt lj_strscan_scan(const uint8_t *p, MSize len, TValue *o,
       if (dig) {
 	ex = (int32_t)(dp-(p-1)); dp = p-1;
 	while (ex < 0 && *dp-- == '0') ex++, dig--;  /* Skip trailing zeros. */
+	if (ex <= -STRSCAN_MAXEXP) return STRSCAN_ERROR;
 	if (base == 16) ex *= 4;
       }
     }
@@ -462,7 +464,8 @@ StrScanFmt lj_strscan_scan(const uint8_t *p, MSize len, TValue *o,
       if (!lj_char_isdigit(*p)) return STRSCAN_ERROR;
       xx = (*p++ & 15);
       while (lj_char_isdigit(*p)) {
-	if (xx < 65536) xx = xx * 10 + (*p & 15);
+	xx = xx * 10 + (*p & 15);
+	if (xx >= STRSCAN_MAXEXP) return STRSCAN_ERROR;
 	p++;
       }
       ex += negx ? -(int32_t)xx : (int32_t)xx;
diff --git a/test/tarantool-tests/lj-788-limit-exponents-range.test.lua b/test/tarantool-tests/lj-788-limit-exponents-range.test.lua
new file mode 100644
index 00000000..8ab31600
--- /dev/null
+++ b/test/tarantool-tests/lj-788-limit-exponents-range.test.lua
@@ -0,0 +1,29 @@
+local tap = require('tap')
+
+-- Test file to demonstrate incorrect behaviour of exponent number
+-- form parsing.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/788.
+local test = tap.test('lj-788-limit-exponents-range')
+test:plan(2)
+
+-- Before the patch, the powers greater than (1 << 16) * 10
+-- (655360) were parsed incorrectly. After the patch, powers
+-- greater than 1 << 20 (1048576 `STRSCAN_MAXEXP`) are considered
+-- invalid. See <src/lj_strscan.c> for details.
+-- Choose the first value between these values and the second
+-- value bigger than `STRSCAN_MAXEXP` to check parsing correctness
+-- for the first one, and `STRSCAN_ERROR` for the second case.
+local PARSABLE_EXP_POWER  = 1000000
+local TOO_LARGE_EXP_POWER = 1050000
+
+local function form_exp_string(n)
+  return '0.' .. string.rep('0', n - 1) .. '1e' .. tostring(n)
+end
+
+test:is(tonumber(form_exp_string(PARSABLE_EXP_POWER)), 1,
+        'correct parsing of large exponent')
+
+test:is(tonumber(form_exp_string(TOO_LARGE_EXP_POWER)), nil,
+        'too big exponent power is not parsed')
+
+test:done(true)
-- 
2.42.1


-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Limit exponent range in number parsing.
  2024-01-23 21:14 ` Sergey Kaplun via Tarantool-patches
@ 2024-01-25  7:47   ` Sergey Bronnikov via Tarantool-patches
  2024-01-25 10:06     ` Sergey Kaplun via Tarantool-patches
  0 siblings, 1 reply; 7+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-01-25  7:47 UTC (permalink / raw)
  To: Sergey Kaplun, Maxim Kokryashkin; +Cc: tarantool-patches

Hi, Sergey!

thanks for the patch!


On 1/24/24 00:14, Sergey Kaplun wrote:


<snipped>

> +++ b/test/tarantool-tests/lj-788-limit-exponents-range.test.lua
> @@ -0,0 +1,29 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate incorrect behaviour of exponent number
> +-- form parsing.
> +-- See also: https://github.com/LuaJIT/LuaJIT/issues/788.
> +local test = tap.test('lj-788-limit-exponents-range')
> +test:plan(2)
> +
> +-- Before the patch, the powers greater than (1 << 16) * 10
> +-- (655360) were parsed incorrectly. After the patch, powers
> +-- greater than 1 << 20 (1048576 `STRSCAN_MAXEXP`) are considered
> +-- invalid. See <src/lj_strscan.c> for details.
> +-- Choose the first value between these values and the second
> +-- value bigger than `STRSCAN_MAXEXP` to check parsing correctness

typical values on testing boundaries [1] are: value before the boundary,

boundary value and value after the boundary. So I propose to test these 
three values.

1: https://en.wikipedia.org/wiki/Boundary-value_analysis

> +-- for the first one, and `STRSCAN_ERROR` for the second case.
> +local PARSABLE_EXP_POWER  = 1000000
> +local TOO_LARGE_EXP_POWER = 1050000
> +
> +local function form_exp_string(n)
> +  return '0.' .. string.rep('0', n - 1) .. '1e' .. tostring(n)
> +end
> +
> +test:is(tonumber(form_exp_string(PARSABLE_EXP_POWER)), 1,
> +        'correct parsing of large exponent')
> +
> +test:is(tonumber(form_exp_string(TOO_LARGE_EXP_POWER)), nil,
> +        'too big exponent power is not parsed')
> +
> +test:done(true)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Limit exponent range in number parsing.
  2024-01-25  7:47   ` Sergey Bronnikov via Tarantool-patches
@ 2024-01-25 10:06     ` Sergey Kaplun via Tarantool-patches
  2024-01-25 16:31       ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 1 reply; 7+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-01-25 10:06 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

Hi, Sergey!
Thanks for the review!

Fixed you comment, see the patch below.
Branch is force-pushed.

On 25.01.24, Sergey Bronnikov wrote:
> Hi, Sergey!
> 
> thanks for the patch!
> 
> 
> On 1/24/24 00:14, Sergey Kaplun wrote:
> 
> 
> <snipped>
> 
> > +++ b/test/tarantool-tests/lj-788-limit-exponents-range.test.lua
> > @@ -0,0 +1,29 @@
> > +local tap = require('tap')
> > +
> > +-- Test file to demonstrate incorrect behaviour of exponent number
> > +-- form parsing.
> > +-- See also: https://github.com/LuaJIT/LuaJIT/issues/788.
> > +local test = tap.test('lj-788-limit-exponents-range')
> > +test:plan(2)
> > +
> > +-- Before the patch, the powers greater than (1 << 16) * 10
> > +-- (655360) were parsed incorrectly. After the patch, powers
> > +-- greater than 1 << 20 (1048576 `STRSCAN_MAXEXP`) are considered
> > +-- invalid. See <src/lj_strscan.c> for details.
> > +-- Choose the first value between these values and the second
> > +-- value bigger than `STRSCAN_MAXEXP` to check parsing correctness
> 
> typical values on testing boundaries [1] are: value before the boundary,
> 
> boundary value and value after the boundary. So I propose to test these 
> three values.
> 
> 1: https://en.wikipedia.org/wiki/Boundary-value_analysis

Thanks! Good to know:).

I added the boundary test case and updated tests names as you suggested.

===================================================================
diff --git a/test/tarantool-tests/lj-788-limit-exponents-range.test.lua b/test/tarantool-tests/lj-788-limit-exponents-range.test.lua
index 8ab31600..0af584fd 100644
--- a/test/tarantool-tests/lj-788-limit-exponents-range.test.lua
+++ b/test/tarantool-tests/lj-788-limit-exponents-range.test.lua
@@ -4,7 +4,7 @@ local tap = require('tap')
 -- form parsing.
 -- See also: https://github.com/LuaJIT/LuaJIT/issues/788.
 local test = tap.test('lj-788-limit-exponents-range')
-test:plan(2)
+test:plan(3)
 
 -- Before the patch, the powers greater than (1 << 16) * 10
 -- (655360) were parsed incorrectly. After the patch, powers
@@ -14,6 +14,7 @@ test:plan(2)
 -- value bigger than `STRSCAN_MAXEXP` to check parsing correctness
 -- for the first one, and `STRSCAN_ERROR` for the second case.
 local PARSABLE_EXP_POWER  = 1000000
+local STRSCAN_MAXEXP      = 1048576
 local TOO_LARGE_EXP_POWER = 1050000
 
 local function form_exp_string(n)
@@ -21,9 +22,12 @@ local function form_exp_string(n)
 end
 
 test:is(tonumber(form_exp_string(PARSABLE_EXP_POWER)), 1,
-        'correct parsing of large exponent')
+        'correct parsing of large exponent before the boundary')
+
+test:is(tonumber(form_exp_string(STRSCAN_MAXEXP)), nil,
+        'boundary power of exponent is not parsed')
 
 test:is(tonumber(form_exp_string(TOO_LARGE_EXP_POWER)), nil,
-        'too big exponent power is not parsed')
+        'too big exponent power after the boundary is not parsed')
 
 test:done(true)
===================================================================

> 
> > +-- for the first one, and `STRSCAN_ERROR` for the second case.
> > +local PARSABLE_EXP_POWER  = 1000000
> > +local TOO_LARGE_EXP_POWER = 1050000
> > +
> > +local function form_exp_string(n)
> > +  return '0.' .. string.rep('0', n - 1) .. '1e' .. tostring(n)
> > +end
> > +
> > +test:is(tonumber(form_exp_string(PARSABLE_EXP_POWER)), 1,
> > +        'correct parsing of large exponent')
> > +
> > +test:is(tonumber(form_exp_string(TOO_LARGE_EXP_POWER)), nil,
> > +        'too big exponent power is not parsed')
> > +
> > +test:done(true)

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Limit exponent range in number parsing.
  2024-01-25 10:06     ` Sergey Kaplun via Tarantool-patches
@ 2024-01-25 16:31       ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 0 replies; 7+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-01-25 16:31 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Thanks! LGTM now

On 1/25/24 13:06, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
>
> Fixed you comment, see the patch below.
> Branch is force-pushed.
>
> On 25.01.24, Sergey Bronnikov wrote:
>> Hi, Sergey!
>>
>> thanks for the patch!
>>
>>
>> On 1/24/24 00:14, Sergey Kaplun wrote:
>>
>>
>> <snipped>
>>
>>> +++ b/test/tarantool-tests/lj-788-limit-exponents-range.test.lua
>>> @@ -0,0 +1,29 @@
>>> +local tap = require('tap')
>>> +
>>> +-- Test file to demonstrate incorrect behaviour of exponent number
>>> +-- form parsing.
>>> +-- See also: https://github.com/LuaJIT/LuaJIT/issues/788.
>>> +local test = tap.test('lj-788-limit-exponents-range')
>>> +test:plan(2)
>>> +
>>> +-- Before the patch, the powers greater than (1 << 16) * 10
>>> +-- (655360) were parsed incorrectly. After the patch, powers
>>> +-- greater than 1 << 20 (1048576 `STRSCAN_MAXEXP`) are considered
>>> +-- invalid. See <src/lj_strscan.c> for details.
>>> +-- Choose the first value between these values and the second
>>> +-- value bigger than `STRSCAN_MAXEXP` to check parsing correctness
>> typical values on testing boundaries [1] are: value before the boundary,
>>
>> boundary value and value after the boundary. So I propose to test these
>> three values.
>>
>> 1: https://en.wikipedia.org/wiki/Boundary-value_analysis
> Thanks! Good to know:).
>
> I added the boundary test case and updated tests names as you suggested.
>
> ===================================================================
> diff --git a/test/tarantool-tests/lj-788-limit-exponents-range.test.lua b/test/tarantool-tests/lj-788-limit-exponents-range.test.lua
> index 8ab31600..0af584fd 100644
> --- a/test/tarantool-tests/lj-788-limit-exponents-range.test.lua
> +++ b/test/tarantool-tests/lj-788-limit-exponents-range.test.lua
> @@ -4,7 +4,7 @@ local tap = require('tap')
>   -- form parsing.
>   -- See also: https://github.com/LuaJIT/LuaJIT/issues/788.
>   local test = tap.test('lj-788-limit-exponents-range')
> -test:plan(2)
> +test:plan(3)
>   
>   -- Before the patch, the powers greater than (1 << 16) * 10
>   -- (655360) were parsed incorrectly. After the patch, powers
> @@ -14,6 +14,7 @@ test:plan(2)
>   -- value bigger than `STRSCAN_MAXEXP` to check parsing correctness
>   -- for the first one, and `STRSCAN_ERROR` for the second case.
>   local PARSABLE_EXP_POWER  = 1000000
> +local STRSCAN_MAXEXP      = 1048576
>   local TOO_LARGE_EXP_POWER = 1050000
>   
>   local function form_exp_string(n)
> @@ -21,9 +22,12 @@ local function form_exp_string(n)
>   end
>   
>   test:is(tonumber(form_exp_string(PARSABLE_EXP_POWER)), 1,
> -        'correct parsing of large exponent')
> +        'correct parsing of large exponent before the boundary')
> +
> +test:is(tonumber(form_exp_string(STRSCAN_MAXEXP)), nil,
> +        'boundary power of exponent is not parsed')
>   
>   test:is(tonumber(form_exp_string(TOO_LARGE_EXP_POWER)), nil,
> -        'too big exponent power is not parsed')
> +        'too big exponent power after the boundary is not parsed')
>   
>   test:done(true)
> ===================================================================
>
>>> +-- for the first one, and `STRSCAN_ERROR` for the second case.
>>> +local PARSABLE_EXP_POWER  = 1000000
>>> +local TOO_LARGE_EXP_POWER = 1050000
>>> +
>>> +local function form_exp_string(n)
>>> +  return '0.' .. string.rep('0', n - 1) .. '1e' .. tostring(n)
>>> +end
>>> +
>>> +test:is(tonumber(form_exp_string(PARSABLE_EXP_POWER)), 1,
>>> +        'correct parsing of large exponent')
>>> +
>>> +test:is(tonumber(form_exp_string(TOO_LARGE_EXP_POWER)), nil,
>>> +        'too big exponent power is not parsed')
>>> +
>>> +test:done(true)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Limit exponent range in number parsing.
  2023-11-21  8:52 [Tarantool-patches] [PATCH luajit] Limit exponent range in number parsing Sergey Kaplun via Tarantool-patches
  2023-11-21 23:14 ` Maxim Kokryashkin via Tarantool-patches
  2024-01-23 21:14 ` Sergey Kaplun via Tarantool-patches
@ 2024-02-15 13:50 ` Igor Munkin via Tarantool-patches
  2 siblings, 0 replies; 7+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2024-02-15 13:50 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Sergey,

I've checked the patchset into all long-term branches in
tarantool/luajit and bumped a new version in master, release/3.0 and
release/2.11.

On 21.11.23, Sergey Kaplun via Tarantool-patches wrote:
> From: Mike Pall <mike>
> 
> Reported by XmiliaH.
> 
> (cherry-picked from commit e56048753634c32ea6eeedf74cef6f9cfea5f4ed)
> 
> When parsing exponent powers greater than (1 << 16) * 10 == (65536 *
> 10), the exponent values are cut without handling any values greater.
> This patch fixes the behaviour, but restricts the power maximum value by
> `STRSCAN_MAXEXP` (1 << 20).
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#9145
> ---
> 
> Branch:
> https://github.com/tarantool/luajit/tree/skaplun/lj-788-limit-exponents-range
> Tarantool PR: https://github.com/tarantool/tarantool/pull/9386
> Related issues:
> * https://github.com/LuaJIT/LuaJIT/issues/788
> * https://github.com/tarantool/tarantool/issues/9145
> 
> 
>  src/lj_strscan.c                              |  5 +++-
>  .../lj-788-limit-exponents-range.test.lua     | 29 +++++++++++++++++++
>  2 files changed, 33 insertions(+), 1 deletion(-)
>  create mode 100644 test/tarantool-tests/lj-788-limit-exponents-range.test.lua
> 

<snipped>

> -- 
> 2.42.1
> 

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-02-15 14:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-21  8:52 [Tarantool-patches] [PATCH luajit] Limit exponent range in number parsing Sergey Kaplun via Tarantool-patches
2023-11-21 23:14 ` Maxim Kokryashkin via Tarantool-patches
2024-01-23 21:14 ` Sergey Kaplun via Tarantool-patches
2024-01-25  7:47   ` Sergey Bronnikov via Tarantool-patches
2024-01-25 10:06     ` Sergey Kaplun via Tarantool-patches
2024-01-25 16:31       ` Sergey Bronnikov via Tarantool-patches
2024-02-15 13:50 ` Igor Munkin via Tarantool-patches

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