From a1b068ec31a042ff1e10a522d8fdf0b8869d53ca Mon Sep 17 00:00:00 2001 From: Luke Tomlinson Date: Fri, 21 May 2021 17:01:42 -0400 Subject: [PATCH] Bugfix: Fix issue with interactive unzip on Linux (#807) * Add new powershell commands for windows unzip * Test fails to overwrite file * Add new windows commands for unzip * Add Test for failing case for both pwsh and powershell * Modify test to confirm overwrite behavior for xar * Delete ._test.txt * Add fallback case for older windows systems * Remove try * Run Tests on windows-2016 * Update tar tests to handle existing files * Lint * Update tool-cache.test.ts * Update tool-cache.test.ts * Update tool-cache.test.ts * Update tool-cache.test.ts * Update from PR feedback --- .../tool-cache/__tests__/tool-cache.test.ts | 177 +++++++++++++----- packages/tool-cache/src/tool-cache.ts | 64 +++++-- 2 files changed, 175 insertions(+), 66 deletions(-) diff --git a/packages/tool-cache/__tests__/tool-cache.test.ts b/packages/tool-cache/__tests__/tool-cache.test.ts index b1a9635a..16c1a5b4 100644 --- a/packages/tool-cache/__tests__/tool-cache.test.ts +++ b/packages/tool-cache/__tests__/tool-cache.test.ts @@ -239,6 +239,10 @@ describe('@actions/tool-cache', function() { const _7zFile: string = path.join(tempDir, 'test.7z') await io.cp(path.join(__dirname, 'data', 'test.7z'), _7zFile) + const destDir = path.join(tempDir, 'destination') + await io.mkdirP(destDir) + fs.writeFileSync(path.join(destDir, 'file.txt'), 'overwriteMe') + // extract/cache const extPath: string = await tc.extract7z(_7zFile) await tc.cacheDir(extPath, 'my-7z-contents', '1.1.0') @@ -247,6 +251,9 @@ describe('@actions/tool-cache', function() { expect(fs.existsSync(toolPath)).toBeTruthy() expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy() expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy() + expect(fs.readFileSync(path.join(toolPath, 'file.txt'), 'utf8')).toBe( + 'file.txt contents' + ) expect( fs.existsSync(path.join(toolPath, 'file-with-ç-character.txt')) ).toBeTruthy() @@ -343,6 +350,22 @@ describe('@actions/tool-cache', function() { await io.rmRF(tempDir) } }) + it.each(['pwsh', 'powershell'])( + 'unzip properly fails with bad path (%s)', + async powershellTool => { + const originalPath = process.env['PATH'] + try { + if (powershellTool === 'powershell' && IS_WINDOWS) { + //remove pwsh from PATH temporarily to test fallback case + process.env['PATH'] = removePWSHFromPath(process.env['PATH']) + } + + await expect(tc.extractZip('badPath')).rejects.toThrow() + } finally { + process.env['PATH'] = originalPath + } + } + ) } else if (IS_MAC) { it('extract .xar', async () => { const tempDir = path.join(tempPath, 'test-install.xar') @@ -356,14 +379,21 @@ describe('@actions/tool-cache', function() { cwd: sourcePath }) + const destDir = path.join(tempDir, 'destination') + await io.mkdirP(destDir) + fs.writeFileSync(path.join(destDir, 'file.txt'), 'overwriteMe') + // extract/cache - const extPath: string = await tc.extractXar(targetPath, undefined, '-x') + const extPath: string = await tc.extractXar(targetPath, destDir, ['-x']) await tc.cacheDir(extPath, 'my-xar-contents', '1.1.0') const toolPath: string = tc.find('my-xar-contents', '1.1.0') expect(fs.existsSync(toolPath)).toBeTruthy() expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy() expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy() + expect(fs.readFileSync(path.join(toolPath, 'file.txt'), 'utf8')).toBe( + 'file.txt contents' + ) expect( fs.existsSync(path.join(toolPath, 'file-with-ç-character.txt')) ).toBeTruthy() @@ -458,14 +488,23 @@ describe('@actions/tool-cache', function() { const _tgzFile: string = path.join(tempDir, 'test.tar.gz') await io.cp(path.join(__dirname, 'data', 'test.tar.gz'), _tgzFile) + //Create file to overwrite + const destDir = path.join(tempDir, 'extract-dest') + await io.rmRF(destDir) + await io.mkdirP(destDir) + fs.writeFileSync(path.join(destDir, 'file.txt'), 'overwriteMe') + // extract/cache - const extPath: string = await tc.extractTar(_tgzFile) + const extPath: string = await tc.extractTar(_tgzFile, destDir) await tc.cacheDir(extPath, 'my-tgz-contents', '1.1.0') const toolPath: string = tc.find('my-tgz-contents', '1.1.0') expect(fs.existsSync(toolPath)).toBeTruthy() expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy() expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy() + expect(fs.readFileSync(path.join(toolPath, 'file.txt'), 'utf8')).toBe( + 'file.txt contents' + ) expect( fs.existsSync(path.join(toolPath, 'file-with-ç-character.txt')) ).toBeTruthy() @@ -496,6 +535,9 @@ describe('@actions/tool-cache', function() { expect(fs.existsSync(toolPath)).toBeTruthy() expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy() expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy() + expect(fs.readFileSync(path.join(toolPath, 'file.txt'), 'utf8')).toBe( + 'file.txt contents' + ) expect( fs.existsSync(path.join(toolPath, 'file-with-ç-character.txt')) ).toBeTruthy() @@ -516,8 +558,14 @@ describe('@actions/tool-cache', function() { const _txzFile: string = path.join(tempDir, 'test.tar.xz') await io.cp(path.join(__dirname, 'data', 'test.tar.xz'), _txzFile) + //Create file to overwrite + const destDir = path.join(tempDir, 'extract-dest') + await io.rmRF(destDir) + await io.mkdirP(destDir) + fs.writeFileSync(path.join(destDir, 'file.txt'), 'overwriteMe') + // extract/cache - const extPath: string = await tc.extractTar(_txzFile, undefined, 'x') + const extPath: string = await tc.extractTar(_txzFile, destDir, 'x') await tc.cacheDir(extPath, 'my-txz-contents', '1.1.0') const toolPath: string = tc.find('my-txz-contents', '1.1.0') @@ -530,58 +578,70 @@ describe('@actions/tool-cache', function() { ).toBe('foo/hello: world') }) - it('installs a zip and finds it', async () => { - const tempDir = path.join(__dirname, 'test-install-zip') - try { - await io.mkdirP(tempDir) + it.each(['pwsh', 'powershell'])( + 'installs a zip and finds it (%s)', + async powershellTool => { + const tempDir = path.join(__dirname, 'test-install-zip') + const originalPath = process.env['PATH'] + try { + await io.mkdirP(tempDir) - // stage the layout for a zip file: - // file.txt - // folder/nested-file.txt - const stagingDir = path.join(tempDir, 'zip-staging') - await io.mkdirP(path.join(stagingDir, 'folder')) - fs.writeFileSync(path.join(stagingDir, 'file.txt'), '') - fs.writeFileSync(path.join(stagingDir, 'folder', 'nested-file.txt'), '') + // stage the layout for a zip file: + // file.txt + // folder/nested-file.txt + const stagingDir = path.join(tempDir, 'zip-staging') + await io.mkdirP(path.join(stagingDir, 'folder')) + fs.writeFileSync(path.join(stagingDir, 'file.txt'), '') + fs.writeFileSync(path.join(stagingDir, 'folder', 'nested-file.txt'), '') - // create the zip - const zipFile = path.join(tempDir, 'test.zip') - await io.rmRF(zipFile) - if (IS_WINDOWS) { - const escapedStagingPath = stagingDir.replace(/'/g, "''") // double-up single quotes - const escapedZipFile = zipFile.replace(/'/g, "''") - const powershellPath = - (await io.which('pwsh', false)) || - (await io.which('powershell', true)) - const args = [ - '-NoLogo', - '-Sta', - '-NoProfile', - '-NonInteractive', - '-ExecutionPolicy', - 'Unrestricted', - '-Command', - `$ErrorActionPreference = 'Stop' ; Add-Type -AssemblyName System.IO.Compression.FileSystem ; [System.IO.Compression.ZipFile]::CreateFromDirectory('${escapedStagingPath}', '${escapedZipFile}')` - ] - await exec.exec(`"${powershellPath}"`, args) - } else { - const zipPath: string = await io.which('zip', true) - await exec.exec(`"${zipPath}`, [zipFile, '-r', '.'], {cwd: stagingDir}) + // create the zip + const zipFile = path.join(tempDir, 'test.zip') + await io.rmRF(zipFile) + if (IS_WINDOWS) { + const escapedStagingPath = stagingDir.replace(/'/g, "''") // double-up single quotes + const escapedZipFile = zipFile.replace(/'/g, "''") + const powershellPath = + (await io.which('pwsh', false)) || + (await io.which('powershell', true)) + const args = [ + '-NoLogo', + '-Sta', + '-NoProfile', + '-NonInteractive', + '-ExecutionPolicy', + 'Unrestricted', + '-Command', + `$ErrorActionPreference = 'Stop' ; Add-Type -AssemblyName System.IO.Compression.FileSystem ; [System.IO.Compression.ZipFile]::CreateFromDirectory('${escapedStagingPath}', '${escapedZipFile}')` + ] + await exec.exec(`"${powershellPath}"`, args) + } else { + const zipPath: string = await io.which('zip', true) + await exec.exec(`"${zipPath}`, [zipFile, '-r', '.'], { + cwd: stagingDir + }) + } + + if (powershellTool === 'powershell' && IS_WINDOWS) { + //remove pwsh from PATH temporarily to test fallback case + process.env['PATH'] = removePWSHFromPath(process.env['PATH']) + } + + const extPath: string = await tc.extractZip(zipFile) + await tc.cacheDir(extPath, 'foo', '1.1.0') + const toolPath: string = tc.find('foo', '1.1.0') + + expect(fs.existsSync(toolPath)).toBeTruthy() + expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy() + expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy() + expect( + fs.existsSync(path.join(toolPath, 'folder', 'nested-file.txt')) + ).toBeTruthy() + } finally { + await io.rmRF(tempDir) + process.env['PATH'] = originalPath } - - const extPath: string = await tc.extractZip(zipFile) - await tc.cacheDir(extPath, 'foo', '1.1.0') - const toolPath: string = tc.find('foo', '1.1.0') - - expect(fs.existsSync(toolPath)).toBeTruthy() - expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy() - expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy() - expect( - fs.existsSync(path.join(toolPath, 'folder', 'nested-file.txt')) - ).toBeTruthy() - } finally { - await io.rmRF(tempDir) } - }) + ) it('installs a zip and extracts it to specified directory', async function() { const tempDir = path.join(__dirname, 'test-install-zip') @@ -593,7 +653,7 @@ describe('@actions/tool-cache', function() { // folder/nested-file.txt const stagingDir = path.join(tempDir, 'zip-staging') await io.mkdirP(path.join(stagingDir, 'folder')) - fs.writeFileSync(path.join(stagingDir, 'file.txt'), '') + fs.writeFileSync(path.join(stagingDir, 'file.txt'), 'originalText') fs.writeFileSync(path.join(stagingDir, 'folder', 'nested-file.txt'), '') // create the zip @@ -625,12 +685,16 @@ describe('@actions/tool-cache', function() { await io.rmRF(destDir) await io.mkdirP(destDir) try { + fs.writeFileSync(path.join(destDir, 'file.txt'), 'overwriteMe') const extPath: string = await tc.extractZip(zipFile, destDir) await tc.cacheDir(extPath, 'foo', '1.1.0') const toolPath: string = tc.find('foo', '1.1.0') expect(fs.existsSync(toolPath)).toBeTruthy() expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy() expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy() + expect(fs.readFileSync(path.join(toolPath, 'file.txt'), 'utf8')).toBe( + 'originalText' + ) expect( fs.existsSync(path.join(toolPath, 'folder', 'nested-file.txt')) ).toBeTruthy() @@ -839,3 +903,12 @@ function setGlobal(key: string, value: T | undefined): void { g[key] = value } } + +function removePWSHFromPath(pathEnv: string | undefined): string { + return (pathEnv || '') + .split(';') + .filter(segment => { + return !segment.startsWith(`C:\\Program Files\\PowerShell`) + }) + .join(';') +} diff --git a/packages/tool-cache/src/tool-cache.ts b/packages/tool-cache/src/tool-cache.ts index e9696f4e..92fd519a 100644 --- a/packages/tool-cache/src/tool-cache.ts +++ b/packages/tool-cache/src/tool-cache.ts @@ -272,6 +272,7 @@ export async function extractTar( if (isGnuTar) { // Suppress warnings when using GNU tar to extract archives created by BSD tar args.push('--warning=no-unknown-keyword') + args.push('--overwrite') } args.push('-C', destArg, '-f', fileArg) @@ -344,21 +345,55 @@ async function extractZipWin(file: string, dest: string): Promise { // build the powershell command const escapedFile = file.replace(/'/g, "''").replace(/"|\n|\r/g, '') // double-up single quotes, remove double quotes and newlines const escapedDest = dest.replace(/'/g, "''").replace(/"|\n|\r/g, '') - const command = `$ErrorActionPreference = 'Stop' ; try { Add-Type -AssemblyName System.IO.Compression.FileSystem } catch { } ; [System.IO.Compression.ZipFile]::ExtractToDirectory('${escapedFile}', '${escapedDest}')` + const pwshPath = await io.which('pwsh', false) - // run powershell - const powershellPath = await io.which('powershell', true) - const args = [ - '-NoLogo', - '-Sta', - '-NoProfile', - '-NonInteractive', - '-ExecutionPolicy', - 'Unrestricted', - '-Command', - command - ] - await exec(`"${powershellPath}"`, args) + //To match the file overwrite behavior on nix systems, we use the overwrite = true flag for ExtractToDirectory + //and the -Force flag for Expand-Archive as a fallback + if (pwshPath) { + //attempt to use pwsh with ExtractToDirectory, if this fails attempt Expand-Archive + const pwshCommand = [ + `$ErrorActionPreference = 'Stop' ;`, + `try { Add-Type -AssemblyName System.IO.Compression.ZipFile } catch { } ;`, + `try { [System.IO.Compression.ZipFile]::ExtractToDirectory('${escapedFile}', '${escapedDest}', $true) }`, + `catch { if (($_.Exception.GetType().FullName -eq 'System.Management.Automation.MethodException') -or ($_.Exception.GetType().FullName -eq 'System.Management.Automation.RuntimeException') ){ Expand-Archive -LiteralPath '${escapedFile}' -DestinationPath '${escapedDest}' -Force } else { throw $_ } } ;` + ].join(' ') + + const args = [ + '-NoLogo', + '-NoProfile', + '-NonInteractive', + '-ExecutionPolicy', + 'Unrestricted', + '-Command', + pwshCommand + ] + + core.debug(`Using pwsh at path: ${pwshPath}`) + await exec(`"${pwshPath}"`, args) + } else { + const powershellCommand = [ + `$ErrorActionPreference = 'Stop' ;`, + `try { Add-Type -AssemblyName System.IO.Compression.FileSystem } catch { } ;`, + `if ((Get-Command -Name Expand-Archive -Module Microsoft.PowerShell.Archive -ErrorAction Ignore)) { Expand-Archive -LiteralPath '${escapedFile}' -DestinationPath '${escapedDest}' -Force }`, + `else {[System.IO.Compression.ZipFile]::ExtractToDirectory('${escapedFile}', '${escapedDest}', $true) }` + ].join(' ') + + const args = [ + '-NoLogo', + '-Sta', + '-NoProfile', + '-NonInteractive', + '-ExecutionPolicy', + 'Unrestricted', + '-Command', + powershellCommand + ] + + const powershellPath = await io.which('powershell', true) + core.debug(`Using powershell at path: ${powershellPath}`) + + await exec(`"${powershellPath}"`, args) + } } async function extractZipNix(file: string, dest: string): Promise { @@ -367,6 +402,7 @@ async function extractZipNix(file: string, dest: string): Promise { if (!core.isDebug()) { args.unshift('-q') } + args.unshift('-o') //overwrite with -o, otherwise a prompt is shown which freezes the run await exec(`"${unzipPath}"`, args, {cwd: dest}) }