Don't use exec() for file

app/src/cache.js

@@ -19,11 +19,98 @@ import fs from 'fs/promises'
 import path from 'path'
 import util from 'util'
 
-import {SUPPORTED_FORMATS, escape_shell_single_quoted} from './common.js'
+import {SUPPORTED_FORMATS} 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}` }
+    }
+    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 }
+}
+
+function unzip_error(err) {
+    if (err.signal) {
+        return { failed:true, stdout:err.stdout, stderr:`SIGNAL ${err.signal}\n${err.stderr}` }
+    }
+    // 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 }
+    }
+    return { stdout:err.stdout, stderr:err.stderr }
+}
+
+/* Callback-based function to spawn a process and pipe its output into
+   /usr/bin/file.
+
+   The callback is of the form callback(err, value). On success, 
+   value will be { stdout, stderr }. If something went wrong, the 
+   return object will be the first argument, and will look like
+   { failed:true, stdout, stderr }.
+
+   (This is slightly awkward, sorry. It's meant to be used with
+   util.promisify(); see below.)
+*/
+function spawn_pipe_file_cb(command, args, callback) {
+    const unproc = child_process.spawn(command, args)
+    const fileproc = child_process.spawn('file', ['-i', '-'])
+
+    // Accumulated output of the file command
+    let stdout = ''
+
+    // Accumulated error output
+    // (Both unzip and file send their stderr here, which means they could interleave weirdly, but in practice it will be one or the other.)
+    let stderr = ''
+
+    // status code of the unzip/untar process
+    let uncode = null
+
+    // Send unzip stdout to file stdin
+    unproc.stdout.on('data', data => { fileproc.stdin.write(data) })
+    // Add stderr to our accumulator.
+    unproc.stderr.on('data', data => { stderr += data })
+    unproc.on('close', code => {
+        // Record the status code of the unzip process
+        uncode = code
+        // Again, unzip code 1 is okay
+        if (command === 'unzip' && code === 1)
+            uncode = 0
+        fileproc.stdin.end()
+    })
+
+    // Ignore errors sending data to file stdin. (It likes to close its input, which results in an EPIPE error.)
+    fileproc.stdin.on('error', () => {})
+    // Add stdout and stderr to our accumulators.
+    fileproc.stdout.on('data', data => { stdout += data })
+    fileproc.stderr.on('data', data => { stderr += data })
+
+    fileproc.on('close', code => {
+        // All done; call the callback. Fill in the first argument for failure, second arguent for success.
+        if (uncode)
+            callback({ failed:true, stdout:stdout, stderr:stderr }, undefined)
+        else if (code)
+            callback({ failed:true, stdout:stdout, stderr:stderr }, undefined)
+        else
+            callback(undefined, { stdout:stdout, stderr:stderr })
+    })
+}
+
+// An async, promise-based version of the above.
+const spawn_pipe_file = util.promisify(spawn_pipe_file_cb)
+
 class CacheEntry {
     constructor (contents, date, size, type) {
         this.contents = contents
@@ -84,13 +171,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')
         }
@@ -166,21 +253,24 @@ 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('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()}`)
+            console.log(`${file_path}: ${command} error: ${results.stderr.toString()}`)
+        }
+        if (results.failed) {
+            throw new Error(`${file_path}: ${command} error: ${results.stderr.toString()}`)
         }
         return results.stdout
     }
@@ -201,7 +291,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
     }
@@ -213,22 +303,25 @@ export default class FileCache {
         switch (type) {
             case 'tar.gz':
             case 'tgz':
-                command = 'tar'
-                results = await exec(`tar -xOzf ${zip_path} '${escape_shell_single_quoted(file_path)}' | file -i -`)
+                command = 'tar|file'
+                results = await spawn_pipe_file('tar', ['-xOzf', zip_path, file_path]).catch(err => { return err })
                 break
             case 'tar.z':
-                command = 'tar'
-                results = await exec(`tar -xOZf ${zip_path} '${escape_shell_single_quoted(file_path)}' | file -i -`)
+                command = 'tar|file'
+                results = await spawn_pipe_file('tar', ['-xOZf', zip_path, file_path]).catch(err => { return err })
                 break
             case 'zip':
-                command = 'unzip'
-                results = await exec(`unzip -p ${zip_path} '${escape_shell_single_quoted(file_path)}' | file -i -`)
+                command = 'unzip|file'
+                results = await spawn_pipe_file('unzip', ['-p', zip_path, file_path]).catch(err => { return err })
                 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()}`)
+            console.log(`${file_path}: ${command} error: ${results.stderr.toString()}`)
+        }
+        if (results.failed) {
+            throw new Error(`${file_path}: ${command} error: ${results.stderr.toString()}`)
         }
         // Trim '/dev/stdin:'
         return results.stdout.trim().substring(12)
@@ -249,17 +342,20 @@ 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('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}`)
+            console.log(`${path}: ${command} error: ${results.stderr.toString()}`)
+        }
+        if (results.failed) {
+            throw new Error(`${path}: ${command} error: ${results.stderr}`)
         }
         return results.stdout.trim().split('\n').filter(line => !line.endsWith('/')).sort()
     }

app/src/common.js

@@ -57,7 +57,3 @@ export function escape_regexp(str) {
     return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') // $& means the whole matched string
 }
 
-// Escape for use inside of a single quoted shell argument
-export function escape_shell_single_quoted(str) {
-    return str.replace(/'/g, `'\\''`)
-}