Fixed issue in POSIX layer that could truncate write streams (#235)

PowerShell/Win32-OpenSSH#908
This commit is contained in:
Manoj Ampalam 2017-11-06 21:38:14 -08:00 committed by GitHub
parent c546971ca8
commit 4edff78b9d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 53 additions and 7 deletions

View File

@ -628,7 +628,7 @@ WriteCompletionRoutine(_In_ DWORD dwErrorCode,
pio->write_details.error = dwErrorCode;
/* TODO - assert that remaining == dwNumberOfBytesTransfered */
if ((dwErrorCode == 0) && (pio->write_details.remaining != dwNumberOfBytesTransfered)) {
debug3("WriteCB - ERROR: broken assumption, io:%p, wrote:%d, remaining:%d", pio,
error("WriteCB - ERROR: broken assumption, io:%p, wrote:%d, remaining:%d", pio,
dwNumberOfBytesTransfered, pio->write_details.remaining);
DebugBreak();
}
@ -902,6 +902,22 @@ fileio_close(struct w32_io* pio)
return 0;
}
/*
* we report to POSIX app that an async write has completed as soon its
* copied to internal buffer. The app may subsequently try to close the
* fd thinking everything is written. IF the Windows handle is closed
* now, the pipe/file io write operation may terminate prematurely.
* To compensate for the discrepency
* wait here until async write has completed.
* If you see any process waiting here indefinitely - its because no one
* is draining from other end of the pipe/file. This is an unfortunate
* consequence that should otherwise have very little impact on practical
* scenarios.
*/
while (pio->write_details.pending)
if (0 != wait_for_any_event(NULL, 0, INFINITE))
return -1;
CancelIo(WINHANDLE(pio));
/* let queued APCs (if any) drain */
SleepEx(0, TRUE);

View File

@ -205,7 +205,7 @@ sw_process_pending_signals()
sigdelset(&pending_tmp, exp[i]);
if (pending_tmp) {
/* unexpected signals queued up */
debug3("process_signals() - ERROR unexpected signals in queue: %d", pending_tmp);
error("process_signals() - ERROR unexpected signals in queue: %d", pending_tmp);
errno = ENOTSUP;
DebugBreak();
return -1;

View File

@ -499,7 +499,7 @@ CALLBACK WSASendCompletionRoutine(IN DWORD dwError,
pio->write_details.error = dwError;
/* TODO - assert that remaining == cbTransferred */
if ((dwError == 0) && (pio->write_details.remaining != cbTransferred)) {
debug3("WSASendCB - ERROR: broken assumption, io:%p, sent:%d, remaining:%d", pio,
error("WSASendCB - ERROR: broken assumption, io:%p, sent:%d, remaining:%d", pio,
cbTransferred, pio->write_details.remaining);
DebugBreak();
}
@ -633,7 +633,7 @@ socketio_close(struct w32_io* pio)
SleepEx(0, TRUE);
if ((pio->internal.state == SOCK_READY) &&
(pio->read_details.pending || pio->write_details.pending)) {
debug4("close - IO is still pending on closed socket. read:%d, write:%d, io:%p",
error("close - IO is still pending on closed socket. read:%d, write:%d, io:%p",
pio->read_details.pending, pio->write_details.pending, pio);
DebugBreak();
}

View File

@ -58,7 +58,7 @@ 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.pending, pio->sync_read_status.error);
pio->read_details.error = pio->sync_read_status.error;
pio->read_details.remaining = pio->sync_read_status.transferred;
pio->read_details.completed = 0;
@ -170,7 +170,7 @@ 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.pending, pio->sync_write_status.error);
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 */
@ -212,7 +212,7 @@ WriteThread(_In_ LPVOID lpParameter)
if (0 == QueueUserAPC(WriteAPCProc, main_thread, (ULONG_PTR)pio)) {
debug3("WriteThread thread - ERROR QueueUserAPC failed %d, io:%p", GetLastError(), pio);
error("WriteThread thread - ERROR QueueUserAPC failed %d, io:%p", GetLastError(), pio);
pio->write_details.pending = FALSE;
pio->write_details.error = GetLastError();
DebugBreak();

View File

@ -22,6 +22,11 @@ Describe "E2E scenarios for ssh client" -Tags "CI" {
{
$null = New-Item $testDir -ItemType directory -Force -ErrorAction SilentlyContinue
}
$acl = Get-Acl $testDir
$rights = [System.Security.AccessControl.FileSystemRights]"Read, Write"
$accessRule = New-Object System.Security.AccessControl.FileSystemAccessRule($ssouser, $rights, "ContainerInherit,Objectinherit", "None", "Allow")
$acl.SetAccessRule($accessRule)
Set-Acl -Path $testDir -AclObject $acl
$platform = Get-Platform
$skip = ($platform -eq [PlatformType]::Windows) -and ($PSVersionTable.PSVersion.Major -le 2)
@ -151,6 +156,31 @@ Describe "E2E scenarios for ssh client" -Tags "CI" {
$o = $h | ssh test_target PowerShell -NoProfile -NonInteractive -ExecutionPolicy Unrestricted -EncodedCommand $EncodedText 2> $null
$o | Should Be "8"
}
It "$tC.$tI - stream file in and out" {
# prep a file of size > 10KB (https://github.com/PowerShell/Win32-OpenSSH/issues/908 was caught with such file size)
$str = ""
(1..100) | foreach {$str += "1234567890"}
#strem file from local to remote
$testsrc = Join-Path $testDir "$tC.$tI.testsrc"
$testdst1 = Join-Path $testDir "$tC.$tI.testdst1"
$null | Set-Content $testsrc
$null | Set-Content $testdst1
(1..105) | foreach {Add-Content -Encoding Ascii -Path $testsrc -Value $str}
# execute this script that dumps input stream in target file, on the remote end
$str = "begin {} process { Add-Content -Encoding Ascii -path $testdst1 -Value ([string]`$input)} end { }"
$EncodedText =[Convert]::ToBase64String([System.Text.Encoding]::Unicode.GetBytes($str))
# ignore error stream using 2> $null
get-content $testsrc | ssh test_target PowerShell -NoProfile -NonInteractive -ExecutionPolicy Unrestricted -EncodedCommand $EncodedText 2> $null
(dir $testdst1).Length | Should Be (dir $testsrc).Length
# stream file from remote to local
$testdst2 = Join-Path $testDir "$tC.$tI.testdst2"
$null | Set-Content $testdst2
(ssh test_target powershell get-content $testdst1 -Encoding Ascii) | Set-Content $testdst2 -Encoding ASCII
(dir $testdst2).Length | Should Be (dir $testsrc).Length
}
}
Context "$tC - configure default shell Scenarios" {