Skip to content

fix(codec): harden FlatBuffers reflection codec for automatic (de)serialization #178

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.

Sign up for GitHub

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

Open
16bit-ykiko wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix(codec): harden FlatBuffers reflection codec for automatic (de)serialization #178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f7ecc96
fix(codec): harden FlatBuffers reflection codec for automatic (de)ser…
16bit-ykiko Jul 5, 2026
b4620e9
fix(codec): widen FlatBuffers inline-struct criterion and decode cons…
16bit-ykiko Jul 5, 2026
5882279
refactor(codec): order map entries with meta::lt instead of requiring…
16bit-ykiko Jul 6, 2026
5ff9bf1
feat(codec): value-mode serialize_visit specializations via to_wire/f…
16bit-ykiko Jul 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
405 changes: 395 additions & 10 deletions include/kota/codec/fbs/decode.h
View file
Open in desktop

Large diffs are not rendered by default.

202 changes: 122 additions & 80 deletions include/kota/codec/fbs/encode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <cstddef>
#include <cstdint>
#include <iterator>
#include <numeric>
#include <optional>
#include <ranges>
#include <span>
Expand All @@ -13,6 +14,7 @@
#include <variant>
#include <vector>

#include "kota/meta/compare.h"
#include "kota/meta/type_kind.h"
#include "kota/codec/fbs/type.h"
#include "kota/codec/visit/config.h"
Expand Down Expand Up @@ -52,12 +54,11 @@ struct boxed_table_collector;
struct byte_collector;

struct map_entry_collector;
struct key_capture_visitor;

struct root_visitor;

template <typename Body>
inline auto two_pass(builder_t& fbb, Body&& body) -> table_offset_t;
inline auto two_pass(builder_t& fbb, Body&& body) -> std::optional<table_offset_t>;

struct alloc_field_visitor {
builder_t& fbb;
Expand Down Expand Up @@ -187,7 +188,12 @@ struct write_field_visitor {

template <typename T>
bool visit_float(T v) {
fbb.AddElement<T>(sid, v);
// Decoders read non-float/double floats as double; keep widths in sync.
if constexpr(std::same_as<T, float> || std::same_as<T, double>) {
fbb.AddElement<T>(sid, v);
} else {
fbb.AddElement<double>(sid, static_cast<double>(v));
}
return true;
}

Expand Down Expand Up @@ -495,7 +501,10 @@ struct boxed_table_collector {

auto start = fbb.StartTable();
write_field_visitor wv{fbb, detail::first_field, av.stored_offset};
KOTA_CODEC_TRY(writer(wv));
if(!writer(wv)) {
fbb.EndTable(start);
return false;
}
table_offsets.push_back(table_offset_t(fbb.EndTable(start)));
return true;
}
Expand Down Expand Up @@ -546,51 +555,9 @@ struct byte_collector {
}
};

struct key_capture_visitor {
std::string captured;

using error_type = rich_error;
constexpr static bool human_readable = false;

bool visit_bool(bool v) {
captured = v ? "true" : "false";
return true;
}

template <typename T>
bool visit_int(T v) {
captured = std::to_string(static_cast<std::int64_t>(v));
return true;
}

template <typename T>
bool visit_uint(T v) {
captured = std::to_string(static_cast<std::uint64_t>(v));
return true;
}

template <typename T>
bool visit_float(T v) {
captured = std::to_string(static_cast<double>(v));
return true;
}

template <typename T>
bool visit_str(const T& v) {
captured = std::string(std::string_view(v));
return true;
}

template <typename T>
bool visit_char(T v) {
captured = std::string(1, static_cast<char>(v));
return true;
}
};

struct map_entry_collector {
builder_t& fbb;
std::vector<std::pair<std::string, table_offset_t>> entries;
std::vector<table_offset_t> entries;

template <typename KF, typename VF>
inline bool visit_entry(KF&& key_fn, VF&& value_fn);
Expand Down Expand Up @@ -619,7 +586,11 @@ struct root_visitor {

template <typename T>
bool visit_float(T v) {
return box_root_scalar<T>(v);
if constexpr(std::same_as<T, float> || std::same_as<T, double>) {
return box_root_scalar<T>(v);
} else {
return box_root_scalar<double>(static_cast<double>(v));
}
}

template <typename T>
Expand Down Expand Up @@ -680,35 +651,77 @@ struct root_visitor {
};

template <typename Body>
auto two_pass(builder_t& fbb, Body&& body) -> table_offset_t {
auto two_pass(builder_t& fbb, Body&& body) -> std::optional<table_offset_t> {
alloc_table_visitor av{fbb, {}, 0};
if(!body(av))
return table_offset_t{0};
return std::nullopt;

auto start = fbb.StartTable();
write_table_visitor wv{fbb, av.offsets, 0};
if(!body(wv)) {
fbb.EndTable(start);
return table_offset_t{0};
return std::nullopt;
}
return table_offset_t(fbb.EndTable(start));
}

template <typename Body>
inline bool encode_sorted_map(builder_t& fbb, Body&& body, uoffset_t& out_offset) {
// Reorder collected entry offsets so that wire order follows the keys'
// ordering under meta::lt — operator< when the key defines one, otherwise the
// reflection-synthesized field-by-field order. map_view::find binary-searches
// with the same comparison.
template <typename Key>
inline void sort_entries_by_key(std::vector<table_offset_t>& offsets, std::vector<Key>& keys) {
if(keys.size() != offsets.size()) {
return;
}
std::vector<std::uint32_t> order(offsets.size());
std::iota(order.begin(), order.end(), 0U);
std::stable_sort(order.begin(), order.end(), [&](std::uint32_t a, std::uint32_t b) {
return meta::lt(keys[a], keys[b]);
});
std::vector<table_offset_t> sorted;
sorted.reserve(offsets.size());
for(auto i: order) {
sorted.push_back(offsets[i]);
}
offsets = std::move(sorted);
}

template <typename Container, typename Body>
inline bool encode_sorted_map(builder_t& fbb,
const Container& m,
Body&& body,
uoffset_t& out_offset) {
map_entry_collector coll{fbb, {}};
KOTA_CODEC_TRY(body(coll));

std::sort(coll.entries.begin(), coll.entries.end(), [](const auto& a, const auto& b) {
return a.first < b.first;
});
auto offsets = std::move(coll.entries);

std::vector<table_offset_t> sorted_offsets;
sorted_offsets.reserve(coll.entries.size());
for(auto& entry: coll.entries) {
sorted_offsets.push_back(entry.second);
using entry_t = std::ranges::range_value_t<Container>;
using key_t = kota::map_entry_key_t<entry_t>;
Comment on lines +700 to +701

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Sort adapted map keys by their wire keys

When a map key type is encoded through a value-mode serialize_visit to a different sortable wire type, this uses the raw container key type for ordering and may leave entries unsorted if the raw key itself is not orderable. map_view later reads keys as deep_clean_t<K> (the wire type) and binary-searches by that value, so an unsorted map such as ticket keys written as uint32_t can make valid lookups miss; collect/sort the same wire keys that are written to the entry table.

Useful? React with 👍 / 👎.


if constexpr(std::convertible_to<const key_t&, std::string_view>) {
// String-like keys compare lexicographically; views stay valid because
// they point into key storage owned by the container.
std::vector<std::string_view> keys;
keys.reserve(offsets.size());
for(const auto& entry: m) {
keys.emplace_back(std::string_view(kota::detail::map_entry_key(entry)));

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Own string keys returned by value before sorting

For a keyed-entry container whose getKey() returns an owning std::string by value, this stores a string_view to that temporary and then sorts after the temporary has been destroyed. That makes wire ordering undefined or incorrect for those map-like containers; store an owning key here unless the accessor result is known to reference container-owned storage.

Useful? React with 👍 / 👎.

}
Comment on lines +708 to +710

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid a second pass over input-only map ranges

When the encoded map-like container is only an input_range (which the codec concepts accept), body(coll) has already iterated it once to build the entry offsets, and this second loop may be empty or produce a different sequence of keys. In that case the offsets are left unsorted or sorted against the wrong keys, so map_view::find's binary search can miss entries from valid buffers; collect each key in the same visit_entry pass as its offset instead of re-iterating m.

Useful? React with 👍 / 👎.

sort_entries_by_key(offsets, keys);
} else if constexpr(std::copy_constructible<key_t> &&
meta::synthesized_lt_with<key_t, key_t>) {
std::vector<key_t> keys;
keys.reserve(offsets.size());
for(const auto& entry: m) {
keys.emplace_back(kota::detail::map_entry_key(entry));
}
sort_entries_by_key(offsets, keys);
}
out_offset = fbb.CreateVector(sorted_offsets.data(), sorted_offsets.size()).o;
// Keys that meta::lt cannot order keep iteration order; map_view rejects
// lookups on such key types at compile time.

out_offset = fbb.CreateVector(offsets.data(), offsets.size()).o;
return true;
}

Expand All @@ -724,7 +737,10 @@ inline bool
auto start = fbb.StartTable();
fbb.AddElement<std::uint32_t>(detail::first_field, static_cast<std::uint32_t>(index));
write_field_visitor payload_write{fbb, payload_slot, payload_alloc.stored_offset};
body(payload_write);
if(!body(payload_write)) {
fbb.EndTable(start);
return false;
}
out_offset = fbb.EndTable(start);
return true;
}
Expand Down Expand Up @@ -769,7 +785,14 @@ bool seq_encode_impl(builder_t& fbb, const Container& c, Body&& body, uoffset_t&
}
} else if constexpr(meta::int_like<element_t> || meta::uint_like<element_t> ||
meta::floating_like<element_t> || meta::char_like<element_t>) {
using wire_t = std::conditional_t<meta::char_like<element_t>, std::int8_t, element_t>;
using wire_t = std::conditional_t<
meta::char_like<element_t>,
std::int8_t,
std::conditional_t<meta::floating_like<element_t> &&
!std::same_as<element_t, float> &&
!std::same_as<element_t, double>,
double,
element_t>>;
if constexpr(std::ranges::contiguous_range<Container> &&
std::ranges::sized_range<Container> && std::same_as<element_t, wire_t>) {
auto data = std::ranges::data(c);
Expand Down Expand Up @@ -868,7 +891,10 @@ bool alloc_field_visitor::visit_struct(const T&, Body&& body) {
return true;
} else {
auto off = two_pass(fbb, std::forward<Body>(body));
stored_offset = off.o;
if(!off) {
return false;
}
stored_offset = off->o;
return true;
}
}
Expand All @@ -881,13 +907,16 @@ bool alloc_field_visitor::visit_seq(const Container& c, Body&& body) {
template <typename T, typename Body>
bool alloc_field_visitor::visit_tuple(const T&, Body&& body) {
auto off = two_pass(fbb, std::forward<Body>(body));
stored_offset = off.o;
if(!off) {
return false;
}
stored_offset = off->o;
return true;
}

template <typename Container, typename Body>
bool alloc_field_visitor::visit_map(const Container&, Body&& body) {
return encode_sorted_map(fbb, std::forward<Body>(body), stored_offset);
bool alloc_field_visitor::visit_map(const Container& m, Body&& body) {
return encode_sorted_map(fbb, m, std::forward<Body>(body), stored_offset);
}

template <typename Body>
Expand All @@ -898,14 +927,20 @@ bool alloc_field_visitor::visit_variant(std::size_t index, Body&& body) {
template <typename T, typename Body>
bool table_elem_visitor::visit_struct(const T&, Body&& body) {
auto off = two_pass(fbb, std::forward<Body>(body));
table_offsets.push_back(off);
if(!off) {
return false;
}
table_offsets.push_back(*off);
return true;
}

template <typename T, typename Body>
bool table_elem_visitor::visit_tuple(const T&, Body&& body) {
auto off = two_pass(fbb, std::forward<Body>(body));
table_offsets.push_back(off);
if(!off) {
return false;
}
table_offsets.push_back(*off);
return true;
}

Expand All @@ -929,9 +964,9 @@ bool table_elem_visitor::visit_seq(const Container& c, Body&& body) {
}

template <typename Container, typename Body>
bool table_elem_visitor::visit_map(const Container&, Body&& body) {
bool table_elem_visitor::visit_map(const Container& m, Body&& body) {
uoffset_t vec_off = 0;
KOTA_CODEC_TRY(encode_sorted_map(fbb, std::forward<Body>(body), vec_off));
KOTA_CODEC_TRY(encode_sorted_map(fbb, m, std::forward<Body>(body), vec_off));

auto start = fbb.StartTable();
fbb.AddOffset(detail::first_field, offset_t<void>(vec_off));
Expand All @@ -941,9 +976,6 @@ bool table_elem_visitor::visit_map(const Container&, Body&& body) {

template <typename KF, typename VF>
bool map_entry_collector::visit_entry(KF&& key_fn, VF&& value_fn) {
key_capture_visitor capture;
KOTA_CODEC_TRY(key_fn(capture));

auto table_off = two_pass(fbb, [&](auto& sv) -> bool {
KOTA_CODEC_TRY(sv.visit_field(std::integral_constant<std::size_t, 0>{},
std::string_view{"key"},
Expand All @@ -953,14 +985,20 @@ bool map_entry_collector::visit_entry(KF&& key_fn, VF&& value_fn) {
[&](auto& vv) -> bool { return value_fn(vv); }));
return true;
});

entries.emplace_back(std::move(capture.captured), table_off);
if(!table_off) {
return false;
}
entries.push_back(*table_off);
return true;
}

template <typename T, typename Body>
bool root_visitor::visit_struct(const T&, Body&& body) {
root_off = two_pass(fbb, std::forward<Body>(body));
auto off = two_pass(fbb, std::forward<Body>(body));
if(!off) {
return false;
}
root_off = *off;
return true;
}

Expand All @@ -977,14 +1015,18 @@ bool root_visitor::visit_seq(const Container& c, Body&& body) {

template <typename T, typename Body>
bool root_visitor::visit_tuple(const T&, Body&& body) {
root_off = two_pass(fbb, std::forward<Body>(body));
auto off = two_pass(fbb, std::forward<Body>(body));
if(!off) {
return false;
}
root_off = *off;
return true;
}

template <typename Container, typename Body>
bool root_visitor::visit_map(const Container&, Body&& body) {
bool root_visitor::visit_map(const Container& m, Body&& body) {
uoffset_t vec_off = 0;
KOTA_CODEC_TRY(encode_sorted_map(fbb, std::forward<Body>(body), vec_off));
KOTA_CODEC_TRY(encode_sorted_map(fbb, m, std::forward<Body>(body), vec_off));

auto start = fbb.StartTable();
fbb.AddOffset(detail::first_field, offset_t<void>(vec_off));
Expand Down
Loading
Loading