From 7be823bb11ad2aa93ffd4d85cc7f11469a867fa2 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Fri, 16 Mar 2012 17:01:31 +0000 Subject: [PATCH] Treat messages received by email as plain text Move creation of a message from an email into the message model and adjust the logic to treat messages received by email as plain text. --- Gemfile | 3 +- Gemfile.lock | 12 +++-- app/models/message.rb | 23 +++++++++ script/deliver-message | 14 +----- test/unit/message_test.rb | 103 +++++++++++++++++++++++++++++++++++++- 5 files changed, 135 insertions(+), 20 deletions(-) diff --git a/Gemfile b/Gemfile index e28317997..de8664828 100644 --- a/Gemfile +++ b/Gemfile @@ -31,8 +31,9 @@ gem 'iconv', :platforms => :ruby_18 # Load libxml support for XML parsing and generation gem 'libxml-ruby', '>= 2.0.5', :require => 'libxml' -# Load HTML sanitizer +# Use for HTML sanitisation gem 'sanitize' +gem 'htmlentities' # Load SystemTimer for implementing request timeouts gem 'SystemTimer', '>= 1.1.3', :require => 'system_timer', :platforms => :ruby_18 diff --git a/Gemfile.lock b/Gemfile.lock index 0dcb5ccff..9b6f5ca2a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -40,7 +40,7 @@ GEM coffee-script-source execjs coffee-script-source (1.2.0) - composite_primary_keys (5.0.1) + composite_primary_keys (5.0.2) activerecord (~> 3.2.0) deadlock_retry (1.2.0) dynamic_form (1.1.4) @@ -52,6 +52,7 @@ GEM multipart-post (~> 1.1) rack (~> 1.1) hike (1.2.1) + htmlentities (4.3.1) http_accept_language (1.0.2) httpclient (2.2.4) i18n (0.6.0) @@ -63,7 +64,7 @@ GEM json (1.6.5) libv8 (3.3.10.4) libxml-ruby (2.2.2) - mail (2.4.1) + mail (2.4.4) i18n (>= 0.4.0) mime-types (~> 1.16) treetop (~> 1.4.8) @@ -71,7 +72,7 @@ GEM mime-types (1.17.2) multi_json (1.1.0) multipart-post (1.1.5) - nokogiri (1.5.0) + nokogiri (1.5.2) oauth (0.4.5) oauth-plugin (0.4.0.rc2) multi_json @@ -91,7 +92,7 @@ GEM pg (0.13.2) polyglot (0.3.3) rack (1.4.1) - rack-cache (1.1) + rack-cache (1.2) rack (>= 0.4) rack-openid (1.3.1) rack (>= 1.1.0) @@ -142,7 +143,7 @@ GEM treetop (1.4.10) polyglot polyglot (>= 0.3.1) - tzinfo (0.3.31) + tzinfo (0.3.32) uglifier (1.2.3) execjs (>= 0.3.0) multi_json (>= 1.0.2) @@ -157,6 +158,7 @@ DEPENDENCIES composite_primary_keys (>= 5.0.0) deadlock_retry (>= 1.2.0) dynamic_form + htmlentities http_accept_language (>= 1.0.2) httpclient iconv diff --git a/app/models/message.rb b/app/models/message.rb index feceec5c0..f897af3c2 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -13,6 +13,29 @@ class Message < ActiveRecord::Base after_initialize :set_defaults + def self.from_mail(mail, from, to) + if mail.multipart? + if mail.text_part + body = mail.text_part.decoded + elsif mail.html_part + body = HTMLEntities.new.decode(Sanitize.clean(mail.html_part.decoded)) + end + elsif mail.text? and mail.sub_type == "html" + body = HTMLEntities.new.decode(Sanitize.clean(mail.decoded)) + else + body = mail.decoded + end + + message = Message.new({ + :sender => from, + :recipient => to, + :sent_on => mail.date.new_offset(0), + :title => mail.subject.sub(/\[OpenStreetMap\] */, ""), + :body => body, + :body_format => "text" + }, :without_protection => true) + end + def body RichText.new(read_attribute(:body_format), read_attribute(:body)) end diff --git a/script/deliver-message b/script/deliver-message index 6dffea96c..3fd641f8d 100755 --- a/script/deliver-message +++ b/script/deliver-message @@ -22,19 +22,7 @@ message.update_attribute(:message_read, true) if message mail = Mail.new(STDIN.readlines.join) -if mail.multipart? - body = mail.html_part || mail.text_part -else - body = mail -end - -message = Message.new({ - :sender => from, - :recipient => to, - :sent_on => mail.date.new_offset(0), - :title => mail.subject.sub(/\[OpenStreetMap\] */, ""), - :body => body.decoded -}, :without_protection => true) +message = Message.from_mail(mail, from, to) message.save! Notifier.message_notification(message).deliver diff --git a/test/unit/message_test.rb b/test/unit/message_test.rb index 58523ca88..863b1a4f2 100644 --- a/test/unit/message_test.rb +++ b/test/unit/message_test.rb @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- require File.dirname(__FILE__) + '/../test_helper' class MessageTest < ActiveSupport::TestCase @@ -81,6 +82,107 @@ class MessageTest < ActiveSupport::TestCase end end + def test_from_mail_plain + mail = Mail.new do + from "from@example.com" + to "to@example.com" + subject "Test message" + date Time.now + content_type 'text/plain; charset=utf-8' + body "This is a test & a message" + end + message = Message.from_mail(mail, users(:normal_user), users(:public_user)) + assert_equal users(:normal_user), message.sender + assert_equal users(:public_user), message.recipient + assert_equal mail.date, message.sent_on + assert_equal "Test message", message.title + assert_equal "This is a test & a message", message.body + assert_equal "text", message.body_format + end + + def test_from_mail_html + mail = Mail.new do + from "from@example.com" + to "to@example.com" + subject "Test message" + date Time.now + content_type 'text/html; charset=utf-8' + body "

This is a test & a message

" + end + message = Message.from_mail(mail, users(:normal_user), users(:public_user)) + assert_equal users(:normal_user), message.sender + assert_equal users(:public_user), message.recipient + assert_equal mail.date, message.sent_on + assert_equal "Test message", message.title + assert_match /^ *This is a test & a message *$/, message.body + assert_equal "text", message.body_format + end + + def test_from_mail_multipart + mail = Mail.new do + from "from@example.com" + to "to@example.com" + subject "Test message" + date Time.now + + text_part do + content_type 'text/plain; charset=utf-8' + body "This is a test & a message in text format" + end + + html_part do + content_type 'text/html; charset=utf-8' + body "

This is a test & a message in HTML format

" + end + end + message = Message.from_mail(mail, users(:normal_user), users(:public_user)) + assert_equal users(:normal_user), message.sender + assert_equal users(:public_user), message.recipient + assert_equal mail.date, message.sent_on + assert_equal "Test message", message.title + assert_equal "This is a test & a message in text format", message.body + assert_equal "text", message.body_format + + mail = Mail.new do + from "from@example.com" + to "to@example.com" + subject "Test message" + date Time.now + + html_part do + content_type 'text/html; charset=utf-8' + body "

This is a test & a message in HTML format

" + end + end + message = Message.from_mail(mail, users(:normal_user), users(:public_user)) + assert_equal users(:normal_user), message.sender + assert_equal users(:public_user), message.recipient + assert_equal mail.date, message.sent_on + assert_equal "Test message", message.title + assert_match /^ *This is a test & a message in HTML format *$/, message.body + assert_equal "text", message.body_format + end + + def test_from_mail_prefix + mail = Mail.new do + from "from@example.com" + to "to@example.com" + subject "[OpenStreetMap] Test message" + date Time.now + content_type 'text/plain; charset=utf-8' + body "This is a test & a message" + end + message = Message.from_mail(mail, users(:normal_user), users(:public_user)) + assert_equal users(:normal_user), message.sender + assert_equal users(:public_user), message.recipient + assert_equal mail.date, message.sent_on + assert_equal "Test message", message.title + assert_equal "This is a test & a message", message.body + assert_equal "text", message.body_format + end + +private + def make_message(char, count) message = messages(:one) message.title = char * count @@ -93,5 +195,4 @@ class MessageTest < ActiveSupport::TestCase response = message.class.find(message.id) # stand by for some über-generalisation... assert_equal char * count, response.title, "message with #{count} #{char} chars (i.e. #{char.length*count} bytes) fails" end - end -- 2.39.5