Re: [Tarantool-patches] [PATCH] lua: handle uri.format empty input properly

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

Hi! Thanks for the patch!

On 15/02/2020 10:31, olegrok@tarantool.org wrote:
> From: Oleg Babin <babinoleg@mail.ru>
> 
> After 7fd6c8096343ca254fec10c2ce8a8233ebe47028
> (buffer: port static allocator to Lua) uri started to use
> static_allocator - cyclic buffer that also is used in
> several modules.

Yeah, my fault, sorry.

> However situation then uri.format output is zero-length

then -> when.

> string was not handled properly and ffi.string could
> return data that was previously written in static buffer
> because use as string terminator the first zero byte.
> 
> To prevent such situation let's pass result length explicitly.
> 
> Closes #4779
> ---
> Issue: https://github.com/tarantool/tarantool/issues/4779
> Branch: https://github.com/tarantool/tarantool/tree/olegrok/4779-uri-format-junk-output
> diff --git a/test/app-tap/uri.test.lua b/test/app-tap/uri.test.lua
> index 5405b0873..27efe6bc6 100755
> --- a/test/app-tap/uri.test.lua
> +++ b/test/app-tap/uri.test.lua
> @@ -64,8 +66,22 @@ local function test_format(test)
>      test:is(uri.format(u, true), "user:password@localhost", "password kept")
>  end
>  
> +local function test_static_alloc(test)
> +    -- gh-4779 uri.format returns junk output

Lets add a dot to the end of the sentence, and elaborate
a little about why do we call static_alloc() before this
test and fill it with something. You and me are in the context,
but for someone, who doesn't know the issue and looks at this
test it may look strange.

Otherwise the patch is good.

> +    local buffer_size = 12288
> +    local buf = static_alloc('char', buffer_size)
> +    ffi.fill(buf, 512, string.byte('x'))
> +
> +    local iterations = 10
> +    test:plan(iterations)
> +    for _ = 1, iterations do
> +        test:is(uri.format({}), '')
> +    end
> +end
> +
>  tap.test("uri", function(test)
> -    test:plan(2)
> +    test:plan(3)
>      test:test("parse", test_parse)
>      test:test("format", test_format)
> +    test:test("static_alloc", test_static_alloc)
>  end)
>