From 9f503f1e9f857d90a06d53122c2ba20cb6388375 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 21 Sep 2017 13:25:55 +0100 Subject: [PATCH 01/12] First pass at encoding changes & comments in ranges --- .../app/coffee/WebsocketController.coffee | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/services/real-time/app/coffee/WebsocketController.coffee b/services/real-time/app/coffee/WebsocketController.coffee index e0242d4208..e4057baf7c 100644 --- a/services/real-time/app/coffee/WebsocketController.coffee +++ b/services/real-time/app/coffee/WebsocketController.coffee @@ -101,6 +101,30 @@ module.exports = WebsocketController = logger.err {err, project_id, doc_id, fromVersion, line, client_id: client.id}, "error encoding line uri component" return callback(err) escapedLines.push line + + if ranges.comments + escapedComments = [] + for comment in ranges.comments + try + comment.op.c = unescape(encodeURIComponent(comment.op.c)) + catch err + logger.err {err, project_id, doc_id, fromVersion, comment, client_id: client.id}, "error encoding comment uri component" + return callback(err) + escapedComments.push comment + ranges.comments = escapedComments + + if ranges.changes + escapedChanges = [] + for change in ranges.changes + try + change.op.i = unescape(encodeURIComponent(change.op.i)) if change.op.i + change.op.d = unescape(encodeURIComponent(change.op.d)) if change.op.d + catch err + logger.err {err, project_id, doc_id, fromVersion, change, client_id: client.id}, "error encoding change uri component" + return callback(err) + escapedChanges.push change + ranges.changes = escapedChanges + AuthorizationManager.addAccessToDoc client, doc_id client.join(doc_id) callback null, escapedLines, version, ops, ranges From aa6e0d0d6977c5a25998b3258943eee1ea20540b Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 21 Sep 2017 14:23:16 +0100 Subject: [PATCH 02/12] Only encode ranges if option passed --- services/real-time/app/coffee/Router.coffee | 4 +- .../app/coffee/WebsocketController.coffee | 47 +++++++++---------- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/services/real-time/app/coffee/Router.coffee b/services/real-time/app/coffee/Router.coffee index 2cc655eafc..8821748a35 100644 --- a/services/real-time/app/coffee/Router.coffee +++ b/services/real-time/app/coffee/Router.coffee @@ -80,13 +80,13 @@ module.exports = Router = Router._handleError null, err, client, "leaveProject" - client.on "joinDoc", (doc_id, fromVersion, callback) -> + client.on "joinDoc", (doc_id, options, fromVersion, callback) -> # fromVersion is optional if typeof fromVersion == "function" callback = fromVersion fromVersion = -1 - WebsocketController.joinDoc client, doc_id, fromVersion, (err, args...) -> + WebsocketController.joinDoc client, doc_id, options, fromVersion, (err, args...) -> if err? Router._handleError callback, err, client, "joinDoc", {doc_id, fromVersion} else diff --git a/services/real-time/app/coffee/WebsocketController.coffee b/services/real-time/app/coffee/WebsocketController.coffee index e4057baf7c..dec4d3d142 100644 --- a/services/real-time/app/coffee/WebsocketController.coffee +++ b/services/real-time/app/coffee/WebsocketController.coffee @@ -80,7 +80,7 @@ module.exports = WebsocketController = callback() , WebsocketController.FLUSH_IF_EMPTY_DELAY - joinDoc: (client, doc_id, fromVersion = -1, callback = (error, doclines, version, ops, ranges) ->) -> + joinDoc: (client, doc_id, options, fromVersion = -1, callback = (error, doclines, version, ops, ranges) ->) -> metrics.inc "editor.join-doc" Utils.getClientAttributes client, ["project_id", "user_id"], (error, {project_id, user_id}) -> return callback(error) if error? @@ -101,29 +101,28 @@ module.exports = WebsocketController = logger.err {err, project_id, doc_id, fromVersion, line, client_id: client.id}, "error encoding line uri component" return callback(err) escapedLines.push line - - if ranges.comments - escapedComments = [] - for comment in ranges.comments - try - comment.op.c = unescape(encodeURIComponent(comment.op.c)) - catch err - logger.err {err, project_id, doc_id, fromVersion, comment, client_id: client.id}, "error encoding comment uri component" - return callback(err) - escapedComments.push comment - ranges.comments = escapedComments - - if ranges.changes - escapedChanges = [] - for change in ranges.changes - try - change.op.i = unescape(encodeURIComponent(change.op.i)) if change.op.i - change.op.d = unescape(encodeURIComponent(change.op.d)) if change.op.d - catch err - logger.err {err, project_id, doc_id, fromVersion, change, client_id: client.id}, "error encoding change uri component" - return callback(err) - escapedChanges.push change - ranges.changes = escapedChanges + if options.encodeRanges + if ranges.comments + escapedComments = [] + for comment in ranges.comments + try + comment.op.c = unescape(encodeURIComponent(comment.op.c)) + catch err + logger.err {err, project_id, doc_id, fromVersion, comment, client_id: client.id}, "error encoding comment uri component" + return callback(err) + escapedComments.push comment + ranges.comments = escapedComments + if ranges.changes + escapedChanges = [] + for change in ranges.changes + try + change.op.i = unescape(encodeURIComponent(change.op.i)) if change.op.i + change.op.d = unescape(encodeURIComponent(change.op.d)) if change.op.d + catch err + logger.err {err, project_id, doc_id, fromVersion, change, client_id: client.id}, "error encoding change uri component" + return callback(err) + escapedChanges.push change + ranges.changes = escapedChanges AuthorizationManager.addAccessToDoc client, doc_id client.join(doc_id) From b796879c9f6d4f262349a1c267c9453608d06e26 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 21 Sep 2017 14:58:49 +0100 Subject: [PATCH 03/12] Handle options not being passed --- services/real-time/app/coffee/Router.coffee | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/services/real-time/app/coffee/Router.coffee b/services/real-time/app/coffee/Router.coffee index 8821748a35..e6dda566e0 100644 --- a/services/real-time/app/coffee/Router.coffee +++ b/services/real-time/app/coffee/Router.coffee @@ -79,8 +79,22 @@ module.exports = Router = if err? Router._handleError null, err, client, "leaveProject" - + # Variadic. The possible options: + # doc_id, callback + # doc_id, fromVersion, callback + # doc_id, options, callback + # doc_id, options, fromVersion, callback client.on "joinDoc", (doc_id, options, fromVersion, callback) -> + # options is optional + if typeof options == "function" + options = {} + callback = options + fromVersion = -1 + else if typeof options == "number" + options = {} + fromVersion = options + callback = fromVersion + # fromVersion is optional if typeof fromVersion == "function" callback = fromVersion From 55c880e1dd12a2bd777e38da1116d4d30c9104d7 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 21 Sep 2017 15:07:15 +0100 Subject: [PATCH 04/12] DRY up a bit --- .../app/coffee/WebsocketController.coffee | 34 +++++++------------ 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/services/real-time/app/coffee/WebsocketController.coffee b/services/real-time/app/coffee/WebsocketController.coffee index dec4d3d142..70d325197f 100644 --- a/services/real-time/app/coffee/WebsocketController.coffee +++ b/services/real-time/app/coffee/WebsocketController.coffee @@ -91,38 +91,28 @@ module.exports = WebsocketController = return callback(error) if error? DocumentUpdaterManager.getDocument project_id, doc_id, fromVersion, (error, lines, version, ranges, ops) -> return callback(error) if error? + # Encode any binary bits of data so it can go via WebSockets # See http://ecmanaut.blogspot.co.uk/2006/07/encoding-decoding-utf8-in-javascript.html + encodeForWebsockets = (text) -> unescape(encodeURIComponent(text)) escapedLines = [] for line in lines try - line = unescape(encodeURIComponent(line)) + line = encodeForWebsockets(line) catch err logger.err {err, project_id, doc_id, fromVersion, line, client_id: client.id}, "error encoding line uri component" return callback(err) escapedLines.push line if options.encodeRanges - if ranges.comments - escapedComments = [] - for comment in ranges.comments - try - comment.op.c = unescape(encodeURIComponent(comment.op.c)) - catch err - logger.err {err, project_id, doc_id, fromVersion, comment, client_id: client.id}, "error encoding comment uri component" - return callback(err) - escapedComments.push comment - ranges.comments = escapedComments - if ranges.changes - escapedChanges = [] - for change in ranges.changes - try - change.op.i = unescape(encodeURIComponent(change.op.i)) if change.op.i - change.op.d = unescape(encodeURIComponent(change.op.d)) if change.op.d - catch err - logger.err {err, project_id, doc_id, fromVersion, change, client_id: client.id}, "error encoding change uri component" - return callback(err) - escapedChanges.push change - ranges.changes = escapedChanges + try + for comment in ranges?.comments or [] + comment.op.c = encodeForWebsockets(comment.op.c) + for change in ranges?.changes or [] + change.op.i = encodeForWebsockets(comment.op.i) if change.op.i + change.op.d = encodeForWebsockets(comment.op.d) if change.op.d + catch err + logger.err {err, project_id, doc_id, fromVersion, ranges, client_id: client.id}, "error encoding range uri component" + return callback(err) AuthorizationManager.addAccessToDoc client, doc_id client.join(doc_id) From 790b9ea8edb17c92a49ccdd15c7f818885c672a1 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 21 Sep 2017 15:19:19 +0100 Subject: [PATCH 05/12] Switch order of args --- services/real-time/app/coffee/Router.coffee | 25 +++++++++------------ 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/services/real-time/app/coffee/Router.coffee b/services/real-time/app/coffee/Router.coffee index e6dda566e0..a55147bfb2 100644 --- a/services/real-time/app/coffee/Router.coffee +++ b/services/real-time/app/coffee/Router.coffee @@ -79,26 +79,23 @@ module.exports = Router = if err? Router._handleError null, err, client, "leaveProject" - # Variadic. The possible options: + # Variadic. The possible arguments: # doc_id, callback # doc_id, fromVersion, callback # doc_id, options, callback - # doc_id, options, fromVersion, callback - client.on "joinDoc", (doc_id, options, fromVersion, callback) -> - # options is optional + # doc_id, fromVersion, options, callback + client.on "joinDoc", (doc_id, fromVersion, options, callback) -> + if typeof fromVersion == "function" + fromVersion = -1 + options = {} + callback = fromVersion + else if typeof fromVersion == "object" + fromVersion = -1 + options = fromVersion + callback = options if typeof options == "function" options = {} callback = options - fromVersion = -1 - else if typeof options == "number" - options = {} - fromVersion = options - callback = fromVersion - - # fromVersion is optional - if typeof fromVersion == "function" - callback = fromVersion - fromVersion = -1 WebsocketController.joinDoc client, doc_id, options, fromVersion, (err, args...) -> if err? From 3966e2f85bcfd3fcbf742d331af55eb3c2a9b939 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 21 Sep 2017 16:55:49 +0100 Subject: [PATCH 06/12] Make variadic options more explicit --- services/real-time/app/coffee/Router.coffee | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/services/real-time/app/coffee/Router.coffee b/services/real-time/app/coffee/Router.coffee index a55147bfb2..b3d6ffeb16 100644 --- a/services/real-time/app/coffee/Router.coffee +++ b/services/real-time/app/coffee/Router.coffee @@ -85,17 +85,22 @@ module.exports = Router = # doc_id, options, callback # doc_id, fromVersion, options, callback client.on "joinDoc", (doc_id, fromVersion, options, callback) -> - if typeof fromVersion == "function" - fromVersion = -1 - options = {} + if typeof fromVersion == "function" and !options callback = fromVersion - else if typeof fromVersion == "object" fromVersion = -1 - options = fromVersion - callback = options - if typeof options == "function" options = {} + else if typeof fromVersion == "number" and typeof options == "function" callback = options + options = {} + else if typeof fromVersion == "object" and typeof options == "function" + callback = options + options = fromVersion + fromVersion = -1 + else if typeof fromVersion == "number" and typeof options == "object" + # Called with 4 args, things are as expected + else + logger.error { arguments: arguments }, "unexpected arguments" + return {} # ???? WebsocketController.joinDoc client, doc_id, options, fromVersion, (err, args...) -> if err? From 90d05dc6ddeb93dbb44405880874de1e3bb6470d Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 21 Sep 2017 16:56:09 +0100 Subject: [PATCH 07/12] Make args order consistent --- services/real-time/app/coffee/Router.coffee | 2 +- services/real-time/app/coffee/WebsocketController.coffee | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/real-time/app/coffee/Router.coffee b/services/real-time/app/coffee/Router.coffee index b3d6ffeb16..0fb502bd4f 100644 --- a/services/real-time/app/coffee/Router.coffee +++ b/services/real-time/app/coffee/Router.coffee @@ -102,7 +102,7 @@ module.exports = Router = logger.error { arguments: arguments }, "unexpected arguments" return {} # ???? - WebsocketController.joinDoc client, doc_id, options, fromVersion, (err, args...) -> + WebsocketController.joinDoc client, doc_id, fromVersion, options, (err, args...) -> if err? Router._handleError callback, err, client, "joinDoc", {doc_id, fromVersion} else diff --git a/services/real-time/app/coffee/WebsocketController.coffee b/services/real-time/app/coffee/WebsocketController.coffee index 70d325197f..4b51da8d53 100644 --- a/services/real-time/app/coffee/WebsocketController.coffee +++ b/services/real-time/app/coffee/WebsocketController.coffee @@ -80,7 +80,7 @@ module.exports = WebsocketController = callback() , WebsocketController.FLUSH_IF_EMPTY_DELAY - joinDoc: (client, doc_id, options, fromVersion = -1, callback = (error, doclines, version, ops, ranges) ->) -> + joinDoc: (client, doc_id, fromVersion = -1, options, callback = (error, doclines, version, ops, ranges) ->) -> metrics.inc "editor.join-doc" Utils.getClientAttributes client, ["project_id", "user_id"], (error, {project_id, user_id}) -> return callback(error) if error? From a299d7335d847cec2a85a32fe07b43dded07a894 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 21 Sep 2017 16:56:18 +0100 Subject: [PATCH 08/12] Fix incorrect var --- services/real-time/app/coffee/WebsocketController.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/real-time/app/coffee/WebsocketController.coffee b/services/real-time/app/coffee/WebsocketController.coffee index 4b51da8d53..580e8ac20b 100644 --- a/services/real-time/app/coffee/WebsocketController.coffee +++ b/services/real-time/app/coffee/WebsocketController.coffee @@ -108,8 +108,8 @@ module.exports = WebsocketController = for comment in ranges?.comments or [] comment.op.c = encodeForWebsockets(comment.op.c) for change in ranges?.changes or [] - change.op.i = encodeForWebsockets(comment.op.i) if change.op.i - change.op.d = encodeForWebsockets(comment.op.d) if change.op.d + change.op.i = encodeForWebsockets(change.op.i) if change.op.i + change.op.d = encodeForWebsockets(change.op.d) if change.op.d catch err logger.err {err, project_id, doc_id, fromVersion, ranges, client_id: client.id}, "error encoding range uri component" return callback(err) From 5d8e201732eeaff7da2ffa6e8fec36804f3d2742 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 21 Sep 2017 16:58:03 +0100 Subject: [PATCH 09/12] Don't return obj --- services/real-time/app/coffee/Router.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/real-time/app/coffee/Router.coffee b/services/real-time/app/coffee/Router.coffee index 0fb502bd4f..21d9e34b7d 100644 --- a/services/real-time/app/coffee/Router.coffee +++ b/services/real-time/app/coffee/Router.coffee @@ -100,7 +100,7 @@ module.exports = Router = # Called with 4 args, things are as expected else logger.error { arguments: arguments }, "unexpected arguments" - return {} # ???? + return # ???? WebsocketController.joinDoc client, doc_id, fromVersion, options, (err, args...) -> if err? From 937bf82a2fee9fa166f45df5f210cc0866e75f58 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Fri, 22 Sep 2017 09:25:24 +0100 Subject: [PATCH 10/12] Return callback with guard --- services/real-time/app/coffee/Router.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/real-time/app/coffee/Router.coffee b/services/real-time/app/coffee/Router.coffee index 21d9e34b7d..17107e443a 100644 --- a/services/real-time/app/coffee/Router.coffee +++ b/services/real-time/app/coffee/Router.coffee @@ -100,7 +100,7 @@ module.exports = Router = # Called with 4 args, things are as expected else logger.error { arguments: arguments }, "unexpected arguments" - return # ???? + return callback?(new Error("unexpected arguments")) WebsocketController.joinDoc client, doc_id, fromVersion, options, (err, args...) -> if err? From c67150ea10334f5ab57445a6f1b31b852d5750cf Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Fri, 22 Sep 2017 09:33:29 +0100 Subject: [PATCH 11/12] Ensure falsy value doesn't fail conditional --- services/real-time/app/coffee/WebsocketController.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/real-time/app/coffee/WebsocketController.coffee b/services/real-time/app/coffee/WebsocketController.coffee index 580e8ac20b..63c3ff903a 100644 --- a/services/real-time/app/coffee/WebsocketController.coffee +++ b/services/real-time/app/coffee/WebsocketController.coffee @@ -108,8 +108,8 @@ module.exports = WebsocketController = for comment in ranges?.comments or [] comment.op.c = encodeForWebsockets(comment.op.c) for change in ranges?.changes or [] - change.op.i = encodeForWebsockets(change.op.i) if change.op.i - change.op.d = encodeForWebsockets(change.op.d) if change.op.d + change.op.i = encodeForWebsockets(change.op.i) if change.op.i? + change.op.d = encodeForWebsockets(change.op.d) if change.op.d? catch err logger.err {err, project_id, doc_id, fromVersion, ranges, client_id: client.id}, "error encoding range uri component" return callback(err) From a0505afb23a63b39bf81ed7b4fa27ccaef3996d1 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Fri, 22 Sep 2017 09:34:10 +0100 Subject: [PATCH 12/12] Be defensive on comment text --- services/real-time/app/coffee/WebsocketController.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/real-time/app/coffee/WebsocketController.coffee b/services/real-time/app/coffee/WebsocketController.coffee index 63c3ff903a..53ccecd51b 100644 --- a/services/real-time/app/coffee/WebsocketController.coffee +++ b/services/real-time/app/coffee/WebsocketController.coffee @@ -106,7 +106,7 @@ module.exports = WebsocketController = if options.encodeRanges try for comment in ranges?.comments or [] - comment.op.c = encodeForWebsockets(comment.op.c) + comment.op.c = encodeForWebsockets(comment.op.c) if comment.op.c? for change in ranges?.changes or [] change.op.i = encodeForWebsockets(change.op.i) if change.op.i? change.op.d = encodeForWebsockets(change.op.d) if change.op.d?