Fix RUBY-2210 GridFS does not write data correctly when given String in UTF-8 encoding (#1858)

shallow_symbolize_keys replaced by expanded version

Co-authored-by: Oleg Pudeyev <[email protected]>

Author: p-mongo

Diff for: lib/mongo/grid/file.rb

@@ -104,6 +104,11 @@ def inspect
       #   chunk objects and assemble the data. If we have an IO object, then
       #   it's the original file data and we must split it into chunks and set
       #   the original data itself.
+      #
+      # @param [ IO, String, Array<BSON::Document> ] value The file object,
+      #   file contents or chunk documents.
+      #
+      # @return [ Array<Grid::File::Chunk> ] Array of chunks.
       def initialize_chunks!(value)
         if value.is_a?(Array)
           @chunks = value.map{ |doc| Chunk.new(doc) }

Diff for: lib/mongo/grid/file/chunk.rb

@@ -151,6 +151,7 @@ class << self
           # @return [ String ] The assembled data.
           #
           # @since 2.0.0
+          # @api private
           def assemble(chunks)
             chunks.reduce(''){ |data, chunk| data << chunk.data.data }
           end
@@ -167,6 +168,7 @@ def assemble(chunks)
           # @return [ Array<Chunk> ] The chunks of the data.
           #
           # @since 2.0.0
+          # @api private
           def split(io, file_info, offset = 0)
             io = StringIO.new(io) if io.is_a?(String)
             parts = Enumerator.new { |y| y << io.read(file_info.chunk_size) until io.eof? }

Diff for: lib/mongo/grid/fs_bucket.rb

@@ -177,7 +177,7 @@ def initialize(database, options = {})
       #
       # @since 2.0.0
       def prefix
-        @options[:fs_name] || @options[:bucket_name]|| DEFAULT_ROOT
+        @options[:fs_name] || @options[:bucket_name] || DEFAULT_ROOT
       end
 
       # Remove a single file from the GridFS.
@@ -230,7 +230,8 @@ def delete(id)
       #
       # @since 2.1.0
       def open_download_stream(id, options = nil)
-        read_stream(id, options).tap do |stream|
+        options = Hash[(options || {}).map { |k, v| [k.to_sym, v] }]
+        read_stream(id, **options).tap do |stream|
           if block_given?
             begin
               yield stream
@@ -348,15 +349,15 @@ def download_to_stream_by_name(filename, io, opts = {})
         download_to_stream(open_download_stream_by_name(filename, opts).file_id, io)
       end
 
-      # Opens an upload stream to GridFS to which the contents of a user file came be written.
+      # Opens an upload stream to GridFS to which the contents of a file or
+      # blob can be written.
       #
-      # @example Open a stream to which the contents of a file came be written.
-      #   fs.open_upload_stream('a-file.txt')
-      #
-      # @param [ String ] filename The filename of the file to upload.
+      # @param [ String ] filename The name of the file in GridFS.
       # @param [ Hash ] opts The options for the write stream.
       #
-      # @option opts [ Object ] :file_id An optional unique file id. An ObjectId is generated otherwise.
+      # @option opts [ Object ] :file_id An optional unique file id.
+      #   A BSON::ObjectId is automatically generated if a file id is not
+      #   provided.
       # @option opts [ Integer ] :chunk_size Override the default chunk size.
       # @option opts [ Hash ] :metadata User data for the 'metadata' field of the files
       #   collection document.
@@ -375,7 +376,8 @@ def download_to_stream_by_name(filename, io, opts = {})
       #
       # @since 2.1.0
       def open_upload_stream(filename, opts = {})
-        write_stream(filename, opts).tap do |stream|
+        opts = Hash[opts.map { |k, v| [k.to_sym, v] }]
+        write_stream(filename, **opts).tap do |stream|
           if block_given?
             begin
               yield stream
@@ -462,12 +464,12 @@ def write_concern
       #
       # @option opts [ BSON::Document ] :file_info_doc For internal
       #   driver use only. A BSON document to use as file information.
-      def read_stream(id, opts = nil)
-        Stream.get(self, Stream::READ_MODE, { file_id: id }.update(options).update(opts || {}))
+      def read_stream(id, **opts)
+        Stream.get(self, Stream::READ_MODE, { file_id: id }.update(options).update(opts))
       end
 
-      def write_stream(filename, opts)
-        Stream.get(self, Stream::WRITE_MODE, { filename: filename }.merge!(options).merge!(opts))
+      def write_stream(filename, **opts)
+        Stream.get(self, Stream::WRITE_MODE, { filename: filename }.update(options).update(opts))
       end
 
       def chunks_name

Diff for: lib/mongo/grid/stream/write.rb

@@ -82,20 +82,26 @@ def initialize(fs, options)
             @open = true
           end
 
-          # Write to the GridFS bucket from the source stream.
+          # Write to the GridFS bucket from the source stream or a string.
           #
           # @example Write to GridFS.
           #   stream.write(io)
           #
-          # @param [ IO ] io The source io stream to upload from.
+          # @param [ String | IO ] io The string or IO object to upload from.
           #
           # @return [ Stream::Write ] self The write stream itself.
           #
           # @since 2.1.0
           def write(io)
             ensure_open!
             @indexes ||= ensure_indexes!
-            @length += io.size
+            @length += if io.respond_to?(:bytesize)
+              # String objects
+              io.bytesize
+            else
+              # IO objects
+              io.size
+            end
             chunks = File::Chunk.split(io, file_info, @n)
             @n += chunks.size
             chunks_collection.insert_many(chunks) unless chunks.empty?

Diff for: spec/integration/grid_fs_bucket_spec.rb

@@ -0,0 +1,48 @@
+require 'spec_helper'
+
+describe 'GridFS bucket integration' do
+  let(:fs) do
+    authorized_client.database.fs
+  end
+
+  describe 'UTF-8 string write' do
+    let(:data) { "hello\u2210" }
+
+    before do
+      data.length.should_not == data.bytesize
+    end
+
+    shared_examples 'round-trips' do
+      it 'round-trips' do
+        stream = fs.open_upload_stream('test') do |stream|
+          stream.write(data_to_write)
+        end
+
+        actual = nil
+        fs.open_download_stream(stream.file_id) do |stream|
+          actual = stream.read
+        end
+
+        actual.encoding.name.should == 'ASCII-8BIT'
+        actual.should == data.dup.force_encoding('binary')
+      end
+    end
+
+    context 'in binary encoding' do
+      let(:data_to_write) do
+        data.force_encoding('binary').freeze
+      end
+
+      it_behaves_like 'round-trips'
+    end
+
+    context 'in UTF-8 encoding' do
+      let(:data_to_write) do
+        data.encoding.name.should == 'UTF-8'
+        data.freeze
+      end
+
+      it_behaves_like 'round-trips'
+    end
+  end
+end