diff options
Diffstat (limited to 'activestorage')
24 files changed, 288 insertions, 294 deletions
diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index ea9b10add9..cece15ff06 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,18 @@ +* Permit generating variants of TIFF images. + + *Luciano Sousa* + +* Use base36 (all lowercase) for all new Blob keys to prevent + collisions and undefined behavior with case-insensitive filesystems and + database indices. + + *Julik Tarkhanov* + +* It doesn’t include an `X-CSRF-Token` header if a meta tag is not found on + the page. It previously included one with a value of `undefined`. + + *Cameron Bothner* + * Fix `ArgumentError` when uploading to amazon s3 *Hiroki Sanpei* @@ -123,9 +138,9 @@ *Janko Marohnić* -* Rails 6 requires Ruby 2.4.1 or newer. +* Rails 6 requires Ruby 2.5.0 or newer. - *Jeremy Daer* + *Jeremy Daer*, *Kasper Timm Hansen* Please check [5-2-stable](https://github.com/rails/rails/blob/5-2-stable/activestorage/CHANGELOG.md) for previous changes. diff --git a/activestorage/MIT-LICENSE b/activestorage/MIT-LICENSE index eed89ac398..771376cc7e 100644 --- a/activestorage/MIT-LICENSE +++ b/activestorage/MIT-LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2017-2018 David Heinemeier Hansson, Basecamp +Copyright (c) 2017-2019 David Heinemeier Hansson, Basecamp Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the diff --git a/activestorage/activestorage.gemspec b/activestorage/activestorage.gemspec index 2c8816df25..dfada7054a 100644 --- a/activestorage/activestorage.gemspec +++ b/activestorage/activestorage.gemspec @@ -9,7 +9,7 @@ Gem::Specification.new do |s| s.summary = "Local and cloud file storage framework." s.description = "Attach cloud and local files in Rails applications." - s.required_ruby_version = ">= 2.4.1" + s.required_ruby_version = ">= 2.5.0" s.license = "MIT" diff --git a/activestorage/app/assets/javascripts/activestorage.js b/activestorage/app/assets/javascripts/activestorage.js index b71e251a11..e2bcb520b9 100644 --- a/activestorage/app/assets/javascripts/activestorage.js +++ b/activestorage/app/assets/javascripts/activestorage.js @@ -560,7 +560,10 @@ this.xhr.setRequestHeader("Content-Type", "application/json"); this.xhr.setRequestHeader("Accept", "application/json"); this.xhr.setRequestHeader("X-Requested-With", "XMLHttpRequest"); - this.xhr.setRequestHeader("X-CSRF-Token", getMetaValue("csrf-token")); + var csrfToken = getMetaValue("csrf-token"); + if (csrfToken != undefined) { + this.xhr.setRequestHeader("X-CSRF-Token", csrfToken); + } this.xhr.addEventListener("load", function(event) { return _this.requestDidLoad(event); }); diff --git a/activestorage/app/javascript/activestorage/blob_record.js b/activestorage/app/javascript/activestorage/blob_record.js index ff847892b2..7fbe315f76 100644 --- a/activestorage/app/javascript/activestorage/blob_record.js +++ b/activestorage/app/javascript/activestorage/blob_record.js @@ -17,7 +17,12 @@ export class BlobRecord { this.xhr.setRequestHeader("Content-Type", "application/json") this.xhr.setRequestHeader("Accept", "application/json") this.xhr.setRequestHeader("X-Requested-With", "XMLHttpRequest") - this.xhr.setRequestHeader("X-CSRF-Token", getMetaValue("csrf-token")) + + const csrfToken = getMetaValue("csrf-token") + if (csrfToken != undefined) { + this.xhr.setRequestHeader("X-CSRF-Token", csrfToken) + } + this.xhr.addEventListener("load", event => this.requestDidLoad(event)) this.xhr.addEventListener("error", event => this.requestDidError(event)) } diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index 04f9dbff9f..6ca7d49bc1 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -79,6 +79,15 @@ class ActiveStorage::Blob < ActiveRecord::Base def create_before_direct_upload!(filename:, byte_size:, checksum:, content_type: nil, metadata: nil) create! filename: filename, byte_size: byte_size, checksum: checksum, content_type: content_type, metadata: metadata end + + # To prevent problems with case-insensitive filesystems, especially in combination + # with databases which treat indices as case-sensitive, all blob keys generated are going + # to only contain the base-36 character alphabet and will therefore be lowercase. To maintain + # the same or higher amount of entropy as in the base-58 encoding used by `has_secure_token` + # the number of bytes used is increased to 28 from the standard 24 + def generate_unique_secure_token + SecureRandom.base36(28) + end end # Returns a signed ID for this blob that's suitable for reference on the client-side without fear of tampering. @@ -87,9 +96,10 @@ class ActiveStorage::Blob < ActiveRecord::Base ActiveStorage.verifier.generate(id, purpose: :blob_id) end - # Returns the key pointing to the file on the service that's associated with this blob. The key is in the - # standard secure-token format from Rails. So it'll look like: XTAPjJCJiuDrLk3TmwyJGpUo. This key is not intended - # to be revealed directly to the user. Always refer to blobs using the signed_id or a verified form of the key. + # Returns the key pointing to the file on the service that's associated with this blob. The key is the + # secure-token format from Rails in lower case. So it'll look like: xtapjjcjiudrlk3tmwyjgpuobabd. + # This key is not intended to be revealed directly to the user. + # Always refer to blobs using the signed_id or a verified form of the key. def key # We can't wait until the record is first saved to have a key for it self[:key] ||= self.class.generate_unique_secure_token diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb index 19c7f6be2a..e127b4cd49 100644 --- a/activestorage/lib/active_storage.rb +++ b/activestorage/lib/active_storage.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true #-- -# Copyright (c) 2017-2018 David Heinemeier Hansson, Basecamp +# Copyright (c) 2017-2019 David Heinemeier Hansson, Basecamp # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index b3bcf48d6f..e2e9b70b69 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -27,6 +27,7 @@ module ActiveStorage image/jpg image/jpeg image/pjpeg + image/tiff image/vnd.adobe.photoshop image/vnd.microsoft.icon ) @@ -49,6 +50,7 @@ module ActiveStorage image/gif image/jpg image/jpeg + image/tiff image/vnd.adobe.photoshop image/vnd.microsoft.icon application/pdf diff --git a/activestorage/lib/active_storage/service/azure_storage_service.rb b/activestorage/lib/active_storage/service/azure_storage_service.rb index a2c4b4d57c..17cecd891c 100644 --- a/activestorage/lib/active_storage/service/azure_storage_service.rb +++ b/activestorage/lib/active_storage/service/azure_storage_service.rb @@ -51,12 +51,10 @@ module ActiveStorage def delete(key) instrument :delete, key: key do - begin - blobs.delete_blob(container, key) - rescue Azure::Core::Http::HTTPError => e - raise unless e.type == "BlobNotFound" - # Ignore files already deleted - end + blobs.delete_blob(container, key) + rescue Azure::Core::Http::HTTPError => e + raise unless e.type == "BlobNotFound" + # Ignore files already deleted end end diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb index 2588c41760..67892d43b2 100644 --- a/activestorage/lib/active_storage/service/disk_service.rb +++ b/activestorage/lib/active_storage/service/disk_service.rb @@ -29,35 +29,29 @@ module ActiveStorage end else instrument :download, key: key do - begin - File.binread path_for(key) - rescue Errno::ENOENT - raise ActiveStorage::FileNotFoundError - end + File.binread path_for(key) + rescue Errno::ENOENT + raise ActiveStorage::FileNotFoundError end end end def download_chunk(key, range) instrument :download_chunk, key: key, range: range do - begin - File.open(path_for(key), "rb") do |file| - file.seek range.begin - file.read range.size - end - rescue Errno::ENOENT - raise ActiveStorage::FileNotFoundError + File.open(path_for(key), "rb") do |file| + file.seek range.begin + file.read range.size end + rescue Errno::ENOENT + raise ActiveStorage::FileNotFoundError end end def delete(key) instrument :delete, key: key do - begin - File.delete path_for(key) - rescue Errno::ENOENT - # Ignore files already deleted - end + File.delete path_for(key) + rescue Errno::ENOENT + # Ignore files already deleted end end diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb index e2590aa35d..9c20ed1d10 100644 --- a/activestorage/lib/active_storage/service/gcs_service.rb +++ b/activestorage/lib/active_storage/service/gcs_service.rb @@ -13,16 +13,14 @@ module ActiveStorage def upload(key, io, checksum: nil, content_type: nil, disposition: nil, filename: nil) instrument :upload, key: key, checksum: checksum do - begin - # GCS's signed URLs don't include params such as response-content-type response-content_disposition - # in the signature, which means an attacker can modify them and bypass our effort to force these to - # binary and attachment when the file's content type requires it. The only way to force them is to - # store them as object's metadata. - content_disposition = content_disposition_with(type: disposition, filename: filename) if disposition && filename - bucket.create_file(io, key, md5: checksum, content_type: content_type, content_disposition: content_disposition) - rescue Google::Cloud::InvalidArgumentError - raise ActiveStorage::IntegrityError - end + # GCS's signed URLs don't include params such as response-content-type response-content_disposition + # in the signature, which means an attacker can modify them and bypass our effort to force these to + # binary and attachment when the file's content type requires it. The only way to force them is to + # store them as object's metadata. + content_disposition = content_disposition_with(type: disposition, filename: filename) if disposition && filename + bucket.create_file(io, key, md5: checksum, content_type: content_type, content_disposition: content_disposition) + rescue Google::Cloud::InvalidArgumentError + raise ActiveStorage::IntegrityError end end @@ -33,11 +31,9 @@ module ActiveStorage end else instrument :download, key: key do - begin - file_for(key).download.string - rescue Google::Cloud::NotFoundError - raise ActiveStorage::FileNotFoundError - end + file_for(key).download.string + rescue Google::Cloud::NotFoundError + raise ActiveStorage::FileNotFoundError end end end @@ -53,32 +49,26 @@ module ActiveStorage def download_chunk(key, range) instrument :download_chunk, key: key, range: range do - begin - file_for(key).download(range: range).string - rescue Google::Cloud::NotFoundError - raise ActiveStorage::FileNotFoundError - end + file_for(key).download(range: range).string + rescue Google::Cloud::NotFoundError + raise ActiveStorage::FileNotFoundError end end def delete(key) instrument :delete, key: key do - begin - file_for(key).delete - rescue Google::Cloud::NotFoundError - # Ignore files already deleted - end + file_for(key).delete + rescue Google::Cloud::NotFoundError + # Ignore files already deleted end end def delete_prefixed(prefix) instrument :delete_prefixed, prefix: prefix do bucket.files(prefix: prefix).all do |file| - begin - file.delete - rescue Google::Cloud::NotFoundError - # Ignore concurrently-deleted files - end + file.delete + rescue Google::Cloud::NotFoundError + # Ignore concurrently-deleted files end end end diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb index 90c3ae1b62..382920ef61 100644 --- a/activestorage/lib/active_storage/service/s3_service.rb +++ b/activestorage/lib/active_storage/service/s3_service.rb @@ -18,11 +18,9 @@ module ActiveStorage def upload(key, io, checksum: nil, **) instrument :upload, key: key, checksum: checksum do - begin - object_for(key).put(upload_options.merge(body: io, content_md5: checksum)) - rescue Aws::S3::Errors::BadDigest - raise ActiveStorage::IntegrityError - end + object_for(key).put(upload_options.merge(body: io, content_md5: checksum)) + rescue Aws::S3::Errors::BadDigest + raise ActiveStorage::IntegrityError end end @@ -33,22 +31,18 @@ module ActiveStorage end else instrument :download, key: key do - begin - object_for(key).get.body.string.force_encoding(Encoding::BINARY) - rescue Aws::S3::Errors::NoSuchKey - raise ActiveStorage::FileNotFoundError - end + object_for(key).get.body.string.force_encoding(Encoding::BINARY) + rescue Aws::S3::Errors::NoSuchKey + raise ActiveStorage::FileNotFoundError end end end def download_chunk(key, range) instrument :download_chunk, key: key, range: range do - begin - object_for(key).get(range: "bytes=#{range.begin}-#{range.exclude_end? ? range.end - 1 : range.end}").body.read.force_encoding(Encoding::BINARY) - rescue Aws::S3::Errors::NoSuchKey - raise ActiveStorage::FileNotFoundError - end + object_for(key).get(range: "bytes=#{range.begin}-#{range.exclude_end? ? range.end - 1 : range.end}").body.read.force_encoding(Encoding::BINARY) + rescue Aws::S3::Errors::NoSuchKey + raise ActiveStorage::FileNotFoundError end end diff --git a/activestorage/test/dummy/bin/yarn b/activestorage/test/dummy/bin/yarn index c9b7498378..d0dd7c27ac 100755 --- a/activestorage/test/dummy/bin/yarn +++ b/activestorage/test/dummy/bin/yarn @@ -3,11 +3,9 @@ VENDOR_PATH = File.expand_path("..", __dir__) Dir.chdir(VENDOR_PATH) do - begin - exec "yarnpkg #{ARGV.join(" ")}" - rescue Errno::ENOENT - $stderr.puts "Yarn executable was not detected in the system." - $stderr.puts "Download Yarn at https://yarnpkg.com/en/docs/install" - exit 1 - end + exec "yarnpkg #{ARGV.join(" ")}" +rescue Errno::ENOENT + $stderr.puts "Yarn executable was not detected in the system." + $stderr.puts "Download Yarn at https://yarnpkg.com/en/docs/install" + exit 1 end diff --git a/activestorage/test/dummy/config/application.rb b/activestorage/test/dummy/config/application.rb index bd14ac0b1a..151c8ade4b 100644 --- a/activestorage/test/dummy/config/application.rb +++ b/activestorage/test/dummy/config/application.rb @@ -15,7 +15,7 @@ Bundler.require(*Rails.groups) module Dummy class Application < Rails::Application - config.load_defaults 5.2 + config.load_defaults 6.0 config.active_storage.service = :local end diff --git a/activestorage/test/fixtures/files/racecar.tif b/activestorage/test/fixtures/files/racecar.tif Binary files differnew file mode 100644 index 0000000000..0a11b22896 --- /dev/null +++ b/activestorage/test/fixtures/files/racecar.tif diff --git a/activestorage/test/models/attached/many_test.rb b/activestorage/test/models/attached/many_test.rb index 3b563b3fc8..8fede0e682 100644 --- a/activestorage/test/models/attached/many_test.rb +++ b/activestorage/test/models/attached/many_test.rb @@ -590,7 +590,7 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase assert_equal "town.jpg", @user.highlights.first.filename.to_s assert_equal "funky.jpg", @user.highlights.second.filename.to_s ensure - User.send(:remove_method, :highlights) + User.remove_method :highlights end end end diff --git a/activestorage/test/models/attached/one_test.rb b/activestorage/test/models/attached/one_test.rb index 561c3e9d23..7fb3262781 100644 --- a/activestorage/test/models/attached/one_test.rb +++ b/activestorage/test/models/attached/one_test.rb @@ -507,7 +507,7 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase assert_equal "gpj.yknuf", @user.avatar ensure - User.send(:remove_method, :avatar) + User.remove_method :avatar end end end diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb index 1503f5fc50..54cf9e2b8a 100644 --- a/activestorage/test/models/blob_test.rb +++ b/activestorage/test/models/blob_test.rb @@ -47,6 +47,10 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase assert_equal "text/csv", blob.content_type end + test "create after upload generates a 28-character base36 key" do + assert_match(/^[a-z0-9]{28}$/, create_blob.key) + end + test "image?" do blob = create_file_blob filename: "racecar.jpg" assert_predicate blob, :image? diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb index 8552080e7b..d98935eb9f 100644 --- a/activestorage/test/models/variant_test.rb +++ b/activestorage/test/models/variant_test.rb @@ -26,16 +26,14 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase end test "monochrome with default variant_processor" do - begin - ActiveStorage.variant_processor = nil - - blob = create_file_blob(filename: "racecar.jpg") - variant = blob.variant(monochrome: true).processed - image = read_image(variant) - assert_match(/Gray/, image.colorspace) - ensure - ActiveStorage.variant_processor = :mini_magick - end + ActiveStorage.variant_processor = nil + + blob = create_file_blob(filename: "racecar.jpg") + variant = blob.variant(monochrome: true).processed + image = read_image(variant) + assert_match(/Gray/, image.colorspace) + ensure + ActiveStorage.variant_processor = :mini_magick end test "disabled variation of JPEG blob" do @@ -66,45 +64,41 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase end test "disabled variation using :combine_options" do - begin - ActiveStorage.variant_processor = nil - blob = create_file_blob(filename: "racecar.jpg") - variant = ActiveSupport::Deprecation.silence do - blob.variant(combine_options: { - crop: "100x100+0+0", - monochrome: false - }).processed - end - assert_match(/racecar\.jpg/, variant.service_url) - - image = read_image(variant) - assert_equal 100, image.width - assert_equal 100, image.height - assert_match(/RGB/, image.colorspace) - ensure - ActiveStorage.variant_processor = :mini_magick + ActiveStorage.variant_processor = nil + blob = create_file_blob(filename: "racecar.jpg") + variant = ActiveSupport::Deprecation.silence do + blob.variant(combine_options: { + crop: "100x100+0+0", + monochrome: false + }).processed end + assert_match(/racecar\.jpg/, variant.service_url) + + image = read_image(variant) + assert_equal 100, image.width + assert_equal 100, image.height + assert_match(/RGB/, image.colorspace) + ensure + ActiveStorage.variant_processor = :mini_magick end test "center-weighted crop of JPEG blob using :combine_options" do - begin - ActiveStorage.variant_processor = nil - blob = create_file_blob(filename: "racecar.jpg") - variant = ActiveSupport::Deprecation.silence do - blob.variant(combine_options: { - gravity: "center", - resize: "100x100^", - crop: "100x100+0+0", - }).processed - end - assert_match(/racecar\.jpg/, variant.service_url) - - image = read_image(variant) - assert_equal 100, image.width - assert_equal 100, image.height - ensure - ActiveStorage.variant_processor = :mini_magick + ActiveStorage.variant_processor = nil + blob = create_file_blob(filename: "racecar.jpg") + variant = ActiveSupport::Deprecation.silence do + blob.variant(combine_options: { + gravity: "center", + resize: "100x100^", + crop: "100x100+0+0", + }).processed end + assert_match(/racecar\.jpg/, variant.service_url) + + image = read_image(variant) + assert_equal 100, image.width + assert_equal 100, image.height + ensure + ActiveStorage.variant_processor = :mini_magick end test "center-weighted crop of JPEG blob using :resize_to_fill" do @@ -139,6 +133,17 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase assert_equal 20, image.height end + test "resized variation of TIFF blob" do + blob = create_file_blob(filename: "racecar.tif") + variant = blob.variant(resize: "50x50").processed + assert_match(/racecar\.png/, variant.service_url) + + image = read_image(variant) + assert_equal "PNG", image.type + assert_equal 50, image.width + assert_equal 33, image.height + end + test "optimized variation of GIF blob" do blob = create_file_blob(filename: "image.gif", content_type: "image/gif") @@ -156,22 +161,20 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase test "service_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.service_url.length, :<, 726 + assert_operator variant.service_url.length, :<, 730 end test "works for vips processor" do - begin - ActiveStorage.variant_processor = :vips - blob = create_file_blob(filename: "racecar.jpg") - variant = blob.variant(thumbnail_image: 100).processed - - image = read_image(variant) - assert_equal 100, image.width - assert_equal 67, image.height - rescue LoadError - # libvips not installed - ensure - ActiveStorage.variant_processor = :mini_magick - end + ActiveStorage.variant_processor = :vips + blob = create_file_blob(filename: "racecar.jpg") + variant = blob.variant(thumbnail_image: 100).processed + + image = read_image(variant) + assert_equal 100, image.width + assert_equal 67, image.height + rescue LoadError + # libvips not installed + ensure + ActiveStorage.variant_processor = :mini_magick end end diff --git a/activestorage/test/service/azure_storage_service_test.rb b/activestorage/test/service/azure_storage_service_test.rb index 09c2e7f99c..2b07902d07 100644 --- a/activestorage/test/service/azure_storage_service_test.rb +++ b/activestorage/test/service/azure_storage_service_test.rb @@ -18,20 +18,18 @@ if SERVICE_CONFIGURATIONS[:azure] end test "uploading a tempfile" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - - Tempfile.open do |file| - file.write(data) - file.rewind - @service.upload(key, file) - end - - assert_equal data, @service.download(key) - ensure - @service.delete(key) + key = SecureRandom.base58(24) + data = "Something else entirely!" + + Tempfile.open do |file| + file.write(data) + file.rewind + @service.upload(key, file) end + + assert_equal data, @service.download(key) + ensure + @service.delete(key) end end else diff --git a/activestorage/test/service/gcs_service_test.rb b/activestorage/test/service/gcs_service_test.rb index af27946357..6bca428f50 100644 --- a/activestorage/test/service/gcs_service_test.rb +++ b/activestorage/test/service/gcs_service_test.rb @@ -10,74 +10,66 @@ if SERVICE_CONFIGURATIONS[:gcs] include ActiveStorage::Service::SharedServiceTests test "direct upload" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - checksum = Digest::MD5.base64digest(data) - url = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum) - - uri = URI.parse url - request = Net::HTTP::Put.new uri.request_uri - request.body = data - request.add_field "Content-Type", "" - request.add_field "Content-MD5", checksum - Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http| - http.request request - end - - assert_equal data, @service.download(key) - ensure - @service.delete key + key = SecureRandom.base58(24) + data = "Something else entirely!" + checksum = Digest::MD5.base64digest(data) + url = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum) + + uri = URI.parse url + request = Net::HTTP::Put.new uri.request_uri + request.body = data + request.add_field "Content-Type", "" + request.add_field "Content-MD5", checksum + Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http| + http.request request end + + assert_equal data, @service.download(key) + ensure + @service.delete key end test "upload with content_type and content_disposition" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - - @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain") - - url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html")) - response = Net::HTTP.get_response(URI(url)) - assert_equal "text/plain", response.content_type - assert_match(/attachment;.*test.txt/, response["Content-Disposition"]) - ensure - @service.delete key - end + key = SecureRandom.base58(24) + data = "Something else entirely!" + + @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain") + + url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html")) + response = Net::HTTP.get_response(URI(url)) + assert_equal "text/plain", response.content_type + assert_match(/attachment;.*test.txt/, response["Content-Disposition"]) + ensure + @service.delete key end test "upload with content_type" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - - @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), content_type: "text/plain") - - url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html")) - response = Net::HTTP.get_response(URI(url)) - assert_equal "text/plain", response.content_type - assert_match(/inline;.*test.html/, response["Content-Disposition"]) - ensure - @service.delete key - end + key = SecureRandom.base58(24) + data = "Something else entirely!" + + @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), content_type: "text/plain") + + url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html")) + response = Net::HTTP.get_response(URI(url)) + assert_equal "text/plain", response.content_type + assert_match(/inline;.*test.html/, response["Content-Disposition"]) + ensure + @service.delete key end test "update metadata" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.html"), content_type: "text/html") - - @service.update_metadata(key, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain") - url = @service.url(key, expires_in: 2.minutes, disposition: :attachment, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html")) - - response = Net::HTTP.get_response(URI(url)) - assert_equal "text/plain", response.content_type - assert_match(/inline;.*test.txt/, response["Content-Disposition"]) - ensure - @service.delete key - end + key = SecureRandom.base58(24) + data = "Something else entirely!" + @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.html"), content_type: "text/html") + + @service.update_metadata(key, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain") + url = @service.url(key, expires_in: 2.minutes, disposition: :attachment, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html")) + + response = Net::HTTP.get_response(URI(url)) + assert_equal "text/plain", response.content_type + assert_match(/inline;.*test.txt/, response["Content-Disposition"]) + ensure + @service.delete key end test "signed URL generation" do diff --git a/activestorage/test/service/mirror_service_test.rb b/activestorage/test/service/mirror_service_test.rb index bb502dde60..94c751a4ff 100644 --- a/activestorage/test/service/mirror_service_test.rb +++ b/activestorage/test/service/mirror_service_test.rb @@ -18,22 +18,20 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase include ActiveStorage::Service::SharedServiceTests test "uploading to all services" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - io = StringIO.new(data) - checksum = Digest::MD5.base64digest(data) + key = SecureRandom.base58(24) + data = "Something else entirely!" + io = StringIO.new(data) + checksum = Digest::MD5.base64digest(data) - @service.upload key, io.tap(&:read), checksum: checksum - assert_predicate io, :eof? + @service.upload key, io.tap(&:read), checksum: checksum + assert_predicate io, :eof? - assert_equal data, @service.primary.download(key) - @service.mirrors.each do |mirror| - assert_equal data, mirror.download(key) - end - ensure - @service.delete key + assert_equal data, @service.primary.download(key) + @service.mirrors.each do |mirror| + assert_equal data, mirror.download(key) end + ensure + @service.delete key end test "downloading from primary service" do diff --git a/activestorage/test/service/s3_service_test.rb b/activestorage/test/service/s3_service_test.rb index 0e5b06dd78..0a6004267f 100644 --- a/activestorage/test/service/s3_service_test.rb +++ b/activestorage/test/service/s3_service_test.rb @@ -11,25 +11,23 @@ if SERVICE_CONFIGURATIONS[:s3] include ActiveStorage::Service::SharedServiceTests test "direct upload" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - checksum = Digest::MD5.base64digest(data) - url = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum) - - uri = URI.parse url - request = Net::HTTP::Put.new uri.request_uri - request.body = data - request.add_field "Content-Type", "text/plain" - request.add_field "Content-MD5", checksum - Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http| - http.request request - end + key = SecureRandom.base58(24) + data = "Something else entirely!" + checksum = Digest::MD5.base64digest(data) + url = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum) - assert_equal data, @service.download(key) - ensure - @service.delete key + uri = URI.parse url + request = Net::HTTP::Put.new uri.request_uri + request.body = data + request.add_field "Content-Type", "text/plain" + request.add_field "Content-MD5", checksum + Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http| + http.request request end + + assert_equal data, @service.download(key) + ensure + @service.delete key end test "upload a zero byte file" do diff --git a/activestorage/test/service/shared_service_tests.rb b/activestorage/test/service/shared_service_tests.rb index b97030b320..17f3736056 100644 --- a/activestorage/test/service/shared_service_tests.rb +++ b/activestorage/test/service/shared_service_tests.rb @@ -20,48 +20,42 @@ module ActiveStorage::Service::SharedServiceTests end test "uploading with integrity" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data)) + key = SecureRandom.base58(24) + data = "Something else entirely!" + @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data)) - assert_equal data, @service.download(key) - ensure - @service.delete key - end + assert_equal data, @service.download(key) + ensure + @service.delete key end test "uploading without integrity" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" + key = SecureRandom.base58(24) + data = "Something else entirely!" - assert_raises(ActiveStorage::IntegrityError) do - @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest("bad data")) - end - - assert_not @service.exist?(key) - ensure - @service.delete key + assert_raises(ActiveStorage::IntegrityError) do + @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest("bad data")) end + + assert_not @service.exist?(key) + ensure + @service.delete key end test "uploading with integrity and multiple keys" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - @service.upload( - key, - StringIO.new(data), - checksum: Digest::MD5.base64digest(data), - filename: "racecar.jpg", - content_type: "image/jpg" - ) - - assert_equal data, @service.download(key) - ensure - @service.delete key - end + key = SecureRandom.base58(24) + data = "Something else entirely!" + @service.upload( + key, + StringIO.new(data), + checksum: Digest::MD5.base64digest(data), + filename: "racecar.jpg", + content_type: "image/jpg" + ) + + assert_equal data, @service.download(key) + ensure + @service.delete key end test "downloading" do @@ -129,20 +123,18 @@ module ActiveStorage::Service::SharedServiceTests end test "deleting by prefix" do - begin - @service.upload("a/a/a", StringIO.new(FIXTURE_DATA)) - @service.upload("a/a/b", StringIO.new(FIXTURE_DATA)) - @service.upload("a/b/a", StringIO.new(FIXTURE_DATA)) - - @service.delete_prefixed("a/a/") - assert_not @service.exist?("a/a/a") - assert_not @service.exist?("a/a/b") - assert @service.exist?("a/b/a") - ensure - @service.delete("a/a/a") - @service.delete("a/a/b") - @service.delete("a/b/a") - end + @service.upload("a/a/a", StringIO.new(FIXTURE_DATA)) + @service.upload("a/a/b", StringIO.new(FIXTURE_DATA)) + @service.upload("a/b/a", StringIO.new(FIXTURE_DATA)) + + @service.delete_prefixed("a/a/") + assert_not @service.exist?("a/a/a") + assert_not @service.exist?("a/a/b") + assert @service.exist?("a/b/a") + ensure + @service.delete("a/a/a") + @service.delete("a/a/b") + @service.delete("a/b/a") end end end |