codeninjasllc/discourse
1
0
Fork 0
mirror of https://github.com/codeninjasllc/discourse.git synced 2024-11-27 09:36:19 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions

FIX: consistent and future-proof upload storage pattern

Browse source
This commit is contained in:
Régis Hanol 2015-05-19 12:31:12 +02:00
parent 666b5b40a5
commit 9ded21e4c6
10 changed files with 28 additions and 67 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
app/controllers/uploads_controller.rb
View file

@ -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

2
app/serializers/upload_serializer.rb
View file

@ -1,5 +1,5 @@
class UploadSerializer < ApplicationSerializer
attributes :url, :original_filename, :filesize, :width, :height
attributes :id, :url, :original_filename, :filesize, :width, :height
end

2
config/nginx.sample.conf
View file

@ -136,7 +136,7 @@ server {
try_files $uri =404;
}
# thumbnails & optimized images
location ~ /_optimized/ {
location ~ /_?optimized/ {
try_files $uri =404;
}

3
config/routes.rb
View file

@ -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"

31
lib/file_store/local_store.rb
View file

@ -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)

4
spec/components/cooked_post_processor_spec.rb
View file

@ -118,7 +118,7 @@ describe CookedPostProcessor do
it "generates overlay information" do
cpp.post_process_images
expect(cpp.html).to match_html '<div class="lightbox-wrapper"><a data-download-href="/uploads/default/e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98" href="/uploads/default/1/1234567890123456.jpg" class="lightbox" title="logo.png"><img src="/uploads/default/_optimized/da3/9a3/ee5e6b4b0d_690x1380.png" width="690" height="1380"><div class="meta">
expect(cpp.html).to match_html '<div class="lightbox-wrapper"><a data-download-href="/uploads/default/e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98" href="/uploads/default/1/1234567890123456.jpg" class="lightbox" title="logo.png"><img src="/uploads/default/optimized/d/a/da39a3ee5e6b4b0d3255bfef95601890afd80709_690x1380.png" width="690" height="1380"><div class="meta">
<span class="filename">logo.png</span><span class="informations">1000x2000 1.21 KB</span><span class="expand"></span>
</div></a></div>'
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 '<div class="lightbox-wrapper"><a data-download-href="/uploads/default/e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98" href="/uploads/default/1/1234567890123456.jpg" class="lightbox" title="WAT"><img src="/uploads/default/_optimized/da3/9a3/ee5e6b4b0d_690x1380.png" title="WAT" width="690" height="1380"><div class="meta">
expect(cpp.html).to match_html '<div class="lightbox-wrapper"><a data-download-href="/uploads/default/e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98" href="/uploads/default/1/1234567890123456.jpg" class="lightbox" title="WAT"><img src="/uploads/default/optimized/d/a/da39a3ee5e6b4b0d3255bfef95601890afd80709_690x1380.png" title="WAT" width="690" height="1380"><div class="meta">
<span class="filename">WAT</span><span class="informations">1000x2000 1.21 KB</span><span class="expand"></span>
</div></a></div>'
expect(cpp).to be_dirty

2
spec/components/email/receiver_spec.rb
View file

@ -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 /<img src=['"](\/uploads\/default\/\d+\/\w{16}\.png)['"] width=['"]289['"] height=['"]126['"]>/
expect(topic.posts.last.cooked).to match /<img src=['"](\/uploads\/default\/original\/#{upload_sha[0]}\/#{upload_sha[1]}\/#{upload_sha}\.png)['"] width=['"]289['"] height=['"]126['"]>/
expect(Upload.find_by(sha1: upload_sha)).not_to eq(nil)
end

12
spec/components/file_store/local_store_spec.rb
View file

@ -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

8
spec/components/file_store/s3_store_spec.rb
View file

@ -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

21
spec/controllers/uploads_controller_spec.rb
View file

@ -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