From d3b38c8dc2efa16427e5e36ad0442fe4f0963eb3 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 12 Dec 2014 15:43:21 +0000 Subject: [PATCH 1/4] Revert "Revert test changes for proxied headers when requesting pdfs from clsi" This reverts commit ab31d2c3fdc00a38f6444248011a3871b83dbec2. --- .../Compile/CompileControllerTests.coffee | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee index e2cbec4cb8..b3c004fdfe 100644 --- a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee @@ -137,7 +137,12 @@ describe "CompileController", -> statusCode: 204 headers: { "mock": "header" } @req.method = "mock-method" - + @req.headers = { + 'Mock': 'Headers', + 'Range': 'should be passed - Range' + 'If-Range': 'should be passed - If-Range' + 'If-Modified-Since': 'should be passed - If-Modified-Since' + } describe "user with standard priority", -> @@ -152,6 +157,11 @@ describe "CompileController", -> method: @req.method url: "#{@settings.apis.clsi.url}#{@url}", timeout: 60 * 1000 + headers: { + 'Range': 'should be passed - Range' + 'If-Range': 'should be passed - If-Range' + 'If-Modified-Since': 'should be passed - If-Modified-Since' + } ) .should.equal true @@ -175,6 +185,11 @@ describe "CompileController", -> method: @req.method url: "#{@settings.apis.clsi_priority.url}#{@url}", timeout: 60 * 1000 + headers: { + 'Range': 'should be passed - Range' + 'If-Range': 'should be passed - If-Range' + 'If-Modified-Since': 'should be passed - If-Modified-Since' + } ) .should.equal true From fbf9111d8045139ef5487ec3733eab47955b48f1 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 12 Dec 2014 15:43:36 +0000 Subject: [PATCH 2/4] Revert "revert to original CLSI proxy call" This reverts commit 99b1b0d6598562b6346921504d1d1d37efe3be86. --- .../app/coffee/Features/Compile/CompileController.coffee | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index 054c6f5aaf..663210b092 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -78,7 +78,12 @@ module.exports = CompileController = url = "#{compilerUrl}#{url}" logger.log url: url, "proxying to CLSI" oneMinute = 60 * 1000 - proxy = request(url: url, method: req.method, timeout: oneMinute) + # pass through If-* and Range headers for byte serving pdfs + # do not send any others, potential proxying loop if Host: is passed! + newHeaders = {} + for h, v of req.headers + newHeaders[h] = req.headers[h] if h.match /^(If-|Range)/i + proxy = request(url: url, method: req.method, timeout: oneMinute, headers: newHeaders) proxy.pipe(res) proxy.on "error", (error) -> logger.warn err: error, url: url, "CLSI proxy error" From 660bb75df492da257e1808f85d1288e2dceb3050 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 12 Dec 2014 16:47:43 +0000 Subject: [PATCH 3/4] add query string parameters for compileGroup= and pdfng= compileGroup will bypass mongo check for compileGroup priority pdfng will pass pdf Range: headers through from user request to CLSI --- .../Features/Compile/CompileController.coffee | 42 +++++++++++-------- .../Features/Compile/CompileManager.coffee | 2 +- .../ide/pdf/controllers/PdfController.coffee | 7 +++- .../ide/pdfng/directives/pdfViewer.coffee | 3 +- 4 files changed, 34 insertions(+), 20 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index 663210b092..adde50f289 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -25,12 +25,13 @@ module.exports = CompileController = if req.body?.compiler options.compiler = req.body.compiler logger.log {options, project_id}, "got compile request" - CompileManager.compile project_id, user_id, options, (error, status, outputFiles) -> + CompileManager.compile project_id, user_id, options, (error, status, outputFiles, output, limits) -> return next(error) if error? res.contentType("application/json") res.send 200, JSON.stringify { status: status outputFiles: outputFiles + compileGroup: limits.compileGroup } downloadPdf: (req, res, next = (error) ->)-> @@ -69,23 +70,30 @@ module.exports = CompileController = CompileController.proxyToClsi(req.params.Project_id, req.url, req, res, next) proxyToClsi: (project_id, url, req, res, next = (error) ->) -> - CompileManager.getProjectCompileLimits project_id, (error, limits) -> - return next(error) if error? - if limits.compileGroup == "priority" - compilerUrl = Settings.apis.clsi_priority.url - else - compilerUrl = Settings.apis.clsi.url - url = "#{compilerUrl}#{url}" - logger.log url: url, "proxying to CLSI" - oneMinute = 60 * 1000 - # pass through If-* and Range headers for byte serving pdfs - # do not send any others, potential proxying loop if Host: is passed! + if req.query?.compileGroup + CompileController.proxyToClsiWithLimits(project_id, url, {compileGroup: req.query.compileGroup}, req, res, next) + else + CompileManager.getProjectCompileLimits project_id, (error, limits) -> + return next(error) if error? + CompileController.proxyToClsiWithLimits(project_id, url, limits, req, res, next) + + proxyToClsiWithLimits: (project_id, url, limits, req, res, next = (error) ->) -> + if limits.compileGroup == "priority" + compilerUrl = Settings.apis.clsi_priority.url + else + compilerUrl = Settings.apis.clsi.url + url = "#{compilerUrl}#{url}" + logger.log url: url, "proxying to CLSI" + oneMinute = 60 * 1000 + # pass through If-* and Range headers for byte serving pdfs + # do not send any others, potential proxying loop if Host: is passed! + if req.query?.pdfng newHeaders = {} for h, v of req.headers newHeaders[h] = req.headers[h] if h.match /^(If-|Range)/i proxy = request(url: url, method: req.method, timeout: oneMinute, headers: newHeaders) - proxy.pipe(res) - proxy.on "error", (error) -> - logger.warn err: error, url: url, "CLSI proxy error" - - + else + proxy = request(url: url, method: req.method, timeout: oneMinute) + proxy.pipe(res) + proxy.on "error", (error) -> + logger.warn err: error, url: url, "CLSI proxy error" diff --git a/services/web/app/coffee/Features/Compile/CompileManager.coffee b/services/web/app/coffee/Features/Compile/CompileManager.coffee index d35322077b..0089eb7b34 100755 --- a/services/web/app/coffee/Features/Compile/CompileManager.coffee +++ b/services/web/app/coffee/Features/Compile/CompileManager.coffee @@ -39,7 +39,7 @@ module.exports = CompileManager = ClsiManager.sendRequest project_id, options, (error, status, outputFiles, output) -> return callback(error) if error? logger.log files: outputFiles, "output files" - callback(null, status, outputFiles, output) + callback(null, status, outputFiles, output, limits) deleteAuxFiles: (project_id, callback = (error) ->) -> CompileManager.getProjectCompileLimits project_id, (error, limits) -> diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index ee898dd8f7..610009bf8c 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -38,7 +38,12 @@ define [ $scope.pdf.failure = true fetchLogs() else if response.status == "success" - $scope.pdf.url = "/project/#{$scope.project_id}/output/output.pdf?cache_bust=#{Date.now()}" + if response.compileGroup? + $scope.pdf.compileGroup = response.compileGroup + $scope.pdf.url = "/project/#{$scope.project_id}/output/output.pdf?cache_bust=#{Date.now()}" + + "&compileGroup=#{$scope.pdf.compileGroup}" + else + $scope.pdf.url = "/project/#{$scope.project_id}/output/output.pdf?cache_bust=#{Date.now()}" fetchLogs() IGNORE_FILES = ["output.fls", "output.fdb_latexmk"] diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee index c9c696d087..7f3ac48250 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee @@ -26,7 +26,8 @@ define [ $scope.document.destroy() if $scope.document? - $scope.document = new PDFRenderer($scope.pdfSrc, { + # TODO need a proper url manipulation library to add to query string + $scope.document = new PDFRenderer($scope.pdfSrc + '&pdfng=true' , { scale: 1, navigateFn: (ref) -> # this function captures clicks on the annotation links From 79b560f96b2a09194d61025a4d27ced89eb3e29e Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 16 Dec 2014 09:44:25 +0000 Subject: [PATCH 4/4] update tests for new pdf viewer server fixes --- .../Features/Compile/CompileController.coffee | 2 +- .../Compile/CompileControllerTests.coffee | 176 +++++++++++++----- 2 files changed, 133 insertions(+), 45 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index adde50f289..b95a8ed82c 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -31,7 +31,7 @@ module.exports = CompileController = res.send 200, JSON.stringify { status: status outputFiles: outputFiles - compileGroup: limits.compileGroup + compileGroup: limits?.compileGroup } downloadPdf: (req, res, next = (error) ->)-> diff --git a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee index b3c004fdfe..fae9dec23f 100644 --- a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee @@ -139,59 +139,147 @@ describe "CompileController", -> @req.method = "mock-method" @req.headers = { 'Mock': 'Headers', - 'Range': 'should be passed - Range' - 'If-Range': 'should be passed - If-Range' - 'If-Modified-Since': 'should be passed - If-Modified-Since' + 'Range': '123-456' + 'If-Range': 'abcdef' + 'If-Modified-Since': 'Mon, 15 Dec 2014 15:23:56 GMT' } - describe "user with standard priority", -> + describe "old pdf viewer", -> + describe "user with standard priority", -> + beforeEach -> + @CompileManager.getProjectCompileLimits = sinon.stub().callsArgWith(1, null, {compileGroup: "standard"}) + @CompileController.proxyToClsi(@project_id, @url = "/test", @req, @res, @next) + it "should open a request to the CLSI", -> + @request + .calledWith( + method: @req.method + url: "#{@settings.apis.clsi.url}#{@url}", + timeout: 60 * 1000 + ) + .should.equal true + + it "should pass the request on to the client", -> + @proxy.pipe + .calledWith(@res) + .should.equal true + + it "should bind an error handle to the request proxy", -> + @proxy.on.calledWith("error").should.equal true + + describe "user with priority compile", -> + beforeEach -> + @CompileManager.getProjectCompileLimits = sinon.stub().callsArgWith(1, null, {compileGroup: "priority"}) + @CompileController.proxyToClsi(@project_id, @url = "/test", @req, @res, @next) + + it "should proxy to the priority url if the user has the feature", ()-> + @request + .calledWith( + method: @req.method + url: "#{@settings.apis.clsi_priority.url}#{@url}", + timeout: 60 * 1000 + ) + .should.equal true + + describe "user with standard priority via query string", -> + beforeEach -> + @req.query = {compileGroup: 'standard'} + @CompileController.proxyToClsi(@project_id, @url = "/test", @req, @res, @next) + + it "should open a request to the CLSI", -> + @request + .calledWith( + method: @req.method + url: "#{@settings.apis.clsi.url}#{@url}", + timeout: 60 * 1000 + ) + .should.equal true + + it "should pass the request on to the client", -> + @proxy.pipe + .calledWith(@res) + .should.equal true + + it "should bind an error handle to the request proxy", -> + @proxy.on.calledWith("error").should.equal true + + describe "user with priority compile via query string", -> + beforeEach -> + @req.query = {compileGroup: 'priority'} + @CompileController.proxyToClsi(@project_id, @url = "/test", @req, @res, @next) + + it "should proxy to the priority url if the user has the feature", ()-> + @request + .calledWith( + method: @req.method + url: "#{@settings.apis.clsi_priority.url}#{@url}", + timeout: 60 * 1000 + ) + .should.equal true + + describe "user with non-existent priority via query string", -> + beforeEach -> + @req.query = {compileGroup: 'foobar'} + @CompileController.proxyToClsi(@project_id, @url = "/test", @req, @res, @next) + + it "should proxy to the standard url", ()-> + @request + .calledWith( + method: @req.method + url: "#{@settings.apis.clsi.url}#{@url}", + timeout: 60 * 1000 + ) + .should.equal true + + + describe "new pdf viewer", -> beforeEach -> - @CompileManager.getProjectCompileLimits = sinon.stub().callsArgWith(1, null, {compileGroup: "standard"}) - @CompileController.proxyToClsi(@project_id, @url = "/test", @req, @res, @next) + @req.query = {pdfng: true} + describe "user with standard priority", -> + beforeEach -> + @CompileManager.getProjectCompileLimits = sinon.stub().callsArgWith(1, null, {compileGroup: "standard"}) + @CompileController.proxyToClsi(@project_id, @url = "/test", @req, @res, @next) + it "should open a request to the CLSI", -> + @request + .calledWith( + method: @req.method + url: "#{@settings.apis.clsi.url}#{@url}", + timeout: 60 * 1000 + headers: { + 'Range': '123-456' + 'If-Range': 'abcdef' + 'If-Modified-Since': 'Mon, 15 Dec 2014 15:23:56 GMT' + } + ) + .should.equal true - it "should open a request to the CLSI", -> - @request - .calledWith( - method: @req.method - url: "#{@settings.apis.clsi.url}#{@url}", - timeout: 60 * 1000 - headers: { - 'Range': 'should be passed - Range' - 'If-Range': 'should be passed - If-Range' - 'If-Modified-Since': 'should be passed - If-Modified-Since' - } - ) - .should.equal true + it "should pass the request on to the client", -> + @proxy.pipe + .calledWith(@res) + .should.equal true - it "should pass the request on to the client", -> - @proxy.pipe - .calledWith(@res) - .should.equal true + it "should bind an error handle to the request proxy", -> + @proxy.on.calledWith("error").should.equal true - it "should bind an error handle to the request proxy", -> - @proxy.on.calledWith("error").should.equal true + describe "user with priority compile", -> + beforeEach -> + @CompileManager.getProjectCompileLimits = sinon.stub().callsArgWith(1, null, {compileGroup: "priority"}) + @CompileController.proxyToClsi(@project_id, @url = "/test", @req, @res, @next) - describe "user with priority compile", -> - - beforeEach -> - @CompileManager.getProjectCompileLimits = sinon.stub().callsArgWith(1, null, {compileGroup: "priority"}) - @CompileController.proxyToClsi(@project_id, @url = "/test", @req, @res, @next) - - it "should proxy to the priorty url if the user has the feature", ()-> - @request - .calledWith( - method: @req.method - url: "#{@settings.apis.clsi_priority.url}#{@url}", - timeout: 60 * 1000 - headers: { - 'Range': 'should be passed - Range' - 'If-Range': 'should be passed - If-Range' - 'If-Modified-Since': 'should be passed - If-Modified-Since' - } - ) - .should.equal true + it "should proxy to the priority url if the user has the feature", ()-> + @request + .calledWith( + method: @req.method + url: "#{@settings.apis.clsi_priority.url}#{@url}", + timeout: 60 * 1000 + headers: { + 'Range': '123-456' + 'If-Range': 'abcdef' + 'If-Modified-Since': 'Mon, 15 Dec 2014 15:23:56 GMT' + } + ) + .should.equal true describe "deleteAuxFiles", ->