[PATCH] Fix backspace in the username login prompt

[PATCH] Fix backspace in the username login prompt

[Egor Ignatov]

This patch fixes an old bug when backspace key does not work in the
username login prompt. It allows BIDI type BN characters (control
chars like '\b') in the visual line. And when grub_gfxterm_putchar
recieves this line it successfully processes backspace.

Signed-off-by: Egor Ignatov <egori@altlinux.org>
---
Hi, I'd like to try to propose this patch again. I'm new to the open
source community, so any feedback and suggestions will be appreciated.

 grub/grub-core/normal/charset.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/grub/grub-core/normal/charset.c b/grub/grub-core/normal/charset.c
index b0ab47d73..30e819bdf 100644
--- a/grub/grub-core/normal/charset.c
+++ b/grub/grub-core/normal/charset.c
@@ -925,6 +925,7 @@ grub_bidi_line_logical_to_visual (const grub_uint32_t *logical,
 	    pop_stack ();
 	    break;
 	  case GRUB_BIDI_TYPE_BN:
+	    visual_len++;
 	    break;
 	  case GRUB_BIDI_TYPE_R:
 	  case GRUB_BIDI_TYPE_AL:
-- 
2.25.4

Re: [PATCH] Fix backspace in the username login prompt

[Daniel Kiper]

First of all, sorry for late reply but I am busy...

On Thu, Apr 29, 2021 at 05:43:48PM +0300, Egor Ignatov wrote:
> This patch fixes an old bug when backspace key does not work in the
> username login prompt. It allows BIDI type BN characters (control
> chars like '\b') in the visual line. And when grub_gfxterm_putchar
> recieves this line it successfully processes backspace.

Did you test this patch with other BIDI type BN characters [1]? I am not
convinced it works with them as expected. I think earlier version of your
patch was more promising...

> Signed-off-by: Egor Ignatov <egori@altlinux.org>
> ---
> Hi, I'd like to try to propose this patch again. I'm new to the open
> source community, so any feedback and suggestions will be appreciated.
>
>  grub/grub-core/normal/charset.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/grub/grub-core/normal/charset.c b/grub/grub-core/normal/charset.c
> index b0ab47d73..30e819bdf 100644
> --- a/grub/grub-core/normal/charset.c
> +++ b/grub/grub-core/normal/charset.c
> @@ -925,6 +925,7 @@ grub_bidi_line_logical_to_visual (const grub_uint32_t *logical,
>  	    pop_stack ();
>  	    break;
>  	  case GRUB_BIDI_TYPE_BN:
> +	    visual_len++;
>  	    break;
>  	  case GRUB_BIDI_TYPE_R:
>  	  case GRUB_BIDI_TYPE_AL:

Daniel

[1] https://www.compart.com/en/unicode/bidiclass/BN

[PATCH v2] Fix backspace in the username login prompt

[Egor Ignatov]

This patch fixes a bug when backspace key does not work in the
username login prompt. It allows backspace characters in the BIDI
visual line. Thus grub_gfxterm_putchar receives '\b' chars that
were previously ignored.

Signed-off-by: Egor Ignatov <egori@altlinux.org>
---
> Did you test this patch with other BIDI type BN characters [1]? I am not

You are right, I added a check to handle only backspace characters

> convinced it works with them as expected. I think earlier version of your
> patch was more promising...

Earlier versions of my patch included workaround for special case when
the password is longer than one line. This scenario is not very
realistic and I dont think it's worth complicating the codebase.

Thank you for your response.
Let me know if you have any concerns about this version.

Egor

 grub-core/normal/charset.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/grub-core/normal/charset.c b/grub-core/normal/charset.c
index 4dfcc3107..8e7add1f8 100644
--- a/grub-core/normal/charset.c
+++ b/grub-core/normal/charset.c
@@ -931,6 +931,8 @@ grub_bidi_line_logical_to_visual (const grub_uint32_t *logical,
 	    pop_stack ();
 	    break;
 	  case GRUB_BIDI_TYPE_BN:
+	    if (visual[visual_len].base == '\b')
+	      visual_len++;
 	    break;
 	  case GRUB_BIDI_TYPE_R:
 	  case GRUB_BIDI_TYPE_AL:
-- 
2.29.3

Re: [PATCH v2] Fix backspace in the username login prompt

[Daniel Kiper]

On Tue, May 11, 2021 at 09:52:39AM +0300, Egor Ignatov wrote:
> This patch fixes a bug when backspace key does not work in the
> username login prompt. It allows backspace characters in the BIDI
> visual line. Thus grub_gfxterm_putchar receives '\b' chars that
> were previously ignored.
>
> Signed-off-by: Egor Ignatov <egori@altlinux.org>
> ---
> > Did you test this patch with other BIDI type BN characters [1]? I am not
>
> You are right, I added a check to handle only backspace characters
>
> > convinced it works with them as expected. I think earlier version of your
> > patch was more promising...
>
> Earlier versions of my patch included workaround for special case when
> the password is longer than one line. This scenario is not very
> realistic and I dont think it's worth complicating the codebase.
>
> Thank you for your response.
> Let me know if you have any concerns about this version.

Did you test your patch outside of username login prompt, e.g. CLI or
menu entry editing? I am afraid your patch breaks at least these cases.

Daniel

> Egor
>
>  grub-core/normal/charset.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/grub-core/normal/charset.c b/grub-core/normal/charset.c
> index 4dfcc3107..8e7add1f8 100644
> --- a/grub-core/normal/charset.c
> +++ b/grub-core/normal/charset.c
> @@ -931,6 +931,8 @@ grub_bidi_line_logical_to_visual (const grub_uint32_t *logical,
>  	    pop_stack ();
>  	    break;
>  	  case GRUB_BIDI_TYPE_BN:
> +	    if (visual[visual_len].base == '\b')
> +	      visual_len++;
>  	    break;
>  	  case GRUB_BIDI_TYPE_R:
>  	  case GRUB_BIDI_TYPE_AL:

Re: [PATCH v2] Fix backspace in the username login prompt

[Egor Ignatov]

[-- Attachment #1: Type: text/plain, Size: 324 bytes --]

On 5/17/21 7:34 PM, Daniel Kiper wrote:
> Did you test your patch outside of username login prompt, e.g. CLI or
> menu entry editing? I am afraid your patch breaks at least these cases.
>
> Daniel

Indeed I tested it with both CLI and menu entry editing and it works fine.

Thank you for taking the time to answer me.

Egor

[-- Attachment #2: Type: text/html, Size: 1644 bytes --]

Re: [PATCH v2] Fix backspace in the username login prompt

[egori]

Indeed I tested it with both CLI and menu entry editing and it works fine.

Thank you for taking the time to answer me.

PS Sorry if you received this message more than once.

Egor