Re: [PATCH] Cygwin: console: Fix handling of surrogate pairs

Hi Takashi,

On Tue, 26 May 2026, Takashi Yano wrote:

> The commit 782aac590af7 introduced surrogate-pair handling. However,
> it does not work as expected in the legacy console. This is because
> a KeyDown event for ALT key with UnicodeChar == 0 is inserted the
> between surrogate pair. The current code reads the next key event
> unconditionally for the second UnicodeChar, but it is not correct.
> This patch searches the next appropriate key event with a valid
> UnicodeChar, ensuring that the second code unit is valid.
> 
> Fixes: 782aac590af7 ("Cygwin: console: Handle Unicode surrogate pairs.")
> Signed-off-by: Takashi Yano <[email protected]>
> Reviewed-by:
> ---
>  winsup/cygwin/fhandler/console.cc | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
> index 6fd4cd965..45eff6efe 100644
> --- a/winsup/cygwin/fhandler/console.cc
> +++ b/winsup/cygwin/fhandler/console.cc
> @@ -1452,9 +1452,21 @@ fhandler_console::process_input_message (void)
>  	    }
>  	  else
>  	    {
> -	      WCHAR second = unicode_char >= 0xd800 && unicode_char <= 0xdbff
> -		  && i + 1 < total_read ?
> -		  input_rec[i + 1].Event.KeyEvent.uChar.UnicodeChar : 0;
> +	      WCHAR second = 0;
> +	      DWORD second_pos = i;
> +	      if (unicode_char >= 0xd800 && unicode_char <= 0xdbff)
> +		for (DWORD j = i + 1; j < total_read; j++)
> +		  {
> +		    /* Do not check bKeyDown. bKeyDown is 0 for surrogate
> +		       pair in legacy console */
> +		    if (input_rec[j].EventType == KEY_EVENT &&
> +			input_rec[j].Event.KeyEvent.uChar.UnicodeChar)
> +		      {
> +			second = input_rec[j].Event.KeyEvent.uChar.UnicodeChar;
> +			second_pos = j;
> +			break;
> +		      }
> +		  }
>  
>  	      if (second < 0xdc00 || second > 0xdfff)
>  		{
> @@ -1465,7 +1477,7 @@ fhandler_console::process_input_message (void)

It's a bit unfortunate that the diff here hides the fact that the
following is in the `else` branch...

In any case, the patch makes sense to me, and I really appreciate the
commit message that puts the diff into context.

Thanks!
Johannes

>  		  /* handle surrogate pairs */
>  		  WCHAR pair[2] = { unicode_char, second };
>  		  nread = sys_wcstombs (tmp + 1, 59, pair, 2);
> -		  i++;
> +		  i = second_pos;
>  		}
>  
>  	      /* Determine if the keystroke is modified by META.  The tricky
> -- 
> 2.51.0
> 
>