Patch #18047

MailHandler: Don't use String#respond_to?(:force_encoding) to differentiate between Ruby 1.8 and Ruby 1.9

Added by Felix Schäfer about 4 years ago. Updated about 4 years ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:Email receiving
Target version:2.6.0

Description

In source:/trunk/app/models/mail_handler.rb@13413#L424, whether String#force_encoding is defined is used to make a decision about Mail::RubyVer, which defines pick_encoding on everything but Ruby 1.8.

This is dangerous as it couples knowledge about Ruby 1.8 (String#force_encoding is not defined) with knowledge about the Mail gem internals (Mail::RubyVer.pick_encoding is not defined on 1.8), there is no guarantee that this will always be the case though.

The attached patch checks whether Mail::RubyVer.pick_encoding is defined before using it instead of using the indirection through whether String#force_encoding is defined or not.

check_for_pick_encoding.patch Magnifier (573 Bytes) Felix Schäfer, 2014-10-08 14:31


Related issues

Related to Redmine - Patch #15785: Support more character encodings in incoming emails Closed

Associated revisions

Revision 13432
Added by Toshi MARUYAMA about 4 years ago

MailHandler: Don't use String#respond_to?(:force_encoding) (#18047)

Contributed by Felix Schäfer.

History

#1 Updated by Toshi MARUYAMA about 4 years ago

  • Related to Patch #15785: Support more character encodings in incoming emails added

#2 Updated by Toshi MARUYAMA about 4 years ago

  • Category set to Email receiving
  • Target version set to 2.6.0

#3 Updated by Toshi MARUYAMA about 4 years ago

  • Subject changed from Don't use String#respond_to?(:force_encoding) to differentiate between Ruby 1.8 and Ruby 1.9 to MailHandler: Don't use String#respond_to?(:force_encoding) to differentiate between Ruby 1.8 and Ruby 1.9

#4 Updated by Toshi MARUYAMA about 4 years ago

  • Status changed from New to Closed

Committed in trunk r13432, thanks.

Rails 4 dropped Ruby 1.8 support.
So, Rails 4 branch r13255 removed this check.

#5 Updated by Felix Schäfer about 4 years ago

Great, thanks!

Also available in: Atom PDF