fix: crash/robustness — malformed project/library JSON, S-param path containment, touchstone OOM caps - #58
Merged
Conversation
…containment, touchstone OOM caps
C1: ProjectSerializer::load() caught only JSON *parse* errors; wrong-shape but valid JSON ({}, "components": 5, wrong-typed params, non-object window_state) threw uncaught nlohmann exceptions and std::terminated. Now: 64 MiB size cap before parse, root must be an object, whole load body wrapped (json::exception -> LOG_ERROR + graceful failure), each component deserialize isolated so one bad component is skipped and the rest load. Same crash class fixed at the library boundary (component_library loadFile catches type_error; scan tolerates filesystem_error).
B7: coax deserialize clamped preset_index/length_m/connectors_loss_dB (previously OOB index into kCoaxCablePresets from a corrupted project -> UB).
S1: S-param paths in project files resolve against the project dir and must stay inside it (weakly_canonical + prefix, mirroring the extension manifest parser); escaping/absolute-outside paths are neutralized with a warning. Library data_files entries get the same containment. Save re-relativizes in-project absolute paths for portability.
S2: touchstone parser now rejects > 256 MiB files before reading and enforces the 10M-point cap during the read loop, not after buffering (~960 MB OOM window removed).
Tests: wrong-shape JSON cases, coax clamp, new test_path_containment standalone exe (5 cases), #56 fixture staged in-project.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Crash/robustness fixes from a comprehensive codebase review (findings C1, B7, S1, S2).
ProjectSerializer::load()caught only JSON parse errors; wrong-shape-but-valid JSON ({},"components": 5, wrong-typed params, non-objectwindow_state) threw uncaught nlohmann exceptions andstd::terminated. Now: 64 MiB size cap before parse, root must be an object, the whole load body is guarded (json::exception→ log + graceful failure), and each componentdeserializeis isolated so one bad component is skipped while the rest load. Same crash class fixed at the library boundary (component_libraryloadFilecatchestype_error; scan toleratesfilesystem_error).deserializeclampedpreset_index/length_m/connectors_loss_dB— previously an out-of-range index intokCoaxCablePresetsfrom a corrupted project was UB.weakly_canonical+ prefix, mirroring the extension manifest parser). Absolute/escaping paths are neutralized with a warning. Librarydata_filesentries get the same containment. Save re-relativizes in-project absolute paths so projects stay portable.Verification
tests.exe: 218 test cases / 65534 assertions, all pass (216 baseline + 2 new wrong-shape JSON cases).test_path_containmentexe: 5 cases / 29 assertions.Notes for merging
.s2pdata inside the project directory. Relative paths now resolve against the project dir (portable) instead of the process CWD.refactor/engine-unificationintests/test_project_file.cpp(the Fix S-param mode not reloading on project deserialize #56 test's attenuator/combiner lines: this branch stages the fixture with the old API names, that branch renames the API). Merge this branch first; resolve by keepingsetSParamFilepath(local_s2p)/sparamMode().