From 939eaa2d4bfd34c9474740e74953f9c42a39217b Mon Sep 17 00:00:00 2001
From: James Allen <james@sharelatex.com>
Date: Fri, 2 Mar 2018 10:02:49 +0000
Subject: [PATCH] Don't allow a document to be loaded without a pathname

---
 .../app/coffee/DocumentManager.coffee         |  2 +-
 .../app/coffee/PersistenceManager.coffee      |  2 ++
 .../app/coffee/RedisManager.coffee            |  2 +-
 .../PersistenceManagerTests.coffee            | 29 ++++++++++++++-----
 4 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee
index 3d3f690b5c..dedb5f63aa 100644
--- a/services/document-updater/app/coffee/DocumentManager.coffee
+++ b/services/document-updater/app/coffee/DocumentManager.coffee
@@ -24,7 +24,7 @@ module.exports = DocumentManager =
 				logger.log {project_id, doc_id}, "doc not in redis so getting from persistence API"
 				PersistenceManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname) ->
 					return callback(error) if error?
-					logger.log {project_id, doc_id, lines, version}, "got doc from persistence API"
+					logger.log {project_id, doc_id, lines, version, pathname}, "got doc from persistence API"
 					RedisManager.putDocInMemory project_id, doc_id, lines, version, ranges, pathname, (error) ->
 						return callback(error) if error?
 						callback null, lines, version, ranges, pathname, null, false
diff --git a/services/document-updater/app/coffee/PersistenceManager.coffee b/services/document-updater/app/coffee/PersistenceManager.coffee
index 974dec5a2c..bd5ce5239c 100644
--- a/services/document-updater/app/coffee/PersistenceManager.coffee
+++ b/services/document-updater/app/coffee/PersistenceManager.coffee
@@ -42,6 +42,8 @@ module.exports = PersistenceManager =
 					return callback(new Error("web API response had no doc lines"))
 				if !body.version? or not body.version instanceof Number
 					return callback(new Error("web API response had no valid doc version"))
+				if !body.pathname?
+					return callback(new Error("web API response had no valid doc pathname"))
 				return callback null, body.lines, body.version, body.ranges, body.pathname
 			else if res.statusCode == 404
 				return callback(new Errors.NotFoundError("doc not not found: #{url}"))
diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee
index cd12b497a1..b14c00ae66 100644
--- a/services/document-updater/app/coffee/RedisManager.coffee
+++ b/services/document-updater/app/coffee/RedisManager.coffee
@@ -47,7 +47,7 @@ module.exports = RedisManager =
 			logger.error {err: error, doc_id: doc_id, docLines: docLines}, error.message
 			return callback(error)
 		docHash = RedisManager._computeHash(docLines)
-		logger.log project_id:project_id, doc_id:doc_id, version: version, hash:docHash, "putting doc in redis"
+		logger.log {project_id, doc_id, version, docHash, pathname}, "putting doc in redis"
 		RedisManager._serializeRanges ranges, (error, ranges) ->
 			if error?
 				logger.error {err: error, doc_id, project_id}, error.message
diff --git a/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee b/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee
index 925274ac2f..937dcf3a77 100644
--- a/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee
+++ b/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee
@@ -30,15 +30,17 @@ describe "PersistenceManager", ->
 				pass: @pass = "password"
 
 	describe "getDoc", ->
+		beforeEach ->
+			@webResponse = {
+				lines: @lines,
+				version: @version,
+				ranges: @ranges
+				pathname: @pathname,
+			}
 
 		describe "with a successful response from the web api", ->
 			beforeEach ->
-				@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify({
-					lines: @lines,
-					version: @version,
-					ranges: @ranges
-					pathname: @pathname,
-				}))
+				@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(@webResponse))
 				@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
 
 			it "should call the web api", ->
@@ -98,7 +100,8 @@ describe "PersistenceManager", ->
 
 		describe "when request returns an doc without lines", ->
 			beforeEach ->
-				@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(version: @version))
+				delete @webResponse.lines
+				@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(@webResponse))
 				@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
 
 			it "should return and error", ->
@@ -106,12 +109,22 @@ describe "PersistenceManager", ->
 
 		describe "when request returns an doc without a version", ->
 			beforeEach ->
-				@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(lines: @lines))
+				delete @webResponse.version
+				@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(@webResponse))
 				@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
 
 			it "should return and error", ->
 				@callback.calledWith(new Error("web API response had no valid doc version")).should.equal true
 
+		describe "when request returns an doc without a pathname", ->
+			beforeEach ->
+				delete @webResponse.pathname
+				@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(@webResponse))
+				@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
+
+			it "should return and error", ->
+				@callback.calledWith(new Error("web API response had no valid doc pathname")).should.equal true
+
 	describe "setDoc", ->
 		describe "with a successful response from the web api", ->
 			beforeEach ->