Jelajahi Sumber

Merge pull request #6317 from overleaf/jpa-send-explicit-content-type

[web] send explicit content type in responses

GitOrigin-RevId: d5aeaba57a7d2fc053fbf5adc2299fb46e435341
Jakob Ackermann 4 tahun lalu
induk
melakukan
d720d6affa
43 mengubah file dengan 390 tambahan dan 224 penghapusan
  1. 4 0
      services/web/Makefile
  2. 1 1
      services/web/app/src/Features/Captcha/CaptchaMiddleware.js
  3. 1 1
      services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js
  4. 8 12
      services/web/app/src/Features/Compile/CompileController.js
  5. 1 1
      services/web/app/src/Features/Contacts/ContactController.js
  6. 1 1
      services/web/app/src/Features/Contacts/ContactRouter.js
  7. 2 2
      services/web/app/src/Features/Documents/DocumentController.js
  8. 6 8
      services/web/app/src/Features/Downloads/ProjectDownloadsController.js
  9. 3 2
      services/web/app/src/Features/Errors/ErrorController.js
  10. 8 7
      services/web/app/src/Features/Errors/HttpErrorHandler.js
  11. 3 2
      services/web/app/src/Features/FileStore/FileStoreController.js
  12. 1 6
      services/web/app/src/Features/HealthCheck/HealthCheckController.js
  13. 2 4
      services/web/app/src/Features/History/HistoryController.js
  14. 1 1
      services/web/app/src/Features/InactiveData/InactiveProjectController.js
  15. 49 33
      services/web/app/src/Features/LinkedFiles/LinkedFilesController.js
  16. 1 1
      services/web/app/src/Features/Notifications/NotificationsController.js
  17. 3 3
      services/web/app/src/Features/Project/ProjectController.js
  18. 2 2
      services/web/app/src/Features/Spelling/SpellingController.js
  19. 6 6
      services/web/app/src/Features/Uploads/ProjectUploadController.js
  20. 2 3
      services/web/app/src/Features/UserMembership/UserMembershipController.js
  21. 48 0
      services/web/app/src/infrastructure/Response.js
  22. 14 9
      services/web/app/src/router.js
  23. 47 0
      services/web/bin/lint_flag_res_send_usage
  24. 1 0
      services/web/package.json
  25. 5 2
      services/web/test/acceptance/src/LinkedFilesTests.js
  26. 2 2
      services/web/test/acceptance/src/mocks/MockChatApi.js
  27. 8 5
      services/web/test/acceptance/src/mocks/MockClsiApi.js
  28. 4 1
      services/web/test/acceptance/src/mocks/MockFilestoreApi.js
  29. 4 1
      services/web/test/acceptance/src/mocks/MockProjectHistoryApi.js
  30. 30 14
      services/web/test/acceptance/src/mocks/MockRecurlyApi.js
  31. 13 8
      services/web/test/acceptance/src/mocks/MockV1HistoryApi.js
  32. 1 1
      services/web/test/unit/src/Authentication/AuthenticationControllerTests.js
  33. 2 2
      services/web/test/unit/src/Compile/CompileControllerTests.js
  34. 3 4
      services/web/test/unit/src/Contact/ContactControllerTests.js
  35. 11 12
      services/web/test/unit/src/Downloads/ProjectDownloadsControllerTests.js
  36. 13 19
      services/web/test/unit/src/FileStore/FileStoreControllerTests.js
  37. 1 1
      services/web/test/unit/src/Notifications/NotificationsControllerTests.js
  38. 3 3
      services/web/test/unit/src/Project/ProjectControllerTests.js
  39. 0 1
      services/web/test/unit/src/References/ReferencesControllerTests.js
  40. 4 11
      services/web/test/unit/src/Spelling/SpellingControllerTests.js
  41. 30 20
      services/web/test/unit/src/Uploads/ProjectUploadControllerTests.js
  42. 2 5
      services/web/test/unit/src/UserMembership/UserMembershipControllerTests.js
  43. 39 7
      services/web/test/unit/src/helpers/MockResponse.js

+ 4 - 0
services/web/Makefile

@@ -423,6 +423,10 @@ lint: lint_locales
 lint_locales:
 	bin/lint_locales
 
+lint: lint_flag_res_send_usage
+lint_flag_res_send_usage:
+	bin/lint_flag_res_send_usage
+
 lint_in_docker:
 	$(RUN_LINT_FORMAT) make lint -j --output-sync
 

+ 1 - 1
services/web/app/src/Features/Captcha/CaptchaMiddleware.js

@@ -47,7 +47,7 @@ module.exports = CaptchaMiddleware = {
               { statusCode: response.statusCode, body },
               'failed recaptcha siteverify request'
             )
-            return res.status(400).send({
+            return res.status(400).json({
               errorReason: 'cannot_verify_user_not_robot',
               message: {
                 text: 'Sorry, we could not verify that you are not a robot. Please check that Google reCAPTCHA is not being blocked by an ad blocker or firewall.',

+ 1 - 1
services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js

@@ -131,7 +131,7 @@ module.exports = CollaboratorsInviteController = {
             { projectId, email, sendingUserId },
             'invalid email address'
           )
-          return res.status(400).send({ errorReason: 'invalid_email' })
+          return res.status(400).json({ errorReason: 'invalid_email' })
         }
         return CollaboratorsInviteController._checkRateLimit(
           sendingUserId,

+ 8 - 12
services/web/app/src/Features/Compile/CompileController.js

@@ -117,7 +117,7 @@ module.exports = CompileController = {
       if (error != null) {
         return next(error)
       }
-      return res.status(200).send()
+      return res.sendStatus(200)
     })
   },
 
@@ -159,15 +159,12 @@ module.exports = CompileController = {
         if (error != null) {
           return next(error)
         }
-        res.contentType('application/json')
-        return res.status(200).send(
-          JSON.stringify({
-            status,
-            outputFiles,
-            clsiServerId,
-            validationProblems,
-          })
-        )
+        return res.json({
+          status,
+          outputFiles,
+          clsiServerId,
+          validationProblems,
+        })
       }
     )
   },
@@ -564,8 +561,7 @@ module.exports = CompileController = {
           if (error != null) {
             return next(error)
           }
-          res.contentType('application/json')
-          return res.send(body)
+          return res.json(body)
         }
       )
     })

+ 1 - 1
services/web/app/src/Features/Contacts/ContactController.js

@@ -70,7 +70,7 @@ module.exports = ContactsController = {
                 contacts = contacts.concat(
                   ...Array.from(additional_contacts || [])
                 )
-                return res.send({
+                return res.json({
                   contacts,
                 })
               }

+ 1 - 1
services/web/app/src/Features/Contacts/ContactRouter.js

@@ -11,7 +11,7 @@ function contactsAuthenticationMiddleware() {
       if (SessionManager.isUserLoggedIn(req.session)) {
         next()
       } else {
-        res.send({ contacts: [] })
+        res.json({ contacts: [] })
       }
     }
   }

+ 2 - 2
services/web/app/src/Features/Documents/DocumentController.js

@@ -5,6 +5,7 @@ const ProjectEntityHandler = require('../Project/ProjectEntityHandler')
 const ProjectEntityUpdateHandler = require('../Project/ProjectEntityUpdateHandler')
 const logger = require('@overleaf/logger')
 const _ = require('lodash')
+const { plainTextResponse } = require('../../infrastructure/Response')
 
 function getDocument(req, res, next) {
   const { Project_id: projectId, doc_id: docId } = req.params
@@ -47,8 +48,7 @@ function getDocument(req, res, next) {
                 return next(error)
               }
               if (plain) {
-                res.type('text/plain')
-                res.send(lines.join('\n'))
+                plainTextResponse(res, lines.join('\n'))
               } else {
                 const projectHistoryId = _.get(project, 'overleaf.history.id')
                 const projectHistoryDisplay = _.get(

+ 6 - 8
services/web/app/src/Features/Downloads/ProjectDownloadsController.js

@@ -17,6 +17,7 @@ const Metrics = require('@overleaf/metrics')
 const ProjectGetter = require('../Project/ProjectGetter')
 const ProjectZipStreamManager = require('./ProjectZipStreamManager')
 const DocumentUpdaterHandler = require('../DocumentUpdater/DocumentUpdaterHandler')
+const { prepareZipAttachment } = require('../../infrastructure/Response')
 
 module.exports = ProjectDownloadsController = {
   downloadProject(req, res, next) {
@@ -41,10 +42,7 @@ module.exports = ProjectDownloadsController = {
                 if (error != null) {
                   return next(error)
                 }
-                res.setContentDisposition('attachment', {
-                  filename: `${project.name}.zip`,
-                })
-                res.contentType('application/zip')
+                prepareZipAttachment(res, `${project.name}.zip`)
                 return stream.pipe(res)
               }
             )
@@ -69,10 +67,10 @@ module.exports = ProjectDownloadsController = {
             if (error != null) {
               return next(error)
             }
-            res.setContentDisposition('attachment', {
-              filename: `Overleaf Projects (${project_ids.length} items).zip`,
-            })
-            res.contentType('application/zip')
+            prepareZipAttachment(
+              res,
+              `Overleaf Projects (${project_ids.length} items).zip`
+            )
             return stream.pipe(res)
           }
         )

+ 3 - 2
services/web/app/src/Features/Errors/ErrorController.js

@@ -4,6 +4,7 @@ const logger = require('@overleaf/logger')
 const SessionManager = require('../Authentication/SessionManager')
 const SamlLogHandler = require('../SamlLog/SamlLogHandler')
 const HttpErrorHandler = require('./HttpErrorHandler')
+const { plainTextResponse } = require('../../infrastructure/Response')
 
 module.exports = ErrorController = {
   notFound(req, res) {
@@ -62,11 +63,11 @@ module.exports = ErrorController = {
     } else if (error instanceof Errors.InvalidError) {
       logger.warn({ err: error, url: req.url }, 'invalid error')
       res.status(400)
-      res.send(error.message)
+      plainTextResponse(res, error.message)
     } else if (error instanceof Errors.InvalidNameError) {
       logger.warn({ err: error, url: req.url }, 'invalid name error')
       res.status(400)
-      res.send(error.message)
+      plainTextResponse(res, error.message)
     } else if (error instanceof Errors.SAMLSessionDataMissing) {
       logger.warn(
         { err: error, url: req.url },

+ 8 - 7
services/web/app/src/Features/Errors/HttpErrorHandler.js

@@ -1,5 +1,6 @@
 const logger = require('@overleaf/logger')
 const Settings = require('@overleaf/settings')
+const { plainTextResponse } = require('../../infrastructure/Response')
 
 function renderJSONError(res, message, info = {}) {
   if (info.message) {
@@ -23,7 +24,7 @@ function handleGeneric500Error(req, res, statusCode, message) {
     case 'json':
       return renderJSONError(res, message)
     default:
-      return res.send('internal server error')
+      return plainTextResponse(res, 'internal server error')
   }
 }
 
@@ -38,7 +39,7 @@ function handleGeneric400Error(req, res, statusCode, message, info = {}) {
     case 'json':
       return renderJSONError(res, message, info)
     default:
-      return res.send('client error')
+      return plainTextResponse(res, 'client error')
   }
 }
 
@@ -90,7 +91,7 @@ module.exports = HttpErrorHandler = {
       case 'json':
         return renderJSONError(res, message, info)
       default:
-        return res.send('conflict')
+        return plainTextResponse(res, 'conflict')
     }
   },
 
@@ -102,7 +103,7 @@ module.exports = HttpErrorHandler = {
       case 'json':
         return renderJSONError(res, message, info)
       default:
-        return res.send('restricted')
+        return plainTextResponse(res, 'restricted')
     }
   },
 
@@ -114,7 +115,7 @@ module.exports = HttpErrorHandler = {
       case 'json':
         return renderJSONError(res, message, info)
       default:
-        return res.send('not found')
+        return plainTextResponse(res, 'not found')
     }
   },
 
@@ -129,7 +130,7 @@ module.exports = HttpErrorHandler = {
       case 'json':
         return renderJSONError(res, message, info)
       default:
-        return res.send('unprocessable entity')
+        return plainTextResponse(res, 'unprocessable entity')
     }
   },
 
@@ -155,7 +156,7 @@ module.exports = HttpErrorHandler = {
       case 'json':
         return renderJSONError(res, message, {})
       default:
-        return res.send(message)
+        return plainTextResponse(res, message)
     }
   },
 }

+ 3 - 2
services/web/app/src/Features/FileStore/FileStoreController.js

@@ -3,6 +3,7 @@ const logger = require('@overleaf/logger')
 const FileStoreHandler = require('./FileStoreHandler')
 const ProjectLocator = require('../Project/ProjectLocator')
 const Errors = require('../Errors/Errors')
+const { preparePlainTextResponse } = require('../../infrastructure/Response')
 
 module.exports = {
   getFile(req, res) {
@@ -34,7 +35,7 @@ module.exports = {
             }
             // mobile safari will try to render html files, prevent this
             if (isMobileSafari(userAgent) && isHtml(file)) {
-              res.setHeader('Content-Type', 'text/plain')
+              preparePlainTextResponse(res)
             }
             res.setContentDisposition('attachment', { filename: file.name })
             stream.pipe(res)
@@ -57,7 +58,7 @@ module.exports = {
         }
         return
       }
-      res.set('Content-Length', fileSize)
+      res.setHeader('Content-Length', fileSize)
       res.status(200).end()
     })
   },

+ 1 - 6
services/web/app/src/Features/HealthCheck/HealthCheckController.js

@@ -12,7 +12,6 @@ module.exports = {
   check(req, res, next) {
     if (!settings.siteIsOpen || !settings.editorIsOpen) {
       // always return successful health checks when site is closed
-      res.contentType('application/json')
       res.sendStatus(200)
     } else {
       // detach from express for cleaner stack traces
@@ -92,9 +91,6 @@ module.exports = {
   },
 }
 
-function prettyJSON(blob) {
-  return JSON.stringify(blob, null, 2) + '\n'
-}
 async function runSmokeTestsDetached(req, res) {
   function isAborted() {
     return req.aborted
@@ -120,6 +116,5 @@ async function runSmokeTestsDetached(req, res) {
     response = { stats, error: err.message }
   }
   if (isAborted()) return
-  res.contentType('application/json')
-  res.status(status).send(prettyJSON(response))
+  res.status(status).json(response)
 }

+ 2 - 4
services/web/app/src/Features/History/HistoryController.js

@@ -12,6 +12,7 @@ const ProjectDetailsHandler = require('../Project/ProjectDetailsHandler')
 const ProjectEntityUpdateHandler = require('../Project/ProjectEntityUpdateHandler')
 const RestoreManager = require('./RestoreManager')
 const { pipeline } = require('stream')
+const { prepareZipAttachment } = require('../../infrastructure/Response')
 
 module.exports = HistoryController = {
   selectHistoryApi(req, res, next) {
@@ -414,10 +415,7 @@ module.exports = HistoryController = {
               delete response.headers['content-disposition']
               delete response.headers['content-type']
               res.status(response.statusCode)
-              res.setContentDisposition('attachment', {
-                filename: `${name}.zip`,
-              })
-              res.contentType('application/zip')
+              prepareZipAttachment(res, `${name}.zip`)
               pipeline(response, res, err => {
                 if (err) {
                   logger.warn(

+ 1 - 1
services/web/app/src/Features/InactiveData/InactiveProjectController.js

@@ -26,7 +26,7 @@ module.exports = {
         if (err != null) {
           return res.sendStatus(500)
         } else {
-          return res.send(projectsDeactivated)
+          return res.json(projectsDeactivated)
         }
       }
     )

+ 49 - 33
services/web/app/src/Features/LinkedFiles/LinkedFilesController.js

@@ -37,6 +37,7 @@ const {
   FileCannotRefreshError,
 } = require('./LinkedFilesErrors')
 const Modules = require('../../infrastructure/Modules')
+const { plainTextResponse } = require('../../infrastructure/Response')
 
 module.exports = LinkedFilesController = {
   Agents: _.extend(
@@ -138,55 +139,70 @@ module.exports = LinkedFilesController = {
 
   handleError(error, req, res, next) {
     if (error instanceof AccessDeniedError) {
-      return res.status(403).send('You do not have access to this project')
+      res.status(403)
+      plainTextResponse(res, 'You do not have access to this project')
     } else if (error instanceof BadDataError) {
-      return res.status(400).send('The submitted data is not valid')
+      res.status(400)
+      plainTextResponse(res, 'The submitted data is not valid')
     } else if (error instanceof BadEntityTypeError) {
-      return res.status(400).send('The file is the wrong type')
+      res.status(400)
+      plainTextResponse(res, 'The file is the wrong type')
     } else if (error instanceof SourceFileNotFoundError) {
-      return res.status(404).send('Source file not found')
+      res.status(404)
+      plainTextResponse(res, 'Source file not found')
     } else if (error instanceof ProjectNotFoundError) {
-      return res.status(404).send('Project not found')
+      res.status(404)
+      plainTextResponse(res, 'Project not found')
     } else if (error instanceof V1ProjectNotFoundError) {
-      return res
-        .status(409)
-        .send(
-          'Sorry, the source project is not yet imported to Overleaf v2. Please import it to Overleaf v2 to refresh this file'
-        )
+      res.status(409)
+      plainTextResponse(
+        res,
+        'Sorry, the source project is not yet imported to Overleaf v2. Please import it to Overleaf v2 to refresh this file'
+      )
     } else if (error instanceof CompileFailedError) {
-      return res
-        .status(422)
-        .send(res.locals.translate('generic_linked_file_compile_error'))
+      res.status(422)
+      plainTextResponse(
+        res,
+        res.locals.translate('generic_linked_file_compile_error')
+      )
     } else if (error instanceof OutputFileFetchFailedError) {
-      return res.status(404).send('Could not get output file')
+      res.status(404)
+      plainTextResponse(res, 'Could not get output file')
     } else if (error instanceof UrlFetchFailedError) {
-      return res
-        .status(422)
-        .send(
-          `Your URL could not be reached (${error.statusCode} status code). Please check it and try again.`
-        )
+      res.status(422)
+      plainTextResponse(
+        res,
+        `Your URL could not be reached (${error.statusCode} status code). Please check it and try again.`
+      )
     } else if (error instanceof InvalidUrlError) {
-      return res
-        .status(422)
-        .send('Your URL is not valid. Please check it and try again.')
+      res.status(422)
+      plainTextResponse(
+        res,
+        'Your URL is not valid. Please check it and try again.'
+      )
     } else if (error instanceof NotOriginalImporterError) {
-      return res
-        .status(400)
-        .send('You are not the user who originally imported this file')
+      res.status(400)
+      plainTextResponse(
+        res,
+        'You are not the user who originally imported this file'
+      )
     } else if (error instanceof FeatureNotAvailableError) {
-      return res.status(400).send('This feature is not enabled on your account')
+      res.status(400)
+      plainTextResponse(res, 'This feature is not enabled on your account')
     } else if (error instanceof RemoteServiceError) {
-      return res.status(502).send('The remote service produced an error')
+      res.status(502)
+      plainTextResponse(res, 'The remote service produced an error')
     } else if (error instanceof FileCannotRefreshError) {
-      return res.status(400).send('This file cannot be refreshed')
+      res.status(400)
+      plainTextResponse(res, 'This file cannot be refreshed')
     } else if (error.message === 'project_has_too_many_files') {
-      return res.status(400).send('too many files')
+      res.status(400)
+      plainTextResponse(res, 'too many files')
     } else if (/\bECONNREFUSED\b/.test(error.message)) {
-      return res
-        .status(500)
-        .send('Importing references is not currently available')
+      res.status(500)
+      plainTextResponse(res, 'Importing references is not currently available')
     } else {
-      return next(error)
+      next(error)
     }
   },
 }

+ 1 - 1
services/web/app/src/Features/Notifications/NotificationsController.js

@@ -21,7 +21,7 @@ module.exports = {
             return notification
           }
         )
-        res.send(unreadNotifications)
+        res.json(unreadNotifications)
       }
     )
   },

+ 3 - 3
services/web/app/src/Features/Project/ProjectController.js

@@ -249,7 +249,7 @@ const ProjectController = {
     const { projectName } = req.body
     logger.log({ projectId, projectName }, 'cloning project')
     if (!SessionManager.isUserLoggedIn(req.session)) {
-      return res.send({ redir: '/register' })
+      return res.json({ redir: '/register' })
     }
     const currentUser = SessionManager.getSessionUser(req.session)
     const { first_name: firstName, last_name: lastName, email } = currentUser
@@ -265,7 +265,7 @@ const ProjectController = {
           })
           return next(err)
         }
-        res.send({
+        res.json({
           name: project.name,
           project_id: project._id,
           owner_ref: project.owner_ref,
@@ -306,7 +306,7 @@ const ProjectController = {
         if (err != null) {
           return next(err)
         }
-        res.send({
+        res.json({
           project_id: project._id,
           owner_ref: project.owner_ref,
           owner: {

+ 2 - 2
services/web/app/src/Features/Spelling/SpellingController.js

@@ -27,14 +27,14 @@ module.exports = {
     if (url === '/check') {
       if (!language) {
         logger.error('"language" field should be included for spell checking')
-        return res.status(422).send(JSON.stringify({ misspellings: [] }))
+        return res.status(422).json({ misspellings: [] })
       }
 
       if (!languageCodeIsSupported(language)) {
         // this log statement can be changed to 'error' once projects with
         // unsupported languages are removed from the DB
         logger.info({ language }, 'language not supported')
-        return res.status(422).send(JSON.stringify({ misspellings: [] }))
+        return res.status(422).json({ misspellings: [] })
       }
     }
 

+ 6 - 6
services/web/app/src/Features/Uploads/ProjectUploadController.js

@@ -60,7 +60,7 @@ module.exports = ProjectUploadController = {
             })
           }
         } else {
-          return res.send({ success: true, project_id: project._id })
+          return res.json({ success: true, project_id: project._id })
         }
       }
     )
@@ -77,7 +77,7 @@ module.exports = ProjectUploadController = {
         { projectId: project_id, fileName: name },
         'bad name when trying to upload file'
       )
-      return res.status(422).send({
+      return res.status(422).json({
         success: false,
         error: 'invalid_filename',
       })
@@ -106,20 +106,20 @@ module.exports = ProjectUploadController = {
             'error uploading file'
           )
           if (error.name === 'InvalidNameError') {
-            return res.status(422).send({
+            return res.status(422).json({
               success: false,
               error: 'invalid_filename',
             })
           } else if (error.message === 'project_has_too_many_files') {
-            return res.status(422).send({
+            return res.status(422).json({
               success: false,
               error: 'project_has_too_many_files',
             })
           } else {
-            return res.status(422).send({ success: false })
+            return res.status(422).json({ success: false })
           }
         } else {
-          return res.send({
+          return res.json({
             success: true,
             entity_id: entity != null ? entity._id : undefined,
             entity_type: entity != null ? entity.type : undefined,

+ 2 - 3
services/web/app/src/Features/UserMembership/UserMembershipController.js

@@ -14,6 +14,7 @@ const SessionManager = require('../Authentication/SessionManager')
 const UserMembershipHandler = require('./UserMembershipHandler')
 const Errors = require('../Errors/Errors')
 const EmailHelper = require('../Helpers/EmailHelper')
+const { csvAttachment } = require('../../infrastructure/Response')
 const CSVParser = require('json2csv').Parser
 
 module.exports = {
@@ -146,9 +147,7 @@ module.exports = {
           return next(error)
         }
         const csvParser = new CSVParser({ fields })
-        res.header('Content-Disposition', 'attachment; filename=Group.csv')
-        res.contentType('text/csv')
-        return res.send(csvParser.parse(users))
+        csvAttachment(res, csvParser.parse(users), 'Group.csv')
       }
     )
   },

+ 48 - 0
services/web/app/src/infrastructure/Response.js

@@ -0,0 +1,48 @@
+function csvAttachment(res, body, filename) {
+  if (!filename || !filename.endsWith('.csv')) {
+    throw new Error('filename must end with .csv')
+  }
+  // res.attachment sets both content-type and content-disposition headers.
+  res.attachment(filename)
+  res.setHeader('X-Content-Type-Options', 'nosniff')
+  res.send(body)
+}
+
+function preparePlainTextResponse(res) {
+  res.setHeader('X-Content-Type-Options', 'nosniff')
+  res.contentType('text/plain; charset=utf-8')
+}
+
+function plainTextResponse(res, body) {
+  preparePlainTextResponse(res)
+  res.send(body)
+}
+
+function xmlResponse(res, body) {
+  res.setHeader('X-Content-Type-Options', 'nosniff')
+  res.contentType('application/xml; charset=utf-8')
+  res.send(body)
+}
+
+function prepareZipAttachment(res, filename) {
+  if (!filename || !filename.endsWith('.zip')) {
+    throw new Error('filename must end with .zip')
+  }
+  // res.attachment sets both content-type and content-disposition headers.
+  res.attachment(filename)
+  res.setHeader('X-Content-Type-Options', 'nosniff')
+}
+
+function zipAttachment(res, body, filename) {
+  prepareZipAttachment(res, filename)
+  res.send(body)
+}
+
+module.exports = {
+  csvAttachment,
+  plainTextResponse,
+  preparePlainTextResponse,
+  prepareZipAttachment,
+  xmlResponse,
+  zipAttachment,
+}

+ 14 - 9
services/web/app/src/router.js

@@ -61,6 +61,7 @@ const {
 const logger = require('@overleaf/logger')
 const _ = require('underscore')
 const { expressify } = require('./util/promises')
+const { plainTextResponse } = require('./infrastructure/Response')
 
 module.exports = { initialize }
 
@@ -1018,23 +1019,27 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
     AdminController.unregisterServiceWorker
   )
 
-  privateApiRouter.get('/perfTest', (req, res) => res.send('hello'))
+  privateApiRouter.get('/perfTest', (req, res) => {
+    plainTextResponse(res, 'hello')
+  })
 
   publicApiRouter.get('/status', (req, res) => {
     if (!Settings.siteIsOpen) {
-      res.send('web site is closed (web)')
+      plainTextResponse(res, 'web site is closed (web)')
     } else if (!Settings.editorIsOpen) {
-      res.send('web editor is closed (web)')
+      plainTextResponse(res, 'web editor is closed (web)')
     } else {
-      res.send('web sharelatex is alive (web)')
+      plainTextResponse(res, 'web sharelatex is alive (web)')
     }
   })
-  privateApiRouter.get('/status', (req, res) =>
-    res.send('web sharelatex is alive (api)')
-  )
+  privateApiRouter.get('/status', (req, res) => {
+    plainTextResponse(res, 'web sharelatex is alive (api)')
+  })
 
   // used by kubernetes health-check and acceptance tests
-  webRouter.get('/dev/csrf', (req, res) => res.send(res.locals.csrfToken))
+  webRouter.get('/dev/csrf', (req, res) => {
+    plainTextResponse(res, res.locals.csrfToken)
+  })
 
   publicApiRouter.get(
     '/health_check',
@@ -1085,7 +1090,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
       const projectId = req.params.Project_id
       const sendRes = _.once(function (statusCode, message) {
         res.status(statusCode)
-        res.send(message)
+        plainTextResponse(res, message)
         ClsiCookieManager.clearServerId(projectId)
       }) // force every compile to a new server
       // set a timeout

+ 47 - 0
services/web/bin/lint_flag_res_send_usage

@@ -0,0 +1,47 @@
+#!/bin/bash
+
+set -e
+
+POTENTIAL_SEND_USAGE=$(\
+  grep \
+    --files-with-matches \
+    --recursive \
+      app.js \
+      app/ \
+      modules/*/app \
+      test/acceptance/ \
+      modules/*/test/acceptance/ \
+    --regex "\.send\b" \
+    --regex "\bsend(" \
+)
+HELPER_MODULE="app/src/infrastructure/Response.js"
+if [[ "$POTENTIAL_SEND_USAGE" == "$HELPER_MODULE" ]]; then
+  exit 0
+fi
+
+for file in ${POTENTIAL_SEND_USAGE}; do
+  if [[ "$file" == "$HELPER_MODULE" ]]; then
+    continue
+  fi
+
+  cat <<MSG >&2
+
+ERROR: $file contains a potential use of 'res.send'.
+
+---
+$(grep -n -C 3  "$file" --regex "\.send\b" --regex "\bsend(")
+---
+
+Using 'res.send' is prone to introducing XSS vulnerabilities.
+
+Consider using 'res.json' or one of the helpers in $HELPER_MODULE.
+
+If this is a false-positive, consider using a more specific name than 'send'
+ for your newly introduced function.
+
+Links:
+ - https://github.com/overleaf/internal/issues/6268
+
+MSG
+  exit 1
+done

+ 1 - 0
services/web/package.json

@@ -224,6 +224,7 @@
     "chai-exclude": "^2.0.3",
     "chaid": "^1.0.2",
     "cheerio": "^1.0.0-rc.3",
+    "content-disposition": "^0.5.0",
     "copy-webpack-plugin": "^5.1.1",
     "css-loader": "^3.5.2",
     "es6-promise": "^4.2.8",

+ 5 - 2
services/web/test/acceptance/src/LinkedFilesTests.js

@@ -7,12 +7,15 @@ const Settings = require('@overleaf/settings')
 const User = require('./helpers/User').promises
 
 const express = require('express')
+const {
+  plainTextResponse,
+} = require('../../../app/src/infrastructure/Response')
 const LinkedUrlProxy = express()
 LinkedUrlProxy.get('/', (req, res, next) => {
   if (req.query.url === 'http://example.com/foo') {
-    return res.send('foo foo foo')
+    return plainTextResponse(res, 'foo foo foo')
   } else if (req.query.url === 'http://example.com/bar') {
-    return res.send('bar bar bar')
+    return plainTextResponse(res, 'bar bar bar')
   } else {
     return res.sendStatus(404)
   }

+ 2 - 2
services/web/test/acceptance/src/mocks/MockChatApi.js

@@ -6,7 +6,7 @@ class MockChatApi extends AbstractMockApi {
   }
 
   getGlobalMessages(req, res) {
-    res.send(this.projects[req.params.project_id] || [])
+    res.json(this.projects[req.params.project_id] || [])
   }
 
   sendGlobalMessage(req, res) {
@@ -19,7 +19,7 @@ class MockChatApi extends AbstractMockApi {
     }
     this.projects[projectId] = this.projects[projectId] || []
     this.projects[projectId].push(message)
-    res.sendStatus(201).send(Object.assign({ room_id: projectId }, message))
+    res.json(Object.assign({ room_id: projectId }, message))
   }
 
   applyRoutes() {

+ 8 - 5
services/web/test/acceptance/src/mocks/MockClsiApi.js

@@ -1,8 +1,11 @@
 const AbstractMockApi = require('./AbstractMockApi')
+const {
+  plainTextResponse,
+} = require('../../../../app/src/infrastructure/Response')
 
 class MockClsiApi extends AbstractMockApi {
   static compile(req, res) {
-    res.status(200).send({
+    res.json({
       compile: {
         status: 'success',
         error: null,
@@ -36,9 +39,9 @@ class MockClsiApi extends AbstractMockApi {
       (req, res) => {
         const filename = req.params[0]
         if (filename === 'project.pdf') {
-          res.status(200).send('mock-pdf')
+          plainTextResponse(res, 'mock-pdf')
         } else if (filename === 'project.log') {
-          res.status(200).send('mock-log')
+          plainTextResponse(res, 'mock-log')
         } else {
           res.sendStatus(404)
         }
@@ -48,12 +51,12 @@ class MockClsiApi extends AbstractMockApi {
     this.app.get(
       '/project/:project_id/user/:user_id/build/:build_id/output/:output_path',
       (req, res) => {
-        res.status(200).send('hello')
+        plainTextResponse(res, 'hello')
       }
     )
 
     this.app.get('/project/:project_id/status', (req, res) => {
-      res.status(200).send()
+      res.sendStatus(200)
     })
   }
 }

+ 4 - 1
services/web/test/acceptance/src/mocks/MockFilestoreApi.js

@@ -1,4 +1,7 @@
 const AbstractMockApi = require('./AbstractMockApi')
+const {
+  plainTextResponse,
+} = require('../../../../app/src/infrastructure/Response')
 
 class MockFilestoreApi extends AbstractMockApi {
   reset() {
@@ -24,7 +27,7 @@ class MockFilestoreApi extends AbstractMockApi {
     this.app.get('/project/:projectId/file/:fileId', (req, res) => {
       const { projectId, fileId } = req.params
       const { content } = this.files[projectId][fileId]
-      res.send(content)
+      plainTextResponse(res, content)
     })
 
     // handle file copying

+ 4 - 1
services/web/test/acceptance/src/mocks/MockProjectHistoryApi.js

@@ -1,6 +1,9 @@
 const AbstractMockApi = require('./AbstractMockApi')
 const _ = require('lodash')
 const { ObjectId } = require('mongodb')
+const {
+  plainTextResponse,
+} = require('../../../../app/src/infrastructure/Response')
 
 class MockProjectHistoryApi extends AbstractMockApi {
   reset() {
@@ -64,7 +67,7 @@ class MockProjectHistoryApi extends AbstractMockApi {
         const { projectId, version, pathname } = req.params
         const key = `${projectId}:${version}:${pathname}`
         if (this.oldFiles[key] != null) {
-          res.send(this.oldFiles[key])
+          plainTextResponse(res, this.oldFiles[key])
         } else {
           res.sendStatus(404)
         }

+ 30 - 14
services/web/test/acceptance/src/mocks/MockRecurlyApi.js

@@ -1,5 +1,6 @@
 const AbstractMockApi = require('./AbstractMockApi')
 const SubscriptionController = require('../../../../app/src/Features/Subscription/SubscriptionController')
+const { xmlResponse } = require('../../../../app/src/infrastructure/Response')
 
 class MockRecurlyApi extends AbstractMockApi {
   reset() {
@@ -28,9 +29,11 @@ class MockRecurlyApi extends AbstractMockApi {
     this.app.get('/subscriptions/:id', (req, res) => {
       const subscription = this.getMockSubscriptionById(req.params.id)
       if (!subscription) {
-        res.status(404).end()
+        res.sendStatus(404)
       } else {
-        res.send(`\
+        xmlResponse(
+          res,
+          `\
 <subscription>
 	<plan><plan_code>${subscription.planCode}</plan_code></plan>
 	<currency>${subscription.currency}</currency>
@@ -42,22 +45,26 @@ class MockRecurlyApi extends AbstractMockApi {
 	<account href="accounts/${subscription.account.id}" />
 	<trial_ends_at type="datetime">${subscription.trial_ends_at}</trial_ends_at>
 </subscription>\
-`)
+`
+        )
       }
     })
 
     this.app.get('/accounts/:id', (req, res) => {
       const subscription = this.getMockSubscriptionByAccountId(req.params.id)
       if (!subscription) {
-        res.status(404).end()
+        res.sendStatus(404)
       } else {
-        res.send(`\
+        xmlResponse(
+          res,
+          `\
 <account>
 	<account_code>${req.params.id}</account_code>
 	<hosted_login_token>${subscription.account.hosted_login_token}</hosted_login_token>
 	<email>${subscription.account.email}</email>
 </account>\
-`)
+`
+        )
       }
     })
 
@@ -67,15 +74,18 @@ class MockRecurlyApi extends AbstractMockApi {
       (req, res) => {
         const subscription = this.getMockSubscriptionByAccountId(req.params.id)
         if (!subscription) {
-          res.status(404).end()
+          res.sendStatus(404)
         } else {
           Object.assign(subscription.account, req.body.account)
-          res.send(`\
+          xmlResponse(
+            res,
+            `\
 <account>
 	<account_code>${req.params.id}</account_code>
 	<email>${subscription.account.email}</email>
 </account>\
-`)
+`
+          )
         }
       }
     )
@@ -83,15 +93,18 @@ class MockRecurlyApi extends AbstractMockApi {
     this.app.get('/coupons/:code', (req, res) => {
       const coupon = this.coupons[req.params.code]
       if (!coupon) {
-        res.status(404).end()
+        res.sendStatus(404)
       } else {
-        res.send(`\
+        xmlResponse(
+          res,
+          `\
 <coupon>
 	<coupon_code>${req.params.code}</coupon_code>
 	<name>${coupon.name || ''}</name>
 	<description>${coupon.description || ''}</description>
 </coupon>\
-`)
+`
+        )
       }
     })
 
@@ -107,11 +120,14 @@ class MockRecurlyApi extends AbstractMockApi {
 `
       }
 
-      res.send(`\
+      xmlResponse(
+        res,
+        `\
 <redemptions type="array">
 	${redemptionsListXml}
 </redemptions>\
-`)
+`
+      )
     })
   }
 }

+ 13 - 8
services/web/test/acceptance/src/mocks/MockV1HistoryApi.js

@@ -1,5 +1,9 @@
 const AbstractMockApi = require('./AbstractMockApi')
 const { EventEmitter } = require('events')
+const {
+  zipAttachment,
+  prepareZipAttachment,
+} = require('../../../../app/src/infrastructure/Response')
 
 class MockV1HistoryApi extends AbstractMockApi {
   reset() {
@@ -13,10 +17,10 @@ class MockV1HistoryApi extends AbstractMockApi {
     this.app.get(
       '/api/projects/:project_id/version/:version/zip',
       (req, res, next) => {
-        res.header('content-disposition', 'attachment; name=project.zip')
-        res.header('content-type', 'application/octet-stream')
-        res.send(
-          `Mock zip for ${req.params.project_id} at version ${req.params.version}`
+        zipAttachment(
+          res,
+          `Mock zip for ${req.params.project_id} at version ${req.params.version}`,
+          'project.zip'
         )
       }
     )
@@ -27,13 +31,14 @@ class MockV1HistoryApi extends AbstractMockApi {
         if (!(this.fakeZipCall++ > 0)) {
           return res.sendStatus(404)
         }
-        res.header('content-disposition', 'attachment; name=project.zip')
-        res.header('content-type', 'application/octet-stream')
         if (req.params.version === '42') {
-          return res.send(
-            `Mock zip for ${req.params.project_id} at version ${req.params.version}`
+          return zipAttachment(
+            res,
+            `Mock zip for ${req.params.project_id} at version ${req.params.version}`,
+            'project.zip'
           )
         }
+        prepareZipAttachment(res, 'project.zip')
         const writeChunk = () => {
           res.write('chunk' + this.sentChunks++)
         }

+ 1 - 1
services/web/test/unit/src/Authentication/AuthenticationControllerTests.js

@@ -481,7 +481,7 @@ describe('AuthenticationController', function () {
 
   describe('requireOauth', function () {
     beforeEach(function () {
-      this.res.send = sinon.stub()
+      this.res.json = sinon.stub()
       this.res.status = sinon.stub().returns(this.res)
       this.res.sendStatus = sinon.stub()
       this.middleware = this.AuthenticationController.requireOauth()

+ 2 - 2
services/web/test/unit/src/Compile/CompileControllerTests.js

@@ -784,7 +784,7 @@ describe('CompileController', function () {
         .yields(null, { content: 'body' })
       this.req.params = { Project_id: this.project_id }
       this.req.query = { clsiserverid: 'node-42' }
-      this.res.send = sinon.stub()
+      this.res.json = sinon.stub()
       this.res.contentType = sinon.stub()
       return this.CompileController.wordCount(this.req, this.res, this.next)
     })
@@ -796,7 +796,7 @@ describe('CompileController', function () {
     })
 
     it('should return a 200 and body', function () {
-      return this.res.send.calledWith({ content: 'body' }).should.equal(true)
+      return this.res.json.calledWith({ content: 'body' }).should.equal(true)
     })
   })
 })

+ 3 - 4
services/web/test/unit/src/Contact/ContactControllerTests.js

@@ -15,6 +15,7 @@ const sinon = require('sinon')
 const { assert, expect } = require('chai')
 const modulePath = '../../../../app/src/Features/Contacts/ContactController.js'
 const SandboxedModule = require('sandboxed-module')
+const MockResponse = require('../helpers/MockResponse')
 
 describe('ContactController', function () {
   beforeEach(function () {
@@ -30,9 +31,7 @@ describe('ContactController', function () {
 
     this.next = sinon.stub()
     this.req = {}
-    this.res = {}
-    this.res.status = sinon.stub().returns(this.req)
-    return (this.res.send = sinon.stub())
+    this.res = new MockResponse()
   })
 
   describe('getContacts', function () {
@@ -105,7 +104,7 @@ describe('ContactController', function () {
     })
 
     it('should return a formatted list of contacts in contact list order, without holding accounts', function () {
-      return this.res.send.args[0][0].contacts.should.deep.equal([
+      return this.res.json.args[0][0].contacts.should.deep.equal([
         {
           id: 'contact-1',
           email: 'joe@example.com',

+ 11 - 12
services/web/test/unit/src/Downloads/ProjectDownloadsControllerTests.js

@@ -46,8 +46,6 @@ describe('ProjectDownloadsController', function () {
         .stub()
         .callsArgWith(1, null, this.stream)
       this.req.params = { Project_id: this.project_id }
-      this.res.contentType = sinon.stub()
-      this.res.header = sinon.stub()
       this.project_name = 'project name with accênts'
       this.ProjectGetter.getProject = sinon
         .stub()
@@ -92,9 +90,11 @@ describe('ProjectDownloadsController', function () {
     })
 
     it('should name the downloaded file after the project', function () {
-      return this.res.setContentDisposition
-        .calledWith('attachment', { filename: `${this.project_name}.zip` })
-        .should.equal(true)
+      this.res.headers.should.deep.equal({
+        'Content-Disposition': `attachment; filename="${this.project_name}.zip"`,
+        'Content-Type': 'application/zip',
+        'X-Content-Type-Options': 'nosniff',
+      })
     })
 
     it('should record the action via Metrics', function () {
@@ -110,8 +110,6 @@ describe('ProjectDownloadsController', function () {
         .callsArgWith(1, null, this.stream)
       this.project_ids = ['project-1', 'project-2']
       this.req.query = { project_ids: this.project_ids.join(',') }
-      this.res.contentType = sinon.stub()
-      this.res.header = sinon.stub()
       this.DocumentUpdaterHandler.flushMultipleProjectsToMongo = sinon
         .stub()
         .callsArgWith(1)
@@ -146,11 +144,12 @@ describe('ProjectDownloadsController', function () {
     })
 
     it('should name the downloaded file after the project', function () {
-      return this.res.setContentDisposition
-        .calledWith('attachment', {
-          filename: 'Overleaf Projects (2 items).zip',
-        })
-        .should.equal(true)
+      this.res.headers.should.deep.equal({
+        'Content-Disposition':
+          'attachment; filename="Overleaf Projects (2 items).zip"',
+        'Content-Type': 'application/zip',
+        'X-Content-Type-Options': 'nosniff',
+      })
     })
 
     it('should record the action via Metrics', function () {

+ 13 - 19
services/web/test/unit/src/FileStore/FileStoreControllerTests.js

@@ -2,6 +2,7 @@ const { expect } = require('chai')
 const sinon = require('sinon')
 const SandboxedModule = require('sandboxed-module')
 const Errors = require('../../../../app/src/Features/Errors/Errors')
+const MockResponse = require('../helpers/MockResponse')
 
 const MODULE_PATH =
   '../../../../app/src/Features/FileStore/FileStoreController.js'
@@ -33,12 +34,7 @@ describe('FileStoreController', function () {
         return undefined
       },
     }
-    this.res = {
-      set: sinon.stub().returnsThis(),
-      setHeader: sinon.stub(),
-      setContentDisposition: sinon.stub(),
-      status: sinon.stub().returnsThis(),
-    }
+    this.res = new MockResponse()
     this.file = { name: 'myfile.png' }
   })
 
@@ -108,9 +104,7 @@ describe('FileStoreController', function () {
         describe('from a non-ios browser', function () {
           it('should not set Content-Type', function (done) {
             this.stream.pipe = des => {
-              this.res.setHeader
-                .calledWith('Content-Type', 'text/plain')
-                .should.equal(false)
+              this.res.headers.should.deep.equal({})
               done()
             }
             this.controller.getFile(this.req, this.res)
@@ -128,9 +122,10 @@ describe('FileStoreController', function () {
 
           it("should set Content-Type to 'text/plain'", function (done) {
             this.stream.pipe = des => {
-              this.res.setHeader
-                .calledWith('Content-Type', 'text/plain')
-                .should.equal(true)
+              this.res.headers.should.deep.equal({
+                'Content-Type': 'text/plain; charset=utf-8',
+                'X-Content-Type-Options': 'nosniff',
+              })
               done()
             }
             this.controller.getFile(this.req, this.res)
@@ -148,9 +143,10 @@ describe('FileStoreController', function () {
 
           it("should set Content-Type to 'text/plain'", function (done) {
             this.stream.pipe = des => {
-              this.res.setHeader
-                .calledWith('Content-Type', 'text/plain')
-                .should.equal(true)
+              this.res.headers.should.deep.equal({
+                'Content-Type': 'text/plain; charset=utf-8',
+                'X-Content-Type-Options': 'nosniff',
+              })
               done()
             }
             this.controller.getFile(this.req, this.res)
@@ -183,9 +179,7 @@ describe('FileStoreController', function () {
 
             it('Should not set the Content-type', function (done) {
               this.stream.pipe = des => {
-                this.res.setHeader
-                  .calledWith('Content-Type', 'text/plain')
-                  .should.equal(false)
+                this.res.headers.should.deep.equal({})
                 done()
               }
               this.controller.getFile(this.req, this.res)
@@ -208,7 +202,7 @@ describe('FileStoreController', function () {
 
       this.res.end = () => {
         expect(this.res.status.lastCall.args).to.deep.equal([200])
-        expect(this.res.set.lastCall.args).to.deep.equal([
+        expect(this.res.header.lastCall.args).to.deep.equal([
           'Content-Length',
           expectedFileSize,
         ])

+ 1 - 1
services/web/test/unit/src/Notifications/NotificationsControllerTests.js

@@ -45,7 +45,7 @@ describe('NotificationsController', function () {
       .stub()
       .callsArgWith(1, null, allNotifications)
     this.controller.getAllUnreadNotifications(this.req, {
-      send: body => {
+      json: body => {
         body.should.deep.equal(allNotifications)
         this.handler.getUserNotifications.calledWith(userId).should.equal(true)
         done()

+ 3 - 3
services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -343,7 +343,7 @@ describe('ProjectController', function () {
 
   describe('cloneProject', function () {
     it('should call the project duplicator', function (done) {
-      this.res.send = json => {
+      this.res.json = json => {
         this.ProjectDuplicator.duplicate
           .calledWith(this.user, this.project_id, this.projectName)
           .should.equal(true)
@@ -357,7 +357,7 @@ describe('ProjectController', function () {
   describe('newProject', function () {
     it('should call the projectCreationHandler with createExampleProject', function (done) {
       this.req.body.template = 'example'
-      this.res.send = json => {
+      this.res.json = json => {
         this.ProjectCreationHandler.createExampleProject
           .calledWith(this.user._id, this.projectName)
           .should.equal(true)
@@ -371,7 +371,7 @@ describe('ProjectController', function () {
 
     it('should call the projectCreationHandler with createBasicProject', function (done) {
       this.req.body.template = 'basic'
-      this.res.send = json => {
+      this.res.json = json => {
         this.ProjectCreationHandler.createExampleProject.called.should.equal(
           false
         )

+ 0 - 1
services/web/test/unit/src/References/ReferencesControllerTests.js

@@ -43,7 +43,6 @@ describe('ReferencesController', function () {
     }
     this.res = new MockResponse()
     this.res.json = sinon.stub()
-    this.res.send = sinon.stub()
     this.res.sendStatus = sinon.stub()
     return (this.fakeResponseData = {
       projectId: this.projectId,

+ 4 - 11
services/web/test/unit/src/Spelling/SpellingControllerTests.js

@@ -1,5 +1,6 @@
 const SandboxedModule = require('sandboxed-module')
 const sinon = require('sinon')
+const MockResponse = require('../helpers/MockResponse')
 const modulePath = require('path').join(
   __dirname,
   '../../../../app/src/Features/Spelling/SpellingController.js'
@@ -52,11 +53,7 @@ describe('SpellingController', function () {
       headers: { Host: SPELLING_HOST },
     }
 
-    this.res = {}
-    this.res.send = sinon.stub()
-    this.res.status = sinon.stub().returns(this.res)
-    this.res.end = sinon.stub()
-    this.res.json = sinon.stub()
+    this.res = new MockResponse()
   })
 
   describe('proxyRequestToSpellingApi', function () {
@@ -104,9 +101,7 @@ describe('SpellingController', function () {
         })
 
         it('should return an empty misspellings array', function () {
-          this.res.send
-            .calledWith(JSON.stringify({ misspellings: [] }))
-            .should.equal(true)
+          this.res.json.calledWith({ misspellings: [] }).should.equal(true)
         })
 
         it('should return a 422 status', function () {
@@ -142,9 +137,7 @@ describe('SpellingController', function () {
         })
 
         it('should return an empty misspellings array', function () {
-          this.res.send
-            .calledWith(JSON.stringify({ misspellings: [] }))
-            .should.equal(true)
+          this.res.json.calledWith({ misspellings: [] }).should.equal(true)
         })
 
         it('should return a 422 status', function () {

+ 30 - 20
services/web/test/unit/src/Uploads/ProjectUploadControllerTests.js

@@ -100,10 +100,12 @@ describe('ProjectUploadController', function () {
       })
 
       it('should return a successful response to the FileUploader client', function () {
-        return expect(this.res.body).to.deep.equal({
-          success: true,
-          project_id: this.project_id,
-        })
+        return expect(this.res.body).to.deep.equal(
+          JSON.stringify({
+            success: true,
+            project_id: this.project_id,
+          })
+        )
       })
 
       it('should record the time taken to do the upload', function () {
@@ -200,11 +202,13 @@ describe('ProjectUploadController', function () {
       })
 
       it('should return a successful response to the FileUploader client', function () {
-        return expect(this.res.body).to.deep.equal({
-          success: true,
-          entity_id: this.entity._id,
-          entity_type: 'file',
-        })
+        return expect(this.res.body).to.deep.equal(
+          JSON.stringify({
+            success: true,
+            entity_id: this.entity._id,
+            entity_type: 'file',
+          })
+        )
       })
 
       it('should time the request', function () {
@@ -225,9 +229,11 @@ describe('ProjectUploadController', function () {
       })
 
       it('should return an unsuccessful response to the FileUploader client', function () {
-        return expect(this.res.body).to.deep.equal({
-          success: false,
-        })
+        return expect(this.res.body).to.deep.equal(
+          JSON.stringify({
+            success: false,
+          })
+        )
       })
     })
 
@@ -240,10 +246,12 @@ describe('ProjectUploadController', function () {
       })
 
       it('should return an unsuccessful response to the FileUploader client', function () {
-        return expect(this.res.body).to.deep.equal({
-          success: false,
-          error: 'project_has_too_many_files',
-        })
+        return expect(this.res.body).to.deep.equal(
+          JSON.stringify({
+            success: false,
+            error: 'project_has_too_many_files',
+          })
+        )
       })
     })
 
@@ -254,10 +262,12 @@ describe('ProjectUploadController', function () {
       })
 
       it('should return a a non success response', function () {
-        return expect(this.res.body).to.deep.equal({
-          success: false,
-          error: 'invalid_filename',
-        })
+        return expect(this.res.body).to.deep.equal(
+          JSON.stringify({
+            success: false,
+            error: 'invalid_filename',
+          })
+        )
       })
     })
   })

+ 2 - 5
services/web/test/unit/src/UserMembership/UserMembershipControllerTests.js

@@ -280,9 +280,6 @@ describe('UserMembershipController', function () {
       this.req.entity = this.subscription
       this.req.entityConfig = EntityConfigs.groupManagers
       this.res = new MockResponse()
-      this.res.contentType = sinon.stub()
-      this.res.header = sinon.stub()
-      this.res.send = sinon.stub()
       return this.UserMembershipController.exportCsv(this.req, this.res)
     })
 
@@ -295,14 +292,14 @@ describe('UserMembershipController', function () {
     })
 
     it('should set the correct content type on the request', function () {
-      return assertCalledWith(this.res.contentType, 'text/csv')
+      return assertCalledWith(this.res.contentType, 'text/csv; charset=utf-8')
     })
 
     it('should name the exported csv file', function () {
       return assertCalledWith(
         this.res.header,
         'Content-Disposition',
-        'attachment; filename=Group.csv'
+        'attachment; filename="Group.csv"'
       )
     })
 

+ 39 - 7
services/web/test/unit/src/helpers/MockResponse.js

@@ -12,14 +12,13 @@
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
 const sinon = require('sinon')
+const Path = require('path')
+const contentDisposition = require('content-disposition')
 
 class MockResponse {
   static initClass() {
+    // Added via ExpressLocals.
     this.prototype.setContentDisposition = sinon.stub()
-
-    this.prototype.header = sinon.stub()
-
-    this.prototype.contentType = sinon.stub()
   }
 
   constructor() {
@@ -28,6 +27,18 @@ class MockResponse {
     this.returned = false
     this.headers = {}
     this.locals = {}
+
+    sinon.spy(this, 'contentType')
+    sinon.spy(this, 'header')
+    sinon.spy(this, 'json')
+    sinon.spy(this, 'send')
+    sinon.spy(this, 'sendStatus')
+    sinon.spy(this, 'status')
+    sinon.spy(this, 'render')
+  }
+
+  header(field, val) {
+    this.headers[field] = val
   }
 
   render(template, variables) {
@@ -100,7 +111,7 @@ class MockResponse {
     }
     this.statusCode = status
     this.returned = true
-    this.type = 'application/json'
+    this.contentType('application/json')
     if (status >= 200 && status < 300) {
       this.success = true
     } else {
@@ -120,7 +131,7 @@ class MockResponse {
   }
 
   setHeader(header, value) {
-    return (this.headers[header] = value)
+    this.header(header, value)
   }
 
   setTimeout(timout) {
@@ -133,8 +144,29 @@ class MockResponse {
     }
   }
 
+  attachment(filename) {
+    switch (Path.extname(filename)) {
+      case '.csv':
+        this.contentType('text/csv; charset=utf-8')
+        break
+      case '.zip':
+        this.contentType('application/zip')
+        break
+      default:
+        throw new Error('unexpected extension')
+    }
+    this.header('Content-Disposition', contentDisposition(filename))
+    return this
+  }
+
+  contentType(type) {
+    this.header('Content-Type', type)
+    this.type = type
+    return this
+  }
+
   type(type) {
-    return (this.type = type)
+    return this.contentType(type)
   }
 }
 MockResponse.initClass()