From 9158e6201a7dd7325f47faead8d37e65752dc301 Mon Sep 17 00:00:00 2001 From: Andrew Plotkin Date: Sun, 6 Mar 2022 11:21:02 -0500 Subject: [PATCH 1/8] Rename local "details" to "results", for consistency. --- app/src/cache.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/cache.js b/app/src/cache.js index c346f42..abd2f71 100644 --- a/app/src/cache.js +++ b/app/src/cache.js @@ -84,13 +84,13 @@ export default class FileCache { const url = `https://${this.options.archive_domain}/if-archive/${this.index.hash_to_path.get(hash)}` const type = SUPPORTED_FORMATS.exec(url)[1].toLowerCase() const cache_path = this.file_path(hash, type) - const details = await execFile('curl', [encodeURI(url), '-o', cache_path, '-s', '-S', '-D', '-']) - if (details.stderr) { - throw new Error(`curl error: ${details.stderr}`) + const results = await execFile('curl', [encodeURI(url), '-o', cache_path, '-s', '-S', '-D', '-']) + if (results.stderr) { + throw new Error(`curl error: ${results.stderr}`) } // Parse the date - const date_header = /last-modified:\s+\w+,\s+(\d+\s+\w+\s+\d+)/.exec(details.stdout) + const date_header = /last-modified:\s+\w+,\s+(\d+\s+\w+\s+\d+)/.exec(results.stdout) if (!date_header) { throw new Error('Could not parse last-modified header') } From d857a231195cfd06c908993e015b5ca76a9a842b Mon Sep 17 00:00:00 2001 From: Andrew Plotkin Date: Sun, 6 Mar 2022 12:15:48 -0500 Subject: [PATCH 2/8] Explicit bad-archive-type error. --- app/src/cache.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/cache.js b/app/src/cache.js index abd2f71..a8513c5 100644 --- a/app/src/cache.js +++ b/app/src/cache.js @@ -177,7 +177,7 @@ export default class FileCache { results = await execFile('unzip', ['-p', zip_path, file_path], {encoding: 'buffer', maxBuffer: this.max_buffer}) break default: - throw new Error('Other archive format not yet supported') + throw new Error(`Archive format ${type} not yet supported`) } if (results.stderr.length) { throw new Error(`${command} error: ${results.stderr.toString()}`) @@ -201,7 +201,7 @@ export default class FileCache { child = child_process.spawn('unzip', ['-p', zip_path, file_path]) break default: - throw new Error('Other archive format not yet supported') + throw new Error(`Archive format ${type} not yet supported`) } return child.stdout } @@ -225,7 +225,7 @@ export default class FileCache { results = await exec(`unzip -p ${zip_path} '${escape_shell_single_quoted(file_path)}' | file -i -`) break default: - throw new Error('Other archive format not yet supported') + throw new Error(`Archive format ${type} not yet supported`) } if (results.stderr.length) { throw new Error(`${command}|file error: ${results.stderr.toString()}`) @@ -256,7 +256,7 @@ export default class FileCache { results = await execFile('unzip', ['-Z1', path]) break default: - throw new Error('Other archive format not yet supported') + throw new Error(`Archive format ${type} not yet supported`) } if (results.stderr) { throw new Error(`${command} error: ${results.stderr}`) From 3f136e74aa85e11d9636b6f7a72db5ad3a389cbb Mon Sep 17 00:00:00 2001 From: Andrew Plotkin Date: Sun, 6 Mar 2022 12:24:32 -0500 Subject: [PATCH 3/8] Try this solution to the unzip/untar errors. --- app/src/cache.js | 45 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/app/src/cache.js b/app/src/cache.js index a8513c5..7744a33 100644 --- a/app/src/cache.js +++ b/app/src/cache.js @@ -24,6 +24,26 @@ import {SUPPORTED_FORMATS, escape_shell_single_quoted} from './common.js' const exec = util.promisify(child_process.exec) const execFile = util.promisify(child_process.execFile) +function untar_error(err) { + if (err.signal) { + return { failed:true, stdout:err.stdout, stderr:`SIGNAL ${err.signal}\n${err.stderr}` } + } + if (err.code != 0) { + return { failed:true, stdout:err.stdout, stderr:err.stderr } + } + return { stdout:err.stdout, stderr:err.stderr } +} + +function unzip_error(err) { + if (err.signal) { + return { failed:true, stdout:err.stdout, stderr:`SIGNAL ${err.signal}\n${err.stderr}` } + } + if (err.code != 0) { + return { failed:true, stdout:err.stdout, stderr:err.stderr } + } + return { stdout:err.stdout, stderr:err.stderr } +} + class CacheEntry { constructor (contents, date, size, type) { this.contents = contents @@ -166,20 +186,23 @@ export default class FileCache { case 'tar.gz': case 'tgz': command = 'tar' - results = await execFile('tar', ['-xOzf', zip_path, file_path], {encoding: 'buffer', maxBuffer: this.max_buffer}) + results = await execFile('tar', ['-xOzf', zip_path, file_path], {encoding: 'buffer', maxBuffer: this.max_buffer}).catch(untar_error) break case 'tar.z': command = 'tar' - results = await execFile('tar', ['-xOZf', zip_path, file_path], {encoding: 'buffer', maxBuffer: this.max_buffer}) + results = await execFile('tar', ['-xOZf', zip_path, file_path], {encoding: 'buffer', maxBuffer: this.max_buffer}).catch(untar_error) break case 'zip': command = 'unzip' - results = await execFile('unzip', ['-p', zip_path, file_path], {encoding: 'buffer', maxBuffer: this.max_buffer}) + results = await execFile('unzip', ['-p', zip_path, file_path], {encoding: 'buffer', maxBuffer: this.max_buffer}).catch(unzip_error) break default: throw new Error(`Archive format ${type} not yet supported`) } if (results.stderr.length) { + console.log(`${command} error: ${results.stderr.toString()}`) + } + if (results.failed) { throw new Error(`${command} error: ${results.stderr.toString()}`) } return results.stdout @@ -214,20 +237,23 @@ export default class FileCache { case 'tar.gz': case 'tgz': command = 'tar' - results = await exec(`tar -xOzf ${zip_path} '${escape_shell_single_quoted(file_path)}' | file -i -`) + results = await exec(`tar -xOzf ${zip_path} '${escape_shell_single_quoted(file_path)}' | file -i -`).catch(untar_error) break case 'tar.z': command = 'tar' - results = await exec(`tar -xOZf ${zip_path} '${escape_shell_single_quoted(file_path)}' | file -i -`) + results = await exec(`tar -xOZf ${zip_path} '${escape_shell_single_quoted(file_path)}' | file -i -`).catch(untar_error) break case 'zip': command = 'unzip' - results = await exec(`unzip -p ${zip_path} '${escape_shell_single_quoted(file_path)}' | file -i -`) + results = await exec(`unzip -p ${zip_path} '${escape_shell_single_quoted(file_path)}' | file -i -`).catch(unzip_error) break default: throw new Error(`Archive format ${type} not yet supported`) } if (results.stderr.length) { + console.log(`${command} error: ${results.stderr.toString()}`) + } + if (results.failed) { throw new Error(`${command}|file error: ${results.stderr.toString()}`) } // Trim '/dev/stdin:' @@ -249,16 +275,19 @@ export default class FileCache { case 'tar.z': case 'tgz': command = 'tar' - results = await execFile('tar', ['-tf', path]) + results = await execFile('tar', ['-tf', path]).catch(untar_error) break case 'zip': command = 'unzip' - results = await execFile('unzip', ['-Z1', path]) + results = await execFile('unzip', ['-Z1', path]).catch(unzip_error) break default: throw new Error(`Archive format ${type} not yet supported`) } if (results.stderr) { + console.log(`${command} error: ${results.stderr.toString()}`) + } + if (results.failed) { throw new Error(`${command} error: ${results.stderr}`) } return results.stdout.trim().split('\n').filter(line => !line.endsWith('/')).sort() From 8a20eceb39e55e80f96588383cfc4292a0268448 Mon Sep 17 00:00:00 2001 From: Andrew Plotkin Date: Sun, 6 Mar 2022 12:40:33 -0500 Subject: [PATCH 4/8] Handle status code 1 from unzip --- app/src/cache.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/cache.js b/app/src/cache.js index 7744a33..a79ef8c 100644 --- a/app/src/cache.js +++ b/app/src/cache.js @@ -38,7 +38,7 @@ function unzip_error(err) { if (err.signal) { return { failed:true, stdout:err.stdout, stderr:`SIGNAL ${err.signal}\n${err.stderr}` } } - if (err.code != 0) { + if (err.code != 0 && err.code != 1) { return { failed:true, stdout:err.stdout, stderr:err.stderr } } return { stdout:err.stdout, stderr:err.stderr } From ddecbe7cb07fcbaaab7a223e7ab47578cec3998c Mon Sep 17 00:00:00 2001 From: Andrew Plotkin Date: Sun, 6 Mar 2022 12:48:16 -0500 Subject: [PATCH 5/8] More info in error logs. --- app/src/cache.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/src/cache.js b/app/src/cache.js index a79ef8c..eb1c2aa 100644 --- a/app/src/cache.js +++ b/app/src/cache.js @@ -200,10 +200,10 @@ export default class FileCache { throw new Error(`Archive format ${type} not yet supported`) } if (results.stderr.length) { - console.log(`${command} error: ${results.stderr.toString()}`) + console.log(`${file_path}: ${command} error: ${results.stderr.toString()}`) } if (results.failed) { - throw new Error(`${command} error: ${results.stderr.toString()}`) + throw new Error(`${file_path}: ${command} error: ${results.stderr.toString()}`) } return results.stdout } @@ -251,10 +251,10 @@ export default class FileCache { throw new Error(`Archive format ${type} not yet supported`) } if (results.stderr.length) { - console.log(`${command} error: ${results.stderr.toString()}`) + console.log(`${file_path}: ${command} error: ${results.stderr.toString()}`) } if (results.failed) { - throw new Error(`${command}|file error: ${results.stderr.toString()}`) + throw new Error(`${file_path}: ${command}|file error: ${results.stderr.toString()}`) } // Trim '/dev/stdin:' return results.stdout.trim().substring(12) @@ -285,10 +285,10 @@ export default class FileCache { throw new Error(`Archive format ${type} not yet supported`) } if (results.stderr) { - console.log(`${command} error: ${results.stderr.toString()}`) + console.log(`${path}: ${command} error: ${results.stderr.toString()}`) } if (results.failed) { - throw new Error(`${command} error: ${results.stderr}`) + throw new Error(`${path}: ${command} error: ${results.stderr}`) } return results.stdout.trim().split('\n').filter(line => !line.endsWith('/')).sort() } From 8793ae19ba149606a46adcb819608d0cf5207a9c Mon Sep 17 00:00:00 2001 From: Andrew Plotkin Date: Sun, 6 Mar 2022 12:59:20 -0500 Subject: [PATCH 6/8] Comments. --- app/src/cache.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/src/cache.js b/app/src/cache.js index eb1c2aa..936b958 100644 --- a/app/src/cache.js +++ b/app/src/cache.js @@ -24,6 +24,13 @@ import {SUPPORTED_FORMATS, escape_shell_single_quoted} from './common.js' const exec = util.promisify(child_process.exec) const execFile = util.promisify(child_process.execFile) +/* Promise-rejection handlers for untar/unzip. These return objects of the + form { stdout, stderr } or { failed, stdout, stderr }. + + The caller should log stderr if it exists, but consider the call to + have succeeded unless failed is true. +*/ + function untar_error(err) { if (err.signal) { return { failed:true, stdout:err.stdout, stderr:`SIGNAL ${err.signal}\n${err.stderr}` } @@ -31,6 +38,7 @@ function untar_error(err) { if (err.code != 0) { return { failed:true, stdout:err.stdout, stderr:err.stderr } } + // For certain old tar files, stderr contains an error like "A lone zero block at..." But err.code is zero so we consider it success. return { stdout:err.stdout, stderr:err.stderr } } @@ -38,6 +46,8 @@ function unzip_error(err) { if (err.signal) { return { failed:true, stdout:err.stdout, stderr:`SIGNAL ${err.signal}\n${err.stderr}` } } + // Special case: unzip return status 1 for "unzip succeeded with warnings". (See man page.) We consider this to be a success. + // We see this with certain zip files and warnings like "128 extra bytes at beginning or within zipfile". if (err.code != 0 && err.code != 1) { return { failed:true, stdout:err.stdout, stderr:err.stderr } } From 0cd97c0215706ae0d6230c897a530843db522c29 Mon Sep 17 00:00:00 2001 From: Andrew Plotkin Date: Sun, 6 Mar 2022 13:02:01 -0500 Subject: [PATCH 7/8] Typo. --- app/src/cache.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/cache.js b/app/src/cache.js index 936b958..3e7d3fe 100644 --- a/app/src/cache.js +++ b/app/src/cache.js @@ -46,7 +46,7 @@ function unzip_error(err) { if (err.signal) { return { failed:true, stdout:err.stdout, stderr:`SIGNAL ${err.signal}\n${err.stderr}` } } - // Special case: unzip return status 1 for "unzip succeeded with warnings". (See man page.) We consider this to be a success. + // Special case: unzip returns status 1 for "unzip succeeded with warnings". (See man page.) We consider this to be a success. // We see this with certain zip files and warnings like "128 extra bytes at beginning or within zipfile". if (err.code != 0 && err.code != 1) { return { failed:true, stdout:err.stdout, stderr:err.stderr } From 725d2b460dad0132d8d1ecf7a3533b0c2fa6c3d3 Mon Sep 17 00:00:00 2001 From: Andrew Plotkin Date: Mon, 7 Mar 2022 00:40:34 -0500 Subject: [PATCH 8/8] Lint likes !==. --- app/src/cache.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/cache.js b/app/src/cache.js index 3e7d3fe..3e9119b 100644 --- a/app/src/cache.js +++ b/app/src/cache.js @@ -35,7 +35,7 @@ function untar_error(err) { if (err.signal) { return { failed:true, stdout:err.stdout, stderr:`SIGNAL ${err.signal}\n${err.stderr}` } } - if (err.code != 0) { + if (err.code !== 0) { return { failed:true, stdout:err.stdout, stderr:err.stderr } } // For certain old tar files, stderr contains an error like "A lone zero block at..." But err.code is zero so we consider it success. @@ -48,7 +48,7 @@ function unzip_error(err) { } // Special case: unzip returns status 1 for "unzip succeeded with warnings". (See man page.) We consider this to be a success. // We see this with certain zip files and warnings like "128 extra bytes at beginning or within zipfile". - if (err.code != 0 && err.code != 1) { + if (err.code !== 0 && err.code !== 1) { return { failed:true, stdout:err.stdout, stderr:err.stderr } } return { stdout:err.stdout, stderr:err.stderr }