starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-09-24 07:54:35 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

Conhost doesn't honour the codepage stored in shortcuts #19482

New Issue
Closed
opened 2026-01-31 06:44:42 +00:00 by claunia · 8 comments
claunia commented 2026-01-31 06:44:42 +00:00
Owner
Copy Link

Originally created by @bzzrak on GitHub (Mar 2, 2023).

Windows Terminal version

n/a

Windows build number

10.0.22621.1344

Other Software

cmd.exe

Steps to reproduce

  1. make a shortcut to a CLI program that specifies a code page in it. Link to documentation regarding this
    Or use this one I made:
    cmd-869.zip, for the sake of example I picked code page 869 (Greek), but it can be any code page.

  2. open the shortcut and type "chcp" to check current code page.

Expected Behavior

"chcp" returns the code page in the shortcut.
This is the behaviour if "legacy console mode" is on. (sorry I can't make the text bigger :D)
image

Actual Behavior

The default system code page is returned, the code page in the shortcut has no effect.
I tried setting the system code page to something other than 65001 and it still occurs, so utf-8 is not the issue here.
image

Originally created by @bzzrak on GitHub (Mar 2, 2023). ### Windows Terminal version n/a ### Windows build number 10.0.22621.1344 ### Other Software cmd.exe ### Steps to reproduce 1. make a shortcut to a CLI program that specifies a code page in it. [Link to documentation regarding this](https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-shllink/b959e24d-67c7-4409-b52d-49c00f8bedf9) Or use this one I made: [cmd-869.zip](https://github.com/microsoft/terminal/files/10873195/cmd-869.zip), for the sake of example I picked code page 869 (Greek), but it can be any code page. 2. open the shortcut and type "chcp" to check current code page. ### Expected Behavior "chcp" returns the code page in the shortcut. This is the behaviour if "legacy console mode" is on. (sorry I can't make the text bigger :D) <img width="109" alt="image" src="https://user-images.githubusercontent.com/77467869/222484398-53540a6c-0255-4983-9987-8cc55eea0716.png"> ### Actual Behavior The default system code page is returned, the code page in the shortcut has no effect. I tried setting the system code page to something other than 65001 and it still occurs, so utf-8 is not the issue here. <img width="311" alt="image" src="https://user-images.githubusercontent.com/77467869/222485196-81c3ba8c-29a6-4be0-8be5-5e46c0814ffd.png">
claunia commented 2026-01-31 06:44:45 +00:00
Author
Owner
Copy Link

@DHowett commented on GitHub (Mar 2, 2023):

You know what? I think that this assignment (L168) is backwards! 😦

599b550817/src/propslib/ShortcutSerialization.cpp (L165-L170)

The ones above it fill in pStateInfo. This one fills in the copy of ConsoleFEData it just loaded and is about to delete using garbage from pStateInfo.

@DHowett commented on GitHub (Mar 2, 2023): _You know what?_ I think that this assignment (L168) is backwards! 😦 https://github.com/microsoft/terminal/blob/599b55081762af1594cd8419320e79b9be533944/src/propslib/ShortcutSerialization.cpp#L165-L170 The ones above it fill in `pStateInfo`. This one fills in the copy of `ConsoleFEData` it just loaded and is about to delete using garbage _from_ `pStateInfo`.
claunia commented 2026-01-31 06:44:45 +00:00
Author
Owner
Copy Link

@DHowett commented on GitHub (Mar 2, 2023):

Thanks for the report, and great find!

@DHowett commented on GitHub (Mar 2, 2023): Thanks for the report, and great find!
claunia commented 2026-01-31 06:44:46 +00:00
Author
Owner
Copy Link

@ahmad-04 commented on GitHub (Mar 12, 2023):

Hi can I help with this issue.

@ahmad-04 commented on GitHub (Mar 12, 2023): Hi can I help with this issue.
claunia commented 2026-01-31 06:44:46 +00:00
Author
Owner
Copy Link

@DHowett commented on GitHub (Mar 13, 2023):

@ahmad2901 absolutely! It looks like it is a quick fix -- verifying the place where we load up the codepage -- and we'd love to review a pull request for it! Let us know if you need any specific help.

Since this is in the Windows Console Host, you will be focusing mostly on the "Host.EXE" project and the projects in the same folder in our solution tree.

@DHowett commented on GitHub (Mar 13, 2023): @ahmad2901 absolutely! It looks like it is a quick fix -- verifying the place where we load up the codepage -- and we'd love to review a pull request for it! Let us know if you need any specific help. Since this is in the Windows Console Host, you will be focusing mostly on the "Host.EXE" project and the projects in the same folder in our solution tree.
claunia commented 2026-01-31 06:44:46 +00:00
Author
Owner
Copy Link

@ahmad-04 commented on GitHub (Mar 13, 2023):

Sure i will get started

@ahmad-04 commented on GitHub (Mar 13, 2023): Sure i will get started
claunia commented 2026-01-31 06:44:46 +00:00
Author
Owner
Copy Link

@ahmad-04 commented on GitHub (Mar 14, 2023):

hey @DHowett , I am unsure how to find this error can you give some guidelines?

@ahmad-04 commented on GitHub (Mar 14, 2023): hey @DHowett , I am unsure how to find this error can you give some guidelines?
claunia commented 2026-01-31 06:44:46 +00:00
Author
Owner
Copy Link

@michalnpl commented on GitHub (Apr 4, 2023):

Hello, I have a fix for this issue. It was, in fact, the wrong order of assignment, as pointed out by @DHowett

You know what? I think that this assignment (L168) is backwards! 😦

599b550817/src/propslib/ShortcutSerialization.cpp (L165-L170)

The ones above it fill in pStateInfo. This one fills in the copy of ConsoleFEData it just loaded and is about to delete using garbage from pStateInfo.

I have run tests for release/debug and x86/x64, and there are no differences in test results between the main branch and this fix branch, although there seem to be no tests that reach this particular line of code. I tested manually with multiple .lnk files with different codepages, and the issue is gone on all my samples.

I am, however, worried about backward compatibility and some legacy code out there in the wild taking a dependency on Conhost not respecting the .lnk codepage...

Is it OK if I take over this issue and submit a pull request?

@michalnpl commented on GitHub (Apr 4, 2023): Hello, I have a fix for this issue. It was, in fact, the **wrong order of assignment**, as pointed out by @DHowett > _You know what?_ I think that this assignment (L168) is backwards! 😦 > > https://github.com/microsoft/terminal/blob/599b55081762af1594cd8419320e79b9be533944/src/propslib/ShortcutSerialization.cpp#L165-L170 > > The ones above it fill in `pStateInfo`. This one fills in the copy of `ConsoleFEData` it just loaded and is about to delete using garbage _from_ `pStateInfo`. I have run tests for **release**/**debug** and **x86**/**x64**, and there are no differences in test results between the main branch and this fix branch, although there seem to be no tests that reach this particular line of code. I tested manually with multiple **.lnk** files with different codepages, and the issue is gone on all my samples. I am, however, worried about backward compatibility and some legacy code out there in the wild taking a dependency on **Conhost** not respecting the .lnk codepage... Is it OK if I take over this issue and submit a pull request?
claunia commented 2026-01-31 06:44:47 +00:00
Author
Owner
Copy Link

@DHowett commented on GitHub (Apr 4, 2023):

Absolutely it is, @michalnpl. Thanks.

And, yeah... that is a real and true backward compat risk. It's been broken for pretty much ever (since 2016, and conhost v2), which means that it is practically canon at this point.

I don't have a good feel for what it's going to break to make it more correct¹, but we can absolutely merge it and try it!

¹ In Windows 2000, we added support for word delimiters... and messed up the implementation. 17 years later, we fixed the bug preventing it from working and good gosh we got a lot of phone calls about that!

@DHowett commented on GitHub (Apr 4, 2023): Absolutely it is, @michalnpl. Thanks. And, yeah... that _is_ a real and true backward compat risk. It's been broken for pretty much ever (since 2016, and conhost v2), which means that it is practically canon at this point. I don't have a good feel for what it's going to break to make it more correct¹, but we can absolutely merge it and try it! ¹ In Windows 2000, we added support for word delimiters... and messed up the implementation. 17 years later, we fixed the bug preventing it from working and _good gosh_ we got a lot of phone calls about that!
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label good first issue Help Wanted Impact-Correctness In-PR Issue-Bug Needs-Tag-Fix Priority-3 Product-Conhost
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#19482
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?