mirrors
/
rails
mirror of https://github.com/rails/rails
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #37897 from peterzhu2118/as-public-disk-blob-fixes 

Use DiskController for both public and private files
This commit is contained in:
Gannon McGibbon 2019-12-06 16:41:37 -05:00 committed by GitHub
parent 2963a30cf9 fbb83d78c3
commit 8cf3f9ad23
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 53 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
activestorage/CHANGELOG.md
View File

@ -1,3 +1,10 @@
* Use `DiskController` for both public and private files.
`DiskController` is able to handle multiple services by adding a
`service_name` field in the generated URL in `DiskService`.
*Peter Zhu*
* Variants are tracked in the database to avoid existence checks in the storage service.
*George Claghorn*

10
activestorage/app/controllers/active_storage/disk_controller.rb
View File

@ -11,7 +11,7 @@ class ActiveStorage::DiskController < ActiveStorage::BaseController
def show
if key = decode_verified_key
serve_file disk_service.path_for(key[:key]), content_type: key[:content_type], disposition: key[:disposition]
serve_file named_disk_service(key[:service_name]).path_for(key[:key]), content_type: key[:content_type], disposition: key[:disposition]
else
head :not_found
end
@ -22,7 +22,7 @@ class ActiveStorage::DiskController < ActiveStorage::BaseController
def update
if token = decode_verified_token
if acceptable_content?(token)
disk_service.upload token[:key], request.body, checksum: token[:checksum]
named_disk_service(token[:service_name]).upload token[:key], request.body, checksum: token[:checksum]
else
head :unprocessable_entity
end
@ -34,8 +34,10 @@ class ActiveStorage::DiskController < ActiveStorage::BaseController
end
private
def disk_service
ActiveStorage::Blob.service
def named_disk_service(name)
ActiveStorage::Blob.services.fetch(name) do
ActiveStorage::Blob.service
end
end

20
activestorage/app/controllers/active_storage/public_disk_controller.rb
View File

@ -1,20 +0,0 @@
# frozen_string_literal: true
# Serves files stored in a public disk service using a permanent URL.
class ActiveStorage::PublicDiskController < ActiveStorage::BaseController
include ActiveStorage::FileServer
skip_forgery_protection
def show
if blob = ActiveStorage::Blob.find_by(key: params[:key])
if blob.service.public?
serve_file blob.service.path_for(blob.key), content_type: blob.content_type, disposition: :inline
else
head :unauthorized
end
else
head :not_found
end
end
end

1
activestorage/config/routes.rb
View File

@ -6,7 +6,6 @@ Rails.application.routes.draw do
get "/representations/:signed_blob_id/:variation_key/*filename" => "active_storage/representations#show", as: :rails_blob_representation
get "/disk/public/:key/*filename" => "active_storage/public_disk#show", as: :rails_disk_service_public
get "/disk/:encoded_key/*filename" => "active_storage/disk#show", as: :rails_disk_service
put "/disk/:encoded_token" => "active_storage/disk#update", as: :update_rails_disk_service
post "/direct_uploads" => "active_storage/direct_uploads#create", as: :rails_direct_uploads

25
activestorage/lib/active_storage/service/disk_service.rb
View File

@ -79,7 +79,8 @@ module ActiveStorage
key: key,
content_type: content_type,
content_length: content_length,
checksum: checksum
checksum: checksum,
service_name: name
},
expires_in: expires_in,
purpose: :blob_token
@ -103,12 +104,21 @@ module ActiveStorage
private
def private_url(key, expires_in:, filename:, content_type:, disposition:, **)
generate_url(key, expires_in: expires_in, filename: filename, content_type: content_type, disposition: disposition)
end
def public_url(key, filename:, content_type: nil, disposition: :attachment, **)
generate_url(key, expires_in: nil, filename: filename, content_type: content_type, disposition: disposition)
end
def generate_url(key, expires_in:, filename:, content_type:, disposition:)
content_disposition = content_disposition_with(type: disposition, filename: filename)
verified_key_with_expiration = ActiveStorage.verifier.generate(
{
key: key,
disposition: content_disposition,
content_type: content_type
content_type: content_type,
service_name: name
},
expires_in: expires_in,
purpose: :blob_key
@ -126,17 +136,6 @@ module ActiveStorage
)
end
def public_url(key, filename:, content_type: nil, disposition: :attachment, **)
current_uri = URI.parse(current_host)
url_helpers.rails_disk_service_public_url(key,
protocol: current_uri.scheme,
host: current_uri.host,
port: current_uri.port,
filename: filename
)
end
def stream(key)
File.open(path_for(key), "rb") do |file|

21
activestorage/test/controllers/disk_controller_test.rb
View File

@ -44,6 +44,27 @@ class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest
assert_response :not_found
end
test "showing public blob" do
with_service("local_public") do
blob = create_blob(content_type: "image/jpg")
get blob.url
assert_response :ok
assert_equal "image/jpg", response.headers["Content-Type"]
assert_equal "Hello world!", response.body
end
end
test "showing public blob variant" do
with_service("local_public") do
blob = create_file_blob.variant(resize_to_limit: [100, 100]).processed
get blob.url
assert_response :ok
assert_equal "image/jpeg", response.headers["Content-Type"]
end
end
test "directly uploading blob with integrity" do
data = "Something else entirely!"
blob = create_blob_before_direct_upload byte_size: data.size, checksum: Digest::MD5.base64digest(data)

33
activestorage/test/controllers/public_disk_controller_test.rb
View File

@ -1,33 +0,0 @@
# frozen_string_literal: true
require "test_helper"
require "database/setup"
class ActiveStorage::PublicDiskControllerTest < ActionDispatch::IntegrationTest
test "showing public blob" do
with_service("local_public") do
blob = create_blob(content_type: "image/jpg")
get blob.url
assert_response :ok
assert_equal "image/jpg", response.headers["Content-Type"]
assert_equal "Hello world!", response.body
end
end
test "showing private blob as a public blob" do
blob = create_blob(content_type: "image/jpg")
with_service("local_public") do
get rails_disk_service_public_url(key: blob.key, filename: "foo.jpg")
assert_response :unauthorized
end
end
test "showing public blob with invalid key" do
with_service("local_public") do
get rails_disk_service_public_url(key: "abc123", filename: "foo.jpg")
assert_response :not_found
end
end
end

4
activestorage/test/models/blob_test.rb
View File

@ -261,12 +261,12 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
end
private
def expected_url_for(blob, disposition: :attachment, filename: nil, content_type: nil)
def expected_url_for(blob, disposition: :attachment, filename: nil, content_type: nil, service_name: :local)
filename ||= blob.filename
content_type ||= blob.content_type
query = { disposition: ActionDispatch::Http::ContentDisposition.format(disposition: disposition, filename: filename.sanitized), content_type: content_type }
key_params = { key: blob.key }.merge(query)
key_params = { key: blob.key }.merge(query).merge(service_name: service_name)
"https://example.com/rails/active_storage/disk/#{ActiveStorage.verifier.generate(key_params, expires_in: 5.minutes, purpose: :blob_key)}/#{filename}?#{query.to_param}"
end

2
activestorage/test/models/variant_test.rb
View File

@ -189,7 +189,7 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
test "url doesn't grow in length despite long variant options" do
blob = create_file_blob(filename: "racecar.jpg")
variant = blob.variant(font: "a" * 10_000).processed
assert_operator variant.url.length, :<, 730
assert_operator variant.url.length, :<, 785
end
test "works for vips processor" do

4
activestorage/test/service/disk_public_service_test.rb
View File

@ -12,8 +12,8 @@ class ActiveStorage::Service::DiskPublicServiceTest < ActiveSupport::TestCase
include ActiveStorage::Service::SharedServiceTests
test "public URL generation" do
url = @service.url(@key, filename: ActiveStorage::Filename.new("avatar.png"))
url = @service.url(@key, disposition: :inline, filename: ActiveStorage::Filename.new("avatar.png"), content_type: "image/png")
assert_match(/^https:\/\/example.com\/rails\/active_storage\/disk\/public\/#{@key}/, url)
assert_match(/^https:\/\/example.com\/rails\/active_storage\/disk\/.*\/avatar\.png\?content_type=image%2Fpng&disposition=inline.*/, url)
end
end

1
railties/test/application/configuration_test.rb
View File

@ -2406,7 +2406,6 @@ module ApplicationTests
Prefix Verb URI Pattern Controller#Action
rails_service_blob GET /files/blobs/:signed_id/*filename(.:format) active_storage/blobs#show
rails_blob_representation GET /files/representations/:signed_blob_id/:variation_key/*filename(.:format) active_storage/representations#show
rails_disk_service_public GET /files/disk/public/:key/*filename(.:format) active_storage/public_disk#show
rails_disk_service GET /files/disk/:encoded_key/*filename(.:format) active_storage/disk#show
update_rails_disk_service PUT /files/disk/:encoded_token(.:format) active_storage/disk#update
rails_direct_uploads POST /files/direct_uploads(.:format) active_storage/direct_uploads#create

1
railties/test/application/rake/routes_test.rb
View File

@ -36,7 +36,6 @@ module ApplicationTests
rails_conductor_inbound_email_reroute POST /rails/conductor/action_mailbox/:inbound_email_id/reroute(.:format) rails/conductor/action_mailbox/reroutes#create
rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show
rails_blob_representation GET /rails/active_storage/representations/:signed_blob_id/:variation_key/*filename(.:format) active_storage/representations#show
rails_disk_service_public GET /rails/active_storage/disk/public/:key/*filename(.:format) active_storage/public_disk#show
rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show
update_rails_disk_service PUT /rails/active_storage/disk/:encoded_token(.:format) active_storage/disk#update
rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create

11
railties/test/commands/routes_test.rb
View File

@ -56,7 +56,6 @@ class Rails::Command::RoutesTest < ActiveSupport::TestCase
rails_conductor_inbound_email GET /rails/conductor/action_mailbox/inbound_emails/:id(.:format) rails/conductor/action_mailbox/inbound_emails#show
rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show
rails_blob_representation GET /rails/active_storage/representations/:signed_blob_id/:variation_key/*filename(.:format) active_storage/representations#show
rails_disk_service_public GET /rails/active_storage/disk/public/:key/*filename(.:format) active_storage/public_disk#show
rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show
MESSAGE
@ -182,7 +181,6 @@ class Rails::Command::RoutesTest < ActiveSupport::TestCase
rails_conductor_inbound_email_reroute POST /rails/conductor/action_mailbox/:inbound_email_id/reroute(.:format) rails/conductor/action_mailbox/reroutes#create
rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show
rails_blob_representation GET /rails/active_storage/representations/:signed_blob_id/:variation_key/*filename(.:format) active_storage/representations#show
rails_disk_service_public GET /rails/active_storage/disk/public/:key/*filename(.:format) active_storage/public_disk#show
rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show
update_rails_disk_service PUT /rails/active_storage/disk/:encoded_token(.:format) active_storage/disk#update
rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create
@ -288,21 +286,16 @@ class Rails::Command::RoutesTest < ActiveSupport::TestCase
URI | /rails/active_storage/representations/:signed_blob_id/:variation_key/*filename(.:format)
Controller#Action | active_storage/representations#show
--[ Route 18 ]-------------
Prefix | rails_disk_service_public
Verb | GET
URI | /rails/active_storage/disk/public/:key/*filename(.:format)
Controller#Action | active_storage/public_disk#show
--[ Route 19 ]-------------
Prefix | rails_disk_service
Verb | GET
URI | /rails/active_storage/disk/:encoded_key/*filename(.:format)
Controller#Action | active_storage/disk#show
--[ Route 20 ]-------------
--[ Route 19 ]-------------
Prefix | update_rails_disk_service
Verb | PUT
URI | /rails/active_storage/disk/:encoded_token(.:format)
Controller#Action | active_storage/disk#update
--[ Route 21 ]-------------
--[ Route 20 ]-------------
Prefix | rails_direct_uploads
Verb | POST
URI | /rails/active_storage/direct_uploads(.:format)