From e47f9f20d68eda78a6621917a7e100ae333a1a53 Mon Sep 17 00:00:00 2001 From: Berke Güzel Date: Thu, 15 Oct 2026 12:29:35 +0300 Subject: Reject invalid arguments instead of ignoring them term_feed() returns bool and rejects a NULL terminal, a NULL buffer with a nonzero length, and any flag bit it does not know. Unknown bits are rejected now so that later flags can be added without changing what the old calls mean. term_new() returns NULL for a negative size component, columns above TERM_MAX_COLS, a negative scrollback, or a cursor style outside the enum. Zero still selects the default size and scrollback. It no longer clamps bad values silently. term_resize() rejects columns above TERM_MAX_COLS instead of clamping them, which matches how it already rejects a size below 1. Co-Authored-By: Claude Haiku 5.5 --- benchmarks/term_write_bench.c | 5 +- include/term/term.h | 23 +++++-- src/screen.c | 19 +++--- src/text.c | 14 +++-- tests/api_args_test.c | 140 ++++++++++++++++++++++++++++++++++++++++++ tests/meson.build | 3 +- tests/model.c | 2 +- 7 files changed, 182 insertions(+), 24 deletions(-) create mode 100644 tests/api_args_test.c diff --git a/benchmarks/term_write_bench.c b/benchmarks/term_write_bench.c index 50853f6..7649ed8 100644 --- a/benchmarks/term_write_bench.c +++ b/benchmarks/term_write_bench.c @@ -152,7 +152,10 @@ int main(int argc, char **argv) .size = { .x = cols_val, .y = 40 }, .scrollback = BENCH_SCROLLBACK, }); - if (!t) return EXIT_FAILURE; + if (!t) { + fprintf(stderr, "term_new failed: TERM_COLS must be 0 to %d\n", TERM_MAX_COLS); + return EXIT_FAILURE; + } /* manyline has bare LFs: each one also returns the cursor to * column 0, as a tty with ONLCR does. */ unsigned flags = !strcmp(cases[c].name, "manyline") ? TERM_FEED_ONLCR : 0; diff --git a/include/term/term.h b/include/term/term.h index dc41114..05afbd9 100644 --- a/include/term/term.h +++ b/include/term/term.h @@ -139,9 +139,14 @@ struct term_callbacks { void (*trace)(void *ud, const char *line); }; -/* Options for term_new(). An all-zero value is valid. */ +/* + * Options for term_new(). An all-zero value is valid. A size component of 0 + * selects 80 columns or 24 rows. A negative component, or columns above + * TERM_MAX_COLS, makes term_new() fail. A negative scrollback fails too. + * cursor_style must be a value of enum term_cursor_style. + */ struct term_options { - struct term_vector2i size; /* columns and rows; each is at least 1 */ + struct term_vector2i size; /* columns and rows */ int scrollback; /* history lines; 0 selects TERM_SCROLLBACK_DEFAULT */ struct term_theme theme; enum term_cursor_style cursor_style; @@ -149,7 +154,6 @@ struct term_options { struct term_callbacks callbacks; }; -/* NULL when memory runs out. */ /* The Unicode version of the width tables, such as "18.0.0". A host can * pass it to programs as UNICODE_VERSION. */ TERM_API const char *term_unicode_version(void); @@ -165,6 +169,7 @@ TERM_API size_t term_scan_printable_span(const char *buf, size_t len, unsigned c * and comparisons. */ TERM_API const char *term_isa(void); +/* NULL when memory runs out or an option is invalid. */ TERM_API struct term *term_new(const struct term_options *opt); TERM_API void term_free(struct term *t); @@ -175,8 +180,9 @@ TERM_API void term_free(struct term *t); TERM_API void term_lock(struct term *t); TERM_API void term_unlock(struct term *t); -/* Change the size in cells. Returns false when memory runs out; the terminal - * is then unchanged. The screen reflows to the new width. */ +/* Change the size in cells. Returns false, and the terminal is unchanged, + * when a component is below 1, when cols is above TERM_MAX_COLS, or when + * memory runs out. The screen reflows to the new width. */ TERM_API bool term_resize(struct term *t, struct term_vector2i size); /* Replace the theme. A different theme also drops the colors that the @@ -197,7 +203,12 @@ enum term_feed_flags { TERM_FEED_ONLCR = 1u << 0, }; -TERM_API void term_feed(struct term *t, const char *bytes, size_t len, unsigned flags); +/* + * Feed bytes from the program. Returns false, and changes nothing, when t is + * NULL, when bytes is NULL and len is not 0, or when flags has a bit that is + * not in enum term_feed_flags. A len of 0 is a no-op that returns true. + */ +TERM_API bool term_feed(struct term *t, const char *bytes, size_t len, unsigned flags); /* Report a focus change. The terminal replies only in focus-event mode. */ TERM_API void term_focus(struct term *t, bool focused); diff --git a/src/screen.c b/src/screen.c index 87d6f2c..697e08a 100644 --- a/src/screen.c +++ b/src/screen.c @@ -808,12 +808,11 @@ void term_screen_set_alternate(struct term *t, bool enabled) } } +/* The caller has checked that requested is not negative. */ static int scrollback_lines(int requested) { if (requested == 0) return TERM_SCROLLBACK_DEFAULT; - if (requested < 0) - return 0; return requested > 1000000 ? 1000000 : requested; } @@ -847,10 +846,12 @@ struct term *term_new(const struct term_options *opt) static const struct term_options defaults; if (!opt) opt = &defaults; - int cols = opt->size.x < 1 ? 80 : opt->size.x; - int rows = opt->size.y < 1 ? 24 : opt->size.y; - if (cols > TERM_MAX_COLS) - cols = TERM_MAX_COLS; + if (opt->size.x < 0 || opt->size.y < 0 || opt->size.x > TERM_MAX_COLS) + return NULL; + if (opt->scrollback < 0 || (unsigned)opt->cursor_style > TERM_CURSOR_BAR) + return NULL; + int cols = opt->size.x == 0 ? 80 : opt->size.x; + int rows = opt->size.y == 0 ? 24 : opt->size.y; static _Atomic uint64_t next_serial = 1; struct term *t = calloc(1, sizeof(*t)); @@ -895,7 +896,7 @@ struct term *term_new(const struct term_options *opt) t->cur_fg = TERM_COLOR_FG; t->cur_bg = TERM_COLOR_BG; t->cursor_visible = true; - t->default_cursor_style = opt->cursor_style > TERM_CURSOR_BAR ? TERM_CURSOR_BLOCK : opt->cursor_style; + t->default_cursor_style = opt->cursor_style; t->default_cursor_blink = !opt->cursor_steady; t->cursor_style = t->default_cursor_style; t->cursor_blink = t->default_cursor_blink; @@ -1523,10 +1524,8 @@ bool term_resize(struct term *t, struct term_vector2i size) return false; int cols = size.x; int rows = size.y; - if (cols < 1 || rows < 1) + if (cols < 1 || rows < 1 || cols > TERM_MAX_COLS) return false; - if (cols > TERM_MAX_COLS) - cols = TERM_MAX_COLS; term_write_begin(t); if (cols == t->cols && rows == t->rows) { term_write_end(t); diff --git a/src/text.c b/src/text.c index 43c3ef5..a646195 100644 --- a/src/text.c +++ b/src/text.c @@ -190,10 +190,11 @@ static inline bool try_text_run(struct term *t, const char *buf, size_t len, return wrote; } +/* The feed flags this build knows. term_feed() rejects any other bit. */ +#define TERM_FEED_KNOWN TERM_FEED_ONLCR + static void term_feed_inner(struct term *t, const char *buf, size_t len, unsigned flags) { - if (!t || !buf || len == 0) - return; t->transport_newline = (flags & TERM_FEED_ONLCR) != 0; uint16_t cur_flags = term_current_flags(t); @@ -366,13 +367,16 @@ static void term_feed_inner(struct term *t, const char *buf, size_t len, unsigne } } -void term_feed(struct term *t, const char *buf, size_t len, unsigned flags) +bool term_feed(struct term *t, const char *buf, size_t len, unsigned flags) { - if (!t || !buf || len == 0) - return; + if (!t || (!buf && len > 0) || (flags & ~TERM_FEED_KNOWN)) + return false; + if (len == 0) + return true; term_write_begin(t); term_feed_inner(t, buf, len, flags); term_write_end(t); + return true; } void term_text_repeat(struct term *t, int count) diff --git a/tests/api_args_test.c b/tests/api_args_test.c new file mode 100644 index 0000000..18af728 --- /dev/null +++ b/tests/api_args_test.c @@ -0,0 +1,140 @@ +#include "term_api.h" +#include +#include +#include + +/* A feed that returns false must not move the cursor or change a cell. */ +static void expect_unchanged(struct term *t, int x, int y, uint32_t cp) +{ + assert(tt_cursor_x(t) == x); + assert(tt_cursor_y(t) == y); + assert(tt_cell(t, 0, 0)->text == cp); +} + +static void test_feed_unknown_flag(void) +{ + struct term *t = tt_new(10, 3); + assert(t); + assert(term_feed(t, "abc", 3, 0)); + expect_unchanged(t, 3, 0, 'a'); + + assert(!term_feed(t, "xy", 2, 1u << 1)); + expect_unchanged(t, 3, 0, 'a'); + assert(!term_feed(t, "xy", 2, TERM_FEED_ONLCR | (1u << 4))); + expect_unchanged(t, 3, 0, 'a'); + assert(!term_feed(t, "xy", 2, 1u << 31)); + expect_unchanged(t, 3, 0, 'a'); + /* Unknown bits are rejected even when there are no bytes. */ + assert(!term_feed(t, "", 0, 1u << 1)); + + assert(term_feed(t, "\nz", 2, TERM_FEED_ONLCR)); + assert(tt_cursor_x(t) == 1); + assert(tt_cursor_y(t) == 1); + term_free(t); +} + +static void test_feed_null_buffer(void) +{ + struct term *t = tt_new(10, 3); + assert(t); + assert(term_feed(t, "abc", 3, 0)); + + assert(!term_feed(t, NULL, 1, 0)); + expect_unchanged(t, 3, 0, 'a'); + assert(!term_feed(t, NULL, 4096, TERM_FEED_ONLCR)); + expect_unchanged(t, 3, 0, 'a'); + + /* A zero length is a no-op, so a NULL buffer is fine. */ + assert(term_feed(t, NULL, 0, 0)); + assert(term_feed(t, "", 0, TERM_FEED_ONLCR)); + expect_unchanged(t, 3, 0, 'a'); + + assert(!term_feed(NULL, "a", 1, 0)); + assert(!term_feed(NULL, NULL, 0, 0)); + term_free(t); +} + +static void test_new_rejects_bad_options(void) +{ + struct term_options opt; + + memset(&opt, 0, sizeof(opt)); + opt.size.x = -1; + assert(!term_new(&opt)); + + memset(&opt, 0, sizeof(opt)); + opt.size.y = -1; + assert(!term_new(&opt)); + + memset(&opt, 0, sizeof(opt)); + opt.size.x = TERM_MAX_COLS + 1; + assert(!term_new(&opt)); + + memset(&opt, 0, sizeof(opt)); + opt.scrollback = -1; + assert(!term_new(&opt)); + + memset(&opt, 0, sizeof(opt)); + opt.cursor_style = (enum term_cursor_style)3; + assert(!term_new(&opt)); + + memset(&opt, 0, sizeof(opt)); + opt.cursor_style = (enum term_cursor_style)-1; + assert(!term_new(&opt)); +} + +static void test_new_accepts_defaults(void) +{ + struct term_options opt; + + memset(&opt, 0, sizeof(opt)); + struct term *t = term_new(&opt); + assert(t); + assert(tt_cols(t) == 80); + assert(tt_rows(t) == 24); + assert(term_cursor(t).style == TERM_CURSOR_BLOCK); + term_free(t); + + memset(&opt, 0, sizeof(opt)); + opt.size.x = TERM_MAX_COLS; + opt.size.y = 1; + opt.cursor_style = TERM_CURSOR_BAR; + t = term_new(&opt); + assert(t); + assert(tt_cols(t) == TERM_MAX_COLS); + assert(tt_rows(t) == 1); + assert(term_cursor(t).style == TERM_CURSOR_BAR); + term_free(t); +} + +static void test_resize_rejects_too_wide(void) +{ + struct term *t = tt_new(10, 3); + assert(t); + assert(term_feed(t, "abc", 3, 0)); + + assert(!tt_resize(t, TERM_MAX_COLS + 1, 3)); + assert(tt_cols(t) == 10); + assert(tt_rows(t) == 3); + expect_unchanged(t, 3, 0, 'a'); + + assert(!tt_resize(t, 10, 0)); + assert(!tt_resize(t, 0, 3)); + assert(tt_cols(t) == 10); + assert(tt_rows(t) == 3); + + assert(tt_resize(t, TERM_MAX_COLS, 3)); + assert(tt_cols(t) == TERM_MAX_COLS); + assert(tt_rows(t) == 3); + term_free(t); +} + +int main(void) +{ + test_feed_unknown_flag(); + test_feed_null_buffer(); + test_new_rejects_bad_options(); + test_new_accepts_defaults(); + test_resize_rejects_too_wide(); + return 0; +} diff --git a/tests/meson.build b/tests/meson.build index 69f3c49..82493af 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -30,6 +30,7 @@ term_tests = [ ['term-resize-failure-test', files('resize_failure_test.c'), [dl_dep], ['extlib']], ['dcs-test', files('dcs_test.c'), [], []], ['osc-test', files('osc_test.c'), [], []], + ['api-args-test', files('api_args_test.c'), [], []], ] test_helpers_inc = include_directories('helpers') @@ -83,7 +84,7 @@ term_amalgamation_lib = static_library( foreach entry : term_tests if entry[0] in ['term-test', 'spec-test', 'view-test', 'dcs-test', 'osc-test', 'history-text-test', 'term-input-test', 'model-test', 'model-row-test', - 'resize-property-test'] + 'resize-property-test', 'api-args-test'] test(entry[0] + '-amalgamation', executable(entry[0] + '-amalgamation', entry[1], c_args: test_c_args, diff --git a/tests/model.c b/tests/model.c index 8dead6c..6e368ba 100644 --- a/tests/model.c +++ b/tests/model.c @@ -1290,7 +1290,7 @@ static void resize_plain(struct model *m, int cols, int rows) static bool resize(struct model *m, int cols, int rows) { - if (cols > TERM_MAX_COLS) cols = TERM_MAX_COLS; + if (cols > TERM_MAX_COLS) return false; if (cols == m->cols && rows == m->rows) return true; if (cols > MAX_TABS) return false; m->top = 0; -- cgit v1.2.3