Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broken net_box compability between 2.11 and 2.10 #8745

Closed
godzie44 opened this issue Jun 7, 2023 · 2 comments · Fixed by #8764
Closed

Broken net_box compability between 2.11 and 2.10 #8745

godzie44 opened this issue Jun 7, 2023 · 2 comments · Fixed by #8764
Assignees
Labels
2.10 Target is 2.10 and all newer release/master branches bug Something isn't working iproto netbox

Comments

@godzie44
Copy link
Contributor

godzie44 commented Jun 7, 2023

Originally this issue also reported a replication breakage, but this was moved to the separate issue #8757.

Bug description

After commit 13b0ce9 there is a broken messaging via net_box between tarantool 2.10 and 2.11.

Related versions:

  • Tarantool 2.11.0-12-g807218ded (but you can use any version down to version 13b0ce9 inclusive)
  • Tarantool 2.10.0-32-gd14267c64

Target: Linux-x86_64-RelWithDebInfo
Build options: cmake . -DCMAKE_INSTALL_PREFIX=/usr/local -DENABLE_BACKTRACE=TRUE
Compiler: GNU-11.3.0
C_FLAGS: -fexceptions -funwind-tables -fasynchronous-unwind-tables -fno-common -fopenmp -msse2 -Wformat -Wformat-security -Werror=format-security -fstack-protector-strong -fPIC -fmacro-prefix-map=/home/kostya/CLionProjects/tarantool-fork=. -std=c11 -Wall -Wextra -Wno-gnu-alignof-expression -fno-gnu89-inline -Wno-cast-function-type
CXX_FLAGS: -fexceptions -funwind-tables -fasynchronous-unwind-tables -fno-common -fopenmp -msse2 -Wformat -Wformat-security -Werror=format-security -fstack-protector-strong -fPIC -fmacro-prefix-map=/home/kostya/CLionProjects/tarantool-fork=. -std=c++11 -Wall -Wextra -Wno-invalid-offsetof -Wno-gnu-alignof-expression -Wno-cast-function-type

Steps to reproduce

  1. run and configure tarantool 2.11 instance:
box.cfg{listen=3301}
  1. run and configure tarantool 2.10 instance, then try to use net_box:
box.cfg{listen=3303}
conn = require('net.box').connect(3301)
conn:eval('print(1)')

This code return error:

- error: Invalid MsgPack - request body

Same behaviour with conn:call

@godzie44 godzie44 added the bug Something isn't working label Jun 7, 2023
@kyukhin kyukhin added the netbox label Jun 9, 2023
@locker locker added iproto 2.10 Target is 2.10 and all newer release/master branches labels Jun 9, 2023
@locker
Copy link
Member

locker commented Jun 9, 2023

There's a bug in Tarantool 2.10 because of which it fails to decode an IPROTO_ID packet from Tarantool 2.11. There's no workaround for it. We have to fix it in Tarantool 2.10 and make a new release.

@locker
Copy link
Member

locker commented Jun 9, 2023

The replication and net.box issues are unrelated. I filed the separate issue #8757 for the replication breakages and removed it from this ticket.

locker added a commit to locker/tarantool that referenced this issue Jun 13, 2023
The xrow_decode_* functions are written in such a way that they ignore
unknown IPROTO keys. This is required for connectivity between different
Tarantool version. However, there's bug in the code connected with the
value type checking: we fail if the key is >= iproto_key_MAX. This
worked fine as long as we added new IPROTO keys in the middle of the key
space, without bumping iproto_key_MAX, but this assumption broke when we
added IPROTO_AUTH_TYPE. The issue is exacerbated by the fact that
IPROTO_AUTH_TYPE is used by IPROTO_ID, which is sent unconditionally on
connect. Let's fix the value type check and add some tests.

Notes:
 - xrow_decode_heartbeat turns out to be unused. Drop it.
 - Fix the net.box helpers response_body_decode and netbox_decode_table
   to handle unknown keys and empty body. This is needed to properly
   decode a response to an injection in tests.
 - Testing unknown keys in replication requests would be complicated.
   Instead we add a bunch of unit tests.
 - Convert the xrow unit test to TAP.

Closes tarantool#8745

NO_DOC=bug fix
locker added a commit to locker/tarantool that referenced this issue Jun 14, 2023
The xrow_decode_* functions are written in such a way that they ignore
unknown IPROTO keys. This is required for connectivity between different
Tarantool version. However, there's bug in the code connected with the
value type checking: we fail if the key is >= iproto_key_MAX. This
worked fine as long as we added new IPROTO keys in the middle of the key
space, without bumping iproto_key_MAX, but this assumption broke when we
added IPROTO_AUTH_TYPE. The issue is exacerbated by the fact that
IPROTO_AUTH_TYPE is used by IPROTO_ID, which is sent unconditionally on
connect. Let's fix the value type check and add some tests.

Notes:
 - xrow_decode_heartbeat turns out to be unused. Drop it.
 - Fix the net.box helpers response_body_decode and netbox_decode_table
   to handle unknown keys and empty body. This is needed to properly
   decode a response to an injection in tests.
 - Testing unknown keys in replication requests would be complicated.
   Instead we add a bunch of unit tests.
 - Convert the xrow unit test to TAP.

Closes tarantool#8745

NO_DOC=bug fix
locker added a commit that referenced this issue Jun 15, 2023
The xrow_decode_* functions are written in such a way that they ignore
unknown IPROTO keys. This is required for connectivity between different
Tarantool version. However, there's bug in the code connected with the
value type checking: we fail if the key is >= iproto_key_MAX. This
worked fine as long as we added new IPROTO keys in the middle of the key
space, without bumping iproto_key_MAX, but this assumption broke when we
added IPROTO_AUTH_TYPE. The issue is exacerbated by the fact that
IPROTO_AUTH_TYPE is used by IPROTO_ID, which is sent unconditionally on
connect. Let's fix the value type check and add some tests.

Notes:
 - xrow_decode_heartbeat turns out to be unused. Drop it.
 - Fix the net.box helpers response_body_decode and netbox_decode_table
   to handle unknown keys and empty body. This is needed to properly
   decode a response to an injection in tests.
 - Testing unknown keys in replication requests would be complicated.
   Instead we add a bunch of unit tests.
 - Convert the xrow unit test to TAP.

Closes #8745

NO_DOC=bug fix
locker added a commit that referenced this issue Jun 15, 2023
The xrow_decode_* functions are written in such a way that they ignore
unknown IPROTO keys. This is required for connectivity between different
Tarantool version. However, there's bug in the code connected with the
value type checking: we fail if the key is >= iproto_key_MAX. This
worked fine as long as we added new IPROTO keys in the middle of the key
space, without bumping iproto_key_MAX, but this assumption broke when we
added IPROTO_AUTH_TYPE. The issue is exacerbated by the fact that
IPROTO_AUTH_TYPE is used by IPROTO_ID, which is sent unconditionally on
connect. Let's fix the value type check and add some tests.

Notes:
 - xrow_decode_heartbeat turns out to be unused. Drop it.
 - Fix the net.box helpers response_body_decode and netbox_decode_table
   to handle unknown keys and empty body. This is needed to properly
   decode a response to an injection in tests.
 - Testing unknown keys in replication requests would be complicated.
   Instead we add a bunch of unit tests.
 - Convert the xrow unit test to TAP.

Closes #8745

NO_DOC=bug fix

(cherry picked from commit ee0660b)
locker added a commit that referenced this issue Jun 15, 2023
The xrow_decode_* functions are written in such a way that they ignore
unknown IPROTO keys. This is required for connectivity between different
Tarantool version. However, there's bug in the code connected with the
value type checking: we fail if the key is >= iproto_key_MAX. This
worked fine as long as we added new IPROTO keys in the middle of the key
space, without bumping iproto_key_MAX, but this assumption broke when we
added IPROTO_AUTH_TYPE. The issue is exacerbated by the fact that
IPROTO_AUTH_TYPE is used by IPROTO_ID, which is sent unconditionally on
connect. Let's fix the value type check and add some tests.

Notes:
 - xrow_decode_heartbeat turns out to be unused. Drop it.
 - Fix the net.box helpers response_body_decode and netbox_decode_table
   to handle unknown keys and empty body. This is needed to properly
   decode a response to an injection in tests.
 - Testing unknown keys in replication requests would be complicated.
   Instead we add a bunch of unit tests.
 - Convert the xrow unit test to TAP.

Closes #8745

NO_DOC=bug fix

(cherry picked from commit ee0660b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.10 Target is 2.10 and all newer release/master branches bug Something isn't working iproto netbox
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants