mirror of
https://github.com/mudler/LocalAI.git
synced 2026-07-30 18:09:05 -04:00
backend(audio-cpp): reject out-of-range numeric model options
std::atoi is undefined once the digits exceed long and in practice wraps, so device:2147483648 was accepted and handed the ggml backend selector a device index of -2147483648 from a function whose error text promises a non-negative integer. Parse with strtol and reject on ERANGE, on a value above INT_MAX, and on any unconsumed trailing input. The error strings are unchanged. Name the whole entry in the unknown-key error too: an entry like ':value' has an empty key and left the user nothing to grep for in their YAML. Tests look keys up through a helper instead of map::at, so a prefix off-by-one fails one named check rather than aborting the binary and skipping the rest of the suite, and cover the overflow, negative and non-numeric paths. Assisted-by: Claude:claude-opus-5 [Claude Code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
This commit is contained in:
committed by
localai-org-maint-bot
parent
7f78bbe752
commit
aea4d921df
@@ -1,6 +1,8 @@
|
||||
#include "model_options.h"
|
||||
|
||||
#include <cctype>
|
||||
#include <cerrno>
|
||||
#include <climits>
|
||||
#include <cstdlib>
|
||||
|
||||
namespace audiocpp_backend {
|
||||
@@ -21,7 +23,13 @@ std::string trim(const std::string &value) {
|
||||
}
|
||||
|
||||
// Parses a non-negative integer. Returns false on anything else, including
|
||||
// empty strings, signs, and trailing garbage.
|
||||
// empty strings, signs, trailing garbage, and values too large for int.
|
||||
//
|
||||
// strtol rather than atoi: atoi is undefined behaviour once the digits exceed
|
||||
// long, and in practice it hands back a wrapped value. That would let
|
||||
// "device:2147483648" through as -2147483648 and send a negative index to the
|
||||
// ggml backend selector, from a function whose error text promises the caller a
|
||||
// non-negative integer.
|
||||
bool parse_non_negative_int(const std::string &value, int &out) {
|
||||
if (value.empty()) {
|
||||
return false;
|
||||
@@ -31,7 +39,18 @@ bool parse_non_negative_int(const std::string &value, int &out) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
out = std::atoi(value.c_str());
|
||||
|
||||
errno = 0;
|
||||
char *end = nullptr;
|
||||
const long parsed = std::strtol(value.c_str(), &end, 10);
|
||||
if (errno == ERANGE || end == nullptr || *end != '\0') {
|
||||
return false;
|
||||
}
|
||||
if (parsed < 0 || parsed > INT_MAX) {
|
||||
return false;
|
||||
}
|
||||
|
||||
out = static_cast<int>(parsed);
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -110,7 +129,9 @@ ParsedOptions parse_model_options(const std::vector<std::string> &entries) {
|
||||
return parsed;
|
||||
}
|
||||
} else {
|
||||
parsed.error = "audio-cpp: unknown option key '" + key +
|
||||
// Quotes the whole entry, not just the key: an entry like ":value"
|
||||
// has an empty key and would otherwise leave nothing to grep for.
|
||||
parsed.error = "audio-cpp: unknown option key '" + entry +
|
||||
"'. Known keys: family, task, backend, device, "
|
||||
"threads, model_spec_override, busy_timeout_ms, "
|
||||
"load.<key>, session.<key>";
|
||||
|
||||
@@ -10,6 +10,7 @@
|
||||
#include "model_options.cpp"
|
||||
|
||||
#include <cstdio>
|
||||
#include <map>
|
||||
#include <string>
|
||||
#include <vector>
|
||||
|
||||
@@ -24,6 +25,15 @@ static void check(bool ok, const std::string &name) {
|
||||
}
|
||||
}
|
||||
|
||||
// Returns the mapped value, or an empty string when the key is absent. map::at
|
||||
// would throw on a miss and abort the whole binary, so one prefix off-by-one
|
||||
// would hide every check that follows it instead of failing a single one.
|
||||
static std::string lookup(const std::map<std::string, std::string> &values,
|
||||
const std::string &key) {
|
||||
const auto found = values.find(key);
|
||||
return found == values.end() ? std::string() : found->second;
|
||||
}
|
||||
|
||||
using audiocpp_backend::parse_model_options;
|
||||
|
||||
static void test_defaults() {
|
||||
@@ -73,11 +83,11 @@ static void test_namespaced_options() {
|
||||
});
|
||||
check(r.error.empty(), "namespaced options parse without error");
|
||||
check(r.options.load_options.size() == 1, "one load option");
|
||||
check(r.options.load_options.at("weight_type") == "q8_0", "load prefix stripped");
|
||||
check(lookup(r.options.load_options, "weight_type") == "q8_0", "load prefix stripped");
|
||||
check(r.options.session_options.size() == 2, "two session options");
|
||||
check(r.options.session_options.at("miocodec.weight_type") == "f16",
|
||||
check(lookup(r.options.session_options, "miocodec.weight_type") == "f16",
|
||||
"session prefix stripped, inner dots kept");
|
||||
check(r.options.session_options.at("graph_capacity") == "tiered",
|
||||
check(lookup(r.options.session_options, "graph_capacity") == "tiered",
|
||||
"second session option parsed");
|
||||
}
|
||||
|
||||
@@ -94,11 +104,29 @@ static void test_errors() {
|
||||
"empty load key is rejected");
|
||||
check(!parse_model_options({"session.:x"}).error.empty(),
|
||||
"empty session key is rejected");
|
||||
check(!parse_model_options({"busy_timeout_ms:abc"}).error.empty(),
|
||||
"non-numeric busy_timeout_ms is rejected");
|
||||
check(!parse_model_options({"device:-1"}).error.empty(),
|
||||
"negative device is rejected");
|
||||
check(!parse_model_options({"threads:x"}).error.empty(),
|
||||
"non-numeric threads is rejected");
|
||||
|
||||
// Values too large for int must be rejected, not silently wrapped into a
|
||||
// negative device index that then reaches the ggml backend selector.
|
||||
check(!parse_model_options({"device:2147483648"}).error.empty(),
|
||||
"device above INT_MAX is rejected");
|
||||
check(!parse_model_options({"threads:99999999999999"}).error.empty(),
|
||||
"threads above INT_MAX is rejected");
|
||||
|
||||
// The error text must name the offending entry so a user can fix their YAML.
|
||||
const auto r = parse_model_options({"nonsense:1"});
|
||||
check(r.error.find("nonsense") != std::string::npos,
|
||||
"error names the offending key");
|
||||
|
||||
// An empty key still has to give the user something to grep for.
|
||||
const auto empty_key = parse_model_options({":value"});
|
||||
check(empty_key.error.find(":value") != std::string::npos,
|
||||
"unknown-key error names the entry even when the key is empty");
|
||||
}
|
||||
|
||||
static void test_blank_entries_ignored() {
|
||||
|
||||
Reference in New Issue
Block a user