Hi Michael, thanks for your reply, much appreciated. (Saw your name several times in git blame of encoding.c, so you seem to have more knowledge of that uncommented code than all others who have commented (sic!) on this issue so far. :-)
Michael Schroeder schrieb am Thu, Feb 11, 2021 at 02:20:33PM +0000: > > Thanks, but this seems to break the actual output. > > But isn't the output broken anyway? Maybe. I don't know how the Chinese glyphs in the PoC should have looked like and I can't really tell that good what's just font issues and what's actually terminal issues. When xterm was found to having similar issues, I temporarily switched to kitty as terminal emulator to test that stuff in. (Also can better show combined diacriticals with giant font sizes and scalable fonts. :-) > Two years ago, the line > > c = (c & 255) | (unsigned char)D_rend.font << 8; > > was deleted from RAW_PUTCHAR, but the utf8_handle_comb function in > encode.c was not adapted. Sounds like a possible cause for this issue. > Since then, combining characters cannot have worked. I haven't sent my mail about that yet, but I've tested especially combining and stacked combining diacriticals (two in a row) with screen 4.8.0 in Debian Unstable: And they worked well _before_ any patching of this issue. With my as well as with Taviso's patch, stacked combining diacriticals no more work. One of my (randomly choosen) examples was ASCII letter "e" plus U+0324 COMBINING DIAERESIS BELOW + U+0312 COMBINING TURNED COMMA ABOVE. Looks like this (if mail transport didn't kill it :-): "e̤̒" This works fine with screen 4.8.0-3 in Debian Testing (the one which crashes, without any of the patches in this thread), but no more with 4.8.0-4 which sports my (flawed, but at least no more crashing) patch. Will try to send that half-finished mail maybe in a ¾-finished state after this one. :-) > Do you have a local patch for this? I need to check against the patches we apply in Debian on top. (Most of them have been adopted upstream, but not all.) Have a look at the patches in https://salsa.debian.org/debian/screen/-/tree/master/debian/patches Those starting with 0x to 6x are potentially relevant here, except 52fix_screen_utf8_nfd.patch which is not applied. It was meant to fix > > https://savannah.gnu.org/bugs/index.php?31336 aka > > https://bugs.debian.org/600246 but made worse. but caused a regression elsewhere. Patches starting with 8x are feature patches (all session handling and sorting related). (Actually 58-show-encoding-hardstatus.patch is also a feature patch. I probably should clean up patch names and ordering. But no more before the next Debian stable release. :-) Patches starting with 9x are cherry-picked patches from upstream or patches for CVE issues expected to be fixed upstream very soon, too. But the only patches which touch encoding.c or ansi.c are 99_CVE-2021-26937.patch (encoding.c) and the disabled 52fix_screen_utf8_nfd.patch (ansi.c). > But there's also the underlying issue that comb_tofront() moves entries > between the two lists. So here's a revised patch: Yay, thanks, will test that later today. Much appreciated! Regards, Axel -- ,''`. | Axel Beckert <a...@debian.org>, https://people.debian.org/~abe/ : :' : | Debian Developer, ftp.ch.debian.org Admin `. `' | 4096R: 2517 B724 C5F6 CA99 5329 6E61 2FF9 CD59 6126 16B5 `- | 1024D: F067 EA27 26B9 C3FC 1486 202E C09E 1D89 9593 0EDE
