From 9333a086375a4911f635f6ac1e5836f0ee83e92e Mon Sep 17 00:00:00 2001 From: Manoj Ampalam Date: Wed, 21 Dec 2016 21:17:14 -0800 Subject: [PATCH] SCP Fixes (from Yanbing), realpath_win cleanup and spawn_child fix (that broke progfiles installation) --- contrib/win32/openssh/win32iocompat.vcxproj | 11 +-- .../openssh/win32iocompat.vcxproj.filters | 1 + contrib/win32/win32compat/misc.c | 83 +++++++------------ contrib/win32/win32compat/misc_internal.h | 3 + contrib/win32/win32compat/w32fd.c | 11 +-- contrib/win32/win32compat/win32_dirent.c | 9 +- regress/pesterTests/SCP.Tests.ps1 | 68 +++++++-------- scp.c | 49 +++++------ 8 files changed, 94 insertions(+), 141 deletions(-) create mode 100644 contrib/win32/win32compat/misc_internal.h diff --git a/contrib/win32/openssh/win32iocompat.vcxproj b/contrib/win32/openssh/win32iocompat.vcxproj index 7849190fa..549e52c9d 100644 --- a/contrib/win32/openssh/win32iocompat.vcxproj +++ b/contrib/win32/openssh/win32iocompat.vcxproj @@ -155,11 +155,11 @@ - - - - - + + + + + @@ -198,6 +198,7 @@ + diff --git a/contrib/win32/openssh/win32iocompat.vcxproj.filters b/contrib/win32/openssh/win32iocompat.vcxproj.filters index ee2675f49..21cc7aad3 100644 --- a/contrib/win32/openssh/win32iocompat.vcxproj.filters +++ b/contrib/win32/openssh/win32iocompat.vcxproj.filters @@ -117,6 +117,7 @@ inc + diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index 5f390d4c8..1984249f8 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -36,6 +36,7 @@ #include "inc\sys\time.h" #include #include +#include "misc_internal.h" int usleep(unsigned int useconds) { @@ -302,11 +303,25 @@ spawn_child(char* cmd, int in, int out, int err, DWORD flags) { PROCESS_INFORMATION pi; STARTUPINFOW si; BOOL b; - char* abs_cmd; + char *abs_cmd, *t; wchar_t * cmd_utf16; + int add_module_path = 0; - /* relative ? if so, add current module path to start */ - if (!(cmd && cmd[0] != '\0' && (cmd[1] == ':' || cmd[0] == '\\' || cmd[0] == '.'))) { + /* should module path be added */ + do{ + if(!cmd) + break; + t = cmd; + if (*t = '\"') + t++; + if (t[0] == '\0' || t[0] == '\\' || t[0] == '.' || t[1] == ':') + break; + add_module_path = 1; + + } while (0); + + /* add current module path to start if needed */ + if (add_module_path) { char* ctr; abs_cmd = malloc(strlen(w32_programdir()) + 1 + strlen(cmd) + 1); if (abs_cmd == NULL) { @@ -478,8 +493,6 @@ w32_chown(const char *pathname, unsigned int owner, unsigned int group) { return -1; } -char *realpath_win(const char *path, char resolved[MAX_PATH]); - int w32_utimes(const char *filename, struct timeval *tvp) { struct utimbuf ub; @@ -487,10 +500,7 @@ w32_utimes(const char *filename, struct timeval *tvp) { ub.modtime = tvp[1].tv_sec; int ret; - // Skip the first '/' in the pathname - char resolvedPathName[MAX_PATH]; - realpath_win(filename, resolvedPathName); - wchar_t *resolvedPathName_utf16 = utf8_to_utf16(resolvedPathName); + wchar_t *resolvedPathName_utf16 = utf8_to_utf16(sanitized_path(filename)); if (resolvedPathName_utf16 == NULL) { errno = ENOMEM; return -1; @@ -517,16 +527,9 @@ link(const char *oldpath, const char *newpath) { int w32_rename(const char *old_name, const char *new_name) { - // Skip the first '/' in the pathname - char resolvedOldPathName[MAX_PATH]; - realpath_win(old_name, resolvedOldPathName); - - // Skip the first '/' in the pathname - char resolvedNewPathName[MAX_PATH]; - realpath_win(new_name, resolvedNewPathName); - - wchar_t *resolvedOldPathName_utf16 = utf8_to_utf16(resolvedOldPathName); - wchar_t *resolvedNewPathName_utf16 = utf8_to_utf16(resolvedNewPathName); + wchar_t *resolvedOldPathName_utf16 = utf8_to_utf16(sanitized_path(old_name)); + wchar_t *resolvedNewPathName_utf16 = utf8_to_utf16(sanitized_path(new_name)); + if (NULL == resolvedOldPathName_utf16 || NULL == resolvedNewPathName_utf16) { errno = ENOMEM; return -1; @@ -541,11 +544,8 @@ w32_rename(const char *old_name, const char *new_name) { int w32_unlink(const char *path) { - // Skip the first '/' in the pathname - char resolvedPathName[MAX_PATH]; - realpath_win(path, resolvedPathName); - - wchar_t *resolvedPathName_utf16 = utf8_to_utf16(resolvedPathName); + + wchar_t *resolvedPathName_utf16 = utf8_to_utf16(sanitized_path(path)); if (NULL == resolvedPathName_utf16) { errno = ENOMEM; return -1; @@ -559,11 +559,7 @@ w32_unlink(const char *path) { int w32_rmdir(const char *path) { - // Skip the first '/' in the pathname - char resolvedPathName[MAX_PATH]; - realpath_win(path, resolvedPathName); - - wchar_t *resolvedPathName_utf16 = utf8_to_utf16(resolvedPathName); + wchar_t *resolvedPathName_utf16 = utf8_to_utf16(sanitized_path(path)); if (NULL == resolvedPathName_utf16) { errno = ENOMEM; return -1; @@ -606,11 +602,8 @@ w32_getcwd(char *buffer, int maxlen) { int w32_mkdir(const char *path_utf8, unsigned short mode) { - // Skip the first '/' in the pathname - char resolvedPathName[MAX_PATH]; - realpath_win(path_utf8, resolvedPathName); - - wchar_t *path_utf16 = utf8_to_utf16(resolvedPathName); + + wchar_t *path_utf16 = utf8_to_utf16(sanitized_path(path_utf8)); if (path_utf16 == NULL) { errno = ENOMEM; return -1; @@ -632,22 +625,14 @@ getrnd(u_char *s, size_t len) { int w32_stat(const char *path, struct w32_stat *buf) { - // Skip the first '/' in the pathname - char resolvedPathName[MAX_PATH]; - realpath_win(path, resolvedPathName); - - return fileio_stat(resolvedPathName, (struct _stat64*)buf); + return fileio_stat(sanitized_path(path), (struct _stat64*)buf); } // if file is symbolic link, copy its link into "link" . int readlink(const char *path, char *link, int linklen) { - // Skip the first '/' in the pathname - char resolvedPathName[MAX_PATH]; - realpath_win(path, resolvedPathName); - - strcpy_s(link, linklen, resolvedPathName); + strcpy_s(link, linklen, sanitized_path(path)); return 0; } @@ -694,16 +679,6 @@ realpath(const char *path, char resolved[MAX_PATH]) { return resolved; } -// like realpathWin32() but takes out the first slash so that windows systems can work on the actual file or directory -char * -realpath_win(const char *path, char resolved[MAX_PATH]) { - char tempPath[MAX_PATH]; - realpath(path, tempPath); - - strncpy(resolved, &tempPath[1], sizeof(tempPath) - 1); - return resolved; -} - // Maximum reparse buffer info size. The max user defined reparse // data is 16KB, plus there's a header. #define MAX_REPARSE_SIZE 17000 diff --git a/contrib/win32/win32compat/misc_internal.h b/contrib/win32/win32compat/misc_internal.h new file mode 100644 index 000000000..0cd236921 --- /dev/null +++ b/contrib/win32/win32compat/misc_internal.h @@ -0,0 +1,3 @@ + +/* removes first '/' for Windows paths that are unix styled. Ex: /c:/ab.cd */ +#define sanitized_path(p) (((p)[0] == '/' && (p)[1] != '\0' && (p)[2] == ':')? (p)+1 : (p)) \ No newline at end of file diff --git a/contrib/win32/win32compat/w32fd.c b/contrib/win32/win32compat/w32fd.c index 9353f8cc2..ab6281be3 100644 --- a/contrib/win32/win32compat/w32fd.c +++ b/contrib/win32/win32compat/w32fd.c @@ -40,6 +40,7 @@ #include #include "Shlwapi.h" #include +#include "misc_internal.h" /* internal table that stores the fd to w32_io mapping*/ struct w32fd_table { @@ -347,7 +348,7 @@ w32_pipe(int *pfds) { pio[0]->handle, pio[0], read_index, pio[1]->handle, pio[1], write_index); return 0; } -char *realpath_win(const char *path, char resolved[MAX_PATH]); + int w32_open(const char *pathname, int flags, ...) { int min_index = fd_table_get_min_index(); @@ -357,18 +358,14 @@ w32_open(const char *pathname, int flags, ...) { if (min_index == -1) return -1; - // Skip the first '/' in the pathname - char resolvedPathName[MAX_PATH]; - realpath_win(pathname, resolvedPathName); - - pio = fileio_open(resolvedPathName, flags, 0); + pio = fileio_open(sanitized_path(pathname), flags, 0); if (pio == NULL) return -1; pio->type = NONSOCK_FD; fd_table_set(pio, min_index); debug("open - handle:%p, io:%p, fd:%d", pio->handle, pio, min_index); - debug3("open - path:%s", resolvedPathName); + debug3("open - path:%s", pathname); return min_index; } diff --git a/contrib/win32/win32compat/win32_dirent.c b/contrib/win32/win32compat/win32_dirent.c index e00937533..2418e13d4 100644 --- a/contrib/win32/win32compat/win32_dirent.c +++ b/contrib/win32/win32compat/win32_dirent.c @@ -11,6 +11,7 @@ #include "inc\dirent.h" #include "inc\libgen.h" +#include "misc_internal.h" struct DIR_ { @@ -19,8 +20,6 @@ struct DIR_ { int first; }; -char * realpath_win(const char *path, char resolved[MAX_PATH]); - /* Open a directory stream on NAME. Return a DIR stream on the directory, or NULL if it could not be opened. */ DIR * opendir(const char *name) @@ -32,11 +31,7 @@ DIR * opendir(const char *name) wchar_t* wname = NULL; int needed; - // Skip the first '/' in the pathname - char resolvedPathName[MAX_PATH]; - realpath_win(name, resolvedPathName); - - if ((wname = utf8_to_utf16(resolvedPathName)) == NULL) { + if ((wname = utf8_to_utf16(sanitized_path(name))) == NULL) { errno = ENOMEM; return NULL; } diff --git a/regress/pesterTests/SCP.Tests.ps1 b/regress/pesterTests/SCP.Tests.ps1 index b510df8d8..05faec59d 100644 --- a/regress/pesterTests/SCP.Tests.ps1 +++ b/regress/pesterTests/SCP.Tests.ps1 @@ -26,13 +26,12 @@ Describe "Tests for scp command" -Tags "CI" { $client.SetupClient($server) $server.SetupServer($client) - $testData = @( - <# known issue 340 + $testData = @( @{ Title = 'Simple copy local file to local file' Source = $SourceFilePath Destination = $DestinationFilePath - },#> + }, @{ Title = 'Simple copy local file to remote file' Source = $SourceFilePath @@ -42,13 +41,12 @@ Describe "Tests for scp command" -Tags "CI" { Title = 'Simple copy remote file to local file' Source = "$($server.localAdminUserName)@$($server.MachineName):$SourceFilePath" Destination = $DestinationFilePath - }, - <# known issue 340 + }, @{ Title = 'Simple copy local file to local dir' Source = $SourceFilePath Destination = $DestinationDir - },#> + }, @{ Title = 'simple copy local file to remote dir' Source = $SourceFilePath @@ -66,7 +64,7 @@ Describe "Tests for scp command" -Tags "CI" { Title = 'copy from local dir to remote dir' Source = $sourceDir Destination = "$($server.localAdminUserName)@$($server.MachineName):$DestinationDir" - }, + }, @{ Title = 'copy from local dir to local dir' Source = $sourceDir @@ -121,7 +119,7 @@ Describe "Tests for scp command" -Tags "CI" { $equal | Should Be $true } - <#It 'Directory recursive Copy with -i option: ' -TestCases:$testData1 { + It 'Directory recursive Copy with -i option: <Title> ' -TestCases:$testData1 { param([string]$Title, $Source, $Destination) .\scp -r -i $identifyFile $Source $Destination @@ -129,10 +127,10 @@ Describe "Tests for scp command" -Tags "CI" { $equal = @(Compare-Object (Get-Item -path $SourceDir ) (Get-Item -path (join-path $DestinationDir $SourceDirName) ) -Property Name, Length).Length -eq 0 $equal | Should Be $true - #known issue 364 - #$equal = @(Compare-Object (Get-ChildItem -Recurse -path $SourceDir) (Get-ChildItem -Recurse -path (join-path $DestinationDir $SourceDirName) ) -Property Name, Length).Length -eq 0 - #$equal | Should Be $true - }#> + + $equal = @(Compare-Object (Get-ChildItem -Recurse -path $SourceDir) (Get-ChildItem -Recurse -path (join-path $DestinationDir $SourceDirName) ) -Property Name, Length).Length -eq 0 + $equal | Should Be $true + } } #this context only run on windows @@ -154,32 +152,29 @@ Describe "Tests for scp command" -Tags "CI" { It 'File Copy with -S option (positive)' { .\scp -S .\ssh.exe $SourceFilePath "$($server.localAdminUserName)@$($server.MachineName):$DestinationFilePath" #validate file content. DestPath is the path to the file. - $equal = @(Compare-Object (Get-ChildItem -path $SourceFilePath) (Get-ChildItem -path $DestinationFilePath) -Property Name, Length).Length -eq 0 #todo: add LastWriteTime in comparison when issue is fixed + $equal = @(Compare-Object (Get-ChildItem -path $SourceFilePath) (Get-ChildItem -path $DestinationFilePath) -Property Name, Length).Length -eq 0 $equal | Should Be $true } <#It 'File Copy with -p -c -v option: <Title> ' -TestCases:$testData { param([string]$Title, $Source, $Destination) - .\scp -p -c aes128-ctr -C $Source $Destination #Todo: add -v after it is supported. + .\scp -p -c aes128-ctr -v -C $Source $Destination #validate file content. DestPath is the path to the file. - $equal = @(Compare-Object (Get-ChildItem -path $SourceFilePath) (Get-ChildItem -path $DestinationFilePath) -Property Name, Length).Length -eq 0 #todo: add LastWriteTime in comparison when issue is fixed - $equal | Should Be $true + $equal = @(Compare-Object (Get-ChildItem -path $SourceFilePath) (Get-ChildItem -path $DestinationFilePath) -Property Name, Length, LastWriteTime.DateTime).Length -eq 0 + $equal | Should Be $true }#> - - <# known issue 369 - It 'Directory recursive Copy with -v option: <Title> ' -TestCases:$testData1 { - param([string]$Title, $Source, $Destination) - - .\scp -r -p $Source $Destination + + It 'Directory recursive Copy with -r -p -v option: <Title> ' -TestCases:$testData1 { + param([string]$Title, $Source, $Destination) + .\scp -r -p -v $Source $Destination - $equal = @(Compare-Object (Get-Item -path $SourceDir ) (Get-Item -path (join-path $DestinationDir $SourceDirName) ) -Property Name, Length, LastWriteTime).Length -eq 0 + $equal = @(Compare-Object (Get-Item -path $SourceDir ) (Get-Item -path (join-path $DestinationDir $SourceDirName) ) -Property Name, Length).Length -eq 0 + $equal | Should Be $true + + $equal = @(Compare-Object (Get-ChildItem -Recurse -path $SourceDir) (Get-ChildItem -Recurse -path (join-path $DestinationDir $SourceDirName) ) -Property Name, Length, LastWriteTime.DateTime).Length -eq 0 $equal | Should Be $true - - #known issue 364 - #$equal = @(Compare-Object (Get-ChildItem -Recurse -path $SourceDir) (Get-ChildItem -Recurse -path (join-path $DestinationDir $SourceDirName) ) -Property Name, Length, LastWriteTime).Length -eq 0 - #$equal | Should Be $true - }#> + } } Context "Key based authentication with -i -C -q options. host keys are not secured on server" { @@ -192,23 +187,20 @@ Describe "Tests for scp command" -Tags "CI" { .\scp -i $identifyFile -C -q $Source $Destination #validate file content. DestPath is the path to the file. - $equal = @(Compare-Object (Get-ChildItem -path $SourceFilePath) (Get-ChildItem -path $DestinationFilePath) -Property Name, Length).Length -eq 0 # need to validate LastWriteTime after issue 356 is fixed. + $equal = @(Compare-Object (Get-ChildItem -path $SourceFilePath) (Get-ChildItem -path $DestinationFilePath) -Property Name, Length).Length -eq 0 $equal | Should Be $true } - <#It 'Directory recursive Copy with -i and -q options: <Title> ' -TestCases:$testData1 { + It 'Directory recursive Copy with -i and -q options: <Title> ' -TestCases:$testData1 { param([string]$Title, $Source, $Destination) .\scp -i $identifyFile -r -q $Source $Destination $equal = @(Compare-Object (Get-Item -path $SourceDir ) (Get-Item -path (join-path $DestinationDir $SourceDirName) ) -Property Name, Length).Length -eq 0 $equal | Should Be $true - - #known issue 364 - #$equal = @(Compare-Object (Get-ChildItem -Recurse -path $SourceDir) (Get-ChildItem -Recurse -path (join-path $DestinationDir $SourceDirName) ) -Property Name, Length).Length -eq 0 - #$equal | Should Be $true - }#> - } - + + $equal = @(Compare-Object (Get-ChildItem -Recurse -path $SourceDir) (Get-ChildItem -Recurse -path (join-path $DestinationDir $SourceDirName) ) -Property Name, Length).Length -eq 0 + $equal | Should Be $true + } + } } - diff --git a/scp.c b/scp.c index 57b037e39..0c4fdf840 100644 --- a/scp.c +++ b/scp.c @@ -596,17 +596,11 @@ main(int argc, char **argv) /* * To support both Windows and Unix style paths * convert '\\' to '/' in rest of arguments - */ - { - char *s1, *s2; + */ + { int i; - for (i = 0; i < argc; i++) { - s1 = argv[i]; - while ((s2 = strchr(s1, '\\')) != NULL) { - *s2 = '/'; - s1 = s2 + 1; - } - } + for (i = 0; i < argc; i++) + convertToForwardslash(argv[i]); } #endif /* WINDOWS */ @@ -831,40 +825,34 @@ tolocal(int argc, char **argv) /* local to local on windows - need to use local native copy command */ struct stat stb; int exists; + char *last; exists = stat(argv[i], &stb) == 0; + /* convert '/' to '\\' */ + convertToBackslash(argv[i]); + convertToBackslash(argv[i - 1]); if (exists && (S_ISDIR(stb.st_mode))) { addargs(&alist, "%s", _PATH_XCOPY); if (iamrecursive) addargs(&alist, "/S /E /H"); if (pflag) addargs(&alist, "/K /X"); - addargs(&alist, "/Y /F /I"); - addargs(&alist, "%s", argv[i]); - - char *lastf = NULL, *lastr = NULL, *name; - if ((lastf = strrchr(argv[i], '/')) == NULL && (lastr = strrchr(argv[i], '\\')) == NULL) - name = argv[i]; - else { - if (lastf) - name = lastf; - if (lastr) - name = lastr; - ++name; - } - - char * dest = argv[argc - 1]; - int len = strlen(dest); - char * lastletter = dest + len - 1; + addargs(&alist, "/Y /F /I"); + addargs(&alist, "%s", argv[i]); + if ((last = strrchr(argv[i], '\\')) == NULL) + last = argv[i]; + else + ++last; + addargs(&alist, "%s%s%s", argv[argc - 1], - (lastletter == "\\" || lastletter == "/") ? "" : "\\", name); + strcmp(argv[argc - 1], "\\") ? "\\" : "", last); } else { addargs(&alist, "%s", _PATH_COPY); - addargs(&alist, "/Y"); + addargs(&alist, "/Y"); addargs(&alist, "%s", argv[i]); addargs(&alist, "%s", argv[argc - 1]); - } + } #else /* !WINDOWS */ addargs(&alist, "%s", _PATH_CP); if (iamrecursive) @@ -1541,3 +1529,4 @@ lostconn(int signo) else exit(1); } +