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

Incompatibility when AutoRun injects AutoHotkey, AnsiCon, and Clink #169

Closed
u01jmg3 opened this issue Sep 23, 2021 · 14 comments
Closed

Incompatibility when AutoRun injects AutoHotkey, AnsiCon, and Clink #169

u01jmg3 opened this issue Sep 23, 2021 · 14 comments
Labels
bug Something isn't working

Comments

@u01jmg3
Copy link

u01jmg3 commented Sep 23, 2021

I've installed both 1.2.33 and 1.2.34 but my history only persists whilst my cmd is open - I cannot access any previously run commands. Reverting back to 1.2.32 fixes the issue and brings back my history.


On a side, it would also be helpful to configure the directory clink installs into via the installer.

e.g. C:\Program Files (x86)\clink rather than C:\Program Files (x86)\clink\1.2.34.6cb41a

Reason being I have a custom setting for Computer\HKEY_CURRENT_USER\SOFTWARE\Microsoft\Command Processor\Autorun and every time a new update for clink is released I have to update this setting with the new version number and commit hash of clink. Just a minor annoyance.

Also, one thing many installers allow is skipping the installation of start menu shortcuts (I don't use them). I'm not sure how simple this would be with the "Nullsoft Install System" installer that's in use.

Thanks for all your work.

@chrisant996
Copy link
Owner

I've installed both 1.2.33 and 1.2.34 but my history only persists whilst my cmd is open - I cannot access any previously run commands. Reverting back to 1.2.32 fixes the issue and brings back my history.

Please share your clink_settings file.

History is working fine for me and I haven't knowingly made any changes near history in some time, so knowing more information about your configuration would help track down what might be happening.

@u01jmg3
Copy link
Author

u01jmg3 commented Sep 23, 2021

I haven't knowingly altered it.

# name: Strips CR and LF chars on paste
# type: enum
# options: delete,space,ampersand,crlf
clink.paste_crlf = space

# name: Skip adding lines prefixed with whitespace
# type: boolean
history.ignore_space = False

# name: The number of history lines to save
# type: integer
history.max_lines = 10000

# name: Expand envvars when completing
# type: boolean
match.expand_envvars = True

# name: Esc sends a literal escape character
# type: boolean
terminal.raw_esc = True

# name: Support Windows' Ctrl-Alt substitute for AltGr
# type: boolean
terminal.use_altgr_substitute = True

@chrisant996
Copy link
Owner

First of all, let me be clear: I believe you. 🙂

I need help tracking down why history stopped working for you. I can't find a way to reproduce that, so I need more information about the configuration (and maybe scripts) in your Clink installation.

  • Can you share the custom AutoRun command you have set up for Clink?
  • Also, does history start working again without the AutoRun command?

@chrisant996 chrisant996 added the needs more information The issue needs clarifying information label Sep 23, 2021
@u01jmg3
Copy link
Author

u01jmg3 commented Sep 23, 2021

It is indeed the autorun command I use that's affecting things.

This is my autorun command in full:

"C:\Portable Apps\AutoHotkey\Includes\Cmd.ahk" & 
(if %ANSICON_VER%==^%ANSICON_VER^% "C:\Portable Apps\AnsIcon 1.89 x64\ansicon" -p) & 
"C:\Program Files (x86)\clink\1.2.34.6cb41a\clink.bat" inject --autorun --quiet --profile ~\clink & 
"%USERPROFILE%\Documents\alias.cmd"

It doesn't seem to relate to the contents of the scripts I call (Cmd.ahk and alias.cmd) as I have blanked these files but that does not change whether my history works or not.

One pattern I have found is removing the first command only and my history starts working again. Equally, removing the second command only and my history starts working again. However, removing the fourth command and my history does not work in the same way that calling all four commands together and my history does not work.

Does any of this relate to changes made in 1.2.33?

@chrisant996
Copy link
Owner

Have you noticed that nothing about Clink works? 🙂 History is one thing that doesn't work, but also nothing else about Clink works, either.

During AutoRun you're injecting AutoHotkey, then AnsiCon, then Clink.

  • If I inject only AHK and Clink --> all is well.
  • If I inject only AnsiCon and Clink --> all is well.
  • If I inject all 3 --> then Clink never receives the GetEnvironmentVariableW call that it relies on to finish its injection.

What changed in v1.2.33 is in response to #159, I changed Clink to use Detours hooking, instead of IAT hooking. Another user mentioned that an antivirus suite was interfering with Clink. Analysis of why revealed that the antivirus suite was (probably intentionally) corrupting OS data structures related to IAT hooking, causing Clink to be unable to hook the necessary APIs. So I rewrote it to not use IAT hooking at all, in the hopes of bypassing that particular kind of interference from an antivirus suite.

I've made an adjustment to the Detours hooking, and now Clink seems to be able to inject during AutoRun even when other programs have used IAT hooking.

@chrisant996 chrisant996 changed the title History no longer accessible since the release of 1.2.33 Incompatibility when AutoRun injects AutoHotkey, AnsiCon, and Clink Sep 23, 2021
@chrisant996 chrisant996 added bug Something isn't working and removed needs more information The issue needs clarifying information labels Sep 23, 2021
@chrisant996
Copy link
Owner

chrisant996 commented Sep 23, 2021

By the way:

  1. May I ask why you're using AnsiCon? What version of Windows are you using? ANSI support has been built into Window for many years.
  2. I noticed that injecting AutoHotkey inside AutoRun introduces a significant slowdown on my computer for a variety of operations (especially running build systems). CMD gets launched implicitly quite a lot when spawning other commands. Keeping AutoRun fast might noticeably improve responsiveness and performance on your computer. I have AutoHotkey always running, rather than injecting it separately into every CMD instance (which includes lots of hidden invocations that never have any UI show up, making lots of things run slower).

@chrisant996 chrisant996 reopened this Sep 23, 2021
@chrisant996
Copy link
Owner

The fix was working ... and then it stopped working. Will have to research further.

@chrisant996
Copy link
Owner

chrisant996 commented Sep 24, 2021

The good news is that all of the hooking actually worked: all of the APIs were successfully hooked, and Clink's overrides are reached successfully.

  • The problem is what AnsiCon does.
  • The reason it works in v1.2.33 and earlier is because if Clink uses IAT hooking then it's able to interfere with AnsiCon and handle console output before AnsiCon has a chance.

Here is what's happening:

  1. Clink prepends "C\bL\bI\bN\bK\b" to the %PROMPT% variable, so that Clink can tell when CMD is trying to print the PROMPT to the screen.
  2. CMD prints the PROMPT, so for example:
    • "C\bL\bI\bN\bK\bC:\\users\\chrisant>".
  3. AnsiCon intercepts the WriteConsoleW calls, and splits the one call into a whole bunch of separate calls:
    • "C"
    • "\b"
    • "L"
    • "\b"
    • "I"
    • "\b"
    • "N"
    • "\b"
    • "K"
    • "\bC:\\users\\chrisant>"
  4. And thus Clink isn't able to tell that CMD is trying to print the prompt.

That's inefficient and unnecessary on the part of AnsiCon.

By the way, you may know this already, but if you're using Windows 10, you should stop using AnsiCon. The built in support in Windows 10 is much better than AnsiCon, and AnsiCon can only cause problems. For example, the built in support handles 8-bit color and 24-bit color, among other things.

What I'll do is this:

  1. Revert back to IAT hooking.
  2. Add a setting to optionally use Detours hooking, in case it can help some users circumvent antivirus false positives.

@chrisant996
Copy link
Owner

Fixed by ba71118.

Clink now uses IAT hooking for everything.

Background

Long ago it used jmp hooking for everything. Martin slowly converted various hooks to use IAT hooking, but never converted ReadConsoleW to use IAT hooking. One of the first things I did was fix a jmp hooking problem with ReadConsoleW, and then I integrated the Detours library to do more sophisticated jmp hooking.

But it turns out the simplest and (apparently) best thing to do would have been to just convert ReadConsoleW to use IAT hooking.

The benefit of Detours hooking is it can hook more reliably than IAT hooking, and can hook more kinds of things than IAT can.
The drawback of Detours hooking is it inherently has lower precedence than IAT hooking; all IAT hooks get a chance at handling the API before any Detours hooks do.

That drawback is a big problem for Clink if anything hooks and interferes with normal WriteConsoleW functionality. Which AnsiCon does. Not only does AnsiCon split one WriteConsoleW call into many tiny calls (which isn't good for speed, either), it also intercepts and handles ANSI escape sequences on its own, before Clink gets them. That completely breaks Clink's terminal emulation capabilities.

Resolution

So for best stability and reliability, Clink now uses IAT hooking for all APIs that it hooks.

But if there's no other way to get Clink to work (e.g. #159), then it's possible to use clink inject --detours to force Detours hooking. But it causes other problems and is not officially supported.

@u01jmg3
Copy link
Author

u01jmg3 commented Sep 24, 2021

Have you noticed that nothing about Clink works?

Aha! No, I noticed my history was broken and stopped there before reverting to v1.2.32.

May I ask why you're using AnsiCon? What version of Windows are you using?

Windows 10. I installed AnsiCon a while ago to correct colours that weren't showing properly in my cmd. Since you questioned its use, I've removed it from my AutoRun and things look to be fine. 24-bit colour is nice! 🌈

I noticed that injecting AutoHotkey inside AutoRun introduces a significant slowdown on my computer for a variety of operations (especially running build systems). CMD gets launched implicitly quite a lot when spawning other commands. Keeping AutoRun fast might noticeably improve responsiveness and performance on your computer. I have AutoHotkey always running, rather than injecting it separately into every CMD instance (which includes lots of hidden invocations that never have any UI show up, making lots of things run slower).

I use it to position the cmd window correctly so the AutoHotkey script that runs is fairly minimal but noted, I'll see if I can do without.

By the way, you may know this already, but if you're using Windows 10, you should stop using AnsiCon.

Done ✔️

So for best stability and reliability, Clink now uses IAT hooking for all APIs that it hooks.

Super. Having an opt-in flag for Detours hooking sounds like the best thing to do.


Thanks for fixing this. I can confirm v1.2.35 works great. 👍🏻

@pukkandan
Copy link

I use it to position the cmd window correctly so the AutoHotkey script that runs is fairly minimal but noted, I'll see if I can do without.

Autohotkey scripts, however small, does tend to take some time to load up the interpretter. Normally, this time is unnoticible, but as christan said, cmd tends to be invoked implicitly for many tasks and the delay can become quite large depending on your workflow. I would suggest having a always-on script that watches for the cmd windows, positions them, and adds its hwnd to an ignore-list. Its easily doable with WinWait and GroupAdd

@u01jmg3
Copy link
Author

u01jmg3 commented Sep 24, 2021

Thanks, I'll take a look at that idea. For info, this is my AHK script:

#SingleInstance, Force
#NoEnv

    WinGetTitle, WinTitle, A

    If InStr(WinTitle, "C:\WINDOWS\SYSTEM32\cmd.exe - """) OR InStr(WinTitle, "cmd - """)
    {
        BlockInput, On
        Send, #`` ; Apply Sizer profile: Win+` | http://www.brianapps.net/sizer4/
        BlockInput, Off
    }
Return

@chrisant996
Copy link
Owner

On a side, it would also be helpful to configure the directory clink installs into via the installer.

e.g. C:\Program Files (x86)\clink rather than C:\Program Files (x86)\clink\1.2.34.6cb41a

Reason being I have a custom setting for Computer\HKEY_CURRENT_USER\SOFTWARE\Microsoft\Command Processor\Autorun and every time a new update for clink is released I have to update this setting with the new version number and commit hash of clink. Just a minor annoyance.

Also, is there a reason you don't want to use the %CLINK_DIR% environment variable the installer sets?
I.e. %clink_dir%\clink inject wouldn't need any modifications.

@u01jmg3
Copy link
Author

u01jmg3 commented Nov 11, 2021

Good point - any reason this variable isn't used when Clink updates the autorun registry entry? I see as part of the installer you can choose not to create %CLINK_DIR% so perhaps that is why.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants