Author SHA1 Message Date
weaselbot bf3f2fe810 Avoid nullptr subtraction using intptr_t casts
Andrew's review on the previous fix noted that the nullptr checks produced slightly worse codegen. Replace the pointer subtraction with intptr_t subtraction, which avoids the undefined behaviour of subtracting two null pointers without introducing extra branches.
2026-06-29 15:14:36 -04:00
weaselbot bd53e57b8e Avoid nullptr subtraction when flushing scalars at EOF
When a scalar ends exactly at a chunk boundary, Parser3::parse resets
dataBegin and writeBuf to the new buf at the start of every call. On the
EOF call buf is null, so both pointers become null. The final
flushNumber/flushString then computed len as buf - dataBegin, i.e.
nullptr - nullptr, which is undefined behaviour in C++.

Compute the flush length safely: if dataBegin is null (or, for raw mode,
buf is null), treat the length as zero. Use an empty string literal as a
non-null data pointer for the zero-length, done=true callback so callers
still receive the completion signal.

Add a regression test covering a number that fills its chunk exactly and
is finalized by an EOF call.

Fixes #40
2026-06-29 13:57:58 -04:00
9 changed files with 41 additions and 242 deletions
-9
View File
@@ -68,15 +68,6 @@ int main() {
expectReject(json, "unknown key in strict root");
}
// ---- invalid stack size is rejected without crashing ----
{
RootBuilder b(-1);
char buf[] = "null";
WeaselJsonStatus s = b.feed(buf, sizeof(buf) - 1);
CHECK(s == WeaselJson_REJECT);
printf("ok invalid stack size rejected, not crashed\n");
}
{
std::string json = R"({
"name": "Ada É",
-144
View File
@@ -382,8 +382,6 @@ class SchemagenIntegerBoundaryTest(unittest.TestCase):
("1e-3", "WeaselJson_REJECT", 0),
("1000e-3", "WeaselJson_OK", 1),
("100.0e-2", "WeaselJson_OK", 1),
("0.0001e4", "WeaselJson_OK", 1),
("0.001e3", "WeaselJson_OK", 1),
("123.0", "WeaselJson_OK", 123),
("9e18", "WeaselJson_OK", 9000000000000000000),
("10e18", "WeaselJson_REJECT", 0),
@@ -427,7 +425,6 @@ class SchemagenIntegerBoundaryTest(unittest.TestCase):
('{"age":-9223372036854775809}', "WeaselJson_REJECT", 0),
('{"age":1e3}', "WeaselJson_OK", 1000),
('{"age":2.0}', "WeaselJson_OK", 2),
('{"age":0.0001e4}', "WeaselJson_OK", 1),
('{"age":0.001}', "WeaselJson_REJECT", 0),
(
'{"age":-9223372036854775808.0}',
@@ -471,147 +468,6 @@ class SchemagenIntegerBoundaryTest(unittest.TestCase):
with tempfile.TemporaryDirectory() as tmpdir:
self._compile_harness(tmpdir, schema, harness)
def test_integer_no_quadratic_leading_zero_loop(self):
"""Regression test for issue #34: leading-zero stripping must not be quadratic."""
with tempfile.TemporaryDirectory() as tmpdir:
schema_path = os.path.join(tmpdir, "schema.json")
with open(schema_path, "w") as fp:
json.dump({"type": "integer"}, fp)
result = subprocess.run(
[sys.executable, SCRIPT, schema_path],
capture_output=True,
text=True,
check=False,
)
self.assertEqual(result.returncode, 0, msg=result.stderr)
self.assertIn("parseJsonInt64", result.stdout)
self.assertNotIn("digits.erase(digits.begin())", result.stdout)
def test_integer_large_fractional_leading_zeros(self):
"""Numbers with many leading fractional zeros must parse correctly."""
if not self.compiler:
self.skipTest("C++ compiler not available")
harness = textwrap.dedent(
"""
#include "gen.h"
#include <cstdio>
#include <string>
int main() {
const int n = 100000;
std::string s = std::string("0.") + std::string(n - 1, '0') + "1e" + std::to_string(n);
test_schema::RootBuilder b;
WeaselJsonStatus st = b.feed(s.data(), static_cast<int>(s.size()));
st = b.finish();
if (st != WeaselJson_OK) {
std::printf("expected OK, got %d\\n", st);
return 1;
}
if (b.take() != 1) {
std::printf("expected value 1\\n");
return 2;
}
return 0;
}
"""
)
with tempfile.TemporaryDirectory() as tmpdir:
self._compile_harness(tmpdir, {"type": "integer"}, harness)
class SchemagenStringEscapeTest(unittest.TestCase):
"""Regression tests for issue #35: control characters in string literals."""
def setUp(self):
self.repo_root = os.path.dirname(
os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
)
self.include_dir = os.path.join(self.repo_root, "include")
self.compiler = shutil.which("c++")
def generate_and_compile(self, schema):
"""Run schemagen on schema and syntax-check the resulting header."""
with tempfile.TemporaryDirectory() as tmpdir:
schema_path = os.path.join(tmpdir, "schema.json")
with open(schema_path, "w") as fp:
json.dump(schema, fp)
header_path = os.path.join(tmpdir, "gen.h")
cmd = [
sys.executable,
SCRIPT,
schema_path,
"-o",
header_path,
"--namespace",
"test_schema",
]
result = subprocess.run(cmd, capture_output=True, text=True, check=False)
self.assertEqual(result.returncode, 0, msg=result.stderr)
if self.compiler:
cpp_path = os.path.join(tmpdir, "test.cpp")
with open(cpp_path, "w") as fp:
fp.write(
'#include "gen.h"\n'
"int main() {\n"
" test_schema::RootBuilder b;\n"
" test_schema::Root r = b.take();\n"
" (void)r;\n"
"}\n"
)
comp = subprocess.run(
[
self.compiler,
"-std=c++20",
"-fsyntax-only",
"-I",
self.include_dir,
"-I",
tmpdir,
cpp_path,
],
capture_output=True,
text=True,
check=False,
)
self.assertEqual(comp.returncode, 0, msg=comp.stderr)
with open(header_path) as fp:
return fp.read()
def test_newline_in_property_key(self):
"""A JSON key containing a newline must become a valid C++ literal."""
schema = {"type": "object", "properties": {"a\nb": {"type": "string"}}}
out = self.generate_and_compile(schema)
self.assertIn('// "a\\nb"', out)
self.assertIn('if (key == "a\\nb") return 0;', out)
def test_newline_in_enum_value(self):
"""An enum value containing a newline must become a valid C++ literal."""
schema = {"type": "object", "properties": {"x": {"enum": ["a\nb"]}}}
out = self.generate_and_compile(schema)
self.assertIn('static constexpr const char *X_names[] = { "a\\nb" };', out)
def test_mixed_control_chars_in_enum_value(self):
"""Mixed control characters in an enum value must be escaped."""
schema = {
"type": "object",
"properties": {
"x": {"enum": ["x\ny\rz\tw\vq\x00\x01"]},
},
}
out = self.generate_and_compile(schema)
self.assertIn(
'static constexpr const char *X_names[] = { "x\\ny\\rz\\tw\\vq\\u0000\\u0001" };',
out,
)
def test_backslash_and_quote_still_escaped(self):
"""Existing escaping for backslash and double quote must remain correct."""
schema = {"type": "object", "properties": {'a"b\\c': {"type": "string"}}}
out = self.generate_and_compile(schema)
self.assertIn('// "a\\"b\\\\c"', out)
self.assertIn('if (key == "a\\"b\\\\c") return 0;', out)
if __name__ == "__main__":
unittest.main()
+8 -49
View File
@@ -18,43 +18,6 @@ import keyword
import sys
def _escape_cpp_string(s):
"""Return *s* escaped for use inside a C++ double-quoted string literal.
JSON strings may contain control characters; emitting them verbatim into
generated C++ source breaks tokenization. This helper escapes backslashes
and double quotes, maps common control characters to their short escape
sequences, and uses universal character names (\\u00XX) for any other
character below 0x20.
"""
out = []
for ch in s:
cp = ord(ch)
if cp == 0x09:
out.append("\\t")
elif cp == 0x0A:
out.append("\\n")
elif cp == 0x0B:
out.append("\\v")
elif cp == 0x0C:
out.append("\\f")
elif cp == 0x0D:
out.append("\\r")
elif cp == 0x08:
out.append("\\b")
elif cp == 0x07:
out.append("\\a")
elif cp < 0x20:
out.append(f"\\u{cp:04X}")
elif ch == "\\":
out.append("\\\\")
elif ch == '"':
out.append('\\"')
else:
out.append(ch)
return "".join(out)
class GenError(Exception):
pass
@@ -655,8 +618,7 @@ namespace {ns} {{"""
and f.ty.kind in ("int", "dbl", "bool")
):
init = " = 0" if f.ty.kind != "bool" else " = false"
esc_key = _escape_cpp_string(f.key)
out.append(f' {store} {f.cpp}{init}; // "{esc_key}"')
out.append(f' {store} {f.cpp}{init}; // "{f.key}"')
out.append("};")
out.append("")
return "\n".join(out)
@@ -784,7 +746,7 @@ namespace {ns} {{"""
for name, obj in self.b.objects.items():
lines.append(f" case Kind::{name}:")
for i, fld in enumerate(obj.fields):
esc = _escape_cpp_string(fld.key)
esc = fld.key.replace("\\", "\\\\").replace('"', '\\"')
lines.append(f' if (key == "{esc}") return {i};')
lines.append(" return -1;")
lines.append(" default: return -1;")
@@ -867,7 +829,10 @@ namespace {ns} {{"""
def _enum_name_arrays(self):
out = []
for e in self.b.enums.values():
lits = ", ".join('"' + _escape_cpp_string(v) + '"' for v in e.values)
lits = ", ".join(
'"' + v.replace("\\", "\\\\").replace('"', '\\"') + '"'
for v in e.values
)
out.append(
f" static constexpr const char *{e.name}_names[] = {{ {lits} }};"
)
@@ -927,10 +892,6 @@ public:
explicit RootBuilder(int stackSize = 1024) {{
cb_ = makeCallbacks();
parser_ = WeaselJsonParser_create(stackSize, &cb_, this, 0);
if (!parser_) {{
error_ = true;
return;
}}
{self._ctor_body()}
}}
~RootBuilder() {{ if (parser_) WeaselJsonParser_destroy(parser_); }}
@@ -1035,10 +996,8 @@ private:
}}
int64_t finalExp = exp - fracDigits + trim;
if (finalExp < 0) return false;
size_t leadingZeros = 0;
while (leadingZeros < digits.size() && digits[leadingZeros] == '0') ++leadingZeros;
if (leadingZeros == digits.size()) {{ out = 0; return true; }}
if (leadingZeros > 0) digits.erase(0, leadingZeros);
while (!digits.empty() && digits.front() == '0') digits.erase(digits.begin());
if (digits.empty()) {{ out = 0; return true; }}
constexpr uint64_t kMaxNeg = 9223372036854775808ULL;
constexpr uint64_t kMaxPos = 9223372036854775807ULL;
-1
View File
@@ -1,7 +1,6 @@
#pragma once
#include <cstddef>
#include <cstdint>
#include <map>
#include <memory>
#include <optional>
-6
View File
@@ -29,17 +29,11 @@ WeaselJsonParser_create(int stackSize, const WeaselJsonCallbacks *callbacks,
__attribute__((visibility("default"))) void
WeaselJsonParser_reset(WeaselJsonParser *parser) {
if (parser == nullptr) {
return;
}
((Parser3 *)parser)->reset();
}
__attribute__((visibility("default"))) void
WeaselJsonParser_destroy(WeaselJsonParser *parser) {
if (parser == nullptr) {
return;
}
((Parser3 *)parser)->~Parser3();
free(parser);
}
+8 -6
View File
@@ -83,26 +83,28 @@ struct Parser3 {
[[nodiscard]] WeaselJsonStatus parse(char *buf, int len);
void flushNumber(bool done, char *buf) {
int len = buf - dataBegin;
int len = (intptr_t)buf - (intptr_t)dataBegin;
assert(len >= 0);
if (done || len > 0) {
callbacks->on_number_data(userdata, dataBegin, len, done);
callbacks->on_number_data(userdata, dataBegin ? dataBegin : "", len,
done);
}
}
void flushString(bool done, char *buf) {
int len;
if (!(flags & WeaselJsonRaw)) {
len = writeBuf - dataBegin;
len = (intptr_t)writeBuf - (intptr_t)dataBegin;
} else {
len = buf - dataBegin;
len = (intptr_t)buf - (intptr_t)dataBegin;
}
assert(len >= 0);
if (done || len > 0) {
const char *data = dataBegin ? dataBegin : "";
if (inKey) {
callbacks->on_key_data(userdata, dataBegin, len, done);
callbacks->on_key_data(userdata, data, len, done);
} else {
callbacks->on_string_data(userdata, dataBegin, len, done);
callbacks->on_string_data(userdata, data, len, done);
}
}
}
+18 -14
View File
@@ -246,20 +246,6 @@ TEST_CASE("create rejects too-small stack") {
WeaselJsonParser_destroy(parser);
}
TEST_CASE("reset and destroy accept null parser") {
// Creation can legitimately fail and return null. The cleanup functions must
// tolerate a null pointer the same way free(nullptr) is a no-op.
auto c = noopCallbacks();
WeaselJsonParser *parser = WeaselJsonParser_create(-1, &c, nullptr, 0);
REQUIRE(parser == nullptr);
WeaselJsonParser_reset(parser); // must not crash
WeaselJsonParser_destroy(parser); // must not crash
// Calling reset/destroy on literal nullptr directly must also be safe.
WeaselJsonParser_reset(nullptr);
WeaselJsonParser_destroy(nullptr);
}
TEST_CASE("parse rejects negative length") {
auto c = noopCallbacks();
auto *parser = WeaselJsonParser_create(1024, &c, nullptr, 0);
@@ -317,6 +303,24 @@ TEST_CASE("reset clears inKey and transient state") {
WeaselJsonParser_destroy(parser);
}
TEST_CASE("scalar ending at chunk boundary is finalized at EOF") {
// A number whose digits exactly fill the first chunk must not invoke
// undefined behaviour on the EOF call, and must still signal completion.
auto c = serializeCallbacks();
SerializeState state;
auto *parser = WeaselJsonParser_create(1024, &c, &state, 0);
REQUIRE(parser != nullptr);
std::string chunk = "123";
REQUIRE(WeaselJsonParser_parse(parser, chunk.data(), chunk.size()) ==
WeaselJson_AGAIN);
REQUIRE(WeaselJsonParser_parse(parser, nullptr, 0) == WeaselJson_OK);
CHECK(state.result == "(123)");
WeaselJsonParser_destroy(parser);
}
void doTestUnescapingUtf8(std::string const &escaped,
std::string const &expected, int stride, int flags) {
CAPTURE(escaped);
-12
View File
@@ -95,20 +95,8 @@ def test_create_rejects_too_small_stack():
raise AssertionError(f"expected ValueError for stackSize={stack_size}")
def test_missing_library_raises_oserror():
try:
weaseljson.WeaselJsonParser(
weaseljson.WeaselJsonCallbacksBase(),
build_dir="/nonexistent",
)
except OSError:
return
raise AssertionError("expected OSError when the shared library is missing")
if __name__ == "__main__":
test_object_keys_routed_correctly()
test_mixed_values()
test_create_rejects_too_small_stack()
test_missing_library_raises_oserror()
print("python bindings ok")
+7 -1
View File
@@ -84,7 +84,13 @@ class WeaselJsonParser:
pass
if self._lib is None:
raise OSError(f"Could not load libweaseljson from {build_dir}")
import sys
print(
"Could not find libweaseljson implementation",
file=sys.stderr,
)
sys.exit(1)
self._lib.WeaselJsonParser_create.argtypes = (
ctypes.c_int,