Fix console aliases not working (#14991)

#14745 contains two regressions related to console alias handling:
* When `ProcessAliases` expands the backup buffer into (an) aliased
  command(s) it changes the `_bytesRead` field of `COOKED_READ_DATA`,
  requiring us to re-read it and reconstruct the `input` string-view.
* Multiline aliases are read line-by-line whereas #14745 didn't treat
  them any different from regular single-line inputs.

## Validation Steps Performed
In `cmd.exe` run
```
doskey test=echo foo$Techo bar$Techo baz
test
```
The output should look exactly like this:
```
C:\>doskey test=echo foo$Techo bar$Techo baz

C:\>test
foo

C:\>bar

C:\>baz

C:\>
```
This commit is contained in:
Leonard Hecker 2023-03-17 21:43:44 +01:00 committed by GitHub
parent 2810155046
commit 00af187a97
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 47 additions and 40 deletions

View File

@ -193,7 +193,6 @@ chh
chk
CHT
Cic
CLA
Clcompile
CLE
cleartype
@ -480,7 +479,6 @@ defterm
DELAYLOAD
DELETEONRELEASE
Delt
demoable
depersist
deprioritized
deserializers
@ -537,7 +535,6 @@ DSSCL
DSwap
DTest
DTTERM
DUMMYUNIONNAME
dup'ed
dvi
dwl
@ -590,7 +587,6 @@ ETW
EUDC
EVENTID
eventing
everytime
evflags
evt
execd
@ -802,7 +798,6 @@ HIBYTE
hicon
HIDEWINDOW
hinst
Hirots
HISTORYBUFS
HISTORYNODUP
HISTORYSIZE
@ -903,7 +898,6 @@ INSERTMODE
INTERACTIVITYBASE
INTERCEPTCOPYPASTE
INTERNALNAME
inthread
intsafe
INVALIDARG
INVALIDATERECT
@ -1260,14 +1254,12 @@ ntm
nto
ntrtl
ntstatus
ntsubauth
NTSYSCALLAPI
nttree
nturtl
ntuser
NTVDM
ntverp
NTWIN
nugetversions
nullability
nullness
@ -1306,8 +1298,6 @@ opencode
opencon
openconsole
openconsoleproxy
OPENIF
OPENLINK
openps
openvt
ORIGINALFILENAME
@ -1360,9 +1350,7 @@ pcg
pch
PCIDLIST
PCIS
PCLIENT
PCLONG
PCOBJECT
pcon
PCONSOLE
PCONSOLEENDTASK
@ -1374,7 +1362,6 @@ pcshell
PCSHORT
PCSR
PCSTR
PCUNICODE
PCWCH
PCWCHAR
PCWSTR
@ -1423,7 +1410,6 @@ PLOGICAL
pnm
PNMLINK
pntm
PNTSTATUS
POBJECT
Podcast
POINTSLIST
@ -1442,7 +1428,6 @@ ppf
ppguid
ppidl
PPROC
PPROCESS
ppropvar
ppsi
ppsl
@ -1506,7 +1491,6 @@ ptrs
ptsz
PTYIn
PUCHAR
PUNICODE
pwch
PWDDMCONSOLECONTEXT
pws
@ -1568,7 +1552,6 @@ REGISTEROS
REGISTERVDM
regkey
REGSTR
reingest
RELBINPATH
remoting
renamer
@ -1863,6 +1846,7 @@ TDP
TEAMPROJECT
tearoff
Teb
Techo
tellp
teraflop
terminalcore
@ -2005,7 +1989,6 @@ unittesting
unittests
unk
unknwn
unmark
UNORM
unparseable
unregistering

View File

@ -20,7 +20,7 @@ bool INPUT_READ_HANDLE_DATA::IsInputPending() const
bool INPUT_READ_HANDLE_DATA::IsMultilineInput() const
{
FAIL_FAST_IF(!_isInputPending); // we shouldn't have multiline input without a pending input.
assert(_isInputPending); // we shouldn't have multiline input without a pending input.
return _isMultilineInput;
}

View File

@ -1019,28 +1019,44 @@ void COOKED_READ_DATA::SavePendingInput(const size_t index, const bool multiline
}
Tracing::s_TraceCookedRead(_clientProcess, _backupLimit, base::saturated_cast<ULONG>(idx));
ProcessAliases(LineCount);
// Don't be fooled by ProcessAliases only taking one argument. It rewrites multiple
// class members on return, including `_bytesRead`, requiring us to reconstruct `input`.
ProcessAliases(LineCount);
input = { _backupLimit, _bytesRead / sizeof(wchar_t) };
// The exact reasons for this are unclear to me (the one writing this comment), but this code used to
// split the contents of a multiline alias (for instance `doskey test=echo foo$Techo bar$Techo baz`)
// into multiple separate read outputs, ensuring that the client receives them line by line.
//
// This code first truncates the `input` to only contain the first line, so that Consume() below only
// writes that line into the user buffer. We'll later store the remainder in SaveMultilinePendingInput().
if (LineCount > 1)
{
input = input.substr(0, idx + 1);
// ProcessAliases() is supposed to end each line with \r\n. If it doesn't we might as well fail-fast.
const auto firstLineEnd = input.find(UNICODE_LINEFEED) + 1;
input = input.substr(0, std::min(input.size(), firstLineEnd));
}
}
}
const auto inputSizeBefore = input.size();
GetInputBuffer()->Consume(isUnicode, input, writer);
if (!input.empty())
{
if (LineCount > 1)
{
// This is a continuation of the above identical if condition.
// We've truncated the `input` slice and now we need to restore it.
const auto inputSizeAfter = input.size();
const auto amountConsumed = inputSizeBefore - inputSizeAfter;
input = { _backupLimit, _bytesRead / sizeof(wchar_t) };
input = input.substr(std::min(input.size(), amountConsumed));
GetInputReadHandleData()->SaveMultilinePendingInput(input);
}
else
else if (!input.empty())
{
GetInputReadHandleData()->SavePendingInput(input);
}
}
numBytes = _userBufferSize - writer.size();
controlKeyState = _controlKeyState;

View File

@ -288,28 +288,36 @@ try
{
bytesRead = 0;
auto pending = readHandleState.GetPendingInput();
const auto pending = readHandleState.GetPendingInput();
auto input = pending;
// This is basically the continuation of COOKED_READ_DATA::_handlePostCharInputLoop.
if (readHandleState.IsMultilineInput())
{
const auto idx = pending.find(UNICODE_LINEFEED);
if (idx != decltype(pending)::npos)
{
// +1 to include the newline.
pending = pending.substr(0, idx + 1);
}
const auto firstLineEnd = input.find(UNICODE_LINEFEED) + 1;
input = input.substr(0, std::min(input.size(), firstLineEnd));
}
const auto inputSizeBefore = input.size();
std::span writer{ buffer };
inputBuffer.Consume(unicode, pending, writer);
inputBuffer.Consume(unicode, input, writer);
if (pending.empty())
// Since we truncated `input` to only include the first line,
// we need to restore `input` here to the entirety of the remaining input.
if (readHandleState.IsMultilineInput())
{
const auto inputSizeAfter = input.size();
const auto amountConsumed = inputSizeBefore - inputSizeAfter;
input = pending.substr(std::min(pending.size(), amountConsumed));
}
if (input.empty())
{
readHandleState.CompletePending();
}
else
{
readHandleState.UpdatePending(pending);
readHandleState.UpdatePending(input);
}
bytesRead = buffer.size() - writer.size();