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

Default all G-sets to ASCII unless ISO-2022 is requested #11658

Merged
1 commit merged into from Nov 3, 2021

Conversation

j4james
Copy link
Collaborator

@j4james j4james commented Oct 30, 2021

Summary of the Pull Request

There is a non-zero subset of applications that randomly output Locking Shift escape sequences which will invoke a character set from G2 or G3 into the left half of the code table. If those G-sets are mapped to Latin1, that can result in the terminal producing output that appears to be broken. This PR now defaults all G-sets to ASCII, to prevent an unintentional Locking Shift from having any effect.

PR Checklist

Detailed Description of the Pull Request / Additional comments

Most other modern terminals also default to ASCII in all G-sets, so this shouldn't break any modern applications. Legacy 8-bit applications may still expect the G2 and G3 sets mapped to Latin1, but they would also need to have the ISO-2022 encoding enabled, so we can keep them happy by setting G2 and G3 correctly when the ISO-2022 encoding is requested.

Validation Steps Performed

I've manually confirmed that echo -e "\en" and echo -e "\eo" no longer have any visible effect on the output (at least without first invoking another character set into G2 or G3). I've also confirmed that they do still work as expected (i.e. selecting Latin1) after enabling the ISO-2022 encoding.

@ghost ghost added Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Issue-Task It's a feature request, but it doesn't really need a major design. Priority-2 A description (P2) Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal. labels Oct 30, 2021
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for doing this. 😄

@DHowett DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Nov 3, 2021
@ghost
Copy link

ghost commented Nov 3, 2021

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 27e042b into microsoft:main Nov 3, 2021
@j4james j4james deleted the default-ascii-gsets branch November 6, 2021 20:37
DHowett pushed a commit that referenced this pull request Dec 13, 2021
## Summary of the Pull Request

There is a non-zero subset of applications that randomly output _Locking Shift_ escape sequences which will invoke a character set from G2 or G3 into the left half of the code table. If those G-sets are mapped to Latin1, that can result in the terminal producing output that appears to be broken. This PR now defaults all G-sets to ASCII, to prevent an unintentional _Locking Shift_ from having any effect.

## PR Checklist
* [x] Closes #10408
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. Issue number where discussion took place: #10408

## Detailed Description of the Pull Request / Additional comments

Most other modern terminals also default to ASCII in all G-sets, so this shouldn't break any modern applications. Legacy 8-bit applications may still expect the G2 and G3 sets mapped to Latin1, but they would also need to have the ISO-2022 encoding enabled, so we can keep them happy by setting G2 and G3 correctly when the ISO-2022 encoding is requested.

## Validation Steps Performed

I've manually confirmed that `echo -e "\en"` and `echo -e "\eo"` no longer have any visible effect on the output (at least without first invoking another character set into G2 or G3). I've also confirmed that they do still work as expected (i.e. selecting Latin1) after enabling the ISO-2022 encoding.

(cherry picked from commit 27e042b)
DHowett pushed a commit that referenced this pull request Dec 13, 2021
## Summary of the Pull Request

There is a non-zero subset of applications that randomly output _Locking Shift_ escape sequences which will invoke a character set from G2 or G3 into the left half of the code table. If those G-sets are mapped to Latin1, that can result in the terminal producing output that appears to be broken. This PR now defaults all G-sets to ASCII, to prevent an unintentional _Locking Shift_ from having any effect.

## PR Checklist
* [x] Closes #10408
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. Issue number where discussion took place: #10408

## Detailed Description of the Pull Request / Additional comments

Most other modern terminals also default to ASCII in all G-sets, so this shouldn't break any modern applications. Legacy 8-bit applications may still expect the G2 and G3 sets mapped to Latin1, but they would also need to have the ISO-2022 encoding enabled, so we can keep them happy by setting G2 and G3 correctly when the ISO-2022 encoding is requested.

## Validation Steps Performed

I've manually confirmed that `echo -e "\en"` and `echo -e "\eo"` no longer have any visible effect on the output (at least without first invoking another character set into G2 or G3). I've also confirmed that they do still work as expected (i.e. selecting Latin1) after enabling the ISO-2022 encoding.

(cherry picked from commit 27e042b)
@ghost
Copy link

ghost commented Dec 14, 2021

🎉Windows Terminal Preview v1.12.3472.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented Dec 14, 2021

🎉Windows Terminal v1.11.3471.0 has been released which incorporates this pull request.:tada:

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-VT Virtual Terminal sequence support AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Issue-Task It's a feature request, but it doesn't really need a major design. Priority-2 A description (P2) Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default all G-sets to ASCII unless ISO-2022 is requested (was: Terminal breaks on \x1b O)
3 participants