fix(GUI): don't split robot input by new lines inside properties (#1008)

We've recently PRed a commit that handles multiple IPC messages being
triggered at the same time, confusing `JSON.parse()`. As a solution to
such problem, we are splitting the CLI output on new lines, based on the
assumption that each line represents a different object, however we
didn't consider that in the case of errors, we include an `stacktrace`
property which usually includes new line characters, causing such
information to be completely garbled and cause `JSON.parse()` once again
to get confused.

As a solution, we only split by new lines that are not surrounded by
quotes (since they represent a JSON property).

See: https://github.com/resin-io/etcher/pull/997
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

lib/shared/child-writer/utils.js

@@ -16,6 +16,8 @@
 
 'use strict';
 
+const _ = require('lodash');
+
 /**
  * @summary Get the explicit boolean form of an argument
  * @function
@@ -85,3 +87,29 @@ exports.getCLIWriterArguments = (options) => {
 
   return argv;
 };
+
+/**
+ * @summary Split stringified object lines
+ * @function
+ * @public
+ *
+ * @description
+ * This function takes special care to not consider new lines
+ * inside the object properties.
+ *
+ * @param {String} lines - lines
+ * @returns {String[]} split lines
+ *
+ * @example
+ * const result = utils.splitObjectLines('{"foo":"bar"}\n{"hello":"Hello\nWorld"}');
+ * console.log(result);
+ *
+ * > [ '{"foo":"bar"}', '{"hello":"Hello\nWorld"}' ]
+ */
+exports.splitObjectLines = (lines) => {
+  return _.chain(lines)
+    .split(/((?:[^\n"']|"[^"]*"|'[^']*')+)/)
+    .map(_.trim)
+    .reject(_.isEmpty)
+    .value();
+};

lib/shared/child-writer/writer-proxy.js

@@ -24,6 +24,7 @@ const _ = require('lodash');
 const os = require('os');
 const path = require('path');
 const sudoPrompt = Bluebird.promisifyAll(require('sudo-prompt'));
+const utils = require('./utils');
 const EXIT_CODES = require('../exit-codes');
 const packageJSON = require('../../../package.json');
 
@@ -162,14 +163,8 @@ return isElevated().then((elevated) => {
           // causing several progress lines to come up at once as single message.
           // Trying to parse multiple JSON objects separated by new lines will
           // of course make the parser confused, causing errors later on.
-          //
-          // As a solution, we split the data coming in from the CLI into
-          // separate lines, and only emit a "message" event for the last one.
-          //
-          // Since each line is terminated by a new line, the last string
-          // is an empty string, which we don't want to send to the IPC
-          // server, so we take the last element of every element but the last.
-          const object = _.last(_.initial(_.split(data.toString(), /\r?\n/)));
+          // As a solution, we only consider the last message.
+          const object = _.last(utils.splitObjectLines(data.toString()));
           ipc.of[process.env.IPC_SERVER_ID].emit('message', object);
 
         };

tests/shared/child-writer/utils.spec.js

@@ -0,0 +1,81 @@
+/*
+ * Copyright 2016 resin.io
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+'use strict';
+
+const m = require('mochainon');
+const utils = require('../../../lib/shared/child-writer/utils');
+
+describe('Shared: ChildWriter Utils', function() {
+
+  describe('.splitObjectLines()', function() {
+
+    it('should split multiple object lines', function() {
+      const input = '{"id":"foo"}\n{"id":"bar"}\n{"id":"baz"}';
+      m.chai.expect(utils.splitObjectLines(input)).to.deep.equal([
+        '{"id":"foo"}',
+        '{"id":"bar"}',
+        '{"id":"baz"}'
+      ]);
+    });
+
+    it('should ignore spaces in between', function() {
+      const input = '{"id":"foo"}   \n     {"id":"bar"}\n   {"id":"baz"}';
+      m.chai.expect(utils.splitObjectLines(input)).to.deep.equal([
+        '{"id":"foo"}',
+        '{"id":"bar"}',
+        '{"id":"baz"}'
+      ]);
+    });
+
+    it('should ignore multiple new lines', function() {
+      const input = '{"id":"foo"}\n\n\n\n{"id":"bar"}\n\n{"id":"baz"}';
+      m.chai.expect(utils.splitObjectLines(input)).to.deep.equal([
+        '{"id":"foo"}',
+        '{"id":"bar"}',
+        '{"id":"baz"}'
+      ]);
+    });
+
+    it('should ignore new lines inside properties', function() {
+      const input = '{"id":"foo\nbar"}\n{"id":"\nhello\n"}';
+      m.chai.expect(utils.splitObjectLines(input)).to.deep.equal([
+        '{"id":"foo\nbar"}',
+        '{"id":"\nhello\n"}'
+      ]);
+    });
+
+    it('should handle carriage returns', function() {
+      const input = '{"id":"foo"}\r\n{"id":"bar"}\r\n{"id":"baz"}';
+      m.chai.expect(utils.splitObjectLines(input)).to.deep.equal([
+        '{"id":"foo"}',
+        '{"id":"bar"}',
+        '{"id":"baz"}'
+      ]);
+    });
+
+    it('should ignore multiple carriage returns', function() {
+      const input = '{"id":"foo"}\r\n\r\n{"id":"bar"}\r\n\r\n\r\n{"id":"baz"}';
+      m.chai.expect(utils.splitObjectLines(input)).to.deep.equal([
+        '{"id":"foo"}',
+        '{"id":"bar"}',
+        '{"id":"baz"}'
+      ]);
+    });
+
+  });
+
+});