Validate Content-Length

2 files changed, 46 insertions, 44 deletions

diff --git a/app/controllers/active_storage/disk_controller.rb b/app/controllers/active_storage/disk_controller.rb
index 6be88d2857..76377a0f20 100644
--- a/app/controllers/active_storage/disk_controller.rb
+++ b/app/controllers/active_storage/disk_controller.rb
@@ -45,9 +45,7 @@ class ActiveStorage::DiskController < ActionController::Base
       ActiveStorage.verifier.verified(params[:encoded_token], purpose: :blob_token)
     end
 
-    # FIXME: Validate Content-Length when we're using integration tests. Controller tests don't
-    # populate the header properly when a request body is provided.
     def acceptable_content?(token)
-      token[:content_type] == request.content_type
+      token[:content_type] == request.content_type && token[:content_length] == request.content_length
     end
 end
diff --git a/test/controllers/disk_controller_test.rb b/test/controllers/disk_controller_test.rb
index c79cc97423..83087eff68 100644
--- a/test/controllers/disk_controller_test.rb
+++ b/test/controllers/disk_controller_test.rb
@@ -4,69 +4,73 @@ require "database/setup"
 class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest
   test "showing blob inline" do
     blob = create_blob
+    key  = ActiveStorage.verifier.generate(blob.key, expires_in: 5.minutes, purpose: :blob_key)
 
-    get rails_disk_service_url(
-      filename: "hello.txt",
-      content_type: blob.content_type,
-      encoded_key: ActiveStorage.verifier.generate(blob.key, expires_in: 5.minutes, purpose: :blob_key)
-    )
-
+    get rails_disk_service_url(key, blob.filename, content_type: blob.content_type)
     assert_equal "inline; filename=\"#{blob.filename.base}\"", @response.headers["Content-Disposition"]
     assert_equal "text/plain", @response.headers["Content-Type"]
   end
 
-  test "sending blob as attachment" do
+  test "showing blob as attachment" do
     blob = create_blob
+    key  = ActiveStorage.verifier.generate(blob.key, expires_in: 5.minutes, purpose: :blob_key)
 
-    get rails_disk_service_url(
-      filename: blob.filename,
-      content_type: blob.content_type,
-      encoded_key: ActiveStorage.verifier.generate(blob.key, expires_in: 5.minutes, purpose: :blob_key),
-      disposition: :attachment
-    )
-
+    get rails_disk_service_url(key, blob.filename, content_type: blob.content_type, disposition: :attachment)
     assert_equal "attachment; filename=\"#{blob.filename.base}\"", @response.headers["Content-Disposition"]
     assert_equal "text/plain", @response.headers["Content-Type"]
   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)
-    token = ActiveStorage.verifier.generate(
-      {
-        key: blob.key,
-        content_length: data.size,
-        content_type: "text/plain",
-        checksum: Digest::MD5.base64digest(data)
-      },
-      expires_in: 5.minutes,
-      purpose: :blob_token
-    )
-
-    put update_rails_disk_service_url(encoded_token: token), params: data, headers: { "Content-Type" => "text/plain" }
+    data  = "Something else entirely!"
+    blob  = create_blob_before_direct_upload byte_size: data.size, checksum: Digest::MD5.base64digest(data)
+    token = encode_verified_token_for blob
 
+    put update_rails_disk_service_url(token), params: data, headers: { "Content-Type" => "text/plain" }
     assert_response :no_content
     assert_equal data, blob.download
   end
 
   test "directly uploading blob without integrity" do
-    data = "Something else entirely!"
-    blob = create_blob_before_direct_upload byte_size: data.size, checksum: Digest::MD5.base64digest(data)
+    data  = "Something else entirely!"
+    blob  = create_blob_before_direct_upload byte_size: data.size, checksum: Digest::MD5.base64digest("bad data")
+    token = encode_verified_token_for blob
+
+    put update_rails_disk_service_url(token), params: data
+    assert_response :unprocessable_entity
+    assert_not blob.service.exist?(blob.key)
+  end
+
+  test "directly uploading blob with mismatched content type" do
+    data  = "Something else entirely!"
+    blob  = create_blob_before_direct_upload byte_size: data.size, checksum: Digest::MD5.base64digest(data)
+    token = encode_verified_token_for blob
 
-    token = ActiveStorage.verifier.generate(
-      {
-        key: blob.key,
-        content_length: data.size,
-        content_type: "text/plain",
-        checksum: Digest::MD5.base64digest("bad data")
-      },
-      expires_in: 5.minutes,
-      purpose: :blob_token
-    )
+    put update_rails_disk_service_url(token), params: data, headers: { "Content-Type" => "application/octet-stream" }
+    assert_response :unprocessable_entity
+    assert_not blob.service.exist?(blob.key)
+  end
 
-    put update_rails_disk_service_url(encoded_token: token), params: { body: data }
+  test "directly uploading blob with mismatched content length" do
+    data  = "Something else entirely!"
+    blob  = create_blob_before_direct_upload byte_size: data.size - 1, checksum: Digest::MD5.base64digest(data)
+    token = encode_verified_token_for blob
 
+    put update_rails_disk_service_url(token), params: data, headers: { "Content-Type" => "text/plain" }
     assert_response :unprocessable_entity
     assert_not blob.service.exist?(blob.key)
   end
+
+  private
+    def encode_verified_token_for(blob)
+      ActiveStorage.verifier.generate(
+        {
+          key: blob.key,
+          content_length: blob.byte_size,
+          content_type: blob.content_type,
+          checksum: blob.checksum
+        },
+        expires_in: 5.minutes,
+        purpose: :blob_token
+      )
+    end
 end