Re: Unicode confusables and reordering characters considered harmful, a simple solution

[Daniel Brooks]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Daniel Brooks <db48x@db48x.net>

>> Of course it will show them in the comments and strings.
>
> Then this visual noise will get in the way of people's reading those
> comments and strings, and, for strings, will make it very hard to
> understand what will be presented to the user when those strings are
> output in some UI.
>
>> That’s where the problem is.
>
> No, the problem is elsewhere entirely: it's in the punctuation
> characters unrelated to strings and comments whose directionality is
> overridden, and which thus display in places that cause incorrect
> visual interpretation of the program during a casual read.

Look at the examples again. In many of them, all of the bidi override
characters are inside a string or comment. When that is the case, these
characters are only a problem if they cause characters that are inside
the string or comment to appear to be outside of it, by reordering those
characters relative to the syntactic markers for the string or
comment. In other examples these characters are _outside_ the string or
comment.

Unless Emacs has specific knowledge of the language syntax, showing the
characters is the only sure way to know if there is a problem or not.

> You misunderstand the cause.  The mere presence of these characters is
> NOT the root cause.  These characters are legitimate and helpful when
> used as intended.  See TUTORIAL.he for a pertinent example.

Please don’t presume to tell me what I do or don’t understand. Yes,
there are use cases which are not harmful, but as I have said it must be
up to either the programmer or the compiler to answer that
question. Emacs doesn’t know the syntax of every programming language.

>> Furthermore, I have not suggested that showing the characters needs to
>> preclude any other form of highlighting. If you wish to develop some
>> additional way of warning the developer, please do so.
>
> We are talking about what should be in Emacs.  What you suggest
> shouldn't.

No other suggested feature will be useful to me. This one will. I
suggest to you that you do not know what all users want.

>> However, I suspect that the compilers for most languages currently in
>> active development will develop their own warnings and error messages as
>> well. We have plenty of ways for those messages to show up inside Emacs
>> as highlights.
>
> That's a tangent.  We are discussing what Emacs should do as a
> programmer's editor to flag such suspicious code.  That shouldn't need
> a compiler if we can do the job ourselves.  And we can.

This is not a tangent. Emacs relies heavily on compilers and language
runtimes for many of its features. This is just one more area where
Emacs should not try to be too clever.

>
>> Rust, for example, has already done so. Here’s an example:
>> 
>>     error: unicode codepoint changing visible direction of text present in 
>> comment
>>       --> src/pathmap/path.rs:10:5
>>        |
>>     10 |     /* } if is_admin  begin admins only */
>>        |     ^^-^^-^^^^^^^^^^--^^^^^^^^^^^^^^^^^^^^
>>        |     | |  |          ||
>>        |     | |  |          |'\u{2066}'
>>        |     | |  |          '\u{2069}'
>>        |     | |  '\u{2066}'
>>        |     | '\u{202e}'
>>        |     this comment contains invisible unicode text flow control 
>> codepoints
>>        |
>>        = note: `#[deny(text_direction_codepoint_in_comment)]` on by default
>>        = note: these kind of unicode codepoints change the way text
>> flows on applications that support them, but can cause confusion
>> because they change the order of characters on the screen
>>        = help: if their presence wasn't intentional, you can remove them
>
> Since the Rust compiler evidently does this when it finds these
> characters inside comments (and probably also inside strings), IMNSHO
> this is a terrible misfeature, because it means code that uses those
> controls in legitimate ways cannot be compiled without tweaking
> non-default options.  That's a cop-out, not the way to flag the
> problematic cases.

Your conclusion here is incorrect. Rust has choosen a fast strategy,
where they implement a broad error today (well, four days ago) knowing
that it does not prevent them from introducing a more refined error or
set of errors later.

Rust also has a very flexible annotation system that allows the
programmer to annotate specific statements and language items. If a use
of these characters is determined to be legitimate, the programmer can
annotate the comment, or the function the comment is in, so that this
error is disabled. In projects with strong review culture, seeing that
annotation while doing a code review will be a very strong signal that
something unusual is going on, and that it needs to be considered
carefully. Annotations are are a great feature of Rust that I do not
expect Emacs to take into account.

Instead I think that Emacs should adopt a similar fast
strategy. Anything we do today can be refined later.

>> Naturally that already shows up inside of Emacs just fine; see the
>> attached image.
>
> I think this is terrible.  At best, it only tells you that something
> non-trivial goes on here (but what exactly?).  At worst, it looks like
> corruption of the source.  And while in the malicious case treating
> that as corruption is not such a bad idea, all the valid uses of these
> characters will also look like corruption.  Which means the cure is
> probably worse than the disease, because the malicious cases are a
> tiny fraction of the valid ones.

I cannot believe that you really think this. It shows up with exactly the
same highlighting that your recently–introduced
highlight-confusing-reorderings function uses. It looks nothing like
“corruption of the source”, whatever you may mean by that. The error
message explains _exactly_ what the compiler is guarding against.

Also, thinking about fractions here is irrelevant. The Rust team
examined the source of every Rust crate every published on
https://crates.io, and found only 5 that even used these
characters. With a sample size that small, percentages don’t mean much.

https://blog.rust-lang.org/2021/11/01/cve-2021-42574.html

> It's the same kind of "solution" like the airport security after 9/11:
> because there was a bunch of terrorists, we are all now suspect as
> potential terrorists, and for that reason we are constantly delayed
> for hours and humiliated by endless frisking.

Now I think you are being deliberately insulting. I conclude that your
only purpose in this conversation was to troll people or to say no to
any solution you didn’t think of yourself.

Yours doesn’t even work with `next-error`. Useless.

db48x