From bea41977323049fefbe5a502f720ff5416488aa3 Mon Sep 17 00:00:00 2001
From: Shane Kilkelly <shane@kilkelly.me>
Date: Mon, 15 Aug 2016 08:54:45 +0100
Subject: [PATCH] return an error if `expires` is not a valid date.

---
 .../app/coffee/Notifications.coffee           |  7 +++-
 .../unit/coffee/NotificationsTests.coffee     | 32 ++++++++++++++++++-
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/services/notifications/app/coffee/Notifications.coffee b/services/notifications/app/coffee/Notifications.coffee
index 2a7e043f78..e8edae5aab 100644
--- a/services/notifications/app/coffee/Notifications.coffee
+++ b/services/notifications/app/coffee/Notifications.coffee
@@ -32,7 +32,12 @@ module.exports =
 				# in Mongo, TTL indexes only work on date fields, and ignore the document when that field is missing
 				# see `README.md` for instruction on creating TTL index
 				if notification.expires?
-					doc.expires =  new Date(notification.expires)
+					try
+						doc.expires =  new Date(notification.expires)
+						_testValue = doc.expires.toISOString()
+					catch err
+						logger.error {user_id, expires: notification.expires}, "error converting `expires` field to Date"
+						return callback(err)
 				db.notifications.insert(doc, callback)
 
 	removeNotificationId: (user_id, notification_id, callback)->
diff --git a/services/notifications/test/unit/coffee/NotificationsTests.coffee b/services/notifications/test/unit/coffee/NotificationsTests.coffee
index 144b8407be..f070889de4 100644
--- a/services/notifications/test/unit/coffee/NotificationsTests.coffee
+++ b/services/notifications/test/unit/coffee/NotificationsTests.coffee
@@ -28,7 +28,10 @@ describe 'Notifications Tests', ->
 		@mongojs.ObjectId = ObjectId
 
 		@notifications = SandboxedModule.require modulePath, requires:
-			'logger-sharelatex': log:->
+			'logger-sharelatex': {
+				log:()->
+				error:()->
+			}
 			'settings-sharelatex': {}
 			'mongojs':@mongojs
 
@@ -100,6 +103,33 @@ describe 'Notifications Tests', ->
 					@insertStub.calledWith(@expectedDocument).should.equal true
 					done()
 
+		describe 'when the notification has a nonsensical expires field', () ->
+			beforeEach ->
+				@stubbedNotification = {
+					user_id: ObjectId(user_id),
+					key:"notification-key",
+					messageOpts:"some info",
+					templateKey:"template-key",
+					expires: 'WAT'
+				}
+				@expectedDocument = {
+					user_id: @stubbedNotification.user_id,
+					key:"notification-key",
+					messageOpts:"some info",
+					templateKey:"template-key",
+					expires: new Date(@stubbedNotification.expires),
+				}
+
+			it 'should produce an error', (done)->
+				@insertStub.callsArgWith(1, null)
+				@countStub.callsArgWith(1, null, 0)
+
+				@notifications.addNotification user_id, @stubbedNotification, (err)=>
+					(err instanceof Error).should.equal true
+					@insertStub.callCount.should.equal 0
+					@insertStub.calledWith(@expectedDocument).should.equal false
+					done()
+
 	describe 'removeNotificationId', ->
 		it 'should mark the notification id as read', (done)->
 			@updateStub.callsArgWith(2, null)