iwd.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] client: validate utf-8 before displaying row
@ 2022-10-12 18:35 James Prestwood
  2022-10-12 18:35 ` [PATCH 2/4] client: add extra parameter for new width when printing rows James Prestwood
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: James Prestwood @ 2022-10-12 18:35 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

In theory any input to this function should have valid utf-8 but
just in case the strings should be validated. This removes the
need to check the return of l_utf8_get_codepoint which is useful
since there is no graceful failure path at this point.
---
 client/display.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/client/display.c b/client/display.c
index ddd9551c..03ea33a1 100644
--- a/client/display.c
+++ b/client/display.c
@@ -420,11 +420,7 @@ static char* next_line(char *s, unsigned int *max, char **color_out)
 				last_space = i;
 			sequence_len = l_utf8_get_codepoint(&s[i], s_len - i,
 									&w);
-			if (sequence_len < 0) {
-				sequence_len = 1;
-				sequence_columns = 1;
-			} else
-				sequence_columns = wcwidth(w);
+			sequence_columns = wcwidth(w);
 		}
 
 		/* Compensate max bytes */
@@ -532,9 +528,17 @@ void display_table_row(const char *margin, unsigned int ncolumns, ...)
 
 	for (i = 0; i < ncolumns; i++) {
 		struct table_entry *e = &entries[i];
+		char *v;
 
 		e->width = va_arg(va, unsigned int);
-		e->next = l_strdup(va_arg(va, char*));
+		v = va_arg(va, char *);
+
+		if (!l_utf8_validate(v, strlen(v), NULL)) {
+			display_error("Invalid utf-8 string!");
+			goto done;
+		}
+
+		e->next = l_strdup(v);
 
 		str += entry_append(e, str);
 	}
@@ -565,9 +569,13 @@ void display_table_row(const char *margin, unsigned int ncolumns, ...)
 		str = buf;
 	}
 
+done:
 	for (i = 0; i < ncolumns; i++) {
 		if (entries[i].color)
 			l_free(entries[i].color);
+
+		if (entries[i].next)
+			l_free(entries[i].next);
 	}
 }
 
-- 
2.34.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/4] client: add extra parameter for new width when printing rows
  2022-10-12 18:35 [PATCH 1/4] client: validate utf-8 before displaying row James Prestwood
@ 2022-10-12 18:35 ` James Prestwood
  2022-10-12 18:35 ` [PATCH 3/4] client: fix not accounting for color escapes after line break James Prestwood
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: James Prestwood @ 2022-10-12 18:35 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

The old 'max' parameter was being used both as an input and output
parameter which was confusing. Instead have next_line take the
column width as input, and output a new width which includes any
color escapes and wide characters.
---
 client/display.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/client/display.c b/client/display.c
index 03ea33a1..df0950d2 100644
--- a/client/display.c
+++ b/client/display.c
@@ -388,24 +388,27 @@ static unsigned int color_end(char *s)
 }
 
 /*
- * Finds last space in 's' before 'max' characters, terminates at that index,
+ * Finds last space in 's' before 'width' characters, terminates at that index,
  * and returns a new string to be printed on the next line.
  *
- * 'max' should be set to the column width, but is also an out parameter since
- * this width can be updated if colored escapes are detected.
+ * 'new_width' will be updated to include extra bytes for color escapes or
+ * wide characters if found.
  *
  * Any colored escapes found are set to 'color_out' so they can be re-enabled
  * on the next line.
  */
-static char* next_line(char *s, unsigned int *max, char **color_out)
+static char* next_line(char *s, unsigned int width, unsigned int *new_width,
+			char **color_out)
 {
 	unsigned int i = 0;
 	int last_space = -1;
 	int last_color = -1;
 	unsigned int s_len = strlen(s);
 
+	*new_width = width;
+
 	/* Find the last space before 'max', as well as any color */
-	while (i <= *max && i != s_len) {
+	while (i <= *new_width && i < s_len) {
 		int sequence_len;
 		int sequence_columns;
 		wchar_t w;
@@ -424,17 +427,17 @@ static char* next_line(char *s, unsigned int *max, char **color_out)
 		}
 
 		/* Compensate max bytes */
-		*max += sequence_len - sequence_columns;
+		*new_width += sequence_len - sequence_columns;
 		i += sequence_len;
 	}
 
 	/* Reached the end of the string within the column bounds */
-	if (i <= *max)
+	if (i <= *new_width)
 		return NULL;
 
 	/* Not anywhere nice to split the line */
 	if (last_space == -1)
-		last_space = *max - 1;
+		last_space = *new_width - 1;
 
 	/*
 	 * Only set the color if it occurred prior to the last space. If after,
@@ -465,7 +468,7 @@ static int entry_append(struct table_entry *e, char *line_buf)
 {
 	char *value = e->next;
 	unsigned int ret = 0;
-	unsigned int width = e->width;
+	unsigned int new_width;
 
 	/* Empty line */
 	if (!value)
@@ -479,10 +482,10 @@ static int entry_append(struct table_entry *e, char *line_buf)
 	}
 
 	/* Advance entry to next line, and terminate current */
-	e->next = next_line(value, &width, &e->color);
+	e->next = next_line(value, e->width, &new_width, &e->color);
 
 	/* Append current line */
-	ret += sprintf(line_buf + ret, "%-*s  ", width, value);
+	ret += sprintf(line_buf + ret, "%-*s  ", new_width, value);
 
 	l_free(value);
 
-- 
2.34.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/4] client: fix not accounting for color escapes after line break
  2022-10-12 18:35 [PATCH 1/4] client: validate utf-8 before displaying row James Prestwood
  2022-10-12 18:35 ` [PATCH 2/4] client: add extra parameter for new width when printing rows James Prestwood
@ 2022-10-12 18:35 ` James Prestwood
  2022-10-12 18:35 ` [PATCH 4/4] client: fix missing character for line breaks without spaces James Prestwood
  2022-10-12 18:56 ` [PATCH 1/4] client: validate utf-8 before displaying row Denis Kenzior
  3 siblings, 0 replies; 5+ messages in thread
From: James Prestwood @ 2022-10-12 18:35 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

Each color escape is tracked and the new_width is adjusted
accordingly. But if the color escape comes after a space which breaks
the line, the adjusted width ends up being too long since that escape
sequence isn't appearing on the current line. This causes the next
column to be shifted over.
---
 client/display.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/client/display.c b/client/display.c
index df0950d2..812fa711 100644
--- a/client/display.c
+++ b/client/display.c
@@ -404,8 +404,10 @@ static char* next_line(char *s, unsigned int width, unsigned int *new_width,
 	int last_space = -1;
 	int last_color = -1;
 	unsigned int s_len = strlen(s);
+	unsigned int color_adjust = 0;
 
 	*new_width = width;
+	*color_out = NULL;
 
 	/* Find the last space before 'max', as well as any color */
 	while (i <= *new_width && i < s_len) {
@@ -418,9 +420,22 @@ static char* next_line(char *s, unsigned int width, unsigned int *new_width,
 			/* color escape won't count for column width */
 			sequence_columns = 0;
 			last_color = i;
+
+			/*
+			 * Color after a space. If the line gets broken this
+			 * will need to be removed off new_width since it will
+			 * appear on the next line.
+			 */
+			if (last_space != -1)
+				color_adjust += sequence_len;
+
 		} else {
-			if (s[i] == ' ')
+			if (s[i] == ' ') {
 				last_space = i;
+				/* Any past colors will appear on this line */
+				color_adjust = 0;
+			}
+
 			sequence_len = l_utf8_get_codepoint(&s[i], s_len - i,
 									&w);
 			sequence_columns = wcwidth(w);
@@ -446,8 +461,8 @@ static char* next_line(char *s, unsigned int width, unsigned int *new_width,
 	if (last_color != -1 && last_space >= last_color)
 		*color_out = l_strndup(s + last_color,
 					color_end(s + last_color));
-	else
-		*color_out = NULL;
+	else if (last_color != -1 && last_space < last_color)
+		*new_width -= color_adjust;
 
 	s[last_space] = '\0';
 
-- 
2.34.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 4/4] client: fix missing character for line breaks without spaces
  2022-10-12 18:35 [PATCH 1/4] client: validate utf-8 before displaying row James Prestwood
  2022-10-12 18:35 ` [PATCH 2/4] client: add extra parameter for new width when printing rows James Prestwood
  2022-10-12 18:35 ` [PATCH 3/4] client: fix not accounting for color escapes after line break James Prestwood
@ 2022-10-12 18:35 ` James Prestwood
  2022-10-12 18:56 ` [PATCH 1/4] client: validate utf-8 before displaying row Denis Kenzior
  3 siblings, 0 replies; 5+ messages in thread
From: James Prestwood @ 2022-10-12 18:35 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

In nearly all cases the auto-line breaks can be on spaces in the
string. The only exception (so far) is DPP which displays a very
long URI without any spaces. When this is displayed a single
character was lost on each break. This was due to the current line
being NULL terminated prior to the next line string being returned.

To handle both cases the next line is copied prior to terminating
the current line. The offsets were modified slightly so the line
will be broken *after* the space, not on the space. This leaves
the space at the end of the current line which is invisible to the
user but allows the no-space case to also work without loss of
the last character (since that last character is where the space
normally would be).
---
 client/display.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/client/display.c b/client/display.c
index 812fa711..59086642 100644
--- a/client/display.c
+++ b/client/display.c
@@ -405,6 +405,7 @@ static char* next_line(char *s, unsigned int width, unsigned int *new_width,
 	int last_color = -1;
 	unsigned int s_len = strlen(s);
 	unsigned int color_adjust = 0;
+	char *ret;
 
 	*new_width = width;
 	*color_out = NULL;
@@ -452,7 +453,7 @@ static char* next_line(char *s, unsigned int width, unsigned int *new_width,
 
 	/* Not anywhere nice to split the line */
 	if (last_space == -1)
-		last_space = *new_width - 1;
+		last_space = *new_width;
 
 	/*
 	 * Only set the color if it occurred prior to the last space. If after,
@@ -464,9 +465,11 @@ static char* next_line(char *s, unsigned int width, unsigned int *new_width,
 	else if (last_color != -1 && last_space < last_color)
 		*new_width -= color_adjust;
 
-	s[last_space] = '\0';
+	ret = l_strdup(s + last_space + 1);
 
-	return l_strdup(s + last_space + 1);
+	s[last_space + 1] = '\0';
+
+	return ret;
 }
 
 struct table_entry {
-- 
2.34.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/4] client: validate utf-8 before displaying row
  2022-10-12 18:35 [PATCH 1/4] client: validate utf-8 before displaying row James Prestwood
                   ` (2 preceding siblings ...)
  2022-10-12 18:35 ` [PATCH 4/4] client: fix missing character for line breaks without spaces James Prestwood
@ 2022-10-12 18:56 ` Denis Kenzior
  3 siblings, 0 replies; 5+ messages in thread
From: Denis Kenzior @ 2022-10-12 18:56 UTC (permalink / raw)
  To: James Prestwood, iwd

Hi James,

On 10/12/22 13:35, James Prestwood wrote:
> In theory any input to this function should have valid utf-8 but
> just in case the strings should be validated. This removes the
> need to check the return of l_utf8_get_codepoint which is useful
> since there is no graceful failure path at this point.
> ---
>   client/display.c | 20 ++++++++++++++------
>   1 file changed, 14 insertions(+), 6 deletions(-)
> 

All applied, thanks.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-10-12 18:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-12 18:35 [PATCH 1/4] client: validate utf-8 before displaying row James Prestwood
2022-10-12 18:35 ` [PATCH 2/4] client: add extra parameter for new width when printing rows James Prestwood
2022-10-12 18:35 ` [PATCH 3/4] client: fix not accounting for color escapes after line break James Prestwood
2022-10-12 18:35 ` [PATCH 4/4] client: fix missing character for line breaks without spaces James Prestwood
2022-10-12 18:56 ` [PATCH 1/4] client: validate utf-8 before displaying row Denis Kenzior

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).