Просмотр исходного кода

Ensure that Filestore responds with 404 when a file does not exist.

Shane Kilkelly 11 лет назад
Родитель
Сommit
47fdf3c5c9

+ 9 - 0
services/filestore/app/coffee/Errors.coffee

@@ -0,0 +1,9 @@
+NotFoundError = (message) ->
+	error = new Error(message)
+	error.name = "NotFoundError"
+	error.__proto__ = NotFoundError.prototype
+	return error
+NotFoundError.prototype.__proto__ = Error.prototype
+
+module.exports = Errors =
+	NotFoundError: NotFoundError

+ 3 - 3
services/filestore/app/coffee/FSPersistorManager.coffee

@@ -1,12 +1,12 @@
 logger = require("logger-sharelatex")
 fs = require("fs")
 LocalFileWriter = require("./LocalFileWriter")
+Errors = require('./Errors')
 rimraf = require("rimraf")
-response = require ("response")
 
 filterName = (key) ->
   return key.replace /\//g, "_"
-  
+
 
 module.exports =
   sendFile: ( location, target, source, callback = (err)->) ->
@@ -38,7 +38,7 @@ module.exports =
     sourceStream.on 'error', (err) ->
       logger.err err:err, location:location, name:name, "Error reading from file"
       if err.code = 'ENOENT'
-        callback null, response().html('NoSuchKey: file not found\n')
+        callback new Errors.NotFoundError(err.message), null
       else
         callback err
     sourceStream.on 'readable', () ->

+ 4 - 1
services/filestore/app/coffee/FileController.coffee

@@ -4,6 +4,7 @@ logger = require("logger-sharelatex")
 FileHandler = require("./FileHandler")
 metrics = require("metrics-sharelatex")
 parseRange = require('range-parser')
+Errors = require('./Errors')
 
 oneDayInSeconds = 60 * 60 * 24
 maxSizeInBytes = 1024 * 1024 * 1024 # 1GB
@@ -29,8 +30,10 @@ module.exports = FileController =
 		FileHandler.getFile bucket, key, options, (err, fileStream)->
 			if err?
 				logger.err err:err, key:key, bucket:bucket, format:format, style:style, "problem getting file"
+				if err instanceof Errors.NotFoundError
+					return res.send 404
 				if !res.finished and res?.send?
-					res.send 500
+					return res.send 500
 			else if req.query.cacheWarm
 				logger.log key:key, bucket:bucket, format:format, style:style, "request is only for cache warm so not sending stream"
 				res.send 200

+ 9 - 1
services/filestore/test/acceptence/coffee/SendingFileTest.coffee

@@ -50,6 +50,14 @@ describe "Filestore", ->
 			writeStream.on "end", done
 			fs.createReadStream(@localFileReadPath).pipe writeStream
 
+		it "should return 404 for a non-existant id", (done) ->
+			@timeout(1000 * 20)
+			options =
+				uri: @fileUrl + '___this_is_clearly_wrong___'
+			request.get options, (err, response, body) =>
+				response.statusCode.should.equal 404
+				done()
+
 		it "should be able get the file back", (done)->
 			@timeout(1000 * 10)
 			request.get @fileUrl, (err, response, body)=>
@@ -81,7 +89,7 @@ describe "Filestore", ->
 			request.del @fileUrl, (err, response, body)=>
 				response.statusCode.should.equal 204
 				request.get @fileUrl, (err, response, body)=>
-					body.indexOf("NoSuchKey").should.not.equal -1
+					response.statusCode.should.equal 404
 					done()
 
 		it "should be able to copy files", (done)->