diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 2b0545dfa..40da4f273 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -29,14 +29,10 @@ class UploadsController < ApplicationController return render_404 unless Discourse.store.internal? return render_404 if SiteSetting.prevent_anons_from_downloading_files && current_user.nil? - id = params[:id].to_i - url = request.fullpath - - # the "url" parameter is here to prevent people from scanning the uploads using the id - if upload = (Upload.find_by(id: id, url: url) || Upload.find_by(sha1: params[:sha])) - opts = {filename: upload.original_filename} + if upload = Upload.find_by(sha1: params[:sha]) + opts = { filename: upload.original_filename } opts[:disposition] = 'inline' if params[:inline] - send_file(Discourse.store.path_for(upload),opts) + send_file(Discourse.store.path_for(upload), opts) else render_404 end diff --git a/app/serializers/upload_serializer.rb b/app/serializers/upload_serializer.rb index 1d3812e20..9e0d86624 100644 --- a/app/serializers/upload_serializer.rb +++ b/app/serializers/upload_serializer.rb @@ -1,5 +1,5 @@ class UploadSerializer < ApplicationSerializer - attributes :url, :original_filename, :filesize, :width, :height + attributes :id, :url, :original_filename, :filesize, :width, :height end diff --git a/config/nginx.sample.conf b/config/nginx.sample.conf index ca7f2d1cb..6dbd5a32d 100644 --- a/config/nginx.sample.conf +++ b/config/nginx.sample.conf @@ -136,7 +136,7 @@ server { try_files $uri =404; } # thumbnails & optimized images - location ~ /_optimized/ { + location ~ /_?optimized/ { try_files $uri =404; } diff --git a/config/routes.rb b/config/routes.rb index 7fe30be8f..3af436fad 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -289,8 +289,7 @@ Discourse::Application.routes.draw do get "stylesheets/:name.css" => "stylesheets#show", constraints: {name: /[a-z0-9_]+/} - get "uploads/:site/:id/:sha.:extension" => "uploads#show", constraints: {site: /\w+/, id: /\d+/, sha: /[a-z0-9]{15,16}/i, extension: /\w{2,}/} - get "uploads/:site/:sha" => "uploads#show", constraints: { site: /\w+/, sha: /[a-z0-9]{40}/} + get "uploads/:site/:sha" => "uploads#show", constraints: { site: /\w+/, sha: /[a-f0-9]{40}/} post "uploads" => "uploads#create" get "posts" => "posts#latest" diff --git a/lib/file_store/local_store.rb b/lib/file_store/local_store.rb index 9fbffa800..2fce96c36 100644 --- a/lib/file_store/local_store.rb +++ b/lib/file_store/local_store.rb @@ -51,10 +51,6 @@ module FileStore "#{public_dir}#{upload.url}" end - def avatar_template(avatar) - relative_avatar_template(avatar) - end - def purge_tombstone(grace_period) `find #{tombstone_dir} -mtime +#{grace_period} -type f -delete` end @@ -62,33 +58,16 @@ module FileStore private def get_path_for_upload(file, upload) - unique_sha1 = Digest::SHA1.hexdigest("#{Time.now}#{upload.original_filename}")[0..15] - extension = File.extname(upload.original_filename) - clean_name = "#{unique_sha1}#{extension}" - # path - "#{relative_base_url}/#{upload.id}/#{clean_name}" + get_path_for("original".freeze, upload.sha1, upload.extension) end def get_path_for_optimized_image(file, optimized_image) - # 1234567890ABCDEF_100x200.jpg - filename = [ - optimized_image.sha1[6..15], - "_#{optimized_image.width}x#{optimized_image.height}", - optimized_image.extension, - ].join - # path - "#{relative_base_url}/_optimized/#{optimized_image.sha1[0..2]}/#{optimized_image.sha1[3..5]}/#{filename}" + extension = "_#{optimized_image.width}x#{optimized_image.height}#{optimized_image.extension}" + get_path_for("optimized".freeze, optimized_image.sha1, extension) end - def relative_avatar_template(avatar) - File.join( - relative_base_url, - "avatars", - avatar.sha1[0..2], - avatar.sha1[3..5], - avatar.sha1[6..15], - "{size}#{avatar.extension}" - ) + def get_path_for(type, sha, extension) + "#{relative_base_url}/#{type}/#{sha[0]}/#{sha[1]}/#{sha}#{extension}" end def store_file(file, path) diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb index e9b70107c..33403f154 100644 --- a/spec/components/cooked_post_processor_spec.rb +++ b/spec/components/cooked_post_processor_spec.rb @@ -118,7 +118,7 @@ describe CookedPostProcessor do it "generates overlay information" do cpp.post_process_images - expect(cpp.html).to match_html '
+ expect(cpp.html).to match_html '
logo.png1000x2000 1.21 KB
' expect(cpp).to be_dirty @@ -145,7 +145,7 @@ describe CookedPostProcessor do it "generates overlay information" do cpp.post_process_images - expect(cpp.html).to match_html '
+ expect(cpp.html).to match_html '
WAT1000x2000 1.21 KB
' expect(cpp).to be_dirty diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index ec0d69e0b..ee4693726 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -309,7 +309,7 @@ This is a link http://example.com" receiver.process expect(topic.posts.count).to eq(start_count + 1) - expect(topic.posts.last.cooked).to match // + expect(topic.posts.last.cooked).to match // expect(Upload.find_by(sha1: upload_sha)).not_to eq(nil) end diff --git a/spec/components/file_store/local_store_spec.rb b/spec/components/file_store/local_store_spec.rb index 8cc2d2ea5..f72ebdf28 100644 --- a/spec/components/file_store/local_store_spec.rb +++ b/spec/components/file_store/local_store_spec.rb @@ -18,7 +18,7 @@ describe FileStore::LocalStore do Time.stubs(:now).returns(Time.utc(2013, 2, 17, 12, 0, 0, 0)) upload.stubs(:id).returns(42) store.expects(:copy_file) - expect(store.store_upload(uploaded_file, upload)).to eq("/uploads/default/42/253dc8edf9d4ada1.png") + expect(store.store_upload(uploaded_file, upload)).to eq("/uploads/default/original/e/9/e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98.png") end end @@ -27,7 +27,7 @@ describe FileStore::LocalStore do it "returns a relative url" do store.expects(:copy_file) - expect(store.store_optimized_image({}, optimized_image)).to eq("/uploads/default/_optimized/86f/7e4/37faa5a7fc_100x200.png") + expect(store.store_optimized_image({}, optimized_image)).to eq("/uploads/default/optimized/8/6/86f7e437faa5a7fce15d1ddcb9eaeaea377667b8_100x200.png") end end @@ -109,12 +109,4 @@ describe FileStore::LocalStore do expect(store.external?).to eq(false) end - describe ".avatar_template" do - - it "is present" do - expect(store.avatar_template(avatar)).to eq("/uploads/default/avatars/e9d/71f/5ee7c92d6d/{size}.png") - end - - end - end diff --git a/spec/components/file_store/s3_store_spec.rb b/spec/components/file_store/s3_store_spec.rb index 8b78803d9..b55e851ab 100644 --- a/spec/components/file_store/s3_store_spec.rb +++ b/spec/components/file_store/s3_store_spec.rb @@ -102,14 +102,6 @@ describe FileStore::S3Store do end - describe ".avatar_template" do - - it "is present" do - expect(store.avatar_template(avatar)).to eq("//s3_upload_bucket.s3.amazonaws.com/avatars/e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98/{size}.png") - end - - end - describe ".purge_tombstone" do it "updates tombstone lifecycle" do diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index feef3a60a..0cd03ea57 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -54,8 +54,7 @@ describe UploadsController do it 'correctly sets retain_hours for admins' do log_in :admin xhr :post, :create, file: logo, retain_hours: 100 - url = JSON.parse(response.body)["url"] - id = url.split("/")[3].to_i + id = JSON.parse(response.body)["id"] expect(Upload.find(id).retain_hours).to eq(100) end @@ -121,30 +120,33 @@ describe UploadsController do context '.show' do + let(:site) { "default" } + let(:sha) { Digest::SHA1.hexdigest("discourse") } + it "returns 404 when using external storage" do store = stub(internal?: false) Discourse.stubs(:store).returns(store) Upload.expects(:find_by).never - get :show, site: "default", id: 1, sha: "1234567890abcdef", extension: "pdf" + + get :show, site: site, sha: sha, extension: "pdf" expect(response.response_code).to eq(404) end it "returns 404 when the upload doens't exist" do - Upload.expects(:find_by).with(id: 2, url: "/uploads/default/2/1234567890abcdef.pdf").returns(nil) - Upload.expects(:find_by).with(sha1: "1234567890abcdef").returns(nil) + Upload.expects(:find_by).with(sha1: sha).returns(nil) - get :show, site: "default", id: 2, sha: "1234567890abcdef", extension: "pdf" + get :show, site: site, sha: sha, extension: "pdf" expect(response.response_code).to eq(404) end it 'uses send_file' do upload = build(:upload) - Upload.expects(:find_by).with(id: 42, url: "/uploads/default/42/66b3ed1503efc936.zip").returns(upload) + Upload.expects(:find_by).with(sha1: sha).returns(upload) controller.stubs(:render) controller.expects(:send_file) - get :show, site: "default", id: 42, sha: "66b3ed1503efc936", extension: "zip" + get :show, site: site, sha: sha, extension: "zip" end context "prevent anons from downloading files" do @@ -153,7 +155,8 @@ describe UploadsController do it "returns 404 when an anonymous user tries to download a file" do Upload.expects(:find_by).never - get :show, site: "default", id: 2, sha: "1234567890abcdef", extension: "pdf" + + get :show, site: site, sha: sha, extension: "pdf" expect(response.response_code).to eq(404) end