Vladimir Davydov authored 2 years ago and Дмитрий Кольцов committed 2 years ago

The index directory is created on demand since commit c00ba8e7
("xlog: make log directory if needed") and removed when it becomes
empty. There's no need to create it when an index is created anymore.

Follow-up #8441

NO_DOC=bugfix

Denis Smirnov authored 2 years ago and Дмитрий Кольцов committed 2 years ago

When vinyl space is dropped, its files are left on the file system
until GC removes them. At the moment GC removes only run files,
but not the root directory. These empty directories are never
removed and occupy 4KB on ext-family file systems each. In a case
of many dropped vinyl spaces it can become a serious disk space
and inode leak. Current commit makes gc always remove root directory
if there are no runs in it.

Closes #8441

NO_DOC=bugfix

Denis Smirnov authored 2 years ago and Дмитрий Кольцов committed 2 years ago

BREAKING CHANGE: previously the schema version was a 32-bit unsigned.
But it can become a problem for applications that widely create
vinyl spaces. As far as vinyl can't be set meta-temporary, its
creation always increments the schema. Let's make this counter 64-bit
unsigned to avoid the overflow problem. As a side effect be break
the IPROTO compatibility with the previous versions of Tarantool.

NO_DOC=core feature
NO_TEST=no Lua API

Serge Petrenko authored 2 years ago and Дмитрий Кольцов committed 2 years ago

Every libunwind error during backtrace collection is reported with
`say_error`.
Since changes from
e2b8d9da (19abfd2a - in upstream) "misc: get rid of fiber_gc"
backtraces are collected on each fiber gc allocation, of which there are
plenty.

For some reason (https://github.com/tarantool/tarantool/issues/7980)
each unw_step fails on mac, and an error is spammed to instance logs,
even though the backtrace is actually collected.

Silence the errors, since there is no much use for them anyway. And
silence all of them just to be consistent.

This doesn't close #7980, because that issue still needs a proper fix.
Although its severity is ameliorated now.

In-scope-of #7980

NO_DOC=bugfix
NO_CHANGELOG=bugfix
NO_TEST=nothing to test

Due to inconsistency of Tarantool type casting while using strict
data types as "double" or "unsigned" it is needed
to use "number" data type in a whole bunch of cases.
However "number" may contain "decimal" that will be serialized into
string by JSON builtin module.

This commit adds "encode_decimal_as_number" parameter to json.cfg{}.
That forces to encode `decimal` as JSON number to force type
consistency in JSON output.
Use with catious - most of JSON parsers assume that number is restricted
to float64.

NO_DOC=we do not host doc

Denis Smirnov authored 2 years ago and Дмитрий Кольцов committed 2 years ago

Previously,

select "t1"."a" from (select "a" from "t") as "t1";

returned a result column name `t1` instead of `t1.a` because of
incorrect work of a dequoting function. The reason was that
previously sqlDequote() function finished its work when found the
first closing quote.

Old logic worked for simple selects where the column name doesn't
contain an explicit scan name ("a" -> a).
But for the sub-queries results sqlDequote() finished its work right
after the scan name ("t1"."a" -> t1). Now the function continues its
deqouting till it gets the null terminator at the end of the string.

Closes #7063

NO_DOC=don't change any public API, only a bug fix

Co-authored-by: Mergen Imeev <imeevma@gmail.com>

Denis Smirnov authored 2 years ago and Дмитрий Кольцов committed 2 years ago

Actually there is no reason to throw an error and make a user
manually recreate prepared statement when it expires. A much more
user friendly way is to recreate it under hood when statement's
schema version differs from the box one.

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring

Denis Smirnov authored 2 years ago and Дмитрий Кольцов committed 2 years ago

Problem description.

When we prepare a statement with parameters in the result columns
(for example box.prepare('select ?')) Tarantool has no information
about the type of the output column and set it to default boolean.
Then, on the execution phase, the type would be recalculated during
the parameter binding.

Tarantool expects that there is no way for parameter to appear in the
result tuple other than exactly be mentioned in the final projection.
But it is incorrect - we can easily propagate parameter from the inner
part of the join. For example

box.prepare([[select COLUMN_1 from t1 join (values (?)) as t2 on true]])

In this case column COLUMN_1 in the final projection is not a
parameter, but a "reference" to it and its type depends on the
parameter from the inner part of the join. But as Tarantool
recalculates only binded parameters in the result projection,
it doesn't change the default boolean metadata type of the COLUMN_1
and the query fails on comparison with the actual type of the tuple.

Solution.
As we don't want to patch Vdbe to make COLUMN_1 refer inner parameter,
it was decided to make a simple workaround: change the default
column type from BOOLEAN to ANY for parameters. It fixes the
comparison with the actual tuple type (we do not fail), but in some
cases get ANY column in the results where we would like to have
explicitly defined type. Also NULL parameters would also have ANY
type, though Tarantool prefers to have BOOLEAN in this case.

Closes https://github.com/tarantool/tarantool/issues/7283

NO_DOC=bug fix

godzie44 authored 2 years ago and Дмитрий Кольцов committed 2 years ago

sql: add sql_execute_prepared_ext function, same as sql_execute_prepared but without `region` parameter
closes #2

NO_DOC=minor
NO_TEST=minor

godzie44 authored 2 years ago and Дмитрий Кольцов committed 2 years ago

- add box_tuple_data_offset function (return offset of the messagePack encoded data from the beginning of the tuple)
- add more export functions

closes #1

NO_DOC=build
NO_TEST=build

Andrey Saranchin authored 2 years ago and Дмитрий Кольцов committed 2 years ago

Currently, tarantool uses fiber() macro in crash_collect()
to collect backtraces, which is redundant and leads to
NULL dereferencing if crash signal callback is executed
on thread with no initialized cord.

The patch makes it possible not to use fiber module for
collecting backtraces and gets rid of fiber() macro
in crash_collect().

NO_CHANGELOG=internal
NO_TEST=internal
NO_DOC=internal

Nikolay Shirokovskiy authored 2 years ago and Дмитрий Кольцов committed 2 years ago

As it breaks sane usage of region as a data stack:

	size_t region_svp = region_used(&fiber()->gc);
	/* some allocation on fiber gc and usage of allocated memory. */
	region_truncate(&fiber()->gc, region_svp);

If in the above snippet one calls a function that in turn calls
`fiber_gc` then the snippet code may have use-after-free and later UB
on truncation.

For this reason let's get read of fiber_gc. However we need to make sure
we won't introduce leaks this way. So before actually removing
fiber_gc we make it perform leak check instead and only after fixing
all the leaks the fiber_gc was removed.

In order to find the leak easily the backtrace of the first fiber gc
allocation that is not truncated is saved and then reported.

In order to catch leaks that are not triggered by the current test suit
and to prevent introducing leaks in future patches the leak check is
added on fiber exit/recycle and for long living system fibers on every loop
iteration.

Leak check in release build is on but without leak backtrace info by
default for performance reasons. Backtrace can be provided by using
`fiber.leak_backtrace_enable()` knob before starting leaking fiber.

Regularly leaks are only reported in log but it will not help to
catch errors when running test suits so build option ABORT_ON_LEAK
is added. When it is on we abort on leak. This option is turned off
for all builds that used in CI.

Closes #5665

NO_CHANGELOG=internal
NO_DOC=internal

Nikolay Shirokovskiy authored 2 years ago and Дмитрий Кольцов committed 2 years ago

Currently in space_execute_dml we have some txn related objects
allocated on fiber region. Use txn region as in other places.

NO_DOC=internal
NO_CHANGELOG=internal
NO_TEST=internal

Nikolay Shirokovskiy authored 2 years ago and Дмитрий Кольцов committed 2 years ago

Currently `backtrace_snprint ` indirectly uses `tt_static_buf()` by
itself. As a result its callers cannot use `tt_static_buf()`. With
large enough backtrace stack size buffer passed to `backtrace_snprint`
will be overwritten inside `backtrace_frame_resolve` call.

Part of #5665

NO_DOC=internal
NO_CHANGELOG=internal
NO_TEST=internal

Vladislav Shpilevoy authored 2 years ago and Дмитрий Кольцов committed 2 years ago

main() used to skip most of modules destruction in
tarantool_free(). That got ASAN complaining on clang-13 about a
leak of a fiber on_stop trigger which was allocated in Lua.

The patch makes fiber_free() called for the main cord. It destroys
and frees all the fibers together with their on_stop triggers.

Closes #7259

NO_CHANGELOG=Not a visible change
NO_DOC=Not a visible change
NO_TEST=Not a visible change

Mergen Imeev authored 2 years ago and Дмитрий Кольцов committed 2 years ago

This patch introduces mp_format_on_region() and mp_vformat_on_region()
functions. These functions help to create an encoded value according to
a given format in a buffer allocated on region.

NO_DOC=Refactoring
NO_TEST=Refactoring
NO_CHANGELOG=Refactoring

When function of a functional index is called, the result is
stored in memtx allocator chunks among data tuples. By design,
memtx allocator requires size of allocation for deallocation,
so the size has to be stored along with the data (actually right
before it). By a mistake, when being deleted, the size of data
was retrieved slightly wrong, giving the value of 4 bytes less.
Due to the allocator specific design the size error leads to a
rare crashes when the size of functional index function result
was about 160 bytes (157..160 bytes with default config). It
seems that sizes about 320 etc are affected to.

Fix it by correct size evaluation of functional index chunks.

Hotfix of #6786

NO_DOC=bug fix
NO_TEST=see later commits

Fixes commit 837b0948 ("box: handle region_alloc failure in
tuple_field_map_create_plain").

NO_DOC=build fix
NO_TEST=build fix
NO_CHANGELOG=build fix

Let's use xregion_alloc instead of region_alloc, because memory
allocations from fiber region shouldn't normally fail, see #3534.

Closes tarantool/security#97

NO_DOC=bug fix
NO_TEST=shouldn't normally happen
NO_CHANGELOG=see NO_TEST

(cherry picked from commit c690d708)

Fixed a bug with syslog priority for OS with non-standart `LOG_MAKEPRI`
macro.

Affected all versions for OS: Alpine (including official docker images),
OpenBSD and maybe others.

Fixes #8269

NO_DOC=bugfix
NO_TEST=exists

(cherry picked from commit acca6d7a)

This patch adds a type check of the first argument of the
tuple_field_by_path() function.

Closes tarantool/security#82

NO_DOC=internal
NO_TEST=internal
NO_CHANGELOG=internal

(cherry picked from commit 0d9213aa)

Bump the small submodule and use small_getpagesize(), which is a
wrapper over sysconf(_SC_PAGESIZE) with a proper error checking.

Closes tarantool/security#78

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring

(cherry picked from commit 7932144d)

obuf_alloc(&log->zbuf, XLOG_FIXHEADER_SIZE) can potentially fail,
because there is no obuf_reserve() prior to it.

Closes tarantool/security#74

NO_DOC=bugfix
NO_CHANGELOG=bugfix
NO_TEST=no test harness for checking OOM

(cherry picked from commit 32dfcb3c)

The YAML serializer fails to detect aliases in objects returned by
the __serialize method:

tarantool> x = {}
---
...

tarantool> {a = x, b = x}
---
- a: &0 []
  b: *0
...

tarantool> setmetatable({}, {
         >     __serialize = function() return {a = x, b = x} end,
         > })
---
- a: []
  b: []
...

Fix this by scanning the object returned by the __serialize method
(called by luaL_checkfield) for references.

Closes #8240

NO_DOC=bug fix

(cherry picked from commit b42302f5)

It was possible to execute arbitrary Lua code outside of the setfenv()
environment. Example:

NO_WRAP
```lua
tarantool> box.cfg{replication_synchro_quorum = [=[N / 2 + 1]] _G.test = true --[[]=]}
tarantool> test
---
- true
...
```
NO_WRAP

How it works:

```lua
local expr = [[%s]]
```

Let's assume that `%s` is replaced by `]]<..code..>--[[`. The result is the
following (newlines are added for readability):

```lua
local expr = [[]]
<..code..>
--[[]]
```

This code is executed outside of the setfenv() protected function.

The fix is to pass the expression as an argument instead of using
`snprintf()`.

Fixes https://github.com/tarantool/security/issues/20
Fixes GHSA-74jr-2fq7-vp42

NO_DOC=bugfix

Sometimes the return value of cfg_gets() is checked for NULL, and sometimes
not. Actually this is intended, although a bit confusing. If an option can
have a nil value, it must be checked for NULL, but if it can't be nil,
there is no sense in it. The nil value can be assigned only by default, it
cannot be set via box.cfg{}.

This patch removes the NULL checks for cfg_gets("election_mode") and
cfg_gets("election_fencing_mode") because they are not nil by default.
All other non-nil options (e.g. cfg_gets("bootstrap_strategy")) are
already implemented without the NULL checks.

Follow-up tarantool/security#75

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring

(cherry picked from commit 5a2dc43c)

`mpstream_encode_double`, apparently, has a typo: the result of
`mpstream_reserve` is checked after encoding the double into the result
buffer — fix it.

Closes tarantool/security#63

NO_DOC=bug fix
NO_CHANGELOG=see NO_TEST
NO_TEST=unlikely to happen because malloc shouldn't normally fail, and
	we don't test other mpstream methods for OOM either

(cherry picked from commit ccf3130c)

 - Use tabs instead of spaces as we usually do.
 - Drop pointless coversion of (void *) to (char *).
 - Add missing comments to struct mpstream members.
 - Cleanup header list.
 - Use short licence.

NO_DOC=code cleanup
NO_TEST=code cleanup
NO_CHANGELOG=code cleanup

(cherry picked from commit c2b76592)

`fiber_new_system` can potentially fail — its return value for the watcher
fiber must be checked and an exception must be raised in case it does fail.

Closes tarantool/security#87

NO_CHANGELOG=<security fix>
NO_DOC=<security fix>
NO_TEST=<no test harness for checking OOM>

(cherry picked from commit e9fad4c7)

This patch fixes an issue with checking the result of sql_get_coll_seq()
in sql_expr_coll(). This fix only changes the error if the collation
combination is invalid because sql_get_coll_seq() sets the is_aborted
flag and error will be thrown in any case.

Closes tarantool/security#80

NO_DOC=change of returned error in rare case
NO_CHANGELOG=change of returned error in rare case

(cherry picked from commit e9f1beab)

The main cord's event loop is initialized by fiber_init(), but for some
reason successful initialization is only checked in main() after other
initialization code might try to use the event loop already.

For example, some of the loop users are coio_enable(), signal_init(),
tarantooL_lua_init(), and they are all run before we actually check that
loop is not NULL.

Closes tarantool/security#28

NO_DOC=code health
NO_TEST=code health
NO_CHANGELOG=code health

(cherry picked from commit 579ac6d3)

This patch replaces malloc() with xmalloc() in key_def_dup() to avoid
the possibility of skipping the malloc() return value check.

Closes tarantool/security#81

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring

(cherry picked from commit 8ca94313)

Now, delete in ephemeral space is obviously incorrect - if we try to
delete a tuple, which is not present in index, NULL dereference will
happen. Fortunately, ephemeral spaces are used for internal purposes
only, so, most likely, this never happens. Let's fix this part not to
confuse code analyzers.

Closes https://github.com/tarantool/security/issues/38

NO_TEST=shouldn't normally happen
NO_CHANGELOG=shouldn't normally happen
NO_DOC=shouldn't normally happen

(cherry picked from commit a2d5e54e)

This is useful for example for the analysis of performance complaints
from users, when they claim that one version of Tarantool is slower
than another, in fact comparing debug and release builds.

NO_DOC=minor change
NO_TEST=minor change

(cherry picked from commit 45576088)

When log format is JSON and a Lua table is written to the log, such
messages are saved by the flight recorder as a "json" string. Fix it.

Part of tarantool/tarantool-ee#325

NO_DOC=bugfix
NO_TEST=will be added to EE, because there are no flightrec in CE
NO_CHANGELOG=will be added to EE, because there are no flightrec in CE

Bitset index size calculation uses the cardinality of the 'flag' bitset,
but when the bitset index is empty, i.e., uninitialized, the
'flag' bitset is not allocated, hence we should simply return 0.

Closes #5809

NO_DOC=bugfix

(cherry picked from commit d542a01a)

We didn't take into consideration the fact, that precision
value passed to control the width of nanoseconds part in
datetime_object:format could be more than maximum positive
value, integer may have. Currently it leads to segfault.

```
tarantool> require('datetime').new{}:format('%2147483648f')
```

We should check errno in order to find out, if overflow
occurs. The problem is the fact, that `width` variable must
have int type due to snprintf requirements ("%*d") and
strtol returns long. Errno won't be set if returned value
is in bounds [INT_MAX, LONG_MAX], but it will overflow
int resulting in inconsistent behavior.

So, let's save the result of strotl to the temp value.
If this value doesn't belong to the above-mentioned set,
or errno was set, we assign to `width` maximum value, it
may have: 9.

Closes tarantool/security#31

NO_DOC=bugfix

(cherry picked from commit b6159217)

This patch fixes some possible bugs that may occur due to malloc
failure.

Closes tarantool/security#65
Closes tarantool/security#66
Closes tarantool/security#68

NO_DOC=bugfix
NO_TEST=hard to reproduce due to malloc() failure being an unusual case

Fixing a bug with nodes in 'manual' election mode bumping the term
excessively revealed a hang in election_pre_vote test. Turns out the
test passed thanks to the previous buggy behaviour.

The following behaviour is expected: when a node is configured in manual
election mode, calling box.ctl.promote() on it should make it bump term
once, try to gather votes and fail on timeout.

Once the extra term bump on timeout was removed in commit 5765fdc4
("raft: fix 'manual' nodes bumping the term excessively"),
box.ctl.promote() without a quorum started hanging.

Let's return the correct behaviour: 'manual' nodes should transition
back to follower if an election timeout passes after the promotion
without any term outcome.

Enable the test_promote_no_quorum testcase of election_pre_vote test
back, since it's fixed now.

Follow-up #8168
Closes #8217

NO_DOC=bugfix
NO_CHANGELOG=changes not released behaviour

(cherry picked from commit 352fe0c7)

A tuple update with the first operation creating a new field
somewhere deep in the tuple and the second operation trying to go
into that new field could crash. This happened because the route
branching function xrow_update_route_branch() missed this case.

It can be detected when see that the bar path is already fully
used (the next JSON token is END), and the new operation's path
is still not END.

Closes #8216

NO_DOC=bugfix

(cherry picked from commit d4e92809)