Skip to content

Commit

Permalink
Default log device to binmode when it is a file
Browse files Browse the repository at this point in the history
When the log device is initialized with a filename default to opening
the file using binmode.

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 change the default 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.
  • Loading branch information
rafaelfranca committed Jun 5, 2019
1 parent 185abc2 commit 496c69e
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 0 deletions.
3 changes: 3 additions & 0 deletions lib/logger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@
#
# logger = Logger.new('logfile.log')
#
# *Note*: When a filename is use, the file is opened using the binary mode.
# For more information see IO#binmode.
#
# 3. Create a logger for the specified file.
#
# file = File.open('foo.log', File::WRONLY | File::APPEND)
Expand Down
2 changes: 2 additions & 0 deletions lib/logger/log_device.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ def set_dev(log)
else
@dev = open_logfile(log)
@dev.sync = true
@dev.binmode
@filename = log
end
end
Expand All @@ -101,6 +102,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
add_log_header(logdev)
logdev.flock(File::LOCK_UN)
rescue Errno::EEXIST
Expand Down
17 changes: 17 additions & 0 deletions test/logger/test_logdevice.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def test_initialize
begin
assert_file.exist?(@filename)
assert_predicate(logdev.dev, :sync)
assert_predicate(logdev.dev, :binmode?)
assert_equal(@filename, logdev.filename)
logdev.write('hello')
ensure
Expand All @@ -53,6 +54,8 @@ def test_initialize
# create logfile whitch is already exist.
logdev = d(@filename)
begin
assert_predicate(logdev.dev, :sync)
assert_predicate(logdev.dev, :binmode?)
logdev.write('world')
logfile = File.read(@filename)
assert_equal(2, logfile.split(/\n/).size)
Expand Down Expand Up @@ -103,6 +106,20 @@ class << (stderr = '')
assert_equal "log writing failed. disk is full\n", stderr
end

def test_write_binary_data
logdev = d(@filename)
begin
logdev.write("\x80".force_encoding("ASCII-8BIT"))
logfile = File.open(@filename)
logfile.binmode
logfile = logfile.read
assert_equal(2, logfile.split(/\n/).size)
assert_match(/^\x80$/, logfile)
ensure
logdev.close
end
end

def test_close
r, w = IO.pipe
logdev = d(w)
Expand Down

0 comments on commit 496c69e

Please sign in to comment.