From 8dc0deb2a4466803e590632e3370397ab7301605 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Wed, 21 Oct 2020 12:02:06 +0100 Subject: [PATCH] Desktop, Cli: Fixes #3958: Fixed freeze when importing ENEX as HTML, and fixed potential error when importing resources --- ReactNativeClient/lib/import-enex-html-gen.js | 26 ++++++++++++------- ReactNativeClient/lib/import-enex.js | 16 +++++++++++- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/ReactNativeClient/lib/import-enex-html-gen.js b/ReactNativeClient/lib/import-enex-html-gen.js index 0dcac82127..2d543a4317 100644 --- a/ReactNativeClient/lib/import-enex-html-gen.js +++ b/ReactNativeClient/lib/import-enex-html-gen.js @@ -1,5 +1,5 @@ const stringToStream = require('string-to-stream'); -const cleanHtml = require('clean-html'); +// const cleanHtml = require('clean-html'); const resourceUtils = require('lib/resourceUtils.js'); const { isSelfClosingTag } = require('lib/htmlUtils'); const Entities = require('html-entities').AllHtmlEntities; @@ -161,14 +161,22 @@ async function enexXmlToHtml(xmlString, resources, options = {}) { } const beautifyHtml = (html) => { - return new Promise((resolve) => { - try { - cleanHtml.clean(html, { wrap: 0 }, (...cleanedHtml) => resolve(cleanedHtml)); - } catch (error) { - console.warn(`Could not clean HTML - the "unclean" version will be used: ${error.message}: ${html.trim().substr(0, 512).replace(/[\n\r]/g, ' ')}...`); - resolve([html]); - } - }); + // The clean-html package doesn't appear to be robust enough to deal with the crazy HTML that Evernote can generate. + // In the best case scenario it will throw an error but in some cases it will go into an infinite loop, so + // for that reason we need to disable it. + // + // Fixed https://github.com/laurent22/joplin/issues/3958 + + return [html]; + + // return new Promise((resolve) => { + // try { + // cleanHtml.clean(html, { wrap: 0 }, (...cleanedHtml) => resolve(cleanedHtml)); + // } catch (error) { + // console.warn(`Could not clean HTML - the "unclean" version will be used: ${error.message}: ${html.trim().substr(0, 512).replace(/[\n\r]/g, ' ')}...`); + // resolve([html]); + // } + // }); }; module.exports = { enexXmlToHtml }; diff --git a/ReactNativeClient/lib/import-enex.js b/ReactNativeClient/lib/import-enex.js index afbdc22483..cad47f95a7 100644 --- a/ReactNativeClient/lib/import-enex.js +++ b/ReactNativeClient/lib/import-enex.js @@ -40,6 +40,16 @@ function extractRecognitionObjId(recognitionXml) { } async function decodeBase64File(sourceFilePath, destFilePath) { + // When something goes wrong with streams you can get an error "EBADF, Bad file descriptor" + // with no strack trace to tell where the error happened. + + // Also note that this code is not great because there's a source and a destination stream + // and while one stream might end, the other might throw an error or vice-versa. However + // we can only throw one error from a promise. So before one stream + // could end with resolve(), then another stream would get an error and call reject(), which + // would be ignored. I don't think it's happening anymore, but something to keep in mind + // anyway. + return new Promise(function(resolve, reject) { // Note: we manually handle closing the file so that we can // force flusing it before close. This is needed because @@ -55,13 +65,17 @@ async function decodeBase64File(sourceFilePath, destFilePath) { }); sourceStream.pipe(new Base64Decode()).pipe(destStream); - sourceStream.on('end', () => { + // We wait for the destination stream "finish" event, not the source stream "end" event + // because even if the source has finished sending data, the destination might not have + // finished receiving it and writing it to disk. + destStream.on('finish', () => { fs.fdatasyncSync(destFile); fs.closeSync(destFile); resolve(); }); sourceStream.on('error', (error) => reject(error)); + destStream.on('error', (error) => reject(error)); }); }