[Tarantool-patches] [PATCH] exports: allow to use json tools via FFI

[Tarantool-patches] [PATCH] exports: allow to use json tools via FFI

[olegrok]

From: Oleg Babin <babinoleg@mail.ru>

This patch exports some json functions to be used via FFI. We solve
following problem: currently we don't have any tools to inspect
jsonpaths. E.g. user wants to ban or restrict an access to some
tuple fields, it could be some system fields. Tarantool doesn't
have hidden fields and to solve such problem we should fairly
parse input jsonpaths. Before this patch user should write its own
or some external tools. This patch allows to use functions from
built-in json-lexer directly from Tarantool via FFI.

Part of #5203
---
Issue: https://github.com/tarantool/tarantool/issues/5203
Branch: https://github.com/tarantool/tarantool/tree/olegrok/5203-expose-json-helpers
Also issue contains an example of usage this feature.

@Changelog:
  - Some symbols from tarantool json moudule are exported now
    (gh-5203).

 src/exports.h                              |  4 ++
 test/box-tap/gh-5203-json-exports.test.lua | 82 ++++++++++++++++++++++
 2 files changed, 86 insertions(+)
 create mode 100755 test/box-tap/gh-5203-json-exports.test.lua

diff --git a/src/exports.h b/src/exports.h
index 7cf283e5b..83c90a8bc 100644
--- a/src/exports.h
+++ b/src/exports.h
@@ -152,6 +152,10 @@ EXPORT(ibuf_create)
 EXPORT(ibuf_destroy)
 EXPORT(ibuf_reinit)
 EXPORT(ibuf_reserve_slow)
+EXPORT(json_lexer_next_token)
+EXPORT(json_path_cmp)
+EXPORT(json_path_validate)
+EXPORT(json_path_multikey_offset)
 EXPORT(lbox_socket_local_resolve)
 EXPORT(lbox_socket_nonblock)
 EXPORT(log_format)
diff --git a/test/box-tap/gh-5203-json-exports.test.lua b/test/box-tap/gh-5203-json-exports.test.lua
new file mode 100755
index 000000000..1b8eb9afa
--- /dev/null
+++ b/test/box-tap/gh-5203-json-exports.test.lua
@@ -0,0 +1,82 @@
+#!/usr/bin/env tarantool
+
+local tap = require('tap')
+local ffi = require('ffi')
+ffi.cdef([[
+    void *dlsym(void *handle, const char *symbol);
+    /**
+     * Lexer for JSON paths:
+     * <field>, <.field>, <[123]>, <['field']> and their combinations.
+     */
+    struct json_lexer {
+        /** Source string. */
+        const char *src;
+        /** Length of string. */
+        int src_len;
+        /** Current lexer's offset in bytes. */
+        int offset;
+        /** Current lexer's offset in symbols. */
+        int symbol_count;
+        /**
+         * Base field offset for emitted JSON_TOKEN_NUM tokens,
+         * e.g. 0 for C and 1 for Lua.
+         */
+        int index_base;
+    };
+
+    enum json_token_type {
+        JSON_TOKEN_NUM,
+        JSON_TOKEN_STR,
+        JSON_TOKEN_ANY,
+        /** Lexer reached end of path. */
+        JSON_TOKEN_END,
+    };
+
+    int
+    json_lexer_next_token(struct json_lexer *lexer, struct json_token *token);
+
+    int
+    json_path_cmp(const char *a, int a_len, const char *b, int b_len,
+            int index_base);
+
+    int
+    json_path_validate(const char *path, int path_len, int index_base);
+
+    int
+    json_path_multikey_offset(const char *path, int path_len, int index_base);
+]])
+
+local test = tap.test('json-features')
+test:plan(1)
+
+local RTLD_DEFAULT
+-- See `man 3 dlsym`:
+-- RTLD_DEFAULT
+--   Find  the  first occurrence of the desired symbol using the default
+--   shared object search order.  The search will include global symbols
+--   in the executable and its dependencies, as well as symbols in shared
+--   objects that were dynamically loaded with the RTLD_GLOBAL flag.
+if jit.os == "OSX" then
+    RTLD_DEFAULT = ffi.cast("void *", -2LL)
+else
+    RTLD_DEFAULT = ffi.cast("void *", 0LL)
+end
+
+local json_symbols = {
+    'json_lexer_next_token',
+    'json_path_cmp',
+    'json_path_validate',
+    'json_path_multikey_offset',
+}
+
+test:test('json_symbols', function(t)
+    t:plan(#json_symbols)
+    for _, sym in ipairs(json_symbols) do
+        t:ok(
+            ffi.C.dlsym(RTLD_DEFAULT, sym) ~= nil,
+            ('Symbol %q found'):format(sym)
+        )
+    end
+end)
+
+os.exit(test:check() and 0 or 1)
-- 
2.23.0

Re: [Tarantool-patches] [PATCH] exports: allow to use json tools via FFI

[Timur Safin]

LGTM as fairly trivial patch

Thanks,
Timur

: From: olegrok@tarantool.org <olegrok@tarantool.org>
: Subject: [PATCH] exports: allow to use json tools via FFI
: 
: From: Oleg Babin <babinoleg@mail.ru>
: 
: This patch exports some json functions to be used via FFI. We solve
: following problem: currently we don't have any tools to inspect
: jsonpaths. E.g. user wants to ban or restrict an access to some
: tuple fields, it could be some system fields. Tarantool doesn't
: have hidden fields and to solve such problem we should fairly
: parse input jsonpaths. Before this patch user should write its own
: or some external tools. This patch allows to use functions from
: built-in json-lexer directly from Tarantool via FFI.
: 
: Part of #5203
: ---
: Issue: https://github.com/tarantool/tarantool/issues/5203
: Branch: https://github.com/tarantool/tarantool/tree/olegrok/5203-expose-
: json-helpers
: Also issue contains an example of usage this feature.
: 
: @Changelog:
:   - Some symbols from tarantool json moudule are exported now
:     (gh-5203).
: 
:  src/exports.h                              |  4 ++
:  test/box-tap/gh-5203-json-exports.test.lua | 82 ++++++++++++++++++++++
:  2 files changed, 86 insertions(+)
:  create mode 100755 test/box-tap/gh-5203-json-exports.test.lua
: 
: diff --git a/src/exports.h b/src/exports.h
: index 7cf283e5b..83c90a8bc 100644
: --- a/src/exports.h
: +++ b/src/exports.h
: @@ -152,6 +152,10 @@ EXPORT(ibuf_create)
:  EXPORT(ibuf_destroy)
:  EXPORT(ibuf_reinit)
:  EXPORT(ibuf_reserve_slow)
: +EXPORT(json_lexer_next_token)
: +EXPORT(json_path_cmp)
: +EXPORT(json_path_validate)
: +EXPORT(json_path_multikey_offset)
:  EXPORT(lbox_socket_local_resolve)
:  EXPORT(lbox_socket_nonblock)
:  EXPORT(log_format)
: diff --git a/test/box-tap/gh-5203-json-exports.test.lua b/test/box-tap/gh-
: 5203-json-exports.test.lua
: new file mode 100755
: index 000000000..1b8eb9afa
: --- /dev/null
: +++ b/test/box-tap/gh-5203-json-exports.test.lua
: @@ -0,0 +1,82 @@
: +#!/usr/bin/env tarantool
: +
: +local tap = require('tap')
: +local ffi = require('ffi')
: +ffi.cdef([[
: +    void *dlsym(void *handle, const char *symbol);
: +    /**
: +     * Lexer for JSON paths:
: +     * <field>, <.field>, <[123]>, <['field']> and their combinations.
: +     */
: +    struct json_lexer {
: +        /** Source string. */
: +        const char *src;
: +        /** Length of string. */
: +        int src_len;
: +        /** Current lexer's offset in bytes. */
: +        int offset;
: +        /** Current lexer's offset in symbols. */
: +        int symbol_count;
: +        /**
: +         * Base field offset for emitted JSON_TOKEN_NUM tokens,
: +         * e.g. 0 for C and 1 for Lua.
: +         */
: +        int index_base;
: +    };
: +
: +    enum json_token_type {
: +        JSON_TOKEN_NUM,
: +        JSON_TOKEN_STR,
: +        JSON_TOKEN_ANY,
: +        /** Lexer reached end of path. */
: +        JSON_TOKEN_END,
: +    };
: +
: +    int
: +    json_lexer_next_token(struct json_lexer *lexer, struct json_token
: *token);
: +
: +    int
: +    json_path_cmp(const char *a, int a_len, const char *b, int b_len,
: +            int index_base);
: +
: +    int
: +    json_path_validate(const char *path, int path_len, int index_base);
: +
: +    int
: +    json_path_multikey_offset(const char *path, int path_len, int
: index_base);
: +]])
: +
: +local test = tap.test('json-features')
: +test:plan(1)
: +
: +local RTLD_DEFAULT
: +-- See `man 3 dlsym`:
: +-- RTLD_DEFAULT
: +--   Find  the  first occurrence of the desired symbol using the default
: +--   shared object search order.  The search will include global symbols
: +--   in the executable and its dependencies, as well as symbols in shared
: +--   objects that were dynamically loaded with the RTLD_GLOBAL flag.
: +if jit.os == "OSX" then
: +    RTLD_DEFAULT = ffi.cast("void *", -2LL)
: +else
: +    RTLD_DEFAULT = ffi.cast("void *", 0LL)
: +end
: +
: +local json_symbols = {
: +    'json_lexer_next_token',
: +    'json_path_cmp',
: +    'json_path_validate',
: +    'json_path_multikey_offset',
: +}
: +
: +test:test('json_symbols', function(t)
: +    t:plan(#json_symbols)
: +    for _, sym in ipairs(json_symbols) do
: +        t:ok(
: +            ffi.C.dlsym(RTLD_DEFAULT, sym) ~= nil,
: +            ('Symbol %q found'):format(sym)
: +        )
: +    end
: +end)
: +
: +os.exit(test:check() and 0 or 1)
: --
: 2.23.0

Re: [Tarantool-patches] [PATCH] exports: allow to use json tools via FFI

[Vladislav Shpilevoy]

Hi! Thanks for the patch!

On 21.08.2020 18:26, olegrok@tarantool.org wrote:
> From: Oleg Babin <babinoleg@mail.ru>
> 
> This patch exports some json functions to be used via FFI. We solve
> following problem: currently we don't have any tools to inspect
> jsonpaths. E.g. user wants to ban or restrict an access to some
> tuple fields, it could be some system fields. Tarantool doesn't
> have hidden fields and to solve such problem we should fairly
> parse input jsonpaths. Before this patch user should write its own
> or some external tools. This patch allows to use functions from
> built-in json-lexer directly from Tarantool via FFI.

I understand the problem - you don't want to implement your own tool,
but you must realize, that exporting some internal structs and functions
via FFI is not safe. We won't care about "backward compatibility" of
internal functions. And JSON code is quite mutable, it may change in
future, if we will rework multikeys, for example. And that will make
your code unusable on newer versions. Even probably inside one version,
because we won't bump version number just for the sake of internal code
change.

Even worse is that you took not only general stuff like json_lexer_next_token,
but also json_path_multikey_offset - this is heavily internal, why do you
need it?

I can't upvote this patch, sorry. I propose you to implement a JSON path
parser in Lua, or copy-paste the C code from json.h/c. and create your
own C module. The whole json.c is 666 lines, including the multikey
support and json trees, which occupy most of the space and you don't need
them. The parser is trivial, one day work to implement it from the scratch
with the tests. Please, don't use the internal structs and functions.

You can't just export everything you need if by luck it happened we have it
somewhere internally implemented in C. It is not safe.

The only thing you need to implement the same JSON path parser is utf8 module,
and you have it.

Re: [Tarantool-patches] [PATCH] exports: allow to use json tools via FFI

[Oleg Babin]

Hi! Thanks for your comments. See my answers below.

On 22/08/2020 00:56, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
>
> On 21.08.2020 18:26,olegrok@tarantool.org  wrote:
>> From: Oleg Babin<babinoleg@mail.ru>
>>
>> This patch exports some json functions to be used via FFI. We solve
>> following problem: currently we don't have any tools to inspect
>> jsonpaths. E.g. user wants to ban or restrict an access to some
>> tuple fields, it could be some system fields. Tarantool doesn't
>> have hidden fields and to solve such problem we should fairly
>> parse input jsonpaths. Before this patch user should write its own
>> or some external tools. This patch allows to use functions from
>> built-in json-lexer directly from Tarantool via FFI.
> I understand the problem - you don't want to implement your own tool,
> but you must realize, that exporting some internal structs and functions
> via FFI is not safe. We won't care about "backward compatibility" of
> internal functions. And JSON code is quite mutable, it may change in
> future, if we will rework multikeys, for example. And that will make
> your code unusable on newer versions. Even probably inside one version,
> because we won't bump version number just for the sake of internal code
> change.

I understand it's OK in general for FFI. If I use some internal 
functions it's my responsibility to fix my wrappers/libraries to use it 
if something will change.

> Even worse is that you took not only general stuff like json_lexer_next_token,
> but also json_path_multikey_offset - this is heavily internal, why do you
> need it?

I don't really need it. I exported it just for consistency (to have full 
set of json_path_* functions).

> I can't upvote this patch, sorry. I propose you to implement a JSON path
> parser in Lua, or copy-paste the C code from json.h/c. and create your
> own C module. The whole json.c is 666 lines, including the multikey
> support and json trees, which occupy most of the space and you don't need
> them. The parser is trivial, one day work to implement it from the scratch
> with the tests. Please, don't use the internal structs and functions.
>
> You can't just export everything you need if by luck it happened we have it
> somewhere internally implemented in C. It is not safe.
>
> The only thing you need to implement the same JSON path parser is utf8 module,
> and you have it.

Creation of some new module will add new external dependency that I 
don't want to introduce.

I'm not sure that performance of Lua-implemented module will quite high 
as in case with FFI.

But OK I understand your point.