[Tarantool-patches] [PATCH luajit] Fix binary number literal parsing.

[Tarantool-patches] [PATCH luajit] Fix binary number literal parsing.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Reported by Egor Skriptunoff.

(cherry-picked from commit 377a8488b62a9f1b589bb68875dd1288aa70e76e)

Binary number with fractional part is invalid. Parsing of such binary
numbers returns `STRSCAN_ERROR` for any numbers with non-zero fractional
part, because it gives non-zero power of the exponent (`ex2`) in
`strscan_bin()`. But binary numbers with a zero fractional part
considered as valid numbers. To avoid such inconsistency the check, that
the given base doesn't equal 2, is added, when parsing decimal point
part of the literal.

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

Part of tarantool/tarantool#8825
---

Branch: https://github.com/tarantool/luajit/tree/skaplun/fix-binary-number-parsing
Issue: https://github.com/tarantool/tarantool/issues/8825
ML: https://www.freelists.org/post/luajit/Fractional-binary-number-literals
Tarantool PR: https://github.com/tarantool/tarantool/pull/9028

Tarantool's CI is red, because static build on aarch64 can't fetch the
LuaJIT's submodule commit somehow. Looks unrelated to the commit:).

 src/lj_strscan.c                                |  1 +
 .../fix-binary-number-parsing.test.lua          | 17 +++++++++++++++++
 2 files changed, 18 insertions(+)
 create mode 100644 test/tarantool-tests/fix-binary-number-parsing.test.lua

diff --git a/src/lj_strscan.c b/src/lj_strscan.c
index 11d341ee..a2d92714 100644
--- a/src/lj_strscan.c
+++ b/src/lj_strscan.c
@@ -444,6 +444,7 @@ StrScanFmt lj_strscan_scan(const uint8_t *p, MSize len, TValue *o,
 
     /* Handle decimal point. */
     if (dp) {
+      if (base == 2) return STRSCAN_ERROR;
       fmt = STRSCAN_NUM;
       if (dig) {
 	ex = (int32_t)(dp-(p-1)); dp = p-1;
diff --git a/test/tarantool-tests/fix-binary-number-parsing.test.lua b/test/tarantool-tests/fix-binary-number-parsing.test.lua
new file mode 100644
index 00000000..df82bd0b
--- /dev/null
+++ b/test/tarantool-tests/fix-binary-number-parsing.test.lua
@@ -0,0 +1,17 @@
+local tap = require('tap')
+
+-- Test file to demonstrate incorrect behaviour of binary number
+-- parsing with fractional dot.
+-- See also:
+-- https://www.freelists.org/post/luajit/Fractional-binary-number-literals
+local test = tap.test('fix-binary-number-parsing')
+test:plan(2)
+
+-- Test that incorrect literal with non-0 fractional part still
+-- can't be converted to number.
+test:is(tonumber('0b.1'), nil, '0b.1 is not converted')
+-- Test that incorrect literal with 0 fractional part can't be
+-- converted to number.
+test:is(tonumber('0b.0'), nil, '0b.0 is not converted')
+
+test:done(true)
-- 
2.41.0

Re: [Tarantool-patches] [PATCH luajit] Fix binary number literal parsing.

[Maxim Kokryashkin via Tarantool-patches]

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


Hi, Sergey!
Thanks for the patch!
LGTM, except for a few comments below.
> 
>>From: Mike Pall <mike>
>>
>>Reported by Egor Skriptunoff.
>>
>>(cherry-picked from commit 377a8488b62a9f1b589bb68875dd1288aa70e76e)
>>
>>Binary number with fractional part is invalid. Parsing of such binary
>Typo: s/Binary/ A binary/
>Typo: s/fractional/a fractional/
>>numbers returns `STRSCAN_ERROR` for any numbers with non-zero fractional
>Typo: s/with/with a/
>>part, because it gives non-zero power of the exponent (`ex2`) in
>Typo: s/part,/part/
>Typo: s/non-zero/a non-zero/
>>`strscan_bin()`. But binary numbers with a zero fractional part
>>considered as valid numbers. To avoid such inconsistency the check, that
>Typo: s/considered as/are considered/
>>the given base doesn't equal 2, is added, when parsing decimal point
>Typo: s/parsing/parsing the/
>>part of the literal.
>>
>>Sergey Kaplun:
>>* added the description and the test for the problem
>>
>>Part of tarantool/tarantool#8825
>>---
>>
>>Branch:  https://github.com/tarantool/luajit/tree/skaplun/fix-binary-number-parsing
>>Issue:  https://github.com/tarantool/tarantool/issues/8825
>>ML:  https://www.freelists.org/post/luajit/Fractional-binary-number-literals
>>Tarantool PR:  https://github.com/tarantool/tarantool/pull/9028
>>
>>Tarantool's CI is red, because static build on aarch64 can't fetch the
>>LuaJIT's submodule commit somehow. Looks unrelated to the commit:).
>>
>> src/lj_strscan.c | 1 +
>> .../fix-binary-number-parsing.test.lua | 17 +++++++++++++++++
>> 2 files changed, 18 insertions(+)
>> create mode 100644 test/tarantool-tests/fix-binary-number-parsing.test.lua
>>
>>diff --git a/src/lj_strscan.c b/src/lj_strscan.c
>>index 11d341ee..a2d92714 100644
>>--- a/src/lj_strscan.c
>>+++ b/src/lj_strscan.c
>>@@ -444,6 +444,7 @@ StrScanFmt lj_strscan_scan(const uint8_t *p, MSize len, TValue *o,
>> 
>>     /* Handle decimal point. */
>>     if (dp) {
>>+ if (base == 2) return STRSCAN_ERROR;
>>       fmt = STRSCAN_NUM;
>>       if (dig) {
>>  ex = (int32_t)(dp-(p-1)); dp = p-1;
>>diff --git a/test/tarantool-tests/fix-binary-number-parsing.test.lua b/test/tarantool-tests/fix-binary-number-parsing.test.lua
>>new file mode 100644
>>index 00000000..df82bd0b
>>--- /dev/null
>>+++ b/test/tarantool-tests/fix-binary-number-parsing.test.lua
>>@@ -0,0 +1,17 @@
>>+local tap = require('tap')
>>+
>>+-- Test file to demonstrate incorrect behaviour of binary number
>>+-- parsing with fractional dot.
>>+-- See also:
>>+--  https://www.freelists.org/post/luajit/Fractional-binary-number-literals
>>+local test = tap.test('fix-binary-number-parsing')
>>+test:plan(2)
>>+
>>+-- Test that incorrect literal with non-0 fractional part still
>Typo: s/that/that an/
>Typo: s/with/with a/
>>+-- can't be converted to number.
>Typo: s/number/a number/
>>+test:is(tonumber('0b.1'), nil, '0b.1 is not converted')
>>+-- Test that incorrect literal with 0 fractional part can't be
>Typo: s/that/that an/
>>+-- converted to number.
>Typo: s/number/a number/
>>+test:is(tonumber('0b.0'), nil, '0b.0 is not converted')
>>+
>>+test:done(true)
>>--
>>2.41.0
>--
>Best regards,
>Maxim Kokryashkin
> 

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

Re: [Tarantool-patches] [PATCH luajit] Fix binary number literal parsing.

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim!
Thanks for the review!
Fixed your comments inline, rebased branch on the current master and
force-pushed it.

On 25.08.23, Maxim Kokryashkin wrote:
> 
> Hi, Sergey!
> Thanks for the patch!
> LGTM, except for a few comments below.
> > 
> >>From: Mike Pall <mike>
> >>
> >>Reported by Egor Skriptunoff.
> >>
> >>(cherry-picked from commit 377a8488b62a9f1b589bb68875dd1288aa70e76e)
> >>
> >>Binary number with fractional part is invalid. Parsing of such binary
> >Typo: s/Binary/ A binary/
> >Typo: s/fractional/a fractional/

Fixed.

> >>numbers returns `STRSCAN_ERROR` for any numbers with non-zero fractional
> >Typo: s/with/with a/

Fixed.

> >>part, because it gives non-zero power of the exponent (`ex2`) in
> >Typo: s/part,/part/

Fixed.

> >Typo: s/non-zero/a non-zero/

Fixed.

> >>`strscan_bin()`. But binary numbers with a zero fractional part
> >>considered as valid numbers. To avoid such inconsistency the check, that
> >Typo: s/considered as/are considered/

Fixed, thanks.

> >>the given base doesn't equal 2, is added, when parsing decimal point
> >Typo: s/parsing/parsing the/

Fixed.

> >>part of the literal.
> >>
> >>Sergey Kaplun:
> >>* added the description and the test for the problem
> >>
> >>Part of tarantool/tarantool#8825
> >>---

<snipped>

> >>+
> >>+-- Test that incorrect literal with non-0 fractional part still
> >Typo: s/that/that an/
> >Typo: s/with/with a/

Fixed.

> >>+-- can't be converted to number.
> >Typo: s/number/a number/

Fixed.

> >>+test:is(tonumber('0b.1'), nil, '0b.1 is not converted')
> >>+-- Test that incorrect literal with 0 fractional part can't be
> >Typo: s/that/that an/

Fixed.

> >>+-- converted to number.
> >Typo: s/number/a number/

Fixed.

See the iterative diff below:

===================================================================
diff --git a/test/tarantool-tests/fix-binary-number-parsing.test.lua b/test/tarantool-tests/fix-binary-number-parsing.test.lua
index df82bd0b..9f149082 100644
--- a/test/tarantool-tests/fix-binary-number-parsing.test.lua
+++ b/test/tarantool-tests/fix-binary-number-parsing.test.lua
@@ -7,11 +7,11 @@ local tap = require('tap')
 local test = tap.test('fix-binary-number-parsing')
 test:plan(2)
 
--- Test that incorrect literal with non-0 fractional part still
--- can't be converted to number.
+-- Test that an incorrect literal with a non-0 fractional part
+-- still can't be converted to a number.
 test:is(tonumber('0b.1'), nil, '0b.1 is not converted')
--- Test that incorrect literal with 0 fractional part can't be
--- converted to number.
+-- Test that an incorrect literal with 0 fractional part can't be
+-- converted to a number.
 test:is(tonumber('0b.0'), nil, '0b.0 is not converted')
 
 test:done(true)
===================================================================

> >>+test:is(tonumber('0b.0'), nil, '0b.0 is not converted')
> >>+
> >>+test:done(true)
> >>--
> >>2.41.0
> >--
> >Best regards,
> >Maxim Kokryashkin
> > 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit] Fix binary number literal parsing.

[Sergey Bronnikov via Tarantool-patches]

Sergey, thanks for the patch! LGTM. Sergey

On 8/24/23 13:02, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Reported by Egor Skriptunoff.
>
> (cherry-picked from commit 377a8488b62a9f1b589bb68875dd1288aa70e76e)
>
> Binary number with fractional part is invalid. Parsing of such binary
> numbers returns `STRSCAN_ERROR` for any numbers with non-zero fractional
> part, because it gives non-zero power of the exponent (`ex2`) in
> `strscan_bin()`. But binary numbers with a zero fractional part
> considered as valid numbers. To avoid such inconsistency the check, that
> the given base doesn't equal 2, is added, when parsing decimal point
> part of the literal.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#8825
> ---
>
> Branch: https://github.com/tarantool/luajit/tree/skaplun/fix-binary-number-parsing
> Issue: https://github.com/tarantool/tarantool/issues/8825
> ML: https://www.freelists.org/post/luajit/Fractional-binary-number-literals
> Tarantool PR: https://github.com/tarantool/tarantool/pull/9028
>
> Tarantool's CI is red, because static build on aarch64 can't fetch the
> LuaJIT's submodule commit somehow. Looks unrelated to the commit:).
>
>   src/lj_strscan.c                                |  1 +
>   .../fix-binary-number-parsing.test.lua          | 17 +++++++++++++++++
>   2 files changed, 18 insertions(+)
>   create mode 100644 test/tarantool-tests/fix-binary-number-parsing.test.lua
>
> diff --git a/src/lj_strscan.c b/src/lj_strscan.c
> index 11d341ee..a2d92714 100644
> --- a/src/lj_strscan.c
> +++ b/src/lj_strscan.c
> @@ -444,6 +444,7 @@ StrScanFmt lj_strscan_scan(const uint8_t *p, MSize len, TValue *o,
>   
>       /* Handle decimal point. */
>       if (dp) {
> +      if (base == 2) return STRSCAN_ERROR;
>         fmt = STRSCAN_NUM;
>         if (dig) {
>   	ex = (int32_t)(dp-(p-1)); dp = p-1;
> diff --git a/test/tarantool-tests/fix-binary-number-parsing.test.lua b/test/tarantool-tests/fix-binary-number-parsing.test.lua
> new file mode 100644
> index 00000000..df82bd0b
> --- /dev/null
> +++ b/test/tarantool-tests/fix-binary-number-parsing.test.lua
> @@ -0,0 +1,17 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate incorrect behaviour of binary number
> +-- parsing with fractional dot.
> +-- See also:
> +-- https://www.freelists.org/post/luajit/Fractional-binary-number-literals
> +local test = tap.test('fix-binary-number-parsing')
> +test:plan(2)
> +
> +-- Test that incorrect literal with non-0 fractional part still
> +-- can't be converted to number.
> +test:is(tonumber('0b.1'), nil, '0b.1 is not converted')
> +-- Test that incorrect literal with 0 fractional part can't be
> +-- converted to number.
> +test:is(tonumber('0b.0'), nil, '0b.0 is not converted')
> +
> +test:done(true)

Re: [Tarantool-patches] [PATCH luajit] Fix binary number literal parsing.

[Igor Munkin via Tarantool-patches]

Sergey,

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

On 24.08.23, Sergey Kaplun via Tarantool-patches wrote:
> From: Mike Pall <mike>
> 
> Reported by Egor Skriptunoff.
> 
> (cherry-picked from commit 377a8488b62a9f1b589bb68875dd1288aa70e76e)
> 
> Binary number with fractional part is invalid. Parsing of such binary
> numbers returns `STRSCAN_ERROR` for any numbers with non-zero fractional
> part, because it gives non-zero power of the exponent (`ex2`) in
> `strscan_bin()`. But binary numbers with a zero fractional part
> considered as valid numbers. To avoid such inconsistency the check, that
> the given base doesn't equal 2, is added, when parsing decimal point
> part of the literal.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#8825
> ---
> 
> Branch: https://github.com/tarantool/luajit/tree/skaplun/fix-binary-number-parsing
> Issue: https://github.com/tarantool/tarantool/issues/8825
> ML: https://www.freelists.org/post/luajit/Fractional-binary-number-literals
> Tarantool PR: https://github.com/tarantool/tarantool/pull/9028
> 
> Tarantool's CI is red, because static build on aarch64 can't fetch the
> LuaJIT's submodule commit somehow. Looks unrelated to the commit:).
> 
>  src/lj_strscan.c                                |  1 +
>  .../fix-binary-number-parsing.test.lua          | 17 +++++++++++++++++
>  2 files changed, 18 insertions(+)
>  create mode 100644 test/tarantool-tests/fix-binary-number-parsing.test.lua
> 

<snipped>

> -- 
> 2.41.0
> 

-- 
Best regards,
IM