Generate root-relative paths in Active Storage disk service URL methods

Fixes #32129.

Gemfile

@@ -19,7 +19,6 @@ gem "rack-cache", "~> 1.2"
 gem "coffee-rails"
 gem "sass-rails"
 gem "turbolinks", "~> 5"
-gem "webmock"
 
 # require: false so bcrypt is loaded only when has_secure_password is used.
 # This is to avoid Active Model (and by extension the entire framework)

Gemfile.lock

@@ -187,8 +187,6 @@ GEM
     concurrent-ruby (1.0.5-java)
     connection_pool (2.2.1)
     cookiejar (0.3.3)
-    crack (0.4.3)
-      safe_yaml (~> 1.0.0)
     crass (1.0.3)
     curses (1.0.2)
     daemons (1.2.4)
@@ -261,7 +259,6 @@ GEM
       multi_json (~> 1.11)
       os (~> 0.9)
       signet (~> 0.7)
-    hashdiff (0.3.7)
     hiredis (0.6.1)
     hiredis (0.6.1-java)
     http_parser.rb (0.6.0)
@@ -400,7 +397,6 @@ GEM
     rubyzip (1.2.1)
     rufus-scheduler (3.4.2)
       et-orbi (~> 1.0)
-    safe_yaml (1.0.4)
     sass (3.5.3)
       sass-listen (~> 4.0.0)
     sass-listen (4.0.0)
@@ -482,10 +478,6 @@ GEM
       json (>= 1.8)
       nokogiri (~> 1.6)
     wdm (0.1.1)
-    webmock (3.2.1)
-      addressable (>= 2.3.6)
-      crack (>= 0.3.2)
-      hashdiff
     websocket (1.2.4)
     websocket-driver (0.6.5)
       websocket-extensions (>= 0.1.0)
@@ -563,7 +555,6 @@ DEPENDENCIES
   uglifier (>= 1.3.0)
   w3c_validators
   wdm (>= 0.1.0)
-  webmock
   websocket-client-simple!
 
 BUNDLED WITH

activestorage/activestorage.gemspec

@@ -29,6 +29,4 @@ Gem::Specification.new do |s|
   s.add_dependency "activerecord", version
 
   s.add_dependency "marcel", "~> 0.3.1"
-
-  s.add_development_dependency "webmock", "~> 3.2.1"
 end

activestorage/app/models/active_storage/blob/identifiable.rb

@@ -2,7 +2,7 @@
 
 module ActiveStorage::Blob::Identifiable
   def identify
-    update!(content_type: identification.content_type, identified: true) unless identified?
+    update! content_type: identify_content_type, identified: true unless identified?
   end
 
   def identified?
@@ -10,7 +10,11 @@ module ActiveStorage::Blob::Identifiable
   end
 
   private
-    def identification
-      ActiveStorage::Identification.new self
+    def identify_content_type
+      Marcel::MimeType.for download_identifiable_chunk, name: filename.to_s, declared_type: content_type
+    end
+
+    def download_identifiable_chunk
+      service.download_chunk key, 0...4.kilobytes
     end
 end

activestorage/app/models/active_storage/identification.rb

@@ -1,35 +0,0 @@
-# frozen_string_literal: true
-
-require "net/http"
-
-class ActiveStorage::Identification #:nodoc:
-  attr_reader :blob
-
-  def initialize(blob)
-    @blob = blob
-  end
-
-  def content_type
-    Marcel::MimeType.for(identifiable_chunk, name: filename, declared_type: declared_content_type)
-  end
-
-  private
-    def identifiable_chunk
-      Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == "https") do |client|
-        client.get(uri, "Range" => "bytes=0-4095").body
-      end
-    end
-
-    def uri
-      @uri ||= URI.parse(blob.service_url)
-    end
-
-
-    def filename
-      blob.filename.to_s
-    end
-
-    def declared_content_type
-      blob.content_type
-    end
-end

activestorage/lib/active_storage/service.rb

@@ -73,6 +73,11 @@ module ActiveStorage
       raise NotImplementedError
     end
 
+    # Return the partial content of the file at the +key+ between the +start+ and +stop+ byte offsets.
+    def download_chunk(key, range)
+      raise NotImplementedError
+    end
+
     # Delete the file at the +key+.
     def delete(key)
       raise NotImplementedError

activestorage/lib/active_storage/service/azure_storage_service.rb

@@ -41,6 +41,13 @@ module ActiveStorage
       end
     end
 
+    def download_chunk(key, range)
+      instrument :download_chunk, key: key, range: range do
+        _, io = blobs.get_blob(container, key, start_range: range.begin, end_range: range.exclude_end? ? range.end - 1 : range.end)
+        io.force_encoding(Encoding::BINARY)
+      end
+    end
+
     def delete(key)
       instrument :delete, key: key do
         begin

activestorage/lib/active_storage/service/disk_service.rb

@@ -9,10 +9,10 @@ module ActiveStorage
   # Wraps a local disk path as an Active Storage service. See ActiveStorage::Service for the generic API
   # documentation that applies to all services.
   class Service::DiskService < Service
-    attr_reader :root, :host
+    attr_reader :root
 
-    def initialize(root:, host: "http://localhost:3000")
-      @root, @host = root, host
+    def initialize(root:)
+      @root = root
     end
 
     def upload(key, io, checksum: nil)
@@ -38,6 +38,15 @@ module ActiveStorage
       end
     end
 
+    def download_chunk(key, range)
+      instrument :download_chunk, key: key, range: range do
+        File.open(path_for(key), "rb") do |file|
+          file.seek range.begin
+          file.read range.size
+        end
+      end
+    end
+
     def delete(key)
       instrument :delete, key: key do
         begin
@@ -69,12 +78,11 @@ module ActiveStorage
         verified_key_with_expiration = ActiveStorage.verifier.generate(key, expires_in: expires_in, purpose: :blob_key)
 
         generated_url =
-          Rails.application.routes.url_helpers.rails_disk_service_url(
+          url_helpers.rails_disk_service_path(
             verified_key_with_expiration,
             filename: filename,
             disposition: content_disposition_with(type: disposition, filename: filename),
-            content_type: content_type,
-            host: host
+            content_type: content_type
           )
 
         payload[:url] = generated_url
@@ -96,7 +104,7 @@ module ActiveStorage
           purpose: :blob_token }
         )
 
-        generated_url = Rails.application.routes.url_helpers.update_rails_disk_service_url(verified_token_with_expiration, host: host)
+        generated_url = url_helpers.update_rails_disk_service_path(verified_token_with_expiration)
 
         payload[:url] = generated_url
 
@@ -121,11 +129,17 @@ module ActiveStorage
         path_for(key).tap { |path| FileUtils.mkdir_p File.dirname(path) }
       end
 
+
       def ensure_integrity_of(key, checksum)
         unless Digest::MD5.file(path_for(key)).base64digest == checksum
           delete key
           raise ActiveStorage::IntegrityError
         end
       end
+
+
+      def url_helpers
+        @url_helpers ||= Rails.application.routes.url_helpers
+      end
   end
 end

activestorage/lib/active_storage/service/gcs_service.rb

@@ -3,7 +3,10 @@
 gem "google-cloud-storage", "~> 1.8"
 
 require "google/cloud/storage"
+require "net/http"
+
 require "active_support/core_ext/object/to_query"
+require "active_storage/filename"
 
 module ActiveStorage
   # Wraps the Google Cloud Storage as an Active Storage service. See ActiveStorage::Service for the generic API
@@ -43,6 +46,16 @@ module ActiveStorage
       end
     end
 
+    def download_chunk(key, range)
+      instrument :download_chunk, key: key, range: range do
+        uri = URI(url(key, expires_in: 30.seconds, filename: ActiveStorage::Filename.new(""), content_type: "application/octet-stream", disposition: "inline"))
+
+        Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == "https") do |client|
+          client.get(uri, "Range" => "bytes=#{range.begin}-#{range.exclude_end? ? range.end - 1 : range.end}").body
+        end
+      end
+    end
+
     def delete(key)
       instrument :delete, key: key do
         begin

activestorage/lib/active_storage/service/mirror_service.rb

@@ -9,7 +9,7 @@ module ActiveStorage
   class Service::MirrorService < Service
     attr_reader :primary, :mirrors
 
-    delegate :download, :exist?, :url, to: :primary
+    delegate :download, :download_chunk, :exist?, :url, to: :primary
 
     # Stitch together from named services.
     def self.build(primary:, mirrors:, configurator:, **options) #:nodoc:

activestorage/lib/active_storage/service/s3_service.rb

@@ -38,6 +38,12 @@ module ActiveStorage
       end
     end
 
+    def download_chunk(key, range)
+      instrument :download_chunk, key: key, range: range do
+        object_for(key).get(range: "bytes=#{range.begin}-#{range.exclude_end? ? range.end - 1 : range.end}").body.read.force_encoding(Encoding::BINARY)
+      end
+    end
+
     def delete(key)
       instrument :delete, key: key do
         object_for(key).delete

activestorage/test/models/attachments_test.rb

@@ -126,7 +126,6 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase
     blob = create_blob_before_direct_upload(filename: "racecar.jpg", content_type: "application/octet-stream", byte_size: 1124062, checksum: "7GjDDNEQb4mzMzsW+MS0JQ==")
     ActiveStorage::Blob.service.upload(blob.key, file_fixture("racecar.jpg").open)
 
-    stub_request(:get, %r{localhost:3000/rails/active_storage/disk/.*}).to_return(body: file_fixture("racecar.jpg"))
     @user.avatar.attach(blob)
 
     assert_equal "image/jpeg", @user.avatar.reload.content_type

activestorage/test/models/blob_test.rb

@@ -140,6 +140,6 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
     def expected_url_for(blob, disposition: :inline, filename: nil)
       filename ||= blob.filename
       query_string = { content_type: blob.content_type, disposition: "#{disposition}; #{filename.parameters}" }.to_param
-      "http://localhost:3000/rails/active_storage/disk/#{ActiveStorage.verifier.generate(blob.key, expires_in: 5.minutes, purpose: :blob_key)}/#{filename}?#{query_string}"
+      "/rails/active_storage/disk/#{ActiveStorage.verifier.generate(blob.key, expires_in: 5.minutes, purpose: :blob_key)}/#{filename}?#{query_string}"
     end
 end

activestorage/test/service/configurator_test.rb

@@ -5,10 +5,8 @@ require "service/shared_service_tests"
 class ActiveStorage::Service::ConfiguratorTest < ActiveSupport::TestCase
   test "builds correct service instance based on service name" do
     service = ActiveStorage::Service::Configurator.build(:foo, foo: { service: "Disk", root: "path" })
-
     assert_instance_of ActiveStorage::Service::DiskService, service
     assert_equal "path", service.root
-    assert_equal "http://localhost:3000", service.host
   end
 
   test "raises error when passing non-existent service name" do

activestorage/test/service/shared_service_tests.rb

@@ -60,6 +60,11 @@ module ActiveStorage::Service::SharedServiceTests
       assert_equal [ FIXTURE_DATA ], chunks
     end
 
+    test "downloading partially" do
+      assert_equal "\x10\x00\x00", @service.download_chunk(FIXTURE_KEY, 19..21)
+      assert_equal "\x10\x00\x00", @service.download_chunk(FIXTURE_KEY, 19...22)
+    end
+
     test "existing" do
       assert @service.exist?(FIXTURE_KEY)
       assert_not @service.exist?(FIXTURE_KEY + "nonsense")

activestorage/test/test_helper.rb

@@ -7,7 +7,6 @@ require "bundler/setup"
 require "active_support"
 require "active_support/test_case"
 require "active_support/testing/autorun"
-require "webmock/minitest"
 require "mini_magick"
 
 begin
@@ -42,8 +41,6 @@ ActiveStorage.verifier = ActiveSupport::MessageVerifier.new("Testing")
 class ActiveSupport::TestCase
   self.file_fixture_path = File.expand_path("fixtures/files", __dir__)
 
-  setup { WebMock.allow_net_connect! }
-
   private
     def create_blob(data: "Hello world!", filename: "hello.txt", content_type: "text/plain")
       ActiveStorage::Blob.create_after_upload! io: StringIO.new(data), filename: filename, content_type: content_type

guides/source/active_storage_overview.md

@@ -93,15 +93,6 @@ local:
   root: <%= Rails.root.join("storage") %>
 ```
 
-Optionally specify a host for generating URLs (the default is `http://localhost:3000`):
-
-```yaml
-local:
-  service: Disk
-  root: <%= Rails.root.join("storage") %>
-  host: http://myapp.test
-```
-
 ### Amazon S3 Service
 
 Declare an S3 service in `config/storage.yml`: