From 83f39289aaac2f3beffbc4c1bcca2c4a52dfff24 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Wed, 17 Mar 2021 18:54:57 -0400
Subject: [PATCH] import/export: On export error return 500 instead of crashing

---
 src/node/handler/ExportHandler.js      |  9 ++++-----
 src/node/hooks/express/importexport.js |  2 +-
 src/tests/backend/specs/export.js      | 26 ++++++++++++++++++++++++++
 3 files changed, 31 insertions(+), 6 deletions(-)
 create mode 100644 src/tests/backend/specs/export.js

diff --git a/src/node/handler/ExportHandler.js b/src/node/handler/ExportHandler.js
index 38215f9c1..7dbcf7e2d 100644
--- a/src/node/handler/ExportHandler.js
+++ b/src/node/handler/ExportHandler.js
@@ -33,11 +33,6 @@ const util = require('util');
 const fsp_writeFile = util.promisify(fs.writeFile);
 const fsp_unlink = util.promisify(fs.unlink);
 
-const converter =
-    settings.soffice != null ? require('../utils/LibreOffice')
-    : settings.abiword != null ? require('../utils/Abiword')
-    : null;
-
 const tempDirectory = os.tmpdir();
 
 /**
@@ -99,6 +94,10 @@ exports.doExport = async (req, res, padId, readOnlyId, type) => {
     if (result.length > 0) {
       // console.log("export handled by plugin", destFile);
     } else {
+      const converter =
+          settings.soffice != null ? require('../utils/LibreOffice')
+          : settings.abiword != null ? require('../utils/Abiword')
+          : null;
       // @TODO no Promise interface for converters (yet)
       await util.promisify(converter.convertFile)(srcFile, destFile, type);
     }
diff --git a/src/node/hooks/express/importexport.js b/src/node/hooks/express/importexport.js
index d6f287c6b..ab8d60376 100644
--- a/src/node/hooks/express/importexport.js
+++ b/src/node/hooks/express/importexport.js
@@ -60,7 +60,7 @@ exports.expressCreateServer = (hookName, args, cb) => {
         }
 
         console.log(`Exporting pad "${req.params.pad}" in ${req.params.type} format`);
-        exportHandler.doExport(req, res, padId, readOnlyId, req.params.type);
+        await exportHandler.doExport(req, res, padId, readOnlyId, req.params.type);
       }
     })().catch((err) => next(err || new Error(err)));
   });
diff --git a/src/tests/backend/specs/export.js b/src/tests/backend/specs/export.js
new file mode 100644
index 000000000..d2fcde131
--- /dev/null
+++ b/src/tests/backend/specs/export.js
@@ -0,0 +1,26 @@
+'use strict';
+
+const common = require('../common');
+const padManager = require('../../../node/db/PadManager');
+const settings = require('../../../node/utils/Settings');
+
+describe(__filename, function () {
+  let agent;
+  const settingsBackup = {};
+
+  before(async function () {
+    agent = await common.init();
+    settingsBackup.soffice = settings.soffice;
+    await padManager.getPad('testExportPad', 'test content');
+  });
+
+  after(async function () {
+    Object.assign(settings, settingsBackup);
+  });
+
+  it('returns 500 on export error', async function () {
+    settings.soffice = 'false'; // '/bin/false' doesn't work on Windows
+    await agent.get('/p/testExportPad/export/doc')
+        .expect(500);
+  });
+});