Skip to content

Commit 3e7d11e

Browse files
Александр Сергеевич Целуйкоclaude
Александр Сергеевич Целуйко
and
claude
committed
Keep int bounds exact, like the Decimal ones
`ensure_is_finite_numeric` converted int bounds with `PyLong_AsDouble` and never checked the result. For an int outside the range of a double the `OverflowError` was left pending and the function still returned true, so `Meta(gt=10**400)` surfaced as `SystemError`, and for `multiple_of` the `-1.0` sentinel tripped the `<= 0` check and reported a large positive bound as non-positive. On this branch the misreport got worse: `Meta(gt=10**400, lt=Decimal("5"))` blamed `decimal.Decimal`. Rejecting those ints is not an option here, because `Meta` already treats the two spellings as one value: `Meta(gt=10**400)` and `Meta(gt=Decimal("1E+400"))` compare equal, hash equal, and collapse to a single object inside `Annotated`. Accepting one and refusing the other would also contradict this branch's own argument that a double is the wrong gate for large-but-finite bounds. So drop the double entirely for ints. Every int is finite, and the only remaining check, positivity for `multiple_of`, is done exactly against zero, which is what the Decimal branch already does; that block is factored out into `ensure_is_positive` and shared. Whether a bound is usable stays a per-type question, decided where the annotated type is known, as it already was for `Meta(gt=2**64)`: int64 for `int` targets, float64 for `float` targets, exact for `Decimal` targets. `_constr_as_f64` grows the missing half of that, turning the overflow into the same kind of message `_constr_as_i64` already gives instead of relying on an invariant that only held because the bug crashed first. Co-Authored-By: Claude Opus 5 <[email protected]>
1 parent f71e100 commit 3e7d11e

3 files changed

Lines changed: 100 additions & 27 deletions

File tree

‎docs/constraints.rst‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,14 @@ These constraints are valid on `int`, `float`, or `decimal.Decimal` types:
125125
``Decimal("0.1")``). Use an ``int`` or ``float`` literal instead, or change
126126
the annotated type to `decimal.Decimal`.
127127

128+
.. note::
129+
130+
``int`` bounds are kept exact on `decimal.Decimal` types as well, so a
131+
bound too large for a ``float`` to represent (e.g. ``ge=10**400``) is
132+
valid there. On ``int`` types a bound must still fit in an int64, and on
133+
``float`` types it must fit in a float64; a bound that doesn't raises a
134+
`ValueError` when the annotation is used.
135+
128136
String Constraints
129137
------------------
130138

‎src/msgspec/_core.c‎

Lines changed: 45 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1730,15 +1730,36 @@ ensure_is_nonnegative_integer(PyObject *val, const char *param) {
17301730
return false;
17311731
}
17321732

1733+
static bool
1734+
ensure_is_positive(PyObject *val, const char *param) {
1735+
PyObject *zero = PyLong_FromLong(0);
1736+
if (zero == NULL) return false;
1737+
int above_zero = PyObject_RichCompareBool(val, zero, Py_GT);
1738+
Py_DECREF(zero);
1739+
if (MS_UNLIKELY(above_zero < 0)) return false;
1740+
if (!above_zero) {
1741+
PyErr_Format(PyExc_ValueError, "`%s` must be > 0", param);
1742+
return false;
1743+
}
1744+
return true;
1745+
}
1746+
17331747
static bool
17341748
ensure_is_finite_numeric(PyObject *val, const char *param, bool positive) {
17351749
MsgspecState *mod = msgspec_get_global_state();
1736-
double x;
17371750
if (PyLong_CheckExact(val)) {
1738-
x = PyLong_AsDouble(val);
1751+
/* Every int is finite, so the only check left is positivity, and it
1752+
* is done exactly. Going through a double would overflow for ints
1753+
* outside the range of a double, leaving an `OverflowError` pending
1754+
* and (for `multiple_of`) reporting a large positive bound as
1755+
* non-positive. Whether a bound is *usable* is not decided here but
1756+
* downstream, per annotated type - `Meta(gt=2**64)` is already
1757+
* accepted here and rejected only for `int` types. */
1758+
if (positive && !ensure_is_positive(val, param)) return false;
1759+
return true;
17391760
}
17401761
else if (PyFloat_CheckExact(val)) {
1741-
x = PyFloat_AS_DOUBLE(val);
1762+
double x = PyFloat_AS_DOUBLE(val);
17421763
if (!isfinite(x)) {
17431764
PyErr_Format(
17441765
PyExc_ValueError,
@@ -1747,6 +1768,11 @@ ensure_is_finite_numeric(PyObject *val, const char *param, bool positive) {
17471768
);
17481769
return false;
17491770
}
1771+
if (positive && x <= 0) {
1772+
PyErr_Format(PyExc_ValueError, "`%s` must be > 0", param);
1773+
return false;
1774+
}
1775+
return true;
17501776
}
17511777
else {
17521778
int is_decimal = PyObject_IsInstance(val, mod->DecimalType);
@@ -1792,32 +1818,13 @@ ensure_is_finite_numeric(PyObject *val, const char *param, bool positive) {
17921818
return false;
17931819
}
17941820

1795-
if (positive) {
1796-
PyObject *zero = PyLong_FromLong(0);
1797-
if (zero == NULL) {
1798-
Py_DECREF(exact);
1799-
return false;
1800-
}
1801-
int above_zero = PyObject_RichCompareBool(exact, zero, Py_GT);
1802-
Py_DECREF(zero);
1803-
if (MS_UNLIKELY(above_zero < 0)) {
1804-
Py_DECREF(exact);
1805-
return false;
1806-
}
1807-
if (!above_zero) {
1808-
Py_DECREF(exact);
1809-
PyErr_Format(PyExc_ValueError, "`%s` must be > 0", param);
1810-
return false;
1811-
}
1821+
if (positive && !ensure_is_positive(exact, param)) {
1822+
Py_DECREF(exact);
1823+
return false;
18121824
}
18131825
Py_DECREF(exact);
18141826
return true;
18151827
}
1816-
if (positive && x <= 0) {
1817-
PyErr_Format(PyExc_ValueError, "`%s` must be > 0", param);
1818-
return false;
1819-
}
1820-
return true;
18211828
}
18221829

18231830
PyDoc_STRVAR(Meta__doc__,
@@ -3827,8 +3834,19 @@ static bool
38273834
_constr_as_f64(PyObject *obj, double *target, int offset) {
38283835
/* Use PyFloat_AsDouble to also handle integers */
38293836
double x = PyFloat_AsDouble(obj);
3830-
/* Should never be hit, types already checked */
3831-
if (x == -1.0 && PyErr_Occurred()) return false;
3837+
if (x == -1.0 && PyErr_Occurred()) {
3838+
/* `Meta` accepts any int bound; only here is the annotated type known
3839+
* to be a float, so only here can an int bound outside the range of a
3840+
* double be reported against the type it is invalid for. */
3841+
if (!PyErr_ExceptionMatches(PyExc_OverflowError)) return false;
3842+
PyErr_Clear();
3843+
PyErr_SetString(
3844+
PyExc_ValueError,
3845+
"Bounds constraints (`ge`, `le`, ...) that don't fit in a float64 "
3846+
"are not supported for `float` types"
3847+
);
3848+
return false;
3849+
}
38323850
if (offset == 1) {
38333851
x = nextafter(x, DBL_MAX);
38343852
}

‎tests/unit/test_constraints.py‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,30 @@ def test_numeric_fields_accept_decimal_beyond_float_range(self, field, val):
173173
# to a double, so the checks can't go through `float`.
174174
assert getattr(Meta(**{field: Decimal(val)}), field) == Decimal(val)
175175

176+
@pytest.mark.parametrize("field", ["gt", "ge", "lt", "le"])
177+
@pytest.mark.parametrize("val", [10**400, -(10**400)])
178+
def test_numeric_fields_accept_int_beyond_float_range(self, field, val):
179+
# Every int is finite, so bounds outside the range of a double are
180+
# accepted here, exactly like the equal `Decimal` spelling above.
181+
# Whether such a bound is usable is decided per annotated type.
182+
assert getattr(Meta(**{field: val}), field) == val
183+
184+
def test_multiple_of_int_beyond_float_range(self):
185+
# The positivity check is exact: going through a double would report
186+
# this large positive bound as non-positive.
187+
assert Meta(multiple_of=10**400).multiple_of == 10**400
188+
with pytest.raises(ValueError, match=r"`multiple_of` must be > 0"):
189+
Meta(multiple_of=-(10**400))
190+
191+
@pytest.mark.parametrize("field", ["gt", "ge", "lt", "le", "multiple_of"])
192+
def test_equal_int_and_decimal_bounds_are_interchangeable(self, field):
193+
# `Meta` compares (and hashes) these bounds as equal, so `Meta` must
194+
# not accept one spelling and reject the other.
195+
int_meta = Meta(**{field: 10**400})
196+
dec_meta = Meta(**{field: Decimal("1E+400")})
197+
assert int_meta == dec_meta
198+
assert hash(int_meta) == hash(dec_meta)
199+
176200
def test_decimal_subclass_cannot_bypass_bound_checks(self):
177201
class LyingFinite(Decimal):
178202
def is_finite(self):
@@ -314,6 +338,11 @@ def test_invalid_lt_and_le_conflict(self):
314338

315339

316340
class TestIntConstraints:
341+
@pytest.mark.parametrize("name", ["ge", "gt", "le", "lt", "multiple_of"])
342+
def test_bound_constraint_beyond_int64_errors(self, name):
343+
with pytest.raises(ValueError, match="don't fit in an int64"):
344+
msgspec.json.Decoder(Annotated[int, Meta(**{name: 10**400})])
345+
317346
@pytest.mark.parametrize(
318347
"name, bound, good, bad",
319348
[
@@ -395,6 +424,11 @@ def test_bound_constraint_uint64_valid_for_floats(self, name):
395424
typ = Annotated[float, Meta(**{name: 2**63})]
396425
msgspec.json.Decoder(typ)
397426

427+
@pytest.mark.parametrize("name", ["ge", "gt", "le", "lt", "multiple_of"])
428+
def test_bound_constraint_beyond_float64_errors(self, name):
429+
with pytest.raises(ValueError, match="don't fit in a float64"):
430+
msgspec.json.Decoder(Annotated[float, Meta(**{name: 10**400})])
431+
398432
def get_bounds_cases(self, name, bound):
399433
def ceilp1(x):
400434
return int(math.ceil(x + 1))
@@ -540,6 +574,19 @@ class Ex(msgspec.Struct):
540574
with pytest.raises(msgspec.ValidationError):
541575
dec.decode(proto.encode(Ex(Decimal("100"))))
542576

577+
def test_bounds_beyond_float_range(self, proto):
578+
# `Decimal` bounds are kept exact, so an `int` bound outside the range
579+
# of a double works here just like the equal `Decimal` spelling.
580+
# (`typing.Annotated` collapses the two spellings onto one object,
581+
# since `Meta(gt=10**400) == Meta(gt=Decimal("1E+400"))`.)
582+
class Ex(msgspec.Struct):
583+
x: Annotated[Decimal, Meta(gt=10**400)]
584+
585+
dec = proto.Decoder(Ex)
586+
assert dec.decode(proto.encode(Ex(Decimal("1E+500")))).x == Decimal("1E+500")
587+
with pytest.raises(msgspec.ValidationError, match="Expected `Decimal` >"):
588+
dec.decode(proto.encode(Ex(Decimal("1E+300"))))
589+
543590
def test_convert(self):
544591
typ = Annotated[Decimal, Meta(gt=0)]
545592
assert msgspec.convert(Decimal("1"), typ) == Decimal("1")

0 commit comments

Comments
 (0)