diff --git a/services/web/app/coffee/Features/LinkedFiles/LinkedFilesController.coffee b/services/web/app/coffee/Features/LinkedFiles/LinkedFilesController.coffee index dd0b088674..f2566beb5c 100644 --- a/services/web/app/coffee/Features/LinkedFiles/LinkedFilesController.coffee +++ b/services/web/app/coffee/Features/LinkedFiles/LinkedFilesController.coffee @@ -18,7 +18,8 @@ module.exports = LinkedFilesController = { linkedFileData = Agent.sanitizeData(data) linkedFileData.provider = provider Agent.writeIncomingFileToDisk project_id, linkedFileData, user_id, (error, fsPath) -> - return next(error) if error? + if error? + return Agent.handleError(error, req, res, next) EditorController.upsertFile project_id, parent_folder_id, name, fsPath, linkedFileData, "upload", user_id, (error) -> return next(error) if error? res.send(204) # created diff --git a/services/web/app/coffee/Features/LinkedFiles/UrlAgent.coffee b/services/web/app/coffee/Features/LinkedFiles/UrlAgent.coffee index 7d7b30ff80..98e176bc06 100644 --- a/services/web/app/coffee/Features/LinkedFiles/UrlAgent.coffee +++ b/services/web/app/coffee/Features/LinkedFiles/UrlAgent.coffee @@ -1,17 +1,62 @@ request = require 'request' FileWriter = require('../../infrastructure/FileWriter') +_ = require "underscore" +urlValidator = require 'valid-url' + +UrlFetchFailedError = (message) -> + error = new Error(message) + error.name = 'UrlFetchFailedError' + error.__proto__ = UrlFetchFailedError.prototype + return error +UrlFetchFailedError.prototype.__proto__ = Error.prototype + +InvalidUrlError = (message) -> + error = new Error(message) + error.name = 'InvalidUrlError' + error.__proto__ = InvalidUrlError.prototype + return error +InvalidUrlError.prototype.__proto__ = Error.prototype + module.exports = UrlAgent = { + UrlFetchFailedError: UrlFetchFailedError + InvalidUrlError: InvalidUrlError + sanitizeData: (data) -> return { url: data.url } + _prependHttpIfNeeded: (url) -> + if !url.match('://') + url = 'http://' + url + return url + writeIncomingFileToDisk: (project_id, data, current_user_id, callback = (error, fsPath) ->) -> - # TODO: Check it's a valid URL # TODO: Proxy through external API - # TODO: Error unless valid status code - url = data.url + callback = _.once(callback) + url = @._prependHttpIfNeeded(data.url) + if !urlValidator.isWebUri(url) + return callback(new InvalidUrlError()) readStream = request.get(url) - FileWriter.writeStreamToDisk project_id, readStream, callback + readStream.on "error", callback + readStream.on "response", (response) -> + if 200 <= response.statusCode < 300 + FileWriter.writeStreamToDisk project_id, readStream, callback + else + error = new UrlFetchFailedError() + error.statusCode = response.statusCode + callback(error) + + handleError: (error, req, res, next) -> + if error instanceof UrlFetchFailedError + res.status(422).send( + "Your URL could not be reached (#{error.statusCode} status code). Please check it and try again." + ) + else if error instanceof InvalidUrlError + res.status(422).send( + "Your URL is not valid. Please check it and try again." + ) + else + next(error) } \ No newline at end of file diff --git a/services/web/package.json b/services/web/package.json index ffb75186b7..485450807d 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -85,6 +85,7 @@ "underscore": "1.6.0", "uuid": "^3.0.1", "v8-profiler": "^5.2.3", + "valid-url": "^1.0.9", "xml2js": "0.2.0", "yauzl": "^2.8.0" }, diff --git a/services/web/test/acceptance/coffee/LinkedFilesTests.coffee b/services/web/test/acceptance/coffee/LinkedFilesTests.coffee index f492f0ba52..2c16a0f0e0 100644 --- a/services/web/test/acceptance/coffee/LinkedFilesTests.coffee +++ b/services/web/test/acceptance/coffee/LinkedFilesTests.coffee @@ -1,5 +1,6 @@ async = require "async" expect = require("chai").expect +_ = require 'underscore' MockFileStoreApi = require './helpers/MockFileStoreApi' MockURLSource = require './helpers/MockURLSource' @@ -92,3 +93,87 @@ describe "LinkedFiles", -> expect(response.statusCode).to.equal 200 expect(body).to.equal "bar bar bar" done() + + it "should return an error if the URL does not succeed", (done) -> + @owner.request.post { + url: "/project/#{@project_id}/linked_file", + json: + provider: 'url' + data: { + url: "http://localhost:6543/does-not-exist" + } + parent_folder_id: @root_folder_id + name: 'url-test-file-3' + }, (error, response, body) => + throw error if error? + expect(response.statusCode).to.equal 422 # unprocessable + expect(body).to.equal( + "Your URL could not be reached (404 status code). Please check it and try again." + ) + done() + + it "should return an error if the URL is invalid", (done) -> + @owner.request.post { + url: "/project/#{@project_id}/linked_file", + json: + provider: 'url' + data: { + url: "!^$%" + } + parent_folder_id: @root_folder_id + name: 'url-test-file-4' + }, (error, response, body) => + throw error if error? + expect(response.statusCode).to.equal 422 # unprocessable + expect(body).to.equal( + "Your URL is not valid. Please check it and try again." + ) + done() + + it "should return an error if the URL uses a non-http protocol", (done) -> + @owner.request.post { + url: "/project/#{@project_id}/linked_file", + json: + provider: 'url' + data: { + url: "ftp://localhost" + } + parent_folder_id: @root_folder_id + name: 'url-test-file-5' + }, (error, response, body) => + throw error if error? + expect(response.statusCode).to.equal 422 # unprocessable + expect(body).to.equal( + "Your URL is not valid. Please check it and try again." + ) + done() + + it "should accept a URL withuot a leading http://, and add it", (done) -> + @owner.request.post { + url: "/project/#{@project_id}/linked_file", + json: + provider: 'url' + data: { + url: "http://localhost:6543/foo" + } + parent_folder_id: @root_folder_id + name: 'url-test-file-6' + }, (error, response, body) => + throw error if error? + expect(response.statusCode).to.equal 204 + @owner.getProject @project_id, (error, project) => + throw error if error? + file = _.find project.rootFolder[0].fileRefs, (file) -> + file.name == 'url-test-file-6' + expect(file.linkedFileData).to.deep.equal({ + provider: 'url' + url: "http://localhost:6543/foo" + }) + @owner.request.get "/project/#{@project_id}/file/#{file._id}", (error, response, body) -> + throw error if error? + expect(response.statusCode).to.equal 200 + expect(body).to.equal "foo foo foo" + done() + + # TODO: Add test for asking for host that return ENOTFOUND + # (This will probably end up handled by the proxy) \ No newline at end of file