Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify that highlighting non-basic ASCII characters is done for security #212812

Open
BrianJDrake opened this issue May 15, 2024 · 3 comments
Open
Assignees

Comments

@BrianJDrake
Copy link

It is well-established that highlighting non-basic ASCII characters is done for security (see my latest comment at #147888, which contains many citations). Unfortunately, there is nothing in the VS Code settings that indicates this; VS Code should make it clear.

This omission in VS Code contributes to a misconception that non-basic ASCII characters are an annoyance but not a security issue; I have wasted some time at the issue linked above trying to drive home the point that they are a security issue.

@github-account1111
Copy link

github-account1111 commented May 15, 2024

There was no need to get all worked up and create a separate issue. #147888 explains why your interpretation of the setting is wrong. A basic grasp of English would've also explained it to you:

This document contains many non-basic ASCII unicode characters

The popup doesn't appear for documents with just a few of them. Don't you think if it were a "security issue" it would've popped up no matter whether there were a lot or a few? There is nothing to be made clear. The purpose of the setting has been clear from the get-go.

This issue should be closed. It's a QoL issue, not a security one.

@BrianJDrake
Copy link
Author

@github-account1111 Again, it is not 'my interpretation'. I cited many sources, including the VS Code developers themselves, which clearly indicate that what I said about it being a security issue is correct.

I do think that VS Code should have an setting (perhaps enabled by default) to display the banner for any number of non-basic ASCII characters, if highlighting of such characters is also enabled. This would help to drive home the point that such characters are a security issue. It would also make dealing with them easier: I often go through text samples twice, once with syntax highlighting disabled to make it easier to confirm there is no Unicode highlighting, and again with syntax highlighting enabled to check everything else.

@github-account1111
Copy link

github-account1111 commented May 16, 2024

Again, the setting clearly says:

This document contains many non-basic ASCII unicode characters

Why do you think it says many if not to point out the fact that they're hard to render in terms of hardware resources?

Your citations are tangential. The setting was implemented as a QoL update. Both the issue and the PR you linked say that. The dev who responded to my issue says that. The people upvoting my comments agree with that. What else do you want? If you want a security setting implemented which this one isn't, propose creating one in a separate issue, instead of hijacking an existing setting which has an entirely different purpose. You're making life difficult for yourself and the people interested in getting actual work done and getting annoyed by useless popups.

@microsoft microsoft locked as too heated and limited conversation to collaborators May 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants