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.

(cherry picked from commit 0834378fb9)

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;
 

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 */

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;

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];