isamert / scli

a simple terminal user interface for signal messenger (using signal-cli)
GNU General Public License v3.0
439 stars 40 forks source link

Unicode Formatting Codepoints destroy layout #130

Closed mutax closed 3 years ago

mutax commented 3 years ago

Hey, happy first time user here, noticing that when people put weird characters into names of groups it seems, like the right-to-left indication, this breaks the ui in a spectacular way crashing the ui in the end when selecting a group.

A workaround for this is:

     def _get_name_markup(self):
     + import unicodedata
     …
        if name:
     +    name = "".join(filter(lambda c: unicodedata.category(c) != 'Cf', str(name)))
          markup.append(name)
        else:
           name = self.contact.id
     +     name = "".join(filter(lambda c: unicodedata.category(c) != 'Cf', str(name)))

to filter all this. Not perfect, as the group name now is not spelled out correctly, but at least it works... (seems rtl was added but not ltr/reset)

edit: The UI still breaks when the group is opened as there the name is not 'filtered' - but as this seems to be a more general problem hot patching that there again might not be sensible. Adding a filter at the point where names are received initially seems the better approach.

exquo commented 3 years ago

Thanks for reporting!

Could you give an example of a group name string that causes a crash? I have tried to reproduce with the following group name:

Some ⁧RTL⁩ and مفتاح معايير ar

which contains unicode bidi characters U+2067 and U+2069 around the word "RTL" (ref) and some Arabic text (strongly typed as right-to-left). Scli does not crash when selecting the group, but the UI is indeed a bit messed up. In fact, in exactly the same way and for exactly the same reason as in #115 (non-printable characters messing up urwid's layout).

I'll add a fix to strip non-printable chars from group names as well (currently it's only done for individual contacts).

mutax commented 3 years ago

I think my friends who created the group intentionally messed up, by only including the rtl character, but not the 'reset' or ltr character afterwards. This makes everything in the ui following the group name flipped around and spelled backwards. I guess what would be needed for this not to mess up would be some sort of cleaning and adding a corresponding character to reset the direction after the incriminating string, if the direction was changed.

So using "regular" group names with changed direction is not a problem it seem. Sorry if that was not clear enough.

The crash happened, when I moved the cursor/focus to the group names and tried to open it. But that might also have been the other issue that I opened, I don't remember to be honest.

Otherwise: <3 <3 <3 for the cli. It sucks soooo much less than the Electron app.

exquo commented 3 years ago

I think my friends who created the group intentionally messed up, by only including the rtl character, but not the 'reset' or ltr character afterwards

Were they trying to break your terminal? Mean! :) I guess they're more resourceful than me - I still can't mess up my terminal anywhere that bad. I've tried various combinations of unicode bidi symbols and RTL text on gnome-terminal and mlterm, but at worst got urwid boxes' lines shifted like in #115, and text on the same line out of order (in mlterm).

Anyway, this should be fixed now (in the develop branch): all the non-printable characters in groups' names are stripped during parsing of signal-cli data, before they have a chance to get anywhere near the terminal screen.

Otherwise: <3 <3 <3 for the cli. It sucks soooo much less than the Electron app.

Thanks! That's exactly what we're going for! :)

meonkeys commented 10 months ago

Not exactly a regression (I don't get a crash), but the vertical lines bordering the contact list and conversation view get pushed out of alignment if a group name contains an emoji. For example:

| foo      ||            |
| flarp    ||            |
| flarp    ||            |
| lbozo 🎨  ||            
| baz      ||            |
| biff     ||            |

The vertical bar on the right side of the conversation pane is similarly moved right (out of view) if a message contains an emoji.

I repro'd this on both the tip of main and develop.

Ah, might just be #48