From 6873c1d972e662acb941afd0464f511da48f74d4 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 16 Jun 2016 13:59:24 +0100 Subject: [PATCH 1/4] fix download methods in CompileController --- .../Features/Compile/CompileController.coffee | 59 ++++++++++++------- 1 file changed, 39 insertions(+), 20 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index c927e6b666..5f3edd4de5 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -44,11 +44,20 @@ module.exports = CompileController = } _compileAsUser: (req, callback) -> + # callback with user_id if isolated flag is set on request, undefined otherwise isolated = req.query?.isolated is "true" if isolated - AuthenticationController.getLoggedInUserId req, callback + AuthenticationController.getLoggedInUserId req, callback # -> (error, user_id) else - callback() + callback() # do a per-project compile, not per-user + + _downloadAsUser: (req, callback) -> + # callback with user_id if isolated flag or user_id param is set on request, undefined otherwise + isolated = req.query?.isolated is "true" or req.params.user_id? + if isolated + AuthenticationController.getLoggedInUserId req, callback # -> (error, user_id) + else + callback() # do a per-project compile, not per-user downloadPdf: (req, res, next = (error) ->)-> Metrics.inc "pdf-downloads" @@ -82,7 +91,9 @@ module.exports = CompileController = logger.log project_id:project_id, ip:req.ip, "rate limit hit downloading pdf" res.send 500 else - CompileController.proxyToClsi(project_id, "/project/#{project_id}/output/output.pdf", req, res, next) + CompileController._downloadAsUser req, (error, user_id) -> + url = CompileController._getFileUrl project_id, user_id, req.params.build_id, "output.pdf" + CompileController.proxyToClsi(project_id, url, req, res, next) deleteAuxFiles: (req, res, next) -> project_id = req.params.Project_id @@ -92,29 +103,37 @@ module.exports = CompileController = return next(error) if error? res.sendStatus(200) + # this is only used by templates, so is not called with a user_id compileAndDownloadPdf: (req, res, next)-> project_id = req.params.project_id - CompileController._compileAsUser req, (error, user_id) -> - return next(error) if error? - CompileManager.compile project_id, user_id, {}, (err)-> - if err? - logger.err err:err, project_id:project_id, "something went wrong compile and downloading pdf" - res.sendStatus 500 - url = "/project/#{project_id}/output/output.pdf" - CompileController.proxyToClsi project_id, url, req, res, next + # pass user_id as null, since templates are an "anonymous" compile + CompileManager.compile project_id, null, {}, (err)-> + if err? + logger.err err:err, project_id:project_id, "something went wrong compile and downloading pdf" + res.sendStatus 500 + url = "/project/#{project_id}/output/output.pdf" + CompileController.proxyToClsi project_id, url, req, res, next getFileFromClsi: (req, res, next = (error) ->) -> project_id = req.params.Project_id - build_id = req.params.build_id - user_id = req.params.user_id - if user_id? and build_id? - url = "/project/#{project_id}/user/#{user_id}/build/#{build_id}/output/#{req.params.file}" - else if build_id? - url = "/project/#{project_id}/build/#{build_id}/output/#{req.params.file}" - else - url = "/project/#{project_id}/output/#{req.params.file}" - CompileController.proxyToClsi(project_id, url, req, res, next) + CompileController._downloadAsUser req, (error, user_id) -> + return next(error) if error? + url = CompileController._getFileUrl project_id, user_id, req.params.build_id, req.params.file + CompileController.proxyToClsi(project_id, url, req, res, next) + # compute a GET file url for a given project, user (optional), build (optional) and file + _getFileUrl: (project_id, user_id, build_id, file) -> + if user_id? and build_id? + url = "/project/#{project_id}/user/#{user_id}/build/#{build_id}/output/#{file}" + else if user_id? + url = "/project/#{project_id}/user/#{user_id}/output/#{file}" + else if build_id? + url = "/project/#{project_id}/build/#{build_id}/output/#{file}" + else + url = "/project/#{project_id}/output/#{file}" + return url + + # compute a POST url for a project, user (optional) and action _getUrl: (project_id, user_id, action) -> path = "/project/#{project_id}" path += "/user/#{user_id}" if user_id? From 8a0fa1321d8efbded1f7684ef33ebb6f6a022a37 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 16 Jun 2016 13:59:56 +0100 Subject: [PATCH 2/4] add comments to router, downcase route express has case-insensitive routes --- services/web/app/coffee/router.coffee | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 23df462787..7a3302672e 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -106,7 +106,9 @@ module.exports = class Router webRouter.post '/project/:Project_id/settings/admin', AuthorizationMiddlewear.ensureUserCanAdminProject, ProjectController.updateProjectAdminSettings webRouter.post '/project/:Project_id/compile', AuthorizationMiddlewear.ensureUserCanReadProject, CompileController.compile - webRouter.get '/Project/:Project_id/output/output.pdf', AuthorizationMiddlewear.ensureUserCanReadProject, CompileController.downloadPdf + # Used by the web download buttons, adds filename header + webRouter.get '/project/:Project_id/output/output.pdf', AuthorizationMiddlewear.ensureUserCanReadProject, CompileController.downloadPdf + # Used by the pdf viewers webRouter.get /^\/project\/([^\/]*)\/output\/(.*)$/, ((req, res, next) -> params = From f045eb65d3033feca2d6bb209ef2f50493580288 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 16 Jun 2016 14:03:42 +0100 Subject: [PATCH 3/4] add query string utility function --- .../public/coffee/ide/pdf/controllers/PdfController.coffee | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index 4baa2a22ff..a686e59fff 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -19,6 +19,11 @@ define [ else $scope.modifierKey = "Ctrl" + # utility for making a query string from a hash, could use jquery $.param + createQueryString = (args) -> + qs_args = ("#{k}=#{v}" for k, v of args) + if qs_args.length then "?" + qs_args.join("&") else "" + $scope.$on "project:joined", () -> return if !autoCompile autoCompile = false From 02e3f7a02b5e24fff1fced59ac7bba7c754e41fc Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 16 Jun 2016 14:05:21 +0100 Subject: [PATCH 4/4] change download links so they do not rely on build id prefer the top level files, because the build directories are more ephemeral. In a shared project they can expire if there are multiple compiles. --- .../ide/pdf/controllers/PdfController.coffee | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index a686e59fff..66f47f65e9 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -117,10 +117,12 @@ define [ qs.clsiserverid = response.clsiServerId ide.clsiServerId = response.clsiServerId # convert the qs hash into a query string and append it - qs_args = ("#{k}=#{v}" for k, v of qs) - $scope.pdf.qs = if qs_args.length then "?" + qs_args.join("&") else "" + $scope.pdf.qs = createQueryString qs $scope.pdf.url += $scope.pdf.qs - $scope.pdf.downloadUrl = "/Project/#{$scope.project_id}/output/output.pdf" + $scope.pdf.qs + # special case for the download url + if perUserCompile + qs.isolated = true + $scope.pdf.downloadUrl = "/project/#{$scope.project_id}/output/output.pdf" + createQueryString(qs) fetchLogs(fileByPath['output.log'], fileByPath['output.blg']) @@ -136,10 +138,12 @@ define [ file.name = "#{file.path.replace(/^output\./, "")} file" else file.name = file.path - if not file.url? - file.url = "/project/#{project_id}/output/#{file.path}" + qs = {} + if perUserCompile + qs.isolated = true if response.clsiServerId? - file.url = file.url + "?clsiserverid=#{response.clsiServerId}" + qs.clsiserverid = response.clsiServerId + file.url = "/project/#{project_id}/output/#{file.path}" + createQueryString qs $scope.pdf.outputFiles.push file