Merge pull request #21 from jeremy/disk-binary-io

Disk service: use binary IO throughout, not UTF-8

2 files changed, 8 insertions, 14 deletions

diff --git a/lib/active_storage/service/disk_service.rb b/lib/active_storage/service/disk_service.rb
index 5576b3b125..f6c4fd8c4b 100644
--- a/lib/active_storage/service/disk_service.rb
+++ b/lib/active_storage/service/disk_service.rb
@@ -1,5 +1,6 @@
 require "fileutils"
 require "pathname"
+require "digest/md5"
 require "active_support/core_ext/numeric/bytes"
 
 class ActiveStorage::Service::DiskService < ActiveStorage::Service
@@ -10,24 +11,19 @@ class ActiveStorage::Service::DiskService < ActiveStorage::Service
   end
 
   def upload(key, io, checksum: nil)
-    File.open(make_path_for(key), "wb") do |file|
-      while chunk = io.read(64.kilobytes)
-        file.write(chunk)
-      end
-    end
-
+    IO.copy_stream(io, make_path_for(key))
     ensure_integrity_of(key, checksum) if checksum
   end
 
   def download(key)
     if block_given?
       File.open(path_for(key)) do |file|
-        while data = file.read(64.kilobytes)
+        while data = file.binread(64.kilobytes)
           yield data
         end
       end
     else
-      File.open path_for(key), &:read
+      File.binread path_for(key)
     end
   end
 
diff --git a/test/service/shared_service_tests.rb b/test/service/shared_service_tests.rb
index 99bc252eea..e799c24c35 100644
--- a/test/service/shared_service_tests.rb
+++ b/test/service/shared_service_tests.rb
@@ -13,18 +13,16 @@ module ActiveStorage::Service::SharedServiceTests
   extend ActiveSupport::Concern
 
   FIXTURE_KEY  = SecureRandom.base58(24)
-  FIXTURE_FILE = StringIO.new("Hello world!")
+  FIXTURE_DATA = "\211PNG\r\n\032\n\000\000\000\rIHDR\000\000\000\020\000\000\000\020\001\003\000\000\000%=m\"\000\000\000\006PLTE\000\000\000\377\377\377\245\331\237\335\000\000\0003IDATx\234c\370\377\237\341\377_\206\377\237\031\016\2603\334?\314p\1772\303\315\315\f7\215\031\356\024\203\320\275\317\f\367\201R\314\f\017\300\350\377\177\000Q\206\027(\316]\233P\000\000\000\000IEND\256B`\202".force_encoding(Encoding::BINARY)
 
   included do
     setup do
       @service = self.class.const_get(:SERVICE)
-      @service.upload FIXTURE_KEY, FIXTURE_FILE
-      FIXTURE_FILE.rewind
+      @service.upload FIXTURE_KEY, StringIO.new(FIXTURE_DATA)
     end
 
     teardown do
       @service.delete FIXTURE_KEY
-      FIXTURE_FILE.rewind
     end
 
     test "uploading with integrity" do
@@ -53,7 +51,7 @@ module ActiveStorage::Service::SharedServiceTests
     end
 
     test "downloading" do
-      assert_equal FIXTURE_FILE.read, @service.download(FIXTURE_KEY)
+      assert_equal FIXTURE_DATA, @service.download(FIXTURE_KEY)
     end
 
     test "existing" do
@@ -65,7 +63,7 @@ module ActiveStorage::Service::SharedServiceTests
       @service.delete FIXTURE_KEY
       assert_not @service.exist?(FIXTURE_KEY)
     end
-  
+
     test "deleting nonexistent key" do
       assert_nothing_raised do
         @service.delete SecureRandom.base58(24)