From 65360562d8ef2f096d34e44c47aa4a55c47908a7 Mon Sep 17 00:00:00 2001 From: Steven Atkinson Date: Sun, 9 Aug 2026 12:51:39 -0700 Subject: [PATCH 1/4] test: cover issue #314 --- tools/run_tests.cpp | 2 ++ tools/test/test_get_dsp.cpp | 54 +++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/tools/run_tests.cpp b/tools/run_tests.cpp index 5699bf78..6ecdfebd 100644 --- a/tools/run_tests.cpp +++ b/tools/run_tests.cpp @@ -297,6 +297,8 @@ int main() test_get_dsp::test_gets_input_level(); test_get_dsp::test_gets_output_level(); + test_get_dsp::test_empty_nam_file_throws_validation_error(); + test_get_dsp::test_malformed_nam_file_throws_validation_error(); test_get_dsp::test_null_input_level(); test_get_dsp::test_null_output_level(); test_get_dsp::test_version_patch_one_beyond_supported(); diff --git a/tools/test/test_get_dsp.cpp b/tools/test/test_get_dsp.cpp index 3cfd3687..de20ce84 100644 --- a/tools/test/test_get_dsp.cpp +++ b/tools/test/test_get_dsp.cpp @@ -1,10 +1,13 @@ #include #include #include +#include #include #include #include +#include #include +#include #include #include "json.hpp" @@ -14,6 +17,8 @@ namespace test_get_dsp { +static_assert(std::is_base_of_v); + // Config const std::string basicConfigStr = R"({"version": "0.5.4", "metadata": {"date": {"year": 2024, "month": 10, "day": 9, "hour": 18, "minute": 44, "second": 41}, "loudness": -37.8406867980957, "gain": 0.13508800804658277, "name": "Test LSTM", "modeled_by": "Steve", "gear_type": "amp", "gear_make": "Darkglass Electronics", "gear_model": "Microtubes 900 v2", "tone_type": "clean", "input_level_dbu": 18.3, "output_level_dbu": 12.3, "training": {"settings": {"ignore_checks": false}, "data": {"latency": {"manual": null, "calibration": {"algorithm_version": 1, "delays": [-16], "safety_factor": 1, "recommended": -17, "warnings": {"matches_lookahead": false, "disagreement_too_high": false}}}, "checks": {"version": 3, "passed": true}}, "validation_esr": null}}, "architecture": "LSTM", "config": {"input_size": 1, "hidden_size": 3, "num_layers": 1}, "weights": [-0.21677088737487793, -0.6683622002601624, -0.2560940980911255, -0.3588429093360901, 0.17952610552310944, 0.19445613026618958, -0.01662646047770977, 0.5353694558143616, -0.2536540627479553, -0.5132213234901428, -0.020476307719945908, 0.08592455089092255, -0.6891753673553467, 0.3627359867095947, 0.008421811275184155, 0.3113192617893219, 0.14251480996608734, 0.07989779114723206, -0.18211324512958527, 0.7118963003158569, 0.41084015369415283, -0.6571938395500183, -0.13214066624641418, -0.2698603868484497, 0.49387243390083313, -0.3491725027561188, 0.6353667974472046, -0.5005152225494385, 0.2052856683731079, -0.4301638901233673, -0.15770092606544495, -0.7181791067123413, 0.056290093809366226, -0.49049463868141174, 0.6623441576957703, 0.09029324352741241, 0.34005245566368103, 0.16416560113430023, 0.15520110726356506, -0.4155678153038025, -0.36928507685661316, 0.3211132884025574, -0.6769840121269226, -0.1575538069009781, 0.05268515646457672, -0.4191459119319916, 0.599330484867096, 0.21518059074878693, -4.246325492858887, -3.315647840499878, -4.328850746154785, 4.496089458465576, 5.015639305114746, 3.6492037773132324, 0.14431169629096985, -0.6633821725845337, 0.11673200130462646, -0.1418764889240265, -0.4897872805595398, -0.8689419031143188, -0.06714004278182983, -0.4450395107269287, -0.02142983116209507, -0.15136894583702087, -2.775207996368408, -0.08681213855743408, 0.05702732503414154, 0.670292317867279, 0.31442636251449585, 0.30793967843055725], "sample_rate": 48000})"; @@ -50,6 +55,21 @@ nam::dspData _GetConfig(const std::string& configStr = basicConfigStr) namespace { +class TemporaryNamFile +{ +public: + TemporaryNamFile(const std::string& filename, const std::string& contents) + : path(std::filesystem::temp_directory_path() / filename) + { + std::ofstream output(path); + output << contents; + } + + ~TemporaryNamFile() { std::filesystem::remove(path); } + + const std::filesystem::path path; +}; + constexpr const char* kConstructorResetArchitecture = "ConstructorResetPrewarmPolicyTest"; int gConstructorResetConstructCount = 0; @@ -91,6 +111,40 @@ nlohmann::json build_constructor_reset_config() } // namespace +void test_empty_nam_file_throws_validation_error() +{ + const TemporaryNamFile file("nam_core_empty_file_test.nam", ""); + + try + { + nam::get_dsp(file.path); + assert(false); + } + catch (const nam::NamFileValidationError& error) + { + const std::string message = error.what(); + assert(message.find(file.path.string()) != std::string::npos); + assert(message.find("parse") != std::string::npos); + } +} + +void test_malformed_nam_file_throws_validation_error() +{ + const TemporaryNamFile file("nam_core_malformed_file_test.nam", R"({"version":)"); + + try + { + nam::get_dsp(file.path); + assert(false); + } + catch (const nam::NamFileValidationError& error) + { + const std::string message = error.what(); + assert(message.find(file.path.string()) != std::string::npos); + assert(message.find("parse") != std::string::npos); + } +} + void test_gets_input_level() { nam::dspData config = _GetConfig(); From d2d9a68fe64643ff2e1a671d872f522e42c8bba9 Mon Sep 17 00:00:00 2001 From: Steven Atkinson Date: Sun, 9 Aug 2026 12:54:02 -0700 Subject: [PATCH 2/4] fix: resolve issue #314 --- NAM/get_dsp.cpp | 9 ++++++++- NAM/get_dsp.h | 10 ++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/NAM/get_dsp.cpp b/NAM/get_dsp.cpp index 64393646..563a035d 100644 --- a/NAM/get_dsp.cpp +++ b/NAM/get_dsp.cpp @@ -173,7 +173,14 @@ std::unique_ptr get_dsp(const std::filesystem::path config_filename, dspDat throw std::runtime_error("Config file doesn't exist!\n"); std::ifstream i(config_filename); nlohmann::json j; - i >> j; + try + { + i >> j; + } + catch (const nlohmann::json::parse_error& error) + { + throw NamFileValidationError("Could not parse .nam file [" + config_filename.string() + "]: " + error.what()); + } populate_dsp_data(j, returnedConfig); /*Copy to a new dsp_config object for get_dsp below, diff --git a/NAM/get_dsp.h b/NAM/get_dsp.h index c9e7941b..ffde1ccc 100644 --- a/NAM/get_dsp.h +++ b/NAM/get_dsp.h @@ -3,12 +3,20 @@ #include #include #include +#include #include #include "dsp.h" namespace nam { +/// \brief Indicates that a .nam file failed validation while loading +class NamFileValidationError : public std::runtime_error +{ +public: + using std::runtime_error::runtime_error; +}; + enum class Supported { NO = 0, @@ -81,6 +89,7 @@ struct DspLoadOptions /// \param config_filename Path to the .nam model file /// \param options Loading options /// \return Unique pointer to a DSP object +/// \throws NamFileValidationError If the file does not contain valid JSON std::unique_ptr get_dsp(const std::filesystem::path config_filename, DspLoadOptions options = DspLoadOptions()); /// \brief Get NAM from a provided configuration struct @@ -96,6 +105,7 @@ std::unique_ptr get_dsp(dspData& conf, DspLoadOptions options = DspLoadOpti /// \param returnedConfig Output parameter that will be filled with the model data /// \param options Loading options /// \return Unique pointer to a DSP object +/// \throws NamFileValidationError If the file does not contain valid JSON std::unique_ptr get_dsp(const std::filesystem::path config_filename, dspData& returnedConfig, DspLoadOptions options = DspLoadOptions()); From afc7515bbe94262c940f9f922c132ecc480f791a Mon Sep 17 00:00:00 2001 From: Steven Atkinson Date: Sun, 9 Aug 2026 13:01:44 -0700 Subject: [PATCH 3/4] test: cover NAM file structure validation --- tools/run_tests.cpp | 5 +++ tools/test/test_nam_file.cpp | 82 ++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+) create mode 100644 tools/test/test_nam_file.cpp diff --git a/tools/run_tests.cpp b/tools/run_tests.cpp index 6ecdfebd..94772855 100644 --- a/tools/run_tests.cpp +++ b/tools/run_tests.cpp @@ -29,6 +29,7 @@ #include "test/test_linear.cpp" #include "test/test_lstm.cpp" #include "test/test_lstm_realtime_safe.cpp" +#include "test/test_nam_file.cpp" #include "test/test_wavenet_configurable_gating.cpp" #include "test/test_noncontiguous_blocks.cpp" #include "test/test_extensible.cpp" @@ -312,6 +313,10 @@ int main() test_get_dsp::test_get_dsp_prewarm_option_forces_constructor_reset_prewarm(); test_get_dsp::test_get_dsp_with_returned_config_constructs_once(); + test_nam_file::test_accepts_minimum_valid_file(); + test_nam_file::test_rejects_non_object_json(); + test_nam_file::test_rejects_missing_required_keys(); + // Finally, some end-to-end tests. test_get_dsp::test_load_and_process_nam_files(); diff --git a/tools/test/test_nam_file.cpp b/tools/test/test_nam_file.cpp new file mode 100644 index 00000000..9162e892 --- /dev/null +++ b/tools/test/test_nam_file.cpp @@ -0,0 +1,82 @@ +#include +#include +#include +#include +#include + +#include "json.hpp" + +#include "NAM/nam_file.h" + +namespace test_nam_file +{ +namespace +{ + +class TemporaryNamFile +{ +public: + TemporaryNamFile(const std::string& filename, const std::string& contents) + : path(std::filesystem::temp_directory_path() / filename) + { + std::ofstream output(path); + output << contents; + } + + ~TemporaryNamFile() { std::filesystem::remove(path); } + + const std::filesystem::path path; +}; + +nlohmann::json minimum_valid_nam_file() +{ + return {{"version", "0.7.0"}, + {"architecture", "Test"}, + {"config", nlohmann::json::object()}, + {"weights", nlohmann::json::array()}}; +} + +void assert_validation_error(const TemporaryNamFile& file, const std::string& expected_detail) +{ + try + { + nam::validate_nam_file(file.path); + assert(false); + } + catch (const nam::NamFileValidationError& error) + { + const std::string message = error.what(); + assert(message.find(file.path.string()) != std::string::npos); + assert(message.find(expected_detail) != std::string::npos); + } +} + +} // namespace + +void test_accepts_minimum_valid_file() +{ + const TemporaryNamFile file("nam_core_minimum_valid_file_test.nam", minimum_valid_nam_file().dump()); + const auto config = nam::validate_nam_file(file.path); + + assert(config.is_object()); +} + +void test_rejects_non_object_json() +{ + const TemporaryNamFile file("nam_core_non_object_file_test.nam", "[]"); + assert_validation_error(file, "object"); +} + +void test_rejects_missing_required_keys() +{ + const std::vector required_keys{"version", "architecture", "config", "weights"}; + for (const auto& key : required_keys) + { + auto config = minimum_valid_nam_file(); + config.erase(key); + const TemporaryNamFile file("nam_core_missing_" + key + "_test.nam", config.dump()); + assert_validation_error(file, key); + } +} + +} // namespace test_nam_file From 251d4de559b8725f0de2950593e0bd4c6548875f Mon Sep 17 00:00:00 2001 From: Steven Atkinson Date: Sun, 9 Aug 2026 13:03:59 -0700 Subject: [PATCH 4/4] fix: validate NAM file structure --- NAM/get_dsp.cpp | 14 +------------- NAM/get_dsp.h | 14 +++----------- NAM/nam_file.cpp | 41 +++++++++++++++++++++++++++++++++++++++++ NAM/nam_file.h | 22 ++++++++++++++++++++++ 4 files changed, 67 insertions(+), 24 deletions(-) create mode 100644 NAM/nam_file.cpp create mode 100644 NAM/nam_file.h diff --git a/NAM/get_dsp.cpp b/NAM/get_dsp.cpp index 563a035d..016e10fe 100644 --- a/NAM/get_dsp.cpp +++ b/NAM/get_dsp.cpp @@ -1,4 +1,3 @@ -#include #include #include #include @@ -169,18 +168,7 @@ std::unique_ptr get_dsp(const nlohmann::json& config, DspLoadOptions option std::unique_ptr get_dsp(const std::filesystem::path config_filename, dspData& returnedConfig, DspLoadOptions options) { - if (!std::filesystem::exists(config_filename)) - throw std::runtime_error("Config file doesn't exist!\n"); - std::ifstream i(config_filename); - nlohmann::json j; - try - { - i >> j; - } - catch (const nlohmann::json::parse_error& error) - { - throw NamFileValidationError("Could not parse .nam file [" + config_filename.string() + "]: " + error.what()); - } + const auto j = validate_nam_file(config_filename); populate_dsp_data(j, returnedConfig); /*Copy to a new dsp_config object for get_dsp below, diff --git a/NAM/get_dsp.h b/NAM/get_dsp.h index ffde1ccc..755c89b4 100644 --- a/NAM/get_dsp.h +++ b/NAM/get_dsp.h @@ -1,22 +1,14 @@ #pragma once -#include #include #include -#include #include #include "dsp.h" +#include "nam_file.h" namespace nam { -/// \brief Indicates that a .nam file failed validation while loading -class NamFileValidationError : public std::runtime_error -{ -public: - using std::runtime_error::runtime_error; -}; - enum class Supported { NO = 0, @@ -89,7 +81,7 @@ struct DspLoadOptions /// \param config_filename Path to the .nam model file /// \param options Loading options /// \return Unique pointer to a DSP object -/// \throws NamFileValidationError If the file does not contain valid JSON +/// \throws NamFileValidationError If the file cannot be read or does not contain a minimally valid .nam configuration std::unique_ptr get_dsp(const std::filesystem::path config_filename, DspLoadOptions options = DspLoadOptions()); /// \brief Get NAM from a provided configuration struct @@ -105,7 +97,7 @@ std::unique_ptr get_dsp(dspData& conf, DspLoadOptions options = DspLoadOpti /// \param returnedConfig Output parameter that will be filled with the model data /// \param options Loading options /// \return Unique pointer to a DSP object -/// \throws NamFileValidationError If the file does not contain valid JSON +/// \throws NamFileValidationError If the file cannot be read or does not contain a minimally valid .nam configuration std::unique_ptr get_dsp(const std::filesystem::path config_filename, dspData& returnedConfig, DspLoadOptions options = DspLoadOptions()); diff --git a/NAM/nam_file.cpp b/NAM/nam_file.cpp new file mode 100644 index 00000000..069f8ead --- /dev/null +++ b/NAM/nam_file.cpp @@ -0,0 +1,41 @@ +#include "nam_file.h" + +#include +#include +#include + +namespace nam +{ +nlohmann::json validate_nam_file(const std::filesystem::path& filename) +{ + if (!std::filesystem::exists(filename)) + throw NamFileValidationError("Could not validate .nam file [" + filename.string() + "]: file does not exist."); + + std::ifstream input(filename); + if (!input.is_open()) + throw NamFileValidationError("Could not validate .nam file [" + filename.string() + "]: file could not be read."); + + nlohmann::json config; + try + { + input >> config; + } + catch (const nlohmann::json::parse_error& error) + { + throw NamFileValidationError("Could not parse .nam file [" + filename.string() + "]: " + error.what()); + } + + if (!config.is_object()) + throw NamFileValidationError("Invalid .nam file [" + filename.string() + "]: root JSON value must be an object."); + + constexpr std::array required_keys{"version", "architecture", "config", "weights"}; + for (const auto* key : required_keys) + { + if (config.find(key) == config.end()) + throw NamFileValidationError("Invalid .nam file [" + filename.string() + "]: missing required key \"" + key + + "\"."); + } + + return config; +} +} // namespace nam diff --git a/NAM/nam_file.h b/NAM/nam_file.h new file mode 100644 index 00000000..a4cda049 --- /dev/null +++ b/NAM/nam_file.h @@ -0,0 +1,22 @@ +#pragma once + +#include +#include + +#include "json.hpp" + +namespace nam +{ +/// \brief Indicates that a .nam file failed validation while loading +class NamFileValidationError : public std::runtime_error +{ +public: + using std::runtime_error::runtime_error; +}; + +/// \brief Parse and validate a .nam file +/// \param filename Path to the .nam file +/// \return The parsed model configuration +/// \throws NamFileValidationError If the file cannot be read, parsed, or does not contain the minimum required fields +nlohmann::json validate_nam_file(const std::filesystem::path& filename); +} // namespace nam