Merge pull request #37904 from peterzhu2118/as-remove-query-params

Remove unused query params in DiskService

activestorage/CHANGELOG.md

+*   Remove unused `disposition` and `content_type` query parameters for `DiskService`.
+
+    *Peter Zhu*
+
 *   Use `DiskController` for both public and private files.
 
     `DiskController` is able to handle multiple services by adding a

activestorage/lib/active_storage/service/disk_service.rb

@@ -130,8 +130,6 @@ def generate_url(key, expires_in:, filename:, content_type:, disposition:)
           protocol: current_uri.scheme,
           host: current_uri.host,
           port: current_uri.port,
-          disposition: content_disposition,
-          content_type: content_type,
           filename: filename
         )
       end

activestorage/test/controllers/representations_controller_test.rb

@@ -14,7 +14,9 @@ class ActiveStorage::RepresentationsControllerWithVariantsTest < ActionDispatch:
       signed_blob_id: @blob.signed_id,
       variation_key: ActiveStorage::Variation.encode(resize: "100x100"))
 
-    assert_redirected_to(/racecar\.jpg\?.*disposition=inline/)
+    assert_redirected_to(/racecar\.jpg/)
+    follow_redirect!
+    assert_match(/^inline/, response.headers["Content-Disposition"])
 
     image = read_image(@blob.variant(resize: "100x100"))
     assert_equal 100, image.width
@@ -43,7 +45,9 @@ class ActiveStorage::RepresentationsControllerWithPreviewsTest < ActionDispatch:
       variation_key: ActiveStorage::Variation.encode(resize: "100x100"))
 
     assert_predicate @blob.preview_image, :attached?
-    assert_redirected_to(/report\.png\?.*disposition=inline/)
+    assert_redirected_to(/report\.png/)
+    follow_redirect!
+    assert_match(/^inline/, response.headers["Content-Disposition"])
 
     image = read_image(@blob.preview_image.variant(resize: "100x100"))
     assert_equal 77, image.width

activestorage/test/models/blob_test.rb

@@ -265,9 +265,8 @@ def expected_url_for(blob, disposition: :attachment, filename: nil, content_type
       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).merge(service_name: service_name)
+      key_params = { key: blob.key, disposition: ActionDispatch::Http::ContentDisposition.format(disposition: disposition, filename: filename.sanitized), content_type: content_type, 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}"
+      "https://example.com/rails/active_storage/disk/#{ActiveStorage.verifier.generate(key_params, expires_in: 5.minutes, purpose: :blob_key)}/#{filename}"
     end
 end

activestorage/test/service/disk_public_service_test.rb

@@ -14,6 +14,6 @@ class ActiveStorage::Service::DiskPublicServiceTest < ActiveSupport::TestCase
   test "public URL generation" do
     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\/.*\/avatar\.png\?content_type=image%2Fpng&disposition=inline.*/, url)
+    assert_match(/^https:\/\/example.com\/rails\/active_storage\/disk\/.*\/avatar\.png/, url)
   end
 end

activestorage/test/service/disk_service_test.rb

@@ -16,7 +16,7 @@ class ActiveStorage::Service::DiskServiceTest < ActiveSupport::TestCase
     original_url_options = Rails.application.routes.default_url_options.dup
     Rails.application.routes.default_url_options.merge!(protocol: "http", host: "test.example.com", port: 3001)
     begin
-      assert_match(/^https:\/\/example.com\/rails\/active_storage\/disk\/.*\/avatar\.png\?content_type=image%2Fpng&disposition=inline/,
+      assert_match(/^https:\/\/example.com\/rails\/active_storage\/disk\/.*\/avatar\.png$/,
         @service.url(@key, expires_in: 5.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("avatar.png"), content_type: "image/png"))
     ensure
       Rails.application.routes.default_url_options = original_url_options