From 0ca702bebfad29295e78462c200d201275bb588a Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Sat, 22 Aug 2026 16:09:45 -0400 Subject: [PATCH 1/2] 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) Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW --- test/test_api_v3_non_finite_numbers.py | 73 ++++++++++++++++++++++++++ web_interface/blueprints/api_v3.py | 54 +++++++++---------- 2 files changed, 100 insertions(+), 27 deletions(-) create mode 100644 test/test_api_v3_non_finite_numbers.py diff --git a/test/test_api_v3_non_finite_numbers.py b/test/test_api_v3_non_finite_numbers.py new file mode 100644 index 00000000..c0392912 --- /dev/null +++ b/test/test_api_v3_non_finite_numbers.py @@ -0,0 +1,73 @@ +"""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, body) pairs that returned 500 before OverflowError was caught. +NON_FINITE_CASES = [ + ('/api/v3/config/dim-schedule', '{"dim_brightness": Infinity}'), + ('/api/v3/config/dim-schedule', '{"dim_brightness": -Infinity}'), + ('/api/v3/errors/clear', '{"max_age_hours": Infinity}'), + ('/api/v3/config/main', '{"multiplexing": Infinity}'), + ('/api/v3/config/main', '{"row_address_type": 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): + response = api_v3_client.post(route, data=body, content_type='application/json') + assert response.status_code != 500, ( + f"{route} with {body} raised instead of validating" + ) + assert 400 <= response.status_code < 500, ( + f"{route} answered {response.status_code}; expected a 4xx" + ) + + +@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 400 <= response.status_code < 500 + + +def test_a_valid_number_is_not_rejected_by_the_guard(api_v3_client): + """The widened except must not start swallowing ordinary input. + + Asserting on 2xx is not possible here: every manager is a MagicMock, so + the save path fails downstream whatever is posted. What this can show is + that a valid number gets past *validation* -- it is not answered with a + 400, and nothing in the response mentions the coercion failing. + """ + response = api_v3_client.post( + '/api/v3/config/dim-schedule', + data='{"dim_brightness": 30}', + content_type='application/json', + ) + assert response.status_code != 400, "a valid brightness was rejected" + assert b'must be an integer' not in response.get_data() + assert b'OverflowError' not in response.get_data() diff --git a/web_interface/blueprints/api_v3.py b/web_interface/blueprints/api_v3.py index 37df1141..c437969c 100644 --- a/web_interface/blueprints/api_v3.py +++ b/web_interface/blueprints/api_v3.py @@ -597,7 +597,7 @@ def save_dim_schedule_config(): dim_brightness = 30 else: dim_brightness = int(dim_brightness_raw) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): return error_response( ErrorCode.VALIDATION_ERROR, "dim_brightness must be an integer between 0 and 100", @@ -797,7 +797,7 @@ def save_main_config(): }), 400 try: target_fps = int(raw_target_fps) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): return jsonify({ 'status': 'error', 'message': "Invalid value for target_fps: must be an integer" @@ -867,7 +867,7 @@ def save_main_config(): mux_val = int(data['multiplexing']) 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 - 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 # 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']) 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 - 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 # Handle hardware settings @@ -910,7 +910,7 @@ def save_main_config(): if rp1_val not in (0, 1): return jsonify({'status': 'error', 'message': "rp1_rio must be 0 (PIO) or 1 (RIO)"}), 400 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 # Handle checkboxes - coerce to bool to ensure proper JSON types @@ -963,7 +963,7 @@ def _copies_fits_hardware(copies: int) -> Optional[str]: copies = None try: copies = int(data['double_sided_copies']) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): if enabled: return jsonify({'status': 'error', 'message': "Double-sided copies must be an integer"}), 400 if copies is not None and not (2 <= copies <= 8): @@ -1036,7 +1036,7 @@ def _copies_fits_hardware(copies: int) -> Optional[str]: if data.get('vegas_extend_threshold_screens') not in ('', None): try: screens = float(data['vegas_extend_threshold_screens']) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): return jsonify({ 'status': 'error', 'message': "Invalid value for vegas_extend_threshold_screens: " @@ -1053,7 +1053,7 @@ def _copies_fits_hardware(copies: int) -> Optional[str]: if data.get('vegas_max_plugin_width_ratio') not in ('', None): try: ratio = float(data['vegas_max_plugin_width_ratio']) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): return jsonify({ 'status': 'error', 'message': "Invalid value for vegas_max_plugin_width_ratio: " @@ -1101,7 +1101,7 @@ def _copies_fits_hardware(copies: int) -> Optional[str]: continue try: int_value = int(raw_value) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): return jsonify({ 'status': 'error', 'message': f"Invalid value for {field_name}: must be an integer" @@ -1153,7 +1153,7 @@ def _copies_fits_hardware(copies: int) -> Optional[str]: if not (1024 <= port_val <= 65535): return jsonify({'status': 'error', 'message': "sync_port must be between 1024 and 65535"}), 400 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 if "sync_follower_position" in data: @@ -1197,7 +1197,7 @@ def _copies_fits_hardware(copies: int) -> Optional[str]: raw_value = data.pop(field) try: int_value = int(raw_value) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): return jsonify({'status': 'error', 'message': f"Invalid duration for {field}: must be an integer"}), 400 current_config['display']['display_durations'][field] = int_value @@ -1220,7 +1220,7 @@ def _copies_fits_hardware(copies: int) -> Optional[str]: continue try: int_value = int(raw_value) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): return jsonify({'status': 'error', 'message': f"Invalid duration for mode '{mode_key}': must be an integer"}), 400 current_config['display']['display_durations'][mode_key] = int_value @@ -5118,7 +5118,7 @@ def fix_array_structures(config_dict, schema_props, prefix=''): converted_array.append(int(v)) else: converted_array.append(float(v)) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): converted_array.append(v) else: converted_array.append(v) @@ -5143,7 +5143,7 @@ def fix_array_structures(config_dict, schema_props, prefix=''): converted_array.append(int(v)) else: converted_array.append(float(v)) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): converted_array.append(v) else: converted_array.append(v) @@ -5180,7 +5180,7 @@ def fix_array_structures(config_dict, schema_props, prefix=''): converted_array.append(int(v)) else: converted_array.append(float(v)) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): converted_array.append(v) else: converted_array.append(v) @@ -5204,7 +5204,7 @@ def fix_array_structures(config_dict, schema_props, prefix=''): converted_array.append(int(v)) else: converted_array.append(float(v)) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): converted_array.append(v) else: converted_array.append(v) @@ -5371,7 +5371,7 @@ def _fix_json_arrays(cfg, props): if isinstance(v, str): try: converted.append(int(v) if item_type == 'integer' else float(v)) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): converted.append(v) else: converted.append(v) @@ -5496,7 +5496,7 @@ def normalize_config_values(config, schema_props, prefix=''): try: normalized[key] = int(value_stripped) continue - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): pass elif isinstance(value, (int, float)): normalized[key] = int(value) @@ -5514,7 +5514,7 @@ def normalize_config_values(config, schema_props, prefix=''): try: normalized[key] = float(value_stripped) continue - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): pass elif isinstance(value, (int, float)): normalized[key] = float(value) @@ -5569,7 +5569,7 @@ def normalize_config_values(config, schema_props, prefix=''): try: normalized_array.append(int(v)) continue - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): pass elif isinstance(v, (int, float)): normalized_array.append(int(v)) @@ -5579,7 +5579,7 @@ def normalize_config_values(config, schema_props, prefix=''): try: normalized_array.append(float(v)) continue - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): pass elif isinstance(v, (int, float)): normalized_array.append(float(v)) @@ -5595,7 +5595,7 @@ def normalize_config_values(config, schema_props, prefix=''): if isinstance(v, str): try: normalized_array.append(int(v)) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): normalized_array.append(v) elif isinstance(v, (int, float)): normalized_array.append(int(v)) @@ -5609,7 +5609,7 @@ def normalize_config_values(config, schema_props, prefix=''): if isinstance(v, str): try: normalized_array.append(float(v)) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): normalized_array.append(v) else: normalized_array.append(v) @@ -5632,7 +5632,7 @@ def normalize_config_values(config, schema_props, prefix=''): if isinstance(value, str): try: normalized[key] = int(value) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): normalized[key] = value else: normalized[key] = value @@ -5641,7 +5641,7 @@ def normalize_config_values(config, schema_props, prefix=''): if isinstance(value, str): try: normalized[key] = float(value) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): normalized[key] = value else: normalized[key] = value @@ -6779,7 +6779,7 @@ def get_font_preview() -> tuple[Response, int] | Response: # Safe integer parsing for size try: size = int(request.args.get('size', 12)) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): return jsonify({'status': 'error', 'message': 'Invalid font size'}), 400 if not font_filename: @@ -8360,7 +8360,7 @@ def clear_old_errors(): context={'provided_value': raw_max_age}, status_code=400 ) - except (ValueError, TypeError): + except (ValueError, TypeError, OverflowError): return error_response( error_code=ErrorCode.INVALID_INPUT, message="max_age_hours must be a valid integer", From 6eaa50fd94620198e3072b6f5137e9a814de96d4 Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Sat, 22 Aug 2026 16:47:33 -0400 Subject: [PATCH 2/2] 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) Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW --- test/test_api_v3_non_finite_numbers.py | 54 ++++++++++++++++---------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/test/test_api_v3_non_finite_numbers.py b/test/test_api_v3_non_finite_numbers.py index c0392912..4e6c217b 100644 --- a/test/test_api_v3_non_finite_numbers.py +++ b/test/test_api_v3_non_finite_numbers.py @@ -23,24 +23,34 @@ from test._api_v3_test_helpers import api_v3_client, api_v3_module # noqa: F401,E402 -#: (route, body) pairs that returned 500 before OverflowError was caught. +#: (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 = [ - ('/api/v3/config/dim-schedule', '{"dim_brightness": Infinity}'), - ('/api/v3/config/dim-schedule', '{"dim_brightness": -Infinity}'), - ('/api/v3/errors/clear', '{"max_age_hours": Infinity}'), - ('/api/v3/config/main', '{"multiplexing": Infinity}'), - ('/api/v3/config/main', '{"row_address_type": Infinity}'), + (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 != 500, ( - f"{route} with {body} raised instead of validating" - ) - assert 400 <= response.status_code < 500, ( - f"{route} answered {response.status_code}; expected a 4xx" + assert response.status_code == 400, ( + f"{route} with {body} answered {response.status_code}; expected 400" ) @@ -52,22 +62,24 @@ 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 400 <= response.status_code < 500 + assert response.status_code == 400 -def test_a_valid_number_is_not_rejected_by_the_guard(api_v3_client): - """The widened except must not start swallowing ordinary input. +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 on 2xx is not possible here: every manager is a MagicMock, so - the save path fails downstream whatever is posted. What this can show is - that a valid number gets past *validation* -- it is not answered with a - 400, and nothing in the response mentions the coercion failing. + 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 != 400, "a valid brightness was rejected" - assert b'must be an integer' not in response.get_data() - assert b'OverflowError' not in response.get_data() + assert response.status_code == 200, response.get_data(as_text=True)[:200]