utf8,format-table: make the console width helpers agree with the UTF-8 validators (#43653)
systemd has two UTF-8 decoders, and the console width path uses the
permissive one.
`utf8_encoded_to_unichar()` performs no overlong check, no
`unichar_is_valid()` check, 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. `utf8_char_console_width()` called the bare
decoder directly.
Checking both against `utf8_is_valid_n()` over all 4,244,897,280 NUL
free byte strings of length one to four, which is the entire UTF-8
encoding space:
| decoder | disagreements with `utf8_is_valid_n()` |
| --- | --- |
| `utf8_encoded_valid_unichar()` | 0 |
| `utf8_encoded_to_unichar()` | 8,835,838, always more permissive |
So `utf8_console_width()` returned a finite width for strings
`utf8_is_valid()` rejects. `format-table.c` measures a cell with
`utf8_console_width()` and, when it does not fit, hands it to
`ellipsize()`, which validates strictly and returns NULL. That is
reported as `-ENOMEM`:
```
lead surrogate valid_utf8=0 utf8_console_width=25 ellipsize=NULL
-> table_format FAILS: Cannot allocate memory
```
A surrogate, overlong, noncharacter or five byte sequence in any cell
failed the whole table with a bogus out of memory error, and only once
that column was narrow enough to need truncation, so whether it happened
depended on the width of the user's terminal. At other widths the
invalid sequence was printed to the terminal instead. Invalid cells are
now refused consistently with `-EINVAL`, which is what the sequences
`utf8_encoded_to_unichar()` already rejected always produced.
The second commit fixes a related disagreement. `ellipsize_mem()` and
`align_string_mem()` open coded character width as `unichar_iswide(c) ?
2 : 1`, while `unichar_console_width()` counts a tab as eight cells.
`format-table.c` uses both: it finds a field too wide with
`utf8_console_width()`, then asks `ellipsize()` to fit that many
columns, and `ellipsize()` thought a tab was one cell, so the result
still overflowed the column that the measurement had just asked it to
fit. A table pinned to `table_set_width(20)` emitted lines of 25, 44 and
39 columns. This uses `unichar_console_width()`, which was split out for
exactly this purpose in 5c0d4b5061.
Both commits add tests that fail before the change and pass after. The
full unit suite is otherwise unaffected.
This commit is contained in:
Yu Watanabe
@@ -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;
|
||||
|
||||
|
||||
+10
-8
@@ -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
|
||||
|
||||
+7
-1
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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 */
|
||||
|
||||
@@ -8,9 +8,12 @@
|
||||
#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"
|
||||
#include "utf8.h"
|
||||
|
||||
TEST(issue_9549) {
|
||||
_cleanup_(table_unrefp) Table *table = NULL;
|
||||
@@ -35,6 +38,89 @@ 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(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;
|
||||
|
||||
@@ -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];
|
||||
|
||||
|
||||
+29
-6
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user