From 6f656f6e7d2af94fae9e6b10b54b9285fb6844b1 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 29 Jan 2019 16:47:25 -0500 Subject: [PATCH] FIX: Better error handling if a file cannot be sent If for some reason `Discourse.store.path_for` returns `nil`, the forum would throw an error rather than returning 404. Why would it be `nil`? One cause could be changing the type of file store and having the `url` field no longer be relative. --- app/controllers/uploads_controller.rb | 6 +++++- spec/requests/uploads_controller_spec.rb | 8 ++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 64d08d4e1a9..481d5d9c29b 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -75,7 +75,11 @@ class UploadsController < ApplicationController } opts[:disposition] = "inline" if params[:inline] opts[:disposition] ||= "attachment" unless FileHelper.is_supported_image?(upload.original_filename) - send_file(Discourse.store.path_for(upload), opts) + + file_path = Discourse.store.path_for(upload) + return render_404 unless file_path + + send_file(file_path, opts) else render_404 end diff --git a/spec/requests/uploads_controller_spec.rb b/spec/requests/uploads_controller_spec.rb index db79aabdadf..9128d8515ec 100644 --- a/spec/requests/uploads_controller_spec.rb +++ b/spec/requests/uploads_controller_spec.rb @@ -226,6 +226,14 @@ describe UploadsController do expect(response.status).to eq(404) end + it "returns 404 when the path is nil" do + upload = upload_file("logo.png") + upload.update_column(:url, "invalid-url") + + get "/uploads/#{site}/#{upload.sha1}.#{upload.extension}" + expect(response.status).to eq(404) + end + it 'uses send_file' do upload = upload_file("logo.png") get "/uploads/#{site}/#{upload.sha1}.#{upload.extension}"