tyler.durden
/
openssh-portable
mirror of https://github.com/PowerShell/openssh-portable.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fixed issue with ssh redirected stdin 

PowerShell/Win32-OpenSSH#929
Issue: ReadThread prematurely returns on read io error. This results in APC never getting set and hence the corresponding fd is never set on select resulting the hang seen in issue 929.
Also removed the static instances storing sync io status, since there could be multiple sync fds operating at the same time. Moved the sync io status to w32_io object itself.
This commit is contained in:
Manoj Ampalam 2017-11-03 13:16:23 -07:00 committed by GitHub
parent dce738c33a
commit c546971ca8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 47 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
contrib/win32/win32compat/misc.c
View File

@ -307,7 +307,7 @@ w32_fopen_utf8(const char *path, const char *mode)
* each UTF-16 char may bloat up to 4 utf-8 chars. We cannot determine if the length of
* input unicode string until it is readed and converted to utf8 string.
* There is a risk to miss on unicode char when last unicode char read from console
* does not fit the remain space in str. use cauciously.
* does not fit the remain space in str. use cautiously.
*/
char*
w32_fgets(char *str, int n, FILE *stream) {

53
contrib/win32/win32compat/termio.c
View File

@ -52,13 +52,6 @@
extern int in_raw_mode;
BOOL isFirstTime = TRUE;
struct io_status {
DWORD to_transfer;
DWORD transferred;
DWORD error;
};
static struct io_status read_status, write_status;
/* APC that gets queued on main thread when a sync Read completes on worker thread */
static VOID CALLBACK
ReadAPCProc(_In_ ULONG_PTR dwParam)
@ -66,8 +59,8 @@ ReadAPCProc(_In_ ULONG_PTR dwParam)
struct w32_io* pio = (struct w32_io*)dwParam;
debug5("TermRead CB - io:%p, bytes: %d, pending: %d, error: %d", pio, read_status.transferred,
pio->read_details.pending, read_status.error);
pio->read_details.error = read_status.error;
pio->read_details.remaining = read_status.transferred;
pio->read_details.error = pio->sync_read_status.error;
pio->read_details.remaining = pio->sync_read_status.transferred;
pio->read_details.completed = 0;
pio->read_details.pending = FALSE;
WaitForSingleObject(pio->read_overlapped.hEvent, INFINITE);
@ -83,14 +76,14 @@ ReadThread(_In_ LPVOID lpParameter)
struct w32_io* pio = (struct w32_io*)lpParameter;
debug5("TermRead thread, io:%p", pio);
memset(&read_status, 0, sizeof(read_status));
memset(&pio->sync_read_status, 0, sizeof(pio->sync_read_status));
if (FILETYPE(pio) == FILE_TYPE_CHAR) {
if (in_raw_mode) {
while (nBytesReturned == 0) {
nBytesReturned = ReadConsoleForTermEmul(WINHANDLE(pio),
pio->read_details.buf, pio->read_details.buf_size);
}
read_status.transferred = nBytesReturned;
pio->sync_read_status.transferred = nBytesReturned;
} else {
if (isFirstTime) {
isFirstTime = false;
@ -106,10 +99,10 @@ ReadThread(_In_ LPVOID lpParameter)
}
if (!ReadFile(WINHANDLE(pio), pio->read_details.buf,
pio->read_details.buf_size, &read_status.transferred, NULL)) {
pio->read_details.buf_size, &(pio->sync_read_status.transferred), NULL)) {
debug4("ReadThread - ReadFile failed, error:%d, io:%p", GetLastError(), pio);
read_status.error = GetLastError();
return -1;
pio->sync_read_status.error = GetLastError();
goto done;
}
char *p = NULL;
@ -121,17 +114,19 @@ ReadThread(_In_ LPVOID lpParameter)
if (p) {
*p = '\0';
pio->read_details.buf_size = (DWORD)strlen(pio->read_details.buf);
read_status.transferred = pio->read_details.buf_size;
pio->sync_read_status.transferred = pio->read_details.buf_size;
}
}
} else {
if (!ReadFile(WINHANDLE(pio), pio->read_details.buf,
pio->read_details.buf_size, &read_status.transferred, NULL)) {
pio->read_details.buf_size, &(pio->sync_read_status.transferred), NULL)) {
debug4("ReadThread - ReadFile failed, error:%d, io:%p", GetLastError(), pio);
read_status.error = GetLastError();
return -1;
pio->sync_read_status.error = GetLastError();
goto done;
}
}
done:
if (0 == QueueUserAPC(ReadAPCProc, main_thread, (ULONG_PTR)pio)) {
pio->read_details.pending = FALSE;
pio->read_details.error = GetLastError();
@ -176,8 +171,8 @@ WriteAPCProc(_In_ ULONG_PTR dwParam)
struct w32_io* pio = (struct w32_io*)dwParam;
debug5("TermWrite CB - io:%p, bytes: %d, pending: %d, error: %d", pio, write_status.transferred,
pio->write_details.pending, write_status.error);
pio->write_details.error = write_status.error;
pio->write_details.remaining -= write_status.transferred;
pio->write_details.error = pio->sync_write_status.error;
pio->write_details.remaining -= pio->sync_write_status.transferred;
/* TODO- assert that reamining is 0 by now */
pio->write_details.completed = 0;
pio->write_details.pending = FALSE;
@ -197,20 +192,20 @@ WriteThread(_In_ LPVOID lpParameter)
debug5("WriteThread thread, io:%p", pio);
if (FILETYPE(pio) == FILE_TYPE_CHAR) {
pio->write_details.buf[write_status.to_transfer] = '\0';
pio->write_details.buf[pio->sync_write_status.to_transfer] = '\0';
if (0 == in_raw_mode) {
wchar_t* t = utf8_to_utf16(pio->write_details.buf);
WriteConsoleW(WINHANDLE(pio), t, (DWORD)wcslen(t), 0, 0);
free(t);
} else {
processBuffer(WINHANDLE(pio), pio->write_details.buf, write_status.to_transfer, &respbuf, &resplen);
processBuffer(WINHANDLE(pio), pio->write_details.buf, pio->sync_write_status.to_transfer, &respbuf, &resplen);
/* TODO - respbuf is not null in some cases, this needs to be returned back via read stream */
}
write_status.transferred = write_status.to_transfer;
pio->sync_write_status.transferred = pio->sync_write_status.to_transfer;
} else {
if (!WriteFile(WINHANDLE(pio), pio->write_details.buf, write_status.to_transfer,
&write_status.transferred, NULL)) {
write_status.error = GetLastError();
if (!WriteFile(WINHANDLE(pio), pio->write_details.buf, pio->sync_write_status.to_transfer,
&(pio->sync_write_status.transferred), NULL)) {
pio->sync_write_status.error = GetLastError();
debug4("WriteThread - WriteFile %d, io:%p", GetLastError(), pio);
}
}
@ -232,8 +227,8 @@ syncio_initiate_write(struct w32_io* pio, DWORD num_bytes)
{
HANDLE write_thread;
debug5("syncio_initiate_write initiate io:%p", pio);
memset(&write_status, 0, sizeof(write_status));
write_status.to_transfer = num_bytes;
memset(&(pio->sync_write_status), 0, sizeof(pio->sync_write_status));
pio->sync_write_status.to_transfer = num_bytes;
write_thread = CreateThread(NULL, 0, WriteThread, pio, 0, NULL);
if (write_thread == NULL) {
errno = errno_from_Win32LastError();
@ -246,7 +241,7 @@ syncio_initiate_write(struct w32_io* pio, DWORD num_bytes)
return 0;
}
/* tty close */
/* close */
int
syncio_close(struct w32_io* pio)
{

13
contrib/win32/win32compat/w32fd.h
View File

@ -102,6 +102,19 @@ struct w32_io {
DWORD std_handle; /* ex. STD_INPUT_HANDLE */
};
/*internal state used by synchronous io - terminal handles and external
handles passed through std io*/
struct {
DWORD to_transfer;
DWORD transferred;
DWORD error;
}sync_read_status;
struct {
DWORD to_transfer;
DWORD transferred;
DWORD error;
}sync_write_status;
/*handle specific internal state context, used by sockets and pipes*/
struct {
enum w32_io_sock_state state;

14
regress/pesterTests/SSH.Tests.ps1
View File

@ -142,11 +142,15 @@ Describe "E2E scenarios for ssh client" -Tags "CI" {
$o | Should Be "1234"
}
<#It "$tC.$tI - stdin from PS object" {
#if input redirection doesn't work, this would hang
0 | ssh -p $port $ssouser@$server pause
$true | Should Be $true
}#>
It "$tC.$tI - stdin from PS object" {
# execute this script that dumps the length of input data, on the remote end
$str = "begin {} process { Write-Output `$input.Length} end { }"
$EncodedText =[Convert]::ToBase64String([System.Text.Encoding]::Unicode.GetBytes($str))
$h = "hello123"
# ignore error stream using 2> $null
$o = $h | ssh test_target PowerShell -NoProfile -NonInteractive -ExecutionPolicy Unrestricted -EncodedCommand $EncodedText 2> $null
$o | Should Be "8"
}
}
Context "$tC - configure default shell Scenarios" {