Skip to content

Commit

Permalink
Fix console aliases not working (#14991)
Browse files Browse the repository at this point in the history
#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:\>
```
  • Loading branch information
lhecker committed Mar 17, 2023
1 parent 2810155 commit 00af187
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 40 deletions.
19 changes: 1 addition & 18 deletions .github/actions/spelling/expect/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,6 @@ chh
chk
CHT
Cic
CLA
Clcompile
CLE
cleartype
Expand Down Expand Up @@ -480,7 +479,6 @@ defterm
DELAYLOAD
DELETEONRELEASE
Delt
demoable
depersist
deprioritized
deserializers
Expand Down Expand Up @@ -537,7 +535,6 @@ DSSCL
DSwap
DTest
DTTERM
DUMMYUNIONNAME
dup'ed
dvi
dwl
Expand Down Expand Up @@ -590,7 +587,6 @@ ETW
EUDC
EVENTID
eventing
everytime
evflags
evt
execd
Expand Down Expand Up @@ -802,7 +798,6 @@ HIBYTE
hicon
HIDEWINDOW
hinst
Hirots
HISTORYBUFS
HISTORYNODUP
HISTORYSIZE
Expand Down Expand Up @@ -903,7 +898,6 @@ INSERTMODE
INTERACTIVITYBASE
INTERCEPTCOPYPASTE
INTERNALNAME
inthread
intsafe
INVALIDARG
INVALIDATERECT
Expand Down Expand Up @@ -1260,14 +1254,12 @@ ntm
nto
ntrtl
ntstatus
ntsubauth
NTSYSCALLAPI
nttree
nturtl
ntuser
NTVDM
ntverp
NTWIN
nugetversions
nullability
nullness
Expand Down Expand Up @@ -1306,8 +1298,6 @@ opencode
opencon
openconsole
openconsoleproxy
OPENIF
OPENLINK
openps
openvt
ORIGINALFILENAME
Expand Down Expand Up @@ -1360,9 +1350,7 @@ pcg
pch
PCIDLIST
PCIS
PCLIENT
PCLONG
PCOBJECT
pcon
PCONSOLE
PCONSOLEENDTASK
Expand All @@ -1374,7 +1362,6 @@ pcshell
PCSHORT
PCSR
PCSTR
PCUNICODE
PCWCH
PCWCHAR
PCWSTR
Expand Down Expand Up @@ -1423,7 +1410,6 @@ PLOGICAL
pnm
PNMLINK
pntm
PNTSTATUS
POBJECT
Podcast
POINTSLIST
Expand All @@ -1442,7 +1428,6 @@ ppf
ppguid
ppidl
PPROC
PPROCESS
ppropvar
ppsi
ppsl
Expand Down Expand Up @@ -1506,7 +1491,6 @@ ptrs
ptsz
PTYIn
PUCHAR
PUNICODE
pwch
PWDDMCONSOLECONTEXT
pws
Expand Down Expand Up @@ -1568,7 +1552,6 @@ REGISTEROS
REGISTERVDM
regkey
REGSTR
reingest
RELBINPATH
remoting
renamer
Expand Down Expand Up @@ -1863,6 +1846,7 @@ TDP
TEAMPROJECT
tearoff
Teb
Techo
tellp
teraflop
terminalcore
Expand Down Expand Up @@ -2005,7 +1989,6 @@ unittesting
unittests
unk
unknwn
unmark
UNORM
unparseable
unregistering
Expand Down
2 changes: 1 addition & 1 deletion src/host/inputReadHandleData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
38 changes: 27 additions & 11 deletions src/host/readDataCooked.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1019,27 +1019,43 @@ 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)
{
if (LineCount > 1)
{
GetInputReadHandleData()->SaveMultilinePendingInput(input);
}
else
{
GetInputReadHandleData()->SavePendingInput(input);
}
// 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 if (!input.empty())
{
GetInputReadHandleData()->SavePendingInput(input);
}

numBytes = _userBufferSize - writer.size();
Expand Down
28 changes: 18 additions & 10 deletions src/host/stream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

// 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 (pending.empty())
if (input.empty())
{
readHandleState.CompletePending();
}
else
{
readHandleState.UpdatePending(pending);
readHandleState.UpdatePending(input);
}

bytesRead = buffer.size() - writer.size();
Expand Down

0 comments on commit 00af187

Please sign in to comment.