Compare commits

..
Author SHA1 Message Date
ChuckBuildsandClaude Opus 5 6eaa50fd94 test(web): assert 400 exactly, and prove valid input is accepted
Both review points were right, and the first is the failure mode this file
exists to catch.

Accepting any 4xx meant a 404 would have passed. Renaming one of these routes
would have left the test green while it tested nothing -- the same "looks like
coverage, points somewhere safe" shape that hid the composer injections. Now
asserts exactly 400.

Both infinity signs are exercised for every route. int() raises OverflowError
either way, but only +Infinity was in the original report, and a guard that
special-cased the sign would have passed a one-sided test.

The valid-input test previously asserted "not a 400", which did not show what
it claimed: the mocked save path fails for any input, so that assertion held
whether or not validation had accepted the value. It now gives load_config a
real dict and stubs _save_config_atomic, so the endpoint reaches its success
response and the test can assert 200 -- which only happens if the value passed
validation.

8 of the 11 checks fail with OverflowError removed from the except tuples.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
2026-08-22 16:47:33 -04:00
ChuckBuildsandClaude Opus 5 0ca702bebf fix(web): reject non-finite JSON numbers instead of raising
POST /api/v3/config/dim-schedule with {"dim_brightness": Infinity} answered
500. So did /api/v3/errors/clear with max_age_hours, and /api/v3/config/main
with multiplexing or row_address_type.

json.loads accepts Infinity/-Infinity/NaN by default -- they are not valid
JSON, but Python's parser emits them -- and Flask's get_json passes them
straight through. int(float('inf')) raises OverflowError, which is neither
ValueError nor TypeError, so validation blocks that carefully caught those let
it past and Flask turned it into a 500.

The status code was not the real damage. dim-schedule answered with
CONFIG_SAVE_FAILED and suggested "Check file permissions on config directory"
and "Check available disk space" for what was an invalid number. Every one of
these sites already had a correct 400 response written; they just never
reached it.

NaN already returned 400, because int(nan) raises ValueError. That is why this
only ever showed up for the infinities, and why it survived: the obvious test
case passes.

OverflowError is now caught alongside ValueError/TypeError at the 27 sites in
this file whose try block performs a numeric coercion. An AST sweep confirms
no int()/float() of request-derived data is left outside a block that catches
it.

Verified end to end through Flask's test client rather than by reasoning about
the parser: all four routes returned 500 before and 400 after.

Tests: five Infinity cases (which fail against the previous except tuples),
two NaN cases pinned so narrowing the tuple cannot quietly break them, and a
check that ordinary input is not rejected by the widened guard.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
2026-08-22 16:09:45 -04:00
3 changed files with 114 additions and 35 deletions
+85
View File
@@ -0,0 +1,85 @@
"""Non-finite JSON numbers must be rejected, not raise.
json.loads accepts Infinity/-Infinity/NaN by default (they are not valid JSON,
but Python's parser emits them) and Flask's get_json passes them straight
through. int(float('inf')) raises OverflowError, which is neither ValueError
nor TypeError -- so validation blocks that carefully caught those let it
through and Flask turned it into a 500.
The damage was not the status code. /config/dim-schedule answered with
CONFIG_SAVE_FAILED and suggested "Check file permissions on config directory"
and "Check available disk space" for what was actually an invalid number.
NaN already returned 400 (int(nan) raises ValueError), which is why this only
showed up for the infinities.
"""
import sys
from pathlib import Path
import pytest
sys.path.insert(0, str(Path(__file__).parent.parent))
from test._api_v3_test_helpers import api_v3_client, api_v3_module # noqa: F401,E402
#: (route, field) that returned 500 before OverflowError was caught. Both
#: infinity signs are exercised: int() raises OverflowError for either, but
#: only one of them was in the original report, and a guard that special-cased
#: the sign would pass a one-sided test.
NON_FINITE_ROUTES = [
('/api/v3/config/dim-schedule', 'dim_brightness'),
('/api/v3/errors/clear', 'max_age_hours'),
('/api/v3/config/main', 'multiplexing'),
('/api/v3/config/main', 'row_address_type'),
]
NON_FINITE_CASES = [
(route, '{"%s": %s}' % (field, literal))
for route, field in NON_FINITE_ROUTES
for literal in ('Infinity', '-Infinity')
]
@pytest.mark.parametrize("route,body", NON_FINITE_CASES)
def test_infinity_is_a_client_error_not_a_server_error(api_v3_client, route, body):
"""Exactly 400, not merely "some 4xx".
Accepting any 4xx would let a 404 pass, so renaming one of these routes
would leave the test green while testing nothing -- the failure mode this
whole file exists to catch.
"""
response = api_v3_client.post(route, data=body, content_type='application/json')
assert response.status_code == 400, (
f"{route} with {body} answered {response.status_code}; expected 400"
)
@pytest.mark.parametrize("route,body", [
('/api/v3/config/dim-schedule', '{"dim_brightness": NaN}'),
('/api/v3/errors/clear', '{"max_age_hours": NaN}'),
])
def test_nan_is_also_a_client_error(api_v3_client, route, body):
"""int(nan) raises ValueError so this path already worked -- pinned so a
refactor that narrows the except tuple cannot quietly break it."""
response = api_v3_client.post(route, data=body, content_type='application/json')
assert response.status_code == 400
def test_a_valid_number_is_accepted(api_v3_client, api_v3_module, monkeypatch):
"""Prove the widened except did not start swallowing ordinary input.
Asserting "not a 400" would not show that: the mocked save path fails for
any input, so the assertion would hold even if validation had rejected the
value. Give load_config a real dict and stub the atomic save, and the
endpoint reaches its success response -- which only happens if 30 passed
validation.
"""
api_v3_module.api_v3.config_manager.load_config.return_value = {}
monkeypatch.setattr(api_v3_module, '_save_config_atomic',
lambda *a, **k: (True, ''))
response = api_v3_client.post(
'/api/v3/config/dim-schedule',
data='{"dim_brightness": 30}',
content_type='application/json',
)
assert response.status_code == 200, response.get_data(as_text=True)[:200]
+2 -8
View File
@@ -89,17 +89,11 @@ class TestContextualFormatter:
assert "hello" in out assert "hello" in out
def test_location_toggle(self): def test_location_toggle(self):
# Assert on the whole "module.func:lineno" token, not a bare ":42".
# The formatted line starts with an HH:MM:SS timestamp, so a bare
# ":{lineno}" also matches the clock whenever the minute or second
# happens to equal the line number -- about 3% of runs, which is a
# flaky failure with nothing wrong.
record = make_record() record = make_record()
location = f"{record.module}.{record.funcName}:{record.lineno}"
with_loc = ContextualFormatter(include_location=True).format(record) with_loc = ContextualFormatter(include_location=True).format(record)
without = ContextualFormatter(include_location=False).format(record) without = ContextualFormatter(include_location=False).format(record)
assert location in with_loc assert f":{record.lineno}" in with_loc
assert location not in without assert f":{record.lineno}" not in without
def test_record_not_mutated_no_double_prefix(self): def test_record_not_mutated_no_double_prefix(self):
# Regression: a record is formatted once PER HANDLER. The formatter # Regression: a record is formatted once PER HANDLER. The formatter
+27 -27
View File
@@ -597,7 +597,7 @@ def save_dim_schedule_config():
dim_brightness = 30 dim_brightness = 30
else: else:
dim_brightness = int(dim_brightness_raw) dim_brightness = int(dim_brightness_raw)
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
return error_response( return error_response(
ErrorCode.VALIDATION_ERROR, ErrorCode.VALIDATION_ERROR,
"dim_brightness must be an integer between 0 and 100", "dim_brightness must be an integer between 0 and 100",
@@ -797,7 +797,7 @@ def save_main_config():
}), 400 }), 400
try: try:
target_fps = int(raw_target_fps) target_fps = int(raw_target_fps)
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
return jsonify({ return jsonify({
'status': 'error', 'status': 'error',
'message': "Invalid value for target_fps: must be an integer" 'message': "Invalid value for target_fps: must be an integer"
@@ -867,7 +867,7 @@ def save_main_config():
mux_val = int(data['multiplexing']) mux_val = int(data['multiplexing'])
if mux_val < 0 or mux_val > 22: if mux_val < 0 or mux_val > 22:
return jsonify({'status': 'error', 'message': f"Invalid multiplexing value '{data['multiplexing']}'. Must be an integer from 0 to 22."}), 400 return jsonify({'status': 'error', 'message': f"Invalid multiplexing value '{data['multiplexing']}'. Must be an integer from 0 to 22."}), 400
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
return jsonify({'status': 'error', 'message': f"Invalid multiplexing value '{data['multiplexing']}'. Must be an integer from 0 to 22."}), 400 return jsonify({'status': 'error', 'message': f"Invalid multiplexing value '{data['multiplexing']}'. Must be an integer from 0 to 22."}), 400
# Validate pixel_mapper_config (free-form mapper string, e.g. "U-mapper;Rotate:90") # Validate pixel_mapper_config (free-form mapper string, e.g. "U-mapper;Rotate:90")
@@ -885,7 +885,7 @@ def save_main_config():
rat_val = int(data['row_address_type']) rat_val = int(data['row_address_type'])
if rat_val < 0 or rat_val > 4: if rat_val < 0 or rat_val > 4:
return jsonify({'status': 'error', 'message': f"Invalid row_address_type '{data['row_address_type']}'. Must be an integer from 0 to 4."}), 400 return jsonify({'status': 'error', 'message': f"Invalid row_address_type '{data['row_address_type']}'. Must be an integer from 0 to 4."}), 400
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
return jsonify({'status': 'error', 'message': f"Invalid row_address_type '{data['row_address_type']}'. Must be an integer from 0 to 4."}), 400 return jsonify({'status': 'error', 'message': f"Invalid row_address_type '{data['row_address_type']}'. Must be an integer from 0 to 4."}), 400
# Handle hardware settings # Handle hardware settings
@@ -910,7 +910,7 @@ def save_main_config():
if rp1_val not in (0, 1): if rp1_val not in (0, 1):
return jsonify({'status': 'error', 'message': "rp1_rio must be 0 (PIO) or 1 (RIO)"}), 400 return jsonify({'status': 'error', 'message': "rp1_rio must be 0 (PIO) or 1 (RIO)"}), 400
current_config['display']['runtime']['rp1_rio'] = rp1_val current_config['display']['runtime']['rp1_rio'] = rp1_val
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
return jsonify({'status': 'error', 'message': "rp1_rio must be 0 or 1"}), 400 return jsonify({'status': 'error', 'message': "rp1_rio must be 0 or 1"}), 400
# Handle checkboxes - coerce to bool to ensure proper JSON types # Handle checkboxes - coerce to bool to ensure proper JSON types
@@ -963,7 +963,7 @@ def save_main_config():
copies = None copies = None
try: try:
copies = int(data['double_sided_copies']) copies = int(data['double_sided_copies'])
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
if enabled: if enabled:
return jsonify({'status': 'error', 'message': "Double-sided copies must be an integer"}), 400 return jsonify({'status': 'error', 'message': "Double-sided copies must be an integer"}), 400
if copies is not None and not (2 <= copies <= 8): if copies is not None and not (2 <= copies <= 8):
@@ -1036,7 +1036,7 @@ def save_main_config():
if data.get('vegas_extend_threshold_screens') not in ('', None): if data.get('vegas_extend_threshold_screens') not in ('', None):
try: try:
screens = float(data['vegas_extend_threshold_screens']) screens = float(data['vegas_extend_threshold_screens'])
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
return jsonify({ return jsonify({
'status': 'error', 'status': 'error',
'message': "Invalid value for vegas_extend_threshold_screens: " 'message': "Invalid value for vegas_extend_threshold_screens: "
@@ -1053,7 +1053,7 @@ def save_main_config():
if data.get('vegas_max_plugin_width_ratio') not in ('', None): if data.get('vegas_max_plugin_width_ratio') not in ('', None):
try: try:
ratio = float(data['vegas_max_plugin_width_ratio']) ratio = float(data['vegas_max_plugin_width_ratio'])
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
return jsonify({ return jsonify({
'status': 'error', 'status': 'error',
'message': "Invalid value for vegas_max_plugin_width_ratio: " 'message': "Invalid value for vegas_max_plugin_width_ratio: "
@@ -1101,7 +1101,7 @@ def save_main_config():
continue continue
try: try:
int_value = int(raw_value) int_value = int(raw_value)
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
return jsonify({ return jsonify({
'status': 'error', 'status': 'error',
'message': f"Invalid value for {field_name}: must be an integer" 'message': f"Invalid value for {field_name}: must be an integer"
@@ -1153,7 +1153,7 @@ def save_main_config():
if not (1024 <= port_val <= 65535): if not (1024 <= port_val <= 65535):
return jsonify({'status': 'error', 'message': "sync_port must be between 1024 and 65535"}), 400 return jsonify({'status': 'error', 'message': "sync_port must be between 1024 and 65535"}), 400
current_config['sync']['port'] = port_val current_config['sync']['port'] = port_val
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
return jsonify({'status': 'error', 'message': "sync_port must be an integer"}), 400 return jsonify({'status': 'error', 'message': "sync_port must be an integer"}), 400
if "sync_follower_position" in data: if "sync_follower_position" in data:
@@ -1197,7 +1197,7 @@ def save_main_config():
raw_value = data.pop(field) raw_value = data.pop(field)
try: try:
int_value = int(raw_value) int_value = int(raw_value)
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
return jsonify({'status': 'error', return jsonify({'status': 'error',
'message': f"Invalid duration for {field}: must be an integer"}), 400 'message': f"Invalid duration for {field}: must be an integer"}), 400
current_config['display']['display_durations'][field] = int_value current_config['display']['display_durations'][field] = int_value
@@ -1220,7 +1220,7 @@ def save_main_config():
continue continue
try: try:
int_value = int(raw_value) int_value = int(raw_value)
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
return jsonify({'status': 'error', return jsonify({'status': 'error',
'message': f"Invalid duration for mode '{mode_key}': must be an integer"}), 400 'message': f"Invalid duration for mode '{mode_key}': must be an integer"}), 400
current_config['display']['display_durations'][mode_key] = int_value current_config['display']['display_durations'][mode_key] = int_value
@@ -5118,7 +5118,7 @@ def save_plugin_config():
converted_array.append(int(v)) converted_array.append(int(v))
else: else:
converted_array.append(float(v)) converted_array.append(float(v))
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
converted_array.append(v) converted_array.append(v)
else: else:
converted_array.append(v) converted_array.append(v)
@@ -5143,7 +5143,7 @@ def save_plugin_config():
converted_array.append(int(v)) converted_array.append(int(v))
else: else:
converted_array.append(float(v)) converted_array.append(float(v))
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
converted_array.append(v) converted_array.append(v)
else: else:
converted_array.append(v) converted_array.append(v)
@@ -5180,7 +5180,7 @@ def save_plugin_config():
converted_array.append(int(v)) converted_array.append(int(v))
else: else:
converted_array.append(float(v)) converted_array.append(float(v))
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
converted_array.append(v) converted_array.append(v)
else: else:
converted_array.append(v) converted_array.append(v)
@@ -5204,7 +5204,7 @@ def save_plugin_config():
converted_array.append(int(v)) converted_array.append(int(v))
else: else:
converted_array.append(float(v)) converted_array.append(float(v))
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
converted_array.append(v) converted_array.append(v)
else: else:
converted_array.append(v) converted_array.append(v)
@@ -5371,7 +5371,7 @@ def save_plugin_config():
if isinstance(v, str): if isinstance(v, str):
try: try:
converted.append(int(v) if item_type == 'integer' else float(v)) converted.append(int(v) if item_type == 'integer' else float(v))
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
converted.append(v) converted.append(v)
else: else:
converted.append(v) converted.append(v)
@@ -5496,7 +5496,7 @@ def save_plugin_config():
try: try:
normalized[key] = int(value_stripped) normalized[key] = int(value_stripped)
continue continue
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
pass pass
elif isinstance(value, (int, float)): elif isinstance(value, (int, float)):
normalized[key] = int(value) normalized[key] = int(value)
@@ -5514,7 +5514,7 @@ def save_plugin_config():
try: try:
normalized[key] = float(value_stripped) normalized[key] = float(value_stripped)
continue continue
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
pass pass
elif isinstance(value, (int, float)): elif isinstance(value, (int, float)):
normalized[key] = float(value) normalized[key] = float(value)
@@ -5569,7 +5569,7 @@ def save_plugin_config():
try: try:
normalized_array.append(int(v)) normalized_array.append(int(v))
continue continue
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
pass pass
elif isinstance(v, (int, float)): elif isinstance(v, (int, float)):
normalized_array.append(int(v)) normalized_array.append(int(v))
@@ -5579,7 +5579,7 @@ def save_plugin_config():
try: try:
normalized_array.append(float(v)) normalized_array.append(float(v))
continue continue
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
pass pass
elif isinstance(v, (int, float)): elif isinstance(v, (int, float)):
normalized_array.append(float(v)) normalized_array.append(float(v))
@@ -5595,7 +5595,7 @@ def save_plugin_config():
if isinstance(v, str): if isinstance(v, str):
try: try:
normalized_array.append(int(v)) normalized_array.append(int(v))
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
normalized_array.append(v) normalized_array.append(v)
elif isinstance(v, (int, float)): elif isinstance(v, (int, float)):
normalized_array.append(int(v)) normalized_array.append(int(v))
@@ -5609,7 +5609,7 @@ def save_plugin_config():
if isinstance(v, str): if isinstance(v, str):
try: try:
normalized_array.append(float(v)) normalized_array.append(float(v))
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
normalized_array.append(v) normalized_array.append(v)
else: else:
normalized_array.append(v) normalized_array.append(v)
@@ -5632,7 +5632,7 @@ def save_plugin_config():
if isinstance(value, str): if isinstance(value, str):
try: try:
normalized[key] = int(value) normalized[key] = int(value)
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
normalized[key] = value normalized[key] = value
else: else:
normalized[key] = value normalized[key] = value
@@ -5641,7 +5641,7 @@ def save_plugin_config():
if isinstance(value, str): if isinstance(value, str):
try: try:
normalized[key] = float(value) normalized[key] = float(value)
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
normalized[key] = value normalized[key] = value
else: else:
normalized[key] = value normalized[key] = value
@@ -6779,7 +6779,7 @@ def get_font_preview() -> tuple[Response, int] | Response:
# Safe integer parsing for size # Safe integer parsing for size
try: try:
size = int(request.args.get('size', 12)) size = int(request.args.get('size', 12))
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
return jsonify({'status': 'error', 'message': 'Invalid font size'}), 400 return jsonify({'status': 'error', 'message': 'Invalid font size'}), 400
if not font_filename: if not font_filename:
@@ -8360,7 +8360,7 @@ def clear_old_errors():
context={'provided_value': raw_max_age}, context={'provided_value': raw_max_age},
status_code=400 status_code=400
) )
except (ValueError, TypeError): except (ValueError, TypeError, OverflowError):
return error_response( return error_response(
error_code=ErrorCode.INVALID_INPUT, error_code=ErrorCode.INVALID_INPUT,
message="max_age_hours must be a valid integer", message="max_age_hours must be a valid integer",