Add option to set the binary mode of the log device

rafaelfranca · rafaelfranca · commit 9114b3ac7ee7 · 2019-06-07T16:06:25.000-04:00
Without binmode strings with incompatible encoding can't be written in
the file. This is very common in applications that log user provided
parameters.

We need to allow changing the binnary mode because right now it is impossible to use
the built-in log rotation feature when you provide a File object to the
LogDevice, and if you provide a filename you can't have binmode.
diff --git a/lib/logger.rb b/lib/logger.rb
@@ -365,6 +365,8 @@ def fatal!; self.level = FATAL; end
   #   Logging formatter. Default values is an instance of Logger::Formatter.
   # +datetime_format+::
   #   Date and time format. Default value is '%Y-%m-%d %H:%M:%S'.
+  # +binmode+::
+  #   Use binany mode on the log device. Defaul value is false.
   # +shift_period_suffix+::
   #   The log file suffix format for +daily+, +weekly+ or +monthly+ rotation.
   #   Default is '%Y%m%d'.
@@ -375,17 +377,18 @@ def fatal!; self.level = FATAL; end
   #
   def initialize(logdev, shift_age = 0, shift_size = 1048576, level: DEBUG,
                  progname: nil, formatter: nil, datetime_format: nil,
-                 shift_period_suffix: '%Y%m%d')
+                 binmode: false, shift_period_suffix: '%Y%m%d')
     self.level = level
     self.progname = progname
     @default_formatter = Formatter.new
     self.datetime_format = datetime_format
     self.formatter = formatter
     @logdev = nil
     if logdev
-      @logdev = LogDevice.new(logdev, :shift_age => shift_age,
-        :shift_size => shift_size,
-        :shift_period_suffix => shift_period_suffix)
+      @logdev = LogDevice.new(logdev, shift_age: shift_age,
+        shift_size: shift_size,
+        shift_period_suffix: shift_period_suffix,
+        binmode: binmode)
     end
   end
 
diff --git a/lib/logger/log_device.rb b/lib/logger/log_device.rb
@@ -11,8 +11,9 @@ class LogDevice
     attr_reader :filename
     include MonitorMixin
 
-    def initialize(log = nil, shift_age: nil, shift_size: nil, shift_period_suffix: nil)
+    def initialize(log = nil, shift_age: nil, shift_size: nil, shift_period_suffix: nil, binmode: false)
       @dev = @filename = @shift_age = @shift_size = @shift_period_suffix = nil
+      @binmode = binmode
       mon_initialize
       set_dev(log)
       if @filename
@@ -84,6 +85,7 @@ def set_dev(log)
       else
         @dev = open_logfile(log)
         @dev.sync = true
+        @dev.binmode if @binmode
         @filename = log
       end
     end
@@ -101,6 +103,7 @@ def create_logfile(filename)
         logdev = File.open(filename, (File::WRONLY | File::APPEND | File::CREAT | File::EXCL))
         logdev.flock(File::LOCK_EX)
         logdev.sync = true
+        logdev.binmode if @binmode
         add_log_header(logdev)
         logdev.flock(File::LOCK_UN)
       rescue Errno::EEXIST
diff --git a/test/logger/test_logdevice.rb b/test/logger/test_logdevice.rb
@@ -45,6 +45,7 @@ def test_initialize
     begin
       assert_file.exist?(@filename)
       assert_predicate(logdev.dev, :sync)
+      refute_predicate(logdev.dev, :binmode?)
       assert_equal(@filename, logdev.filename)
       logdev.write('hello')
     ensure
@@ -53,6 +54,8 @@ def test_initialize
     # create logfile whitch is already exist.
     logdev = d(@filename)
     begin
+      assert_predicate(logdev.dev, :sync)
+      refute_predicate(logdev.dev, :binmode?)
       logdev.write('world')
       logfile = File.read(@filename)
       assert_equal(2, logfile.split(/\n/).size)
diff --git a/test/logger/test_logger.rb b/test/logger/test_logger.rb
@@ -240,6 +240,29 @@ def test_add
     assert_equal("false\n", log.msg)
   end
 
+  def test_add_binary_data_with_binmode_logdev
+    EnvUtil.with_default_internal(Encoding::UTF_8) do
+      begin
+        tempfile = Tempfile.new("logger")
+        tempfile.close
+        filename = tempfile.path
+        File.unlink(filename)
+
+        logger = Logger.new filename, binmode: true
+        logger.level = Logger::DEBUG
+
+        str = +"\x80"
+        str.force_encoding("ASCII-8BIT")
+
+        logger.add Logger::DEBUG, str
+        assert_equal(2, File.binread(filename).split(/\n/).size)
+      ensure
+        logger.close
+        tempfile.unlink
+      end
+    end
+  end
+
   def test_level_log
     logger = Logger.new(nil)
     logger.progname = "my_progname"
