Rewrite dissector: modular sources, dual-protocol, version-tested#6
Open
bigbes wants to merge 1 commit into
Open
Rewrite dissector: modular sources, dual-protocol, version-tested#6bigbes wants to merge 1 commit into
bigbes wants to merge 1 commit into
Conversation
3f2b574 to
a24a6a3
Compare
Author
|
Incrorporated other notable changes from #5 |
a24a6a3 to
6178f52
Compare
ligurio
reviewed
Jun 30, 2026
Comment on lines
+12
to
+14
| [0] = 'unknown', [1] = 'decimal', [2] = 'uuid', [3] = 'error', | ||
| [4] = 'datetime', [5] = 'compression', [6] = 'interval', | ||
| [7] = 'tuple', [8] = 'arrow', |
Member
There was a problem hiding this comment.
One entry per line would be better
| -- as negative. Exports the configured msgpack module and the `ext_mt` marker. | ||
| -- Used only by the modern decoder (legacy <= 1.5 is not MsgPack-based). | ||
|
|
||
| local msgpack = require 'MessagePack' |
Member
There was a problem hiding this comment.
usually we use syntax with parentheses (require('MessagePack'))
Comment on lines
+32
to
+50
| -- Read one MsgPack integer at `from`; returns the value and the next index. | ||
| local function read_mp_int(s, from) | ||
| local b = s:byte(from) | ||
| if b <= 0x7f then return b, from + 1 | ||
| elseif b >= 0xe0 then return b - 0x100, from + 1 | ||
| elseif b == 0xcc then return s:byte(from + 1), from + 2 | ||
| elseif b == 0xcd then return be_uint(s, from + 1, 2), from + 3 | ||
| elseif b == 0xce then return be_uint(s, from + 1, 4), from + 5 | ||
| elseif b == 0xd0 then local v = s:byte(from + 1) | ||
| return v >= 0x80 and v - 0x100 or v, from + 2 | ||
| elseif b == 0xd1 then local v = be_uint(s, from + 1, 2) | ||
| return v >= 0x8000 and v - 0x10000 or v, from + 3 | ||
| elseif b == 0xd2 then local v = be_uint(s, from + 1, 4) | ||
| return v >= 0x80000000 and v - 0x100000000 or v, from + 5 | ||
| elseif b == 0xcf then return be_uint(s, from + 1, 8), from + 9 | ||
| elseif b == 0xd3 then return be_uint(s, from + 1, 8), from + 9 | ||
| end | ||
| return 0, from + 1 | ||
| end |
Member
There was a problem hiding this comment.
Use constants instead magic numbers and put a path to C header where these constants are defined
Comment on lines
+20
to
+50
| local function le_uint(s, from, len) | ||
| local v = 0 | ||
| for i = len, 1, -1 do v = v * 256 + s:byte(from + i - 1) end | ||
| return v | ||
| end | ||
|
|
||
| local function be_uint(s, from, len) | ||
| local v = 0 | ||
| for i = 0, len - 1 do v = v * 256 + s:byte(from + i) end | ||
| return v | ||
| end | ||
|
|
||
| -- Read one MsgPack integer at `from`; returns the value and the next index. | ||
| local function read_mp_int(s, from) | ||
| local b = s:byte(from) | ||
| if b <= 0x7f then return b, from + 1 | ||
| elseif b >= 0xe0 then return b - 0x100, from + 1 | ||
| elseif b == 0xcc then return s:byte(from + 1), from + 2 | ||
| elseif b == 0xcd then return be_uint(s, from + 1, 2), from + 3 | ||
| elseif b == 0xce then return be_uint(s, from + 1, 4), from + 5 | ||
| elseif b == 0xd0 then local v = s:byte(from + 1) | ||
| return v >= 0x80 and v - 0x100 or v, from + 2 | ||
| elseif b == 0xd1 then local v = be_uint(s, from + 1, 2) | ||
| return v >= 0x8000 and v - 0x10000 or v, from + 3 | ||
| elseif b == 0xd2 then local v = be_uint(s, from + 1, 4) | ||
| return v >= 0x80000000 and v - 0x100000000 or v, from + 5 | ||
| elseif b == 0xcf then return be_uint(s, from + 1, 8), from + 9 | ||
| elseif b == 0xd3 then return be_uint(s, from + 1, 8), from + 9 | ||
| end | ||
| return 0, from + 1 | ||
| end |
Member
There was a problem hiding this comment.
may be use bitop functions instead? Anyway the module is built into LuaJIT/Tarantool
| digits[#digits + 1] = byte % 16 | ||
| else | ||
| local nibble = byte % 16 -- last low nibble is the sign | ||
| sign = (nibble == 0x0b or nibble == 0x0d) and '-' or '' |
Member
There was a problem hiding this comment.
avoid magic numbers, here and below
Comment on lines
+115
to
+117
| local INTERVAL_FIELD = { | ||
| [0] = 'year', [1] = 'month', [2] = 'week', [3] = 'day', [4] = 'hour', | ||
| [5] = 'min', [6] = 'sec', [7] = 'nsec', [8] = 'adjust', |
Member
There was a problem hiding this comment.
one entry per line, here and below
Comment on lines
+10
to
+24
| paths: | ||
| - "src/**" | ||
| - "dist/**" | ||
| - "tests/**" | ||
| - "MessagePack.lua" | ||
| - "amalgamate.sh" | ||
| - ".github/workflows/tests.yml" | ||
| pull_request: | ||
| paths: | ||
| - "src/**" | ||
| - "dist/**" | ||
| - "tests/**" | ||
| - "MessagePack.lua" | ||
| - "amalgamate.sh" | ||
| - ".github/workflows/tests.yml" |
Member
There was a problem hiding this comment.
Why do you need to limit runs by paths?
| local function map_value_offsets(raw, base_off) | ||
| local b = raw:byte(1) | ||
| local count, first | ||
| if b >= 0x80 and b <= 0x8f then count, first = b - 0x80, 2 |
Single "Tarantool" dissector that auto-detects, per PDU, the modern MsgPack
IPROTO (1.6-3.x) vs the legacy <=1.5 binary protocol, so a mixed capture
decodes in one load.
Structure:
- src/ split into core, msgpack_ext, modern, legacy and the entry point
(entry_all); amalgamate.sh (POSIX sh) inlines the modules -- bundling the
vendored MessagePack -- into one self-contained build,
dist/tarantool.dissector.lua. The build carries a private module registry
instead of the global package.preload, so generic module names ("core", ...)
don't collide with other Lua plugins.
- two interchangeable install forms, both covering all versions and decoding
identically: the amalgamated single file (built by CI and attached to each
GitHub Release; dist/ is not committed), and the modular src/ tree run via the
tarantool.lua loader for development / CLI. The loader loads the modules into
the same kind of private registry (a per-module environment, mutating no global
package.path/package.loaded), so it cannot collide with a co-loaded plugin.
Decoding:
- modern: SQL, streams, id, watchers, structured MP_ERROR stack, replication
(join/subscribe/raft/vclock), and MsgPack ext types (decimal, uuid, datetime,
interval) decoded to real values; pcall-guarded against malformed PDUs; 0xce
framing guard so non-IPROTO bytes don't corrupt reassembly.
- legacy: full 1.5 request/response set; direction detection from the configured
server ports, falling back to the lower-port heuristic; typeless fields
rendered as string / LE integer / blob, consistently across Wireshark versions.
- number model: unsigned uint64 and signed int64 body values (datetime seconds,
intervals) are decoded exactly via raw-byte arithmetic, so they stay correct on
both integer-number Lua (5.3+) and float-number Lua (LuaJIT / 5.1-5.2); box.NULL
in a non-final array position is preserved rather than truncating the tuple.
Preferences:
- "Dissector enabled" (default on) toggles the dissector from the GUI and
re-registers on change; "TCP ports" is a range, e.g. 3301,3311-3313 (default
3301). prefs_changed re-registers the port table; disabling unregisters it.
Tests:
- a Go harness (tests/, testify) drives tshark against committed pcap fixtures --
real captures from Tarantool 1.5, 1.10, 2.11, 3.x, a merged 1.5+3.x, and 3-node
master-master replication (async and sync), plus hand-crafted synthetic-*.pcap
(tests/gen_synthetic_pcaps.py) for box.NULL, exact uint64, negative datetime,
TCP reassembly, and the insert_arrow/nop/CHUNK/opaque-ext/malformed paths. It
asserts concrete decoded values, exact per-type PDU counts, request/response
classification, display-filter usability, the enabled and ports-range
preferences, and runs every scenario against both install forms; a luajit leg
checks number-model exactness the Lua-5.4 reference tshark cannot. CI runs it on
Wireshark 3.x and 4.x, and a separate tag-triggered workflow builds and tests
the amalgamated file before attaching it to the GitHub Release. The capture
generator test.lua guards with "if _TARANTOOL == nil then return end", so it is
a no-op when loaded outside a Tarantool runtime.
Borrowed from Dmitry Pankov's "Wireshark 4.0+ support for tarantool 1.5 proto"
branch (PR #5): the "Dissector enabled" preference
and the test.lua no-op guard.
Removes the old single-format tarantool.dissector.lua and
tarantool15.dissector.lua.
6178f52 to
27c89a9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Single "Tarantool" dissector that auto-detects, per PDU, the modern MsgPack IPROTO (1.6-3.x) vs the legacy <=1.5 binary protocol, so a mixed capture decodes in one load.
Structure
dist/stores the amalgamated resultsrc/split into core, msgpack_ext, modern, legacy and per-build entriesamalgamate.shinlines modules into three self-containeddist/builds (all / modern-only / legacy-only), bundling MessagePack only where needed. Each build carries a private module registry instead of the globalpackage.preload, so generic module names (core, …) don't collide with other Lua plugins and two builds can load in one Wireshark session.tarantool(all),tarantool2(modern),tarantool1(legacy) — so the split builds coexist.Decoding
Preferences
3301,3311-3313. Defaults to 3301 (33013 for the legacy build), so co-loaded split builds stay off the sametcp.portslot (Wireshark binds one dissector per port). A single build can bind a whole cluster / replication mesh.Tests
tests/pcap/holds real captures from Tarantool 1.5, 1.10, 2.11, 3.x, a merged 1.5+3.x, and 3-node master-master replication (async and sync).tests/run.shasserts concrete decoded values (bodies, responses, error text, ext tuples, replication metadata), the enabled/disabled preference, the ports-range preference (binding a 3311-3313 mesh without Decode As), and that the legacy + modern builds co-load without colliding — including that disabling one leaves the other working.dist/is regenerated fromsrc/.Borrowed from #5
Cherry-picked ideas from @dima424658's "Wireshark 4.0+ support for tarantool 1.5 proto" (#5):
tarantool15/tarantool2; adopted here astarantool1/tarantool2/tarantool);test.luaif _TARANTOOL == nil then return endguard, so the capture generator is a no-op outside a Tarantool runtime.Removes the old single-format
tarantool.dissector.luaandtarantool15.dissector.lua.Be not afraid of +6000 lines — it's mostly
dist/(amalgamated dissectors).