From cf48c9472573c9c84afea4c5808dc66d83fa8225 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Fri, 30 Oct 2015 11:35:11 +0000 Subject: [PATCH] rate limit pdf downloads --- .../Features/Compile/CompileController.coffee | 27 +++++++++++- .../Compile/CompileControllerTests.coffee | 43 +++++++++++++++---- 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/CompileController.coffee b/services/web/app/coffee/Features/Compile/CompileController.coffee index 36dd4bb5e6..83a0b3b4b7 100755 --- a/services/web/app/coffee/Features/Compile/CompileController.coffee +++ b/services/web/app/coffee/Features/Compile/CompileController.coffee @@ -7,6 +7,7 @@ request = require "request" Settings = require "settings-sharelatex" AuthenticationController = require "../Authentication/AuthenticationController" UserGetter = require "../User/UserGetter" +RateLimiter = require("../../infrastructure/RateLimiter") module.exports = CompileController = compile: (req, res, next = (error) ->) -> @@ -35,8 +36,11 @@ module.exports = CompileController = } downloadPdf: (req, res, next = (error) ->)-> + Metrics.inc "pdf-downloads" project_id = req.params.Project_id + isPdfjsPartialDownload = req.query?.pdfng + Project.findById project_id, {name: 1}, (err, project)-> res.contentType("application/pdf") if !!req.query.popupDownload @@ -45,7 +49,28 @@ module.exports = CompileController = else logger.log project_id: project_id, "download pdf to embed in browser" res.header('Content-Disposition', "filename=#{project.getSafeProjectName()}.pdf") - CompileController.proxyToClsi(project_id, "/project/#{project_id}/output/output.pdf", req, res, next) + + + if isPdfjsPartialDownload + rateLimitOpts = + endpointName: "partial-pdf-download" + throttle: 500 + else + rateLimitOpts = + endpointName: "full-pdf-download" + throttle: 50 + + rateLimitOpts.subjectName = req.ip + rateLimitOpts.timeInterval = 60 * 5 + RateLimiter.addCount rateLimitOpts, (err, canContinue)-> + if err? + logger.err err:err, "error checking rate limit for pdf download" + return res.send 500 + else if !canContinue + 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) deleteAuxFiles: (req, res, next) -> project_id = req.params.Project_id diff --git a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee index 481650dfc4..d40d10e77c 100644 --- a/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/CompileControllerTests.coffee @@ -15,13 +15,15 @@ describe "CompileController", -> @ClsiManager = {} @UserGetter = getUser:sinon.stub() + @RateLimiter = {addCount:sinon.stub()} + @settings = + apis: + clsi: + url: "clsi.example.com" + clsi_priority: + url: "clsi-priority.example.com" @CompileController = SandboxedModule.require modulePath, requires: - "settings-sharelatex": @settings = - apis: - clsi: - url: "clsi.example.com" - clsi_priority: - url: "clsi-priority.example.com" + "settings-sharelatex": @settings "request": @request = sinon.stub() "../../models/Project": Project: @Project = {} "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() } @@ -30,6 +32,7 @@ describe "CompileController", -> "../User/UserGetter":@UserGetter "./ClsiManager": @ClsiManager "../Authentication/AuthenticationController": @AuthenticationController = {} + "../../infrastructure/RateLimiter":@RateLimiter @project_id = "project-id" @user = features: @@ -94,11 +97,13 @@ describe "CompileController", -> @project = getSafeProjectName: () => @safe_name = "safe-name" + @req.query = {pdfng:true} @Project.findById = sinon.stub().callsArgWith(2, null, @project) describe "when downloading for embedding", -> beforeEach -> @CompileController.proxyToClsi = sinon.stub() + @RateLimiter.addCount.callsArgWith(1, null, true) @CompileController.downloadPdf(@req, @res, @next) it "should look up the project", -> @@ -122,9 +127,29 @@ describe "CompileController", -> .should.equal true it "should proxy the PDF from the CLSI", -> - @CompileController.proxyToClsi - .calledWith(@project_id, "/project/#{@project_id}/output/output.pdf", @req, @res, @next) - .should.equal true + @CompileController.proxyToClsi.calledWith(@project_id, "/project/#{@project_id}/output/output.pdf", @req, @res, @next).should.equal true + + it "should check the rate limiter", -> + @RateLimiter.addCount.args[0][0].throttle.should.equal 500 + + describe "when the pdf is not going to be used in pdfjs viewer", -> + + it "should check the rate limiter when pdfng is not set", (done)-> + @req.query = {} + @RateLimiter.addCount.callsArgWith(1, null, true) + @CompileController.proxyToClsi = (project_id, url)=> + @RateLimiter.addCount.args[0][0].throttle.should.equal 50 + done() + @CompileController.downloadPdf @req, @res + + + it "should check the rate limiter when pdfng is false", (done)-> + @req.query = {pdfng:false} + @RateLimiter.addCount.callsArgWith(1, null, true) + @CompileController.proxyToClsi = (project_id, url)=> + @RateLimiter.addCount.args[0][0].throttle.should.equal 50 + done() + @CompileController.downloadPdf @req, @res describe "proxyToClsi", -> beforeEach ->