From cd51add717322d56599a38d8026716dc327f7b9d Mon Sep 17 00:00:00 2001 From: Armaan Sandhu Date: Thu, 3 Sep 2026 23:13:53 -0400 Subject: [PATCH 1/3] utf8: make utf8_char_console_width() reject what utf8_is_valid() rejects utf8_char_console_width() decoded with utf8_encoded_to_unichar(), which does not check for overlong encodings, does not check unichar_is_valid(), and still accepts the obsolete five and six byte forms. Those checks live in the wrapper utf8_encoded_valid_unichar(), which is what utf8_is_valid() and ellipsize_mem() use. Over all 4244897280 NUL free byte strings of length one to four, that is the whole UTF-8 encoding space, the wrapper agrees with utf8_is_valid_n() everywhere, while the bare decoder disagrees on 8835838 of them, always by accepting more. So utf8_console_width() assigned a finite width to strings that utf8_is_valid() refuses. format-table.c measures a cell with utf8_console_width() and, if it does not fit, hands it to ellipsize(), which validates strictly, returns NULL, and is reported as -ENOMEM. A surrogate, overlong, noncharacter or five byte sequence in any cell therefore failed the whole table with a bogus out of memory error, and only once the column happened to be narrow enough to require truncation, so whether it happened at all depended on the width of the user's terminal. At other widths the invalid sequence was printed to the terminal instead. Decode with utf8_encoded_valid_unichar() so that how wide we think a string is agrees with whether we consider it valid UTF-8 in the first place. Invalid cells are now refused consistently with -EINVAL, the way the sequences that utf8_encoded_to_unichar() already rejected always were. Note that for tables this makes the refusal total rather than per field: table_data_requested_width_height() turns a SIZE_MAX width into -EINVAL, which table_format() propagates, so a cell in one of the newly rejected encodings now fails the whole table at every width. systemd-dissect and repart --dry-run put GPT partition names read straight off the image into cells without filtering them, so a crafted partition name suppresses the whole listing where before it cost one field, and only at some terminal widths. That is the intended trade: the alternative is to keep emitting raw invalid bytes to the terminal at whichever widths happen not to need truncation. Should degrading the cell rather than the table turn out to be preferable, the place for it is where the text enters the table, via utf8_escape_invalid() and friends, not in a helper whose job is to say how wide a string is. This also makes the assert(w >= 0) in utf8_escape_non_printable_full() hold by construction rather than by the accident that its own validation was stricter than the width helper's. --- src/basic/utf8.c | 8 +++++++- src/test/test-format-table.c | 32 ++++++++++++++++++++++++++++++++ src/test/test-utf8.c | 35 +++++++++++++++++++++++++++++------ 3 files changed, 68 insertions(+), 7 deletions(-) diff --git a/src/basic/utf8.c b/src/basic/utf8.c index 0fccc4711c..c4298cfe76 100644 --- a/src/basic/utf8.c +++ b/src/basic/utf8.c @@ -181,10 +181,16 @@ int utf8_char_console_width(const char *str) { assert(str); - r = utf8_encoded_to_unichar(str, &c); + /* Validate the same way utf8_is_valid() does. utf8_encoded_to_unichar() on its own accepts + * overlong encodings, surrogates, noncharacters and the obsolete 5 and 6 byte forms, which would + * make our idea of how wide a string is disagree with our idea of whether it is valid UTF-8 in + * the first place. */ + r = utf8_encoded_valid_unichar(str, SIZE_MAX); if (r < 0) return r; + assert_se(utf8_encoded_to_unichar(str, &c) == r); + return unichar_console_width(c); } diff --git a/src/test/test-format-table.c b/src/test/test-format-table.c index 4c96d6caf0..8a0f93fb6d 100644 --- a/src/test/test-format-table.c +++ b/src/test/test-format-table.c @@ -8,6 +8,8 @@ #include "env-util.h" #include "format-table.h" #include "json-util.h" +#include "string-util.h" +#include "strv.h" #include "terminal-util.h" #include "tests.h" #include "time-util.h" @@ -35,6 +37,36 @@ TEST(issue_9549) { "foooo raw no 673.6M Wed 2018-07-11 00:10:33 J… Wed 2018-07-11 00:16:00 JST\n"); } +TEST(invalid_utf8_cell) { + /* A cell that is not valid UTF-8 must be refused the same way whatever the column width is. + * The width was previously determined with a more permissive decoder than the one ellipsize() + * uses, so a surrogate, overlong, noncharacter or obsolete five byte sequence was measured as + * if it were fine, and then failed inside ellipsize() as a bogus -ENOMEM, but only once the + * column happened to be narrow enough to require truncation. The widths below deliberately go + * past the width of the whole table, so that the last iterations are ones where no truncation + * is needed at all: those are the ones where the old code did not fail but printed the invalid + * sequence to the terminal instead. */ + + FOREACH_STRING(x, + "\xed\xa0\x80", /* UTF-16 surrogate */ + "\xc0\xaf", /* overlong '/' */ + "\xef\xb7\x90", /* noncharacter U+FDD0 */ + "\xf8\x88\x80\x80\x80") /* obsolete five byte form */ + for (size_t w = 8; w <= 64; w += 8) { + _cleanup_(table_unrefp) Table *table = NULL; + _cleanup_free_ char *formatted = NULL, *cell = NULL; + + ASSERT_NOT_NULL((table = table_new("name", "value"))); + ASSERT_NOT_NULL((cell = strjoin("aaaaaaaaaaaaaaaa", x, "aaaaaaaaaaaaaaaa"))); + ASSERT_OK(table_add_many(table, + TABLE_STRING, cell, + TABLE_STRING, "second")); + table_set_width(table, w); + + ASSERT_ERROR(table_format(table, &formatted), EINVAL); + } +} + TEST(multiline) { _cleanup_(table_unrefp) Table *table = NULL; _cleanup_free_ char *formatted = NULL; diff --git a/src/test/test-utf8.c b/src/test/test-utf8.c index 9ab6297681..875700fa7b 100644 --- a/src/test/test-utf8.c +++ b/src/test/test-utf8.c @@ -207,13 +207,36 @@ TEST(utf8_n_codepoints) { assert_se(utf8_n_codepoints("\xF1") == SIZE_MAX); } +TEST(utf8_char_console_width) { + ASSERT_EQ(utf8_char_console_width("a"), 1); + ASSERT_EQ(utf8_char_console_width("串"), 2); + ASSERT_EQ(utf8_char_console_width("\t"), 8); + + /* How wide we think a character is must agree with whether we consider it valid UTF-8 in the + * first place. utf8_encoded_to_unichar(), which this used to be based on, accepts everything + * below except the bare continuation byte, while utf8_is_valid() rejects all of them. */ + FOREACH_STRING(x, + "\x80", /* bare continuation byte */ + "\xed\xa0\x80", /* UTF-16 surrogate */ + "\xc0\xaf", /* overlong '/' */ + "\xef\xb7\x90", /* noncharacter U+FDD0 */ + "\xef\xbf\xbe", /* noncharacter U+FFFE */ + "\xf8\x88\x80\x80\x80", /* obsolete five byte form */ + "\xfc\x84\x80\x80\x80\x80") { /* obsolete six byte form */ + ASSERT_FALSE(utf8_is_valid(x)); + ASSERT_ERROR(utf8_char_console_width(x), EINVAL); + ASSERT_EQ(utf8_console_width(x), SIZE_MAX); + } +} + TEST(utf8_console_width) { - assert_se(utf8_console_width("abc") == 3); - assert_se(utf8_console_width("zażółcić gęślą jaźń") == 19); - assert_se(utf8_console_width("串") == 2); - assert_se(utf8_console_width("") == 0); - assert_se(utf8_console_width("…👊🔪💐…") == 8); - assert_se(utf8_console_width("\xF1") == SIZE_MAX); + ASSERT_EQ(utf8_console_width("abc"), (size_t) 3); + ASSERT_EQ(utf8_console_width("zażółcić gęślą jaźń"), (size_t) 19); + ASSERT_EQ(utf8_console_width("串"), (size_t) 2); + ASSERT_EQ(utf8_console_width(""), (size_t) 0); + ASSERT_EQ(utf8_console_width("…👊🔪💐…"), (size_t) 8); + ASSERT_EQ(utf8_console_width("\xF1"), SIZE_MAX); + ASSERT_EQ(utf8_console_width("\t"), (size_t) 8); } TEST(utf8_to_utf16) { From 0834378fb9b950b1ceb6f054d930674f973189f2 Mon Sep 17 00:00:00 2001 From: Armaan Sandhu Date: Thu, 3 Sep 2026 23:13:53 -0400 Subject: [PATCH 2/3] string-util,format-table: measure console width with unichar_console_width() ellipsize_mem() and align_string_mem() open coded the width of a character as unichar_iswide(c) ? 2 : 1, but unichar_console_width() counts a tab as eight cells. The two disagreed, and format-table.c uses both: it decides a field is too wide with utf8_console_width(), then asks ellipsize() to fit it into that many columns, and ellipsize() believed a tab was one cell. The ellipsized cell then still overflowed the column, measured by the very function that had just asked for it to be truncated. A table pinned to table_set_width(20) emitted lines of 25, 44 and 39 columns. align_string_mem() undercounted the same way and so padded already overlong cells further. Use unichar_console_width(), split out for this purpose in 5c0d4b5061. While at it, let align_string_mem() decode with utf8_char_console_width() as well, so that the second pass shares both the width model and the validity check with the measuring pass, instead of keeping a more permissive decoder of its own around for the next caller to trip over. Its "count invalid chars as 1" fallback now skips the whole rejected sequence rather than one byte, so that the continuation bytes are not charged a cell each of their own; the branch should be unreachable anyway, since our caller only gets there after utf8_console_width() succeeded on the same string. Note that unichar_console_width() charges a tab a flat eight cells, while a terminal advances to the next multiple of eight, so this is deliberately an upper bound: the two agree only when the tab itself starts on a multiple of eight. A per character helper cannot do better, because a tab's width is a function of the column it starts in, and neither ellipsize_mem() nor align_string_mem() is told what column its string starts at. Overestimating is the safe direction: it can leave a column padded short, but it cannot overflow one. ascii_ellipsize_mem() equates bytes with character cells, which holds for ASCII except for the tab, so keep tabs off that fast path. Like the ANSI sequence case just above it, such strings now take the generic path, which means they get the Unicode ellipsis and one reserved cell rather than "..." and three, even in a non UTF-8 locale. --- src/basic/string-util.c | 10 ++++--- src/shared/format-table.c | 17 ++++++++---- src/test/test-format-table.c | 54 ++++++++++++++++++++++++++++++++++++ src/test/test-string-util.c | 43 ++++++++++++++++++++++++++++ 4 files changed, 114 insertions(+), 10 deletions(-) diff --git a/src/basic/string-util.c b/src/basic/string-util.c index efd690b076..26602ac771 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -414,8 +414,10 @@ char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigne bool has_ansi_seq = string_has_ansi_sequence(s, old_length); - /* If no multibyte characters or ANSI sequences, use ascii_ellipsize_mem for speed */ - if (!has_ansi_seq && ascii_is_valid_n(s, old_length)) + /* If no multibyte characters or ANSI sequences, use ascii_ellipsize_mem for speed. It equates + * bytes with character cells, which holds for ASCII except for the tab, which + * unichar_console_width() counts as 8 cells. */ + if (!has_ansi_seq && ascii_is_valid_n(s, old_length) && !memchr(s, '\t', old_length)) return ascii_ellipsize_mem(s, old_length, new_length, percent); x = (new_length - 1) * percent / 100; @@ -436,7 +438,7 @@ char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigne char32_t c; assert_se(utf8_encoded_to_unichar(i, &c) == r); - int w = unichar_iswide(c) ? 2 : 1; + int w = unichar_console_width(c); if (k + w > x) break; @@ -467,7 +469,7 @@ char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigne if (!tt) return NULL; - w = unichar_iswide(c) ? 2 : 1; + w = unichar_console_width(c); if (k + w > new_length) break; diff --git a/src/shared/format-table.c b/src/shared/format-table.c index 88d76b67f0..518ef5bd25 100644 --- a/src/shared/format-table.c +++ b/src/shared/format-table.c @@ -2079,18 +2079,23 @@ static char* align_string_mem(const char *str, const char *url, size_t new_lengt } else clickable_length = old_length; - /* Determine current width on screen */ + /* Determine current width on screen. Use the same helper the measuring pass uses, so that this + * function cannot keep a second, more permissive idea of both validity and width of its own. + * Note that our caller only gets here after utf8_console_width() succeeded on the very same + * string, so the invalid case below should not be reachable in practice. */ p = str; while (p < str + old_length) { - char32_t c; - - if (utf8_encoded_to_unichar(p, &c) < 0) { - p++, w++; /* count invalid chars as 1 */ + int n = utf8_char_console_width(p); + if (n < 0) { + /* Count an invalid sequence as one character of width one. Skip all of it, so + * that its continuation bytes are not charged a cell each of their own. */ + p = utf8_next_char(p); + w++; continue; } p = utf8_next_char(p); - w += unichar_iswide(c) ? 2 : 1; + w += n; } /* Already wider than the target, if so, don't do anything */ diff --git a/src/test/test-format-table.c b/src/test/test-format-table.c index 8a0f93fb6d..0b3ec62953 100644 --- a/src/test/test-format-table.c +++ b/src/test/test-format-table.c @@ -13,6 +13,7 @@ #include "terminal-util.h" #include "tests.h" #include "time-util.h" +#include "utf8.h" TEST(issue_9549) { _cleanup_(table_unrefp) Table *table = NULL; @@ -67,6 +68,59 @@ TEST(invalid_utf8_cell) { } } +TEST(tab_in_cell) { + _cleanup_(table_unrefp) Table *table = NULL; + _cleanup_free_ char *formatted = NULL; + + /* A tab takes up eight character cells, so the padding align_string_mem() adds has to be + * computed with unichar_console_width() too, or the column comes out too wide. + * + * Note that unichar_console_width() charges a tab a flat eight cells, while a terminal + * advances to the next multiple of eight instead. The two only agree if the tab itself starts + * on a multiple of eight, hence the eight characters in front of it here: that way the + * expected output below is what a terminal really renders, too. */ + + ASSERT_NOT_NULL((table = table_new("name", "value"))); + ASSERT_OK(table_add_many(table, + TABLE_STRING, "aaaaaaaa\tb", + TABLE_STRING, "1", + TABLE_STRING, "wide enough to pad the tab cell", + TABLE_STRING, "2")); + + table_set_width(table, 40); + ASSERT_OK(table_format(table, &formatted)); + + printf("%s\n", formatted); + ASSERT_STREQ(formatted, + "NAME VALUE\n" + "aaaaaaaa\tb 1\n" + "wide enough to pad the tab cell 2\n"); + + /* And the case the two passes have to agree on: a cell whose tab makes it the widest one in + * its column, in a table pinned narrower than that. The cell drives the requested column width + * and is then handed to ellipsize(), which used to believe the tab was a single cell and so + * returned something that still overflowed the column it had just been asked to fit into. */ + + _cleanup_(table_unrefp) Table *narrow = NULL; + _cleanup_free_ char *narrow_formatted = NULL; + _cleanup_strv_free_ char **lines = NULL; + + ASSERT_NOT_NULL((narrow = table_new("name", "value"))); + ASSERT_OK(table_add_many(narrow, + TABLE_STRING, "aaaaaaaa\tbbbbbbbb", + TABLE_STRING, "1", + TABLE_STRING, "short", + TABLE_STRING, "2")); + + table_set_width(narrow, 20); + ASSERT_OK(table_format(narrow, &narrow_formatted)); + + printf("%s\n", narrow_formatted); + ASSERT_NOT_NULL((lines = strv_split_newlines(narrow_formatted))); + STRV_FOREACH(l, lines) + ASSERT_LE(utf8_console_width(*l), (size_t) 20); +} + TEST(multiline) { _cleanup_(table_unrefp) Table *table = NULL; _cleanup_free_ char *formatted = NULL; diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index 79b8175c34..732517541f 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -9,6 +9,7 @@ #include "string-util.h" #include "strv.h" #include "tests.h" +#include "utf8.h" #include "version.h" TEST(ellipsize_mem_ansi_short) { @@ -22,6 +23,48 @@ TEST(ellipsize_mem_ansi_short) { assert_se(c); } +TEST(ellipsize_mem_tab) { + /* unichar_console_width() counts a tab as eight character cells. Callers such as + * format-table.c measure a field with utf8_console_width() and then ask ellipsize() to fit it + * into that many columns, so ellipsize() must not count a tab as a single cell: the result + * would still overflow the column that the measurement had just asked us to fit. */ + + unsigned percent; + + FOREACH_STRING(x, + "\t", + "a\tb", + "ab\tcd", + "\t\t\t", + "a\tb\tc\td\te", + "a\t你\tb") /* tabs mixed with a wide character */ + for (size_t w = 1; w <= 24; w++) + /* Sweep the ellipsis placement too: it decides how much of the string the + * forward loop takes and how much is left to the backward one, and the two + * meet at quite different places for an eight cell tab than for a single cell + * character. */ + FOREACH_ARGUMENT(percent, 0u, 30u, 50u, 90u, 100u) { + _cleanup_free_ char *e = ASSERT_NOT_NULL(ellipsize(x, w, percent)); + + ASSERT_LE(utf8_console_width(e), w); + + /* And don't truncate what already fits. */ + if (utf8_console_width(x) <= w) + ASSERT_STREQ(e, x); + } + + /* Pin a couple of concrete results, so that a change to the assumed tab width or to where the + * ellipsis lands shows up in the diff. Note that these are in terms of the flat eight cells + * unichar_console_width() charges a tab, which is what our callers measure with; a terminal + * instead advances to the next multiple of eight, and hence needs one column less for "a\tb" + * at the start of a line. */ + _cleanup_free_ char *a = ASSERT_NOT_NULL(ellipsize("a\tb", 10, 50)); + ASSERT_STREQ(a, "a\tb"); + + _cleanup_free_ char *b = ASSERT_NOT_NULL(ellipsize("a\tb", 9, 50)); + ASSERT_STREQ(b, "a…b"); +} + TEST(xsprintf) { char buf[5]; From 7cc0257ba80317121faf97fda6d69d0e5b2e43af Mon Sep 17 00:00:00 2001 From: Armaan Sandhu Date: Fri, 4 Sep 2026 11:40:48 -0400 Subject: [PATCH 3/3] strv: measure rebroken lines with utf8_char_console_width() too strv_rebreak_lines() was the last place left that decoded with the permissive utf8_encoded_to_unichar() and then asked unichar_console_width() how wide the result is, so it accepted and measured overlong encodings, surrogates, noncharacters and the obsolete five and six byte forms that utf8_console_width() now refuses. That matters because callers mix the two within a single width computation. sd-varlink-idl.c sizes its indent with utf8_console_width() and then breaks the body with strv_rebreak_lines(); verbs.c does the same. One half of the sum refused a sequence that the other half happily assigned a width to. Use utf8_char_console_width(), which does the validation and the width in one step, and advance with utf8_next_char() rather than the decoded length. The function already refused invalid UTF-8, so the only change is which sequences count as invalid; it now matches utf8_is_valid(), as everywhere else. --- src/basic/strv.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/basic/strv.c b/src/basic/strv.c index 443ae9028d..faf413e8ba 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -8,6 +8,7 @@ #include "escape.h" #include "extract-word.h" #include "fileio.h" +#include "gunicode.h" #include "hashmap.h" #include "log.h" #include "memory-util.h" @@ -1281,13 +1282,14 @@ int strv_rebreak_lines(char **l, size_t width, char ***ret) { in_prefix = false; } - char32_t c; - int n = utf8_encoded_to_unichar(p, &c); - if (n < 0) - return log_debug_errno(n, "Line to break contains invalid UTF-8, refusing: %m"); - - int cw = unichar_console_width(c); - assert(cw >= 0); + /* Measure with the same helper utf8_console_width() uses. Our callers mix the + * two: sd-varlink-idl.c and verbs.c size their indent with utf8_console_width() + * and then break the body here, so if we accepted and measured what that one + * refuses we'd be computing a single width from two ideas of what valid UTF-8 + * is. */ + int cw = utf8_char_console_width(p); + if (cw < 0) + return log_debug_errno(cw, "Line to break contains invalid UTF-8, refusing: %m"); w += cw; @@ -1312,7 +1314,7 @@ int strv_rebreak_lines(char **l, size_t width, char ***ret) { continue; } - p += n; + p = utf8_next_char(p); } /* Process rest of the line, except if all that's left after the last newline we already