From d192db441ab6451595830f672a718b48fbb1612c Mon Sep 17 00:00:00 2001 From: Holger Just Date: Thu, 26 Jan 2017 23:20:02 +0100 Subject: [PATCH] Retain the original message in a Message object We only format and convert it after passing it through the formatter. That way, we can ensure that the actually interesting formatted value is valid. --- lib/rackstash/formatter.rb | 2 +- lib/rackstash/message.rb | 7 ++- spec/rackstash/formatter_spec.rb | 8 ++++ spec/rackstash/message_spec.rb | 79 +++++++++++++++++++------------- 4 files changed, 60 insertions(+), 36 deletions(-) diff --git a/lib/rackstash/formatter.rb b/lib/rackstash/formatter.rb index 2d58197..b51378b 100644 --- a/lib/rackstash/formatter.rb +++ b/lib/rackstash/formatter.rb @@ -28,7 +28,7 @@ module Rackstash class RawFormatter def call(_severity, _timestamp, _progname, msg) - msg + msg.is_a?(String) ? msg : msg.inspect end end end diff --git a/lib/rackstash/message.rb b/lib/rackstash/message.rb index 29ce90f..95a5b7d 100644 --- a/lib/rackstash/message.rb +++ b/lib/rackstash/message.rb @@ -34,7 +34,7 @@ module Rackstash progname: PROGNAME, formatter: RAW_FORMATTER ) - @message = cleanup_message(msg) + @message = dup_freeze(msg) @severity = Integer(severity) @severity = 0 if @severity < 0 @@ -54,7 +54,7 @@ module Rackstash end def to_s - @formatter.call(severity_label, @time, @progname, @message) + cleanup @formatter.call(severity_label, @time, @progname, @message) end alias_method :to_str, :to_s alias_method :as_json, :to_s @@ -66,8 +66,7 @@ module Rackstash # # @param msg [#to_s] a message to be added to the buffer # @return [String] the sanitized frozen message - def cleanup_message(msg) - msg = msg.inspect unless msg.is_a?(String) + def cleanup(msg) msg = utf8_encode(msg) # remove useless ANSI color codes msg.gsub!(/\e\[[0-9;]*m/, EMPTY_STRING) diff --git a/spec/rackstash/formatter_spec.rb b/spec/rackstash/formatter_spec.rb index e9085f5..eb050f7 100644 --- a/spec/rackstash/formatter_spec.rb +++ b/spec/rackstash/formatter_spec.rb @@ -54,4 +54,12 @@ describe Rackstash::RawFormatter do msg = 'my message' expect(formatter.call('ERROR', Time.now, 'progname', msg)).to equal msg end + + it 'inspects non-string messages' do + obj = Object.new + + expect(obj).to receive(:inspect).and_return('object') + expect(formatter.call('ERROR', Time.now, 'progname', obj)).to eql 'object' + end + end diff --git a/spec/rackstash/message_spec.rb b/spec/rackstash/message_spec.rb index 0a0db5f..f099ce1 100644 --- a/spec/rackstash/message_spec.rb +++ b/spec/rackstash/message_spec.rb @@ -11,44 +11,20 @@ require 'rackstash/message' describe Rackstash::Message do describe '#message' do it 'dups the message string' do - str = 'a message' + str = 'a message'.encode(Encoding::ASCII_8BIT) message = Rackstash::Message.new(str) expect(message.message).to eql str expect(message.message).not_to equal str + expect(message.message.encoding).to eql Encoding::ASCII_8BIT expect(message.message).to be_frozen end - it 'cleans the message' do - messages = [ - ["First\r\nSecond", "First\nSecond"], - ["First\r\nSecond\n\r", "First\nSecond\n\n"], - ["Foo\r\n\rBar", "Foo\n\nBar"], - ["\r \tWord\n\nPhrase\n", "\n \tWord\n\nPhrase\n"], - ["\e[31mRED TEXT\e[0m", 'RED TEXT'] - ] + it 'accepts non-string objects' do + exception = StandardError.new('An error') + message = Rackstash::Message.new(exception) - messages.each do |msg, clean| - message = Rackstash::Message.new(msg) - expect(message.message).to eql clean - end - end - - it 'encodes the message as UTF-8' do - utf8_str = 'Dönerstraße' - latin_str = utf8_str.encode(Encoding::ISO8859_9) - expect(latin_str.encoding).to eql Encoding::ISO8859_9 - - message = Rackstash::Message.new(latin_str) - expect(message.message).to eql utf8_str - expect(message.message.encoding).to eql Encoding::UTF_8 - end - - it 'does not raise an error on incompatible encodings' do - binary = Digest::SHA256.digest('string') - message = Rackstash::Message.new(binary) - - expect(message.message).to include '�' + expect(message.message).to eq exception end end @@ -127,7 +103,7 @@ describe Rackstash::Message do expect(Rackstash::Message.new('').formatter).to equal Rackstash::Message::RAW_FORMATTER message = Rackstash::Message.new('Beep boop') - expect(message.to_s).to equal message.message + expect(message.to_s).to eql 'Beep boop' end end @@ -153,6 +129,47 @@ describe Rackstash::Message do expect(message.to_s).to eql 'Formatted Message' end + + it 'cleans the message' do + messages = [ + ["First\r\nSecond", "First\nSecond"], + ["First\r\nSecond\n\r", "First\nSecond\n\n"], + ["Foo\r\n\rBar", "Foo\n\nBar"], + ["\r \tWord\n\nPhrase\n", "\n \tWord\n\nPhrase\n"], + ["\e[31mRED TEXT\e[0m", 'RED TEXT'] + ] + + messages.each do |msg, clean| + message = Rackstash::Message.new(msg) + expect(message.to_s).to eql clean + end + end + + it 'encodes the message as UTF-8' do + utf8_str = 'Dönerstraße' + latin_str = utf8_str.encode(Encoding::ISO8859_9) + expect(latin_str.encoding).to eql Encoding::ISO8859_9 + + message = Rackstash::Message.new(latin_str) + expect(message.to_s).to eql utf8_str + expect(message.to_s.encoding).to eql Encoding::UTF_8 + end + + it 'does not raise an error on incompatible encodings' do + binary = Digest::SHA256.digest('string') + message = Rackstash::Message.new(binary) + + expect(message.to_s).to include '�' + expect(message.to_s.encoding).to eql Encoding::UTF_8 + end + + it 'accepts non-string objects' do + exception = StandardError.new('An error') + message = Rackstash::Message.new(exception) + + expect(message.to_s).to eql '#' + end + end describe '#frozen?' do