Check the canvas size before narrowing it to an int
A canvas side was cast to int with no range check, so a number that did not fit came back as one that did: `-s 4294968296x600` rendered a perfectly plausible 1000x600, and a hand-edited project file could ask for a width the reader turned into something else (or, for a double out of range, into undefined behaviour). Both front-ends now check the side before the cast, against one shared bound in chain.hpp — the 1..20000 the GUI spin buttons already offered, so the help text, the CLI error, the project reader and the interface all move together. The tests gain a `canvas size` section covering the round trip at both ends of the range and the refusals on either side of it, through the project reader and through `-s`; the test binary picks up the CLI parser to do it, which stays GUI-free. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+9
-3
@@ -24,7 +24,10 @@ bool parseSize(const std::string& text, int& width, int& height) {
|
||||
long w = 0;
|
||||
long h = 0;
|
||||
if (!parseLong(text.substr(0, sep), w) || !parseLong(text.substr(sep + 1), h)) return false;
|
||||
if (w <= 0 || h <= 0) return false;
|
||||
// Checked as longs, before the narrowing: 4294968296 cast straight to int
|
||||
// is 1000, which would render a plausible canvas nobody asked for.
|
||||
if (w < kMinCanvasSide || w > kMaxCanvasSide) return false;
|
||||
if (h < kMinCanvasSide || h > kMaxCanvasSide) return false;
|
||||
width = static_cast<int>(w);
|
||||
height = static_cast<int>(h);
|
||||
return true;
|
||||
@@ -66,7 +69,9 @@ Options parseOptions(int argc, char** argv) {
|
||||
} else if (arg == "-s" || arg == "--size") {
|
||||
const std::string value = requireValue(argc, argv, i, arg);
|
||||
if (!parseSize(value, options.width, options.height)) {
|
||||
throw EffectError("invalid size '" + value + "', expected e.g. 1920x1080");
|
||||
throw EffectError("invalid size '" + value + "', expected e.g. 1920x1080 with "
|
||||
"each side between " + std::to_string(kMinCanvasSide) + " and " +
|
||||
std::to_string(kMaxCanvasSide));
|
||||
}
|
||||
} else if (arg == "--seed") {
|
||||
const std::string value = requireValue(argc, argv, i, arg);
|
||||
@@ -131,7 +136,8 @@ std::string usage(const char* programName) {
|
||||
<< "Options:\n"
|
||||
<< " -e, --effect NAME effect to apply, repeat to cascade (default: " << defaultEffectName() << ")\n"
|
||||
<< " -o, --output FILE render headlessly to this file and exit\n"
|
||||
<< " -s, --size WxH canvas size (default: 1920x1080)\n"
|
||||
<< " -s, --size WxH canvas size, each side " << kMinCanvasSide << "-"
|
||||
<< kMaxCanvasSide << " (default: 1920x1080)\n"
|
||||
<< " --seed N random seed (default: random)\n"
|
||||
<< " --list-effects list available effects and exit\n"
|
||||
<< " -h, --help show this help and exit\n";
|
||||
|
||||
@@ -10,6 +10,13 @@
|
||||
|
||||
namespace effector {
|
||||
|
||||
// The canvas sides both front-ends accept. The upper bound is not a hardware
|
||||
// limit but the point past which a side stops fitting comfortably in an int
|
||||
// once multiplied out, so both the CLI and the project reader can reject a
|
||||
// number rather than silently wrap it into a plausible-looking one.
|
||||
inline constexpr int kMinCanvasSide = 1;
|
||||
inline constexpr int kMaxCanvasSide = 20000;
|
||||
|
||||
// One link of a cascade: an effect together with the values it runs with.
|
||||
struct ChainStage {
|
||||
EffectPtr effect;
|
||||
|
||||
+17
-3
@@ -283,6 +283,21 @@ std::string relativeTo(const std::string& path, const std::string& directory) {
|
||||
return text.rfind("..", 0) == 0 ? path : text;
|
||||
}
|
||||
|
||||
// A canvas side is checked as a double, before it is narrowed: casting a
|
||||
// number outside the range of an int is undefined, and even where it wraps
|
||||
// quietly it turns a size the file never named into one that looks ordinary.
|
||||
int canvasSide(const Json& value, const char* what) {
|
||||
if (value.kind != Json::Kind::Number || value.number != std::trunc(value.number)) {
|
||||
throw EffectError(std::string("the canvas ") + what + " must be a whole number");
|
||||
}
|
||||
if (!(value.number >= kMinCanvasSide) || !(value.number <= kMaxCanvasSide)) {
|
||||
throw EffectError(std::string("the canvas ") + what + " must be between " +
|
||||
std::to_string(kMinCanvasSide) + " and " +
|
||||
std::to_string(kMaxCanvasSide) + " pixels");
|
||||
}
|
||||
return static_cast<int>(value.number);
|
||||
}
|
||||
|
||||
std::string resolve(const std::string& path, const std::string& directory) {
|
||||
std::filesystem::path file(path);
|
||||
if (file.is_absolute() || directory.empty()) return path;
|
||||
@@ -350,12 +365,11 @@ Project projectFromJson(const std::string& text, const std::string& directory) {
|
||||
Project project;
|
||||
|
||||
if (const auto* canvas = root.member("canvas")) {
|
||||
if (const auto* width = canvas->member("width")) project.width = static_cast<int>(width->number);
|
||||
if (const auto* width = canvas->member("width")) project.width = canvasSide(*width, "width");
|
||||
if (const auto* height = canvas->member("height")) {
|
||||
project.height = static_cast<int>(height->number);
|
||||
project.height = canvasSide(*height, "height");
|
||||
}
|
||||
}
|
||||
if (project.width <= 0 || project.height <= 0) throw EffectError("the canvas size is not positive");
|
||||
|
||||
if (const auto* seed = root.member("seed")) {
|
||||
if (seed->kind == Json::Kind::Number) {
|
||||
|
||||
+2
-2
@@ -176,10 +176,10 @@ Gtk::Widget& EffectorWindow::buildSidebar() {
|
||||
auto* grid = Gtk::make_managed<Gtk::Grid>();
|
||||
grid->set_column_spacing(8);
|
||||
grid->set_row_spacing(6);
|
||||
widthSpin_.set_range(1, 20000);
|
||||
widthSpin_.set_range(kMinCanvasSide, kMaxCanvasSide);
|
||||
widthSpin_.set_increments(1, 100);
|
||||
widthSpin_.set_value(1920);
|
||||
heightSpin_.set_range(1, 20000);
|
||||
heightSpin_.set_range(kMinCanvasSide, kMaxCanvasSide);
|
||||
heightSpin_.set_increments(1, 100);
|
||||
heightSpin_.set_value(1080);
|
||||
for (auto* spin : {&widthSpin_, &heightSpin_}) {
|
||||
|
||||
Reference in New Issue
Block a user