diff options
Diffstat (limited to 'app/models')
| -rw-r--r-- | app/models/about_me_validator.rb | 8 | ||||
| -rw-r--r-- | app/models/application_mailer.rb | 14 | ||||
| -rw-r--r-- | app/models/change_email_validator.rb | 5 | ||||
| -rw-r--r-- | app/models/comment.rb | 41 | ||||
| -rw-r--r-- | app/models/contact_mailer.rb | 2 | ||||
| -rw-r--r-- | app/models/foi_attachment.rb | 73 | ||||
| -rw-r--r-- | app/models/incoming_message.rb | 507 | ||||
| -rw-r--r-- | app/models/info_request.rb | 22 | ||||
| -rw-r--r-- | app/models/info_request_event.rb | 2 | ||||
| -rw-r--r-- | app/models/outgoing_mailer.rb | 3 | ||||
| -rw-r--r-- | app/models/outgoing_message.rb | 56 | ||||
| -rw-r--r-- | app/models/profile_photo.rb | 6 | ||||
| -rw-r--r-- | app/models/public_body.rb | 23 | ||||
| -rw-r--r-- | app/models/request_mailer.rb | 39 | ||||
| -rw-r--r-- | app/models/track_mailer.rb | 2 | ||||
| -rw-r--r-- | app/models/track_thing.rb | 15 | ||||
| -rw-r--r-- | app/models/user.rb | 25 |
17 files changed, 296 insertions, 547 deletions
diff --git a/app/models/about_me_validator.rb b/app/models/about_me_validator.rb index 5e04c2761..8ee505ac8 100644 --- a/app/models/about_me_validator.rb +++ b/app/models/about_me_validator.rb @@ -17,10 +17,14 @@ class AboutMeValidator < ActiveRecord::BaseWithoutTable column :about_me, :text, "I...", false - def validate + # TODO: Switch to built in validations + validate :length_of_about_me + + private + + def length_of_about_me if !self.about_me.blank? && self.about_me.size > 500 errors.add(:about_me, _("Please keep it shorter than 500 characters")) end end - end diff --git a/app/models/application_mailer.rb b/app/models/application_mailer.rb index cdb279c3c..1a97a4bf9 100644 --- a/app/models/application_mailer.rb +++ b/app/models/application_mailer.rb @@ -77,6 +77,17 @@ class ApplicationMailer < ActionMailer::Base # and via template_path, which is created from it) in the create! method when # looking for templates. Our modified version looks for templates in the view_paths # in order. + + # It also has a line converting the mail subject to a string. This is because we + # use translated strings in the subject lines, sometimes in conjunction with + # user input, like request titles. The _() function used for translation + # returns an instance of SafeBuffer, which doesn't handle gsub calls in the block form + # with $ variables - https://github.com/rails/rails/issues/1555. + # Unfortunately ActionMailer uses that form in quoted_printable(), which will be + # called if any part of the subject requires quoting. So we convert the subject + # back to a string via to_str() before passing in to create_mail. There is a test + # for this in spec/models/request_mailer_spec.rb + # Changed lines marked with *** # Initialize the mailer via the given +method_name+. The body will be @@ -139,6 +150,9 @@ class ApplicationMailer < ActionMailer::Base # already set. @mime_version ||= "1.0" if !@parts.empty? + # *** Convert into a string + @subject = @subject.to_str if @subject + # build the mail object itself @mail = create_mail end diff --git a/app/models/change_email_validator.rb b/app/models/change_email_validator.rb index 9ef25217d..2ddebb177 100644 --- a/app/models/change_email_validator.rb +++ b/app/models/change_email_validator.rb @@ -28,12 +28,15 @@ class ChangeEmailValidator < ActiveRecord::BaseWithoutTable validates_presence_of :old_email, :message => N_("Please enter your old email address") validates_presence_of :new_email, :message => N_("Please enter your new email address") validates_presence_of :password, :message => N_("Please enter your password"), :unless => :changing_email + validate :password_and_format_of_email def changing_email() self.user_circumstance == 'change_email' end - def validate + private + + def password_and_format_of_email if !self.old_email.blank? && !MySociety::Validate.is_valid_email(self.old_email) errors.add(:old_email, _("Old email doesn't look like a valid address")) end diff --git a/app/models/comment.rb b/app/models/comment.rb index 5507910e2..70f3ba00d 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -24,13 +24,13 @@ class Comment < ActiveRecord::Base strip_attributes! belongs_to :user - #validates_presence_of :user # breaks during construction of new ones :( - - validates_inclusion_of :comment_type, :in => [ 'request' ] belongs_to :info_request - has_many :info_request_events # in practice only ever has one + #validates_presence_of :user # breaks during construction of new ones :( + validates_inclusion_of :comment_type, :in => [ 'request' ] + validate :body_of_comment + def body ret = read_attribute(:body) if ret.nil? @@ -40,6 +40,7 @@ class Comment < ActiveRecord::Base ret = ret.gsub(/(?:\n\s*){2,}/, "\n\n") # remove excess linebreaks that unnecessarily space it out ret end + def raw_body read_attribute(:body) end @@ -52,23 +53,13 @@ class Comment < ActiveRecord::Base end end - # Check have edited comment - def validate - if self.body.empty? || self.body =~ /^\s+$/ - errors.add(:body, _("Please enter your annotation")) - end - if !MySociety::Validate.uses_mixed_capitals(self.body) - errors.add(:body, _('Please write your annotation using a mixture of capital and lower case letters. This makes it easier for others to read.')) - end - end - # Return body for display as HTML def get_body_for_html_display text = self.body.strip text = CGI.escapeHTML(text) text = MySociety::Format.make_clickable(text, :contract => 1) text = text.gsub(/\n/, '<br>') - return text + return text.html_safe end # When posting a new comment, use this to check user hasn't double submitted. @@ -82,9 +73,21 @@ class Comment < ActiveRecord::Base return Comment.find(:first, :conditions => [ "info_request_id = ? and body = ?", info_request_id, body ]) end end - def for_admin_column - self.class.content_columns.each do |column| - yield(column.human_name, self.send(column.name), column.type.to_s, column.name) + + def for_admin_column + self.class.content_columns.each do |column| + yield(column.human_name, self.send(column.name), column.type.to_s, column.name) + end + end + + private + + def body_of_comment + if self.body.empty? || self.body =~ /^\s+$/ + errors.add(:body, _("Please enter your annotation")) + end + if !MySociety::Validate.uses_mixed_capitals(self.body) + errors.add(:body, _('Please write your annotation using a mixture of capital and lower case letters. This makes it easier for others to read.')) + end end - end end diff --git a/app/models/contact_mailer.rb b/app/models/contact_mailer.rb index 16aae2f15..abde64928 100644 --- a/app/models/contact_mailer.rb +++ b/app/models/contact_mailer.rb @@ -7,7 +7,7 @@ class ContactMailer < ApplicationMailer # Send message to administrator - def message(name, email, subject, message, logged_in_user, last_request, last_body) + def to_admin_message(name, email, subject, message, logged_in_user, last_request, last_body) @from = name + " <" + email + ">" @recipients = contact_from_name_and_email @subject = subject diff --git a/app/models/foi_attachment.rb b/app/models/foi_attachment.rb index a40898aef..bba0b6a8d 100644 --- a/app/models/foi_attachment.rb +++ b/app/models/foi_attachment.rb @@ -38,7 +38,7 @@ class FoiAttachment < ActiveRecord::Base BODY_MAX_DELAY = 5 def directory - rails_env = ENV['RAILS_ENV'] + rails_env = Rails.env if rails_env.nil? || rails_env.empty? raise "$RAILS_ENV is not set" end @@ -67,9 +67,22 @@ class FoiAttachment < ActiveRecord::Base file.write d } update_display_size! + encode_cached_body! @cached_body = d end + # If the original mail part had a charset, it's some kind of string, so assume that + # it should be handled as a string in the stated charset, not a bytearray, and then + # convert it our default encoding. For ruby 1.8 this is a noop. + def encode_cached_body! + if RUBY_VERSION.to_f >= 1.9 + if charset + @cached_body.force_encoding(charset) + @cached_body = @cached_body.encode(Encoding.default_internal, charset) + end + end + end + def body if @cached_body.nil? tries = 0 @@ -90,6 +103,7 @@ class FoiAttachment < ActiveRecord::Base self.incoming_message.parse_raw_email!(force) retry end + encode_cached_body! end return @cached_body end @@ -205,7 +219,7 @@ class FoiAttachment < ActiveRecord::Base def ensure_filename! - if self.filename.nil? + if self.filename.blank? calc_ext = AlaveteliFileTypes.mimetype_to_extension(self.content_type) if !calc_ext calc_ext = "bin" @@ -303,38 +317,47 @@ class FoiAttachment < ActiveRecord::Base text = CGI.escapeHTML(text) text = MySociety::Format.make_clickable(text) html = text.gsub(/\n/, '<br>') - return '<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" - "http://www.w3.org/TR/html4/loose.dtd"><html><head><title></title></head><body>' + html + "</body></html>", wrapper_id + return '<!DOCTYPE html><html><head><title></title></head><body>' + html + "</body></html>", wrapper_id end # the extractions will also produce image files, which go in the # current directory, so change to the directory the function caller # wants everything in - Dir.chdir(dir) do - tempfile = Tempfile.new('foiextract', '.') - tempfile.print self.body - tempfile.flush - - html = nil - if self.content_type == 'application/pdf' - # We set a timeout here, because pdftohtml can spiral out of control - # on some PDF files and we don’t want to crash the whole server. - html = AlaveteliExternalCommand.run("pdftohtml", "-nodrm", "-zoom", "1.0", "-stdout", "-enc", "UTF-8", "-noframes", tempfile.path, :timeout => 30) - elsif self.content_type == 'application/rtf' - html = AlaveteliExternalCommand.run("unrtf", "--html", tempfile.path, :timeout => 120) - end - - if html.nil? - if self.has_google_docs_viewer? - html = '' # force error and using Google docs viewer + + html = nil + if ['application/pdf', 'application/rtf'].include?(self.content_type) + text = self.body + Dir.chdir(dir) do + if RUBY_VERSION.to_f >= 1.9 + tempfile = Tempfile.new('foiextract', '.', :encoding => text.encoding) else - raise "No HTML conversion available for type " + self.content_type + tempfile = Tempfile.new('foiextract', '.') end - end + tempfile.print text + tempfile.flush + - tempfile.close - tempfile.delete + if self.content_type == 'application/pdf' + # We set a timeout here, because pdftohtml can spiral out of control + # on some PDF files and we don't want to crash the whole server. + html = AlaveteliExternalCommand.run("pdftohtml", "-nodrm", "-zoom", "1.0", "-stdout", "-enc", "UTF-8", "-noframes", tempfile.path, :timeout => 30) + elsif self.content_type == 'application/rtf' + html = AlaveteliExternalCommand.run("unrtf", "--html", tempfile.path, :timeout => 120) + end + + tempfile.close + tempfile.delete + end end + if html.nil? + if self.has_google_docs_viewer? + html = '' # force error and using Google docs viewer + else + raise "No HTML conversion available for type " + self.content_type + end + end + + # We need to look at: # a) Any error code diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb index 5205d0a2d..5c845ead3 100644 --- a/app/models/incoming_message.rb +++ b/app/models/incoming_message.rb @@ -38,14 +38,6 @@ require 'zip/zip' require 'mapi/msg' require 'mapi/convert' -# Monkeypatch! Adding some extra members to store extra info in. -module TMail - class Mail - attr_accessor :url_part_number - attr_accessor :rfc822_attachment # when a whole email message is attached as text - attr_accessor :within_rfc822_attachment # for parts within a message attached as text (for getting subject mainly) - end -end class IncomingMessage < ActiveRecord::Base belongs_to :info_request @@ -70,56 +62,35 @@ class IncomingMessage < ActiveRecord::Base 'application/zip' => 1, } - # Return the structured TMail::Mail object - # Documentation at http://i.loveruby.net/en/projects/tmail/doc/ + # Return a cached structured mail object def mail(force = nil) if (!force.nil? || @mail.nil?) && !self.raw_email.nil? - # Hack round bug in TMail's MIME decoding. - # Report of TMail bug: - # http://rubyforge.org/tracker/index.php?func=detail&aid=21810&group_id=4512&atid=17370 - copy_of_raw_data = self.raw_email.data.gsub(/; boundary=\s+"/im,'; boundary="') - - @mail = TMail::Mail.parse(copy_of_raw_data) - @mail.base64_decode + @mail = MailHandler.mail_from_raw_email(self.raw_email.data) end @mail end - def from_address - self.mail.from_addrs[0].address - end - def empty_from_field? self.mail.from_addrs.nil? || self.mail.from_addrs.size == 0 end def from_email - self.mail.from_addrs[0].spec + MailHandler.get_from_address(self.mail) end def addresses - ((self.mail.to || []) + - (self.mail.cc || []) + - (self.mail.envelope_to || [])).uniq + MailHandler.get_all_addresses(self.mail) end def message_id self.mail.message_id end - # Returns the name of the person the incoming message is from, or nil if - # there isn't one or if there is only an email address. XXX can probably - # remove from_name_if_present (which is a monkey patch) by just calling - # .from_addrs[0].name here instead? - # Return false if for some reason this is a message that we shouldn't let them reply to def _calculate_valid_to_reply_to # check validity of email - if empty_from_field? - return false - end email = self.from_email - if !MySociety::Validate.is_valid_email(email) + if email.nil? || !MySociety::Validate.is_valid_email(email) return false end @@ -129,13 +100,13 @@ class IncomingMessage < ActiveRecord::Base prefix = email prefix =~ /^(.*)@/ prefix = $1 - if !prefix.nil? && prefix.downcase.match(/^(postmaster|mailer-daemon|auto_reply|donotreply|no.reply)$/) + if !prefix.nil? && prefix.downcase.match(/^(postmaster|mailer-daemon|auto_reply|do.?not.?reply|no.reply)$/) return false end - if !self.mail['return-path'].nil? && self.mail['return-path'].addr == "<>" + if MailHandler.empty_return_path?(self.mail) return false end - if !self.mail['auto-submitted'].nil? + if !MailHandler.get_auto_submitted(self.mail).nil? return false end return true @@ -153,13 +124,10 @@ class IncomingMessage < ActiveRecord::Base self.extract_attachments! self.sent_at = self.mail.date || self.created_at self.subject = self.mail.subject - # XXX can probably remove from_name_if_present (which is a - # monkey patch) by just calling .from_addrs[0].name here - # instead? - self.mail_from = self.mail.from_name_if_present - begin + self.mail_from = MailHandler.get_from_name(self.mail) + if self.from_email self.mail_from_domain = PublicBody.extract_domain_from_email(self.from_email) - rescue NoMethodError + else self.mail_from_domain = "" end self.valid_to_reply_to = self._calculate_valid_to_reply_to @@ -205,54 +173,8 @@ class IncomingMessage < ActiveRecord::Base super end - # Number the attachments in depth first tree order, for use in URLs. - # XXX This fills in part.rfc822_attachment and part.url_part_number within - # all the parts of the email (see TMail monkeypatch above for how these - # attributes are added). ensure_parts_counted must be called before using - # the attributes. - def ensure_parts_counted - @count_parts_count = 0 - _count_parts_recursive(self.mail) - # we carry on using these numeric ids for attachments uudecoded from within text parts - @count_first_uudecode_count = @count_parts_count - end - def _count_parts_recursive(part) - if part.multipart? - part.parts.each do |p| - _count_parts_recursive(p) - end - else - part_filename = TMail::Mail.get_part_file_name(part) - begin - if part.content_type == 'message/rfc822' - # An email attached as text - # e.g. http://www.whatdotheyknow.com/request/64/response/102 - part.rfc822_attachment = TMail::Mail.parse(part.body) - elsif part.content_type == 'application/vnd.ms-outlook' || part_filename && AlaveteliFileTypes.filename_to_mimetype(part_filename) == 'application/vnd.ms-outlook' - # An email attached as an Outlook file - # e.g. http://www.whatdotheyknow.com/request/chinese_names_for_british_politi - msg = Mapi::Msg.open(StringIO.new(part.body)) - part.rfc822_attachment = TMail::Mail.parse(msg.to_mime.to_s) - elsif part.content_type == 'application/ms-tnef' - # A set of attachments in a TNEF file - part.rfc822_attachment = TNEF.as_tmail(part.body) - end - rescue - # If attached mail doesn't parse, treat it as text part - part.rfc822_attachment = nil - else - unless part.rfc822_attachment.nil? - _count_parts_recursive(part.rfc822_attachment) - end - end - if part.rfc822_attachment.nil? - @count_parts_count += 1 - part.url_part_number = @count_parts_count - end - end - end # And look up by URL part number to get an attachment - # XXX relies on extract_attachments calling ensure_parts_counted + # XXX relies on extract_attachments calling MailHandler.ensure_parts_counted def self.get_attachment_by_url_part_number(attachments, found_url_part_number) attachments.each do |a| if a.url_part_number == found_url_part_number @@ -362,6 +284,7 @@ class IncomingMessage < ActiveRecord::Base # Lotus notes quoting yeuch! def remove_lotus_quoting(text, replacement = "FOLDED_QUOTED_SECTION") text = text.dup + return text if self.info_request.user_name.nil? name = Regexp.escape(self.info_request.user_name) # To end of message sections @@ -473,105 +396,6 @@ class IncomingMessage < ActiveRecord::Base return text end - # Internal function - def _get_part_file_name(mail) - part_file_name = TMail::Mail.get_part_file_name(mail) - if part_file_name.nil? - return nil - end - part_file_name = part_file_name.dup - return part_file_name - end - - # (This risks losing info if the unchosen alternative is the only one to contain - # useful info, but let's worry about that another time) - def get_attachment_leaves - force = true - return _get_attachment_leaves_recursive(self.mail(force)) - end - def _get_attachment_leaves_recursive(curr_mail, within_rfc822_attachment = nil) - leaves_found = [] - if curr_mail.multipart? - if curr_mail.parts.size == 0 - raise "no parts on multipart mail" - end - - if curr_mail.sub_type == 'alternative' - # Choose best part from alternatives - best_part = nil - # Take the last text/plain one, or else the first one - curr_mail.parts.each do |m| - if not best_part - best_part = m - elsif m.content_type == 'text/plain' - best_part = m - end - end - # Take an HTML one as even higher priority. (They tend - # to render better than text/plain, e.g. don't wrap links here: - # http://www.whatdotheyknow.com/request/amount_and_cost_of_freedom_of_in#incoming-72238 ) - curr_mail.parts.each do |m| - if m.content_type == 'text/html' - best_part = m - end - end - leaves_found += _get_attachment_leaves_recursive(best_part, within_rfc822_attachment) - else - # Add all parts - curr_mail.parts.each do |m| - leaves_found += _get_attachment_leaves_recursive(m, within_rfc822_attachment) - end - end - else - # XXX Yuck. this section alters various content_type's. That puts - # it into conflict with ensure_parts_counted which it has to be - # called both before and after. It will fail with cases of - # attachments of attachments etc. - charset = curr_mail.charset # save this, because overwriting content_type also resets charset - # Don't allow nil content_types - if curr_mail.content_type.nil? - curr_mail.content_type = 'application/octet-stream' - end - # PDFs often come with this mime type, fix it up for view code - if curr_mail.content_type == 'application/octet-stream' - part_file_name = self._get_part_file_name(curr_mail) - calc_mime = AlaveteliFileTypes.filename_and_content_to_mimetype(part_file_name, curr_mail.body) - if calc_mime - curr_mail.content_type = calc_mime - end - end - - # Use standard content types for Word documents etc. - curr_mail.content_type = normalise_content_type(curr_mail.content_type) - if curr_mail.content_type == 'message/rfc822' - ensure_parts_counted # fills in rfc822_attachment variable - if curr_mail.rfc822_attachment.nil? - # Attached mail didn't parse, so treat as text - curr_mail.content_type = 'text/plain' - end - end - if curr_mail.content_type == 'application/vnd.ms-outlook' || curr_mail.content_type == 'application/ms-tnef' - ensure_parts_counted # fills in rfc822_attachment variable - if curr_mail.rfc822_attachment.nil? - # Attached mail didn't parse, so treat as binary - curr_mail.content_type = 'application/octet-stream' - end - end - # If the part is an attachment of email - if curr_mail.content_type == 'message/rfc822' || curr_mail.content_type == 'application/vnd.ms-outlook' || curr_mail.content_type == 'application/ms-tnef' - ensure_parts_counted # fills in rfc822_attachment variable - leaves_found += _get_attachment_leaves_recursive(curr_mail.rfc822_attachment, curr_mail.rfc822_attachment) - else - # Store leaf - curr_mail.within_rfc822_attachment = within_rfc822_attachment - leaves_found += [curr_mail] - end - # restore original charset - curr_mail.charset = charset - end - return leaves_found - end - # Removes anything cached about the object in the database, and saves def clear_in_database_caches! self.cached_attachment_text_clipped = nil @@ -634,7 +458,8 @@ class IncomingMessage < ActiveRecord::Base text = "[ Email has no body, please see attachments ]" source_charset = "utf-8" else - text = part.body # by default, TMail converts to UTF8 in this call + # by default, the body (coming from an foi_attachment) should have been converted to utf-8 + text = part.body source_charset = part.charset if part.content_type == 'text/html' # e.g. http://www.whatdotheyknow.com/request/35/response/177 @@ -642,42 +467,31 @@ class IncomingMessage < ActiveRecord::Base # convert to text routine. Could instead call a # sanitize HTML one. - # If the text isn't UTF8, it means TMail had a problem + # If the text isn't UTF8, it means we had a problem # converting it (invalid characters, etc), and we # should instead tell elinks to respect the source # charset use_charset = "utf-8" - begin - text = Iconv.conv('utf-8', 'utf-8', text) - rescue Iconv::IllegalSequence - use_charset = source_charset - end - text = self.class._get_attachment_text_internal_one_file(part.content_type, text, use_charset) - end - end - - # If TMail can't convert text, it just returns it, so we sanitise it. - begin - # Test if it's good UTF-8 - text = Iconv.conv('utf-8', 'utf-8', text) - rescue Iconv::IllegalSequence - # Text looks like unlabelled nonsense, - # strip out anything that isn't UTF-8 - begin - source_charset = 'utf-8' if source_charset.nil? - text = Iconv.conv('utf-8//IGNORE', source_charset, text) + - _("\n\n[ {{site_name}} note: The above text was badly encoded, and has had strange characters removed. ]", - :site_name => Configuration::site_name) - rescue Iconv::InvalidEncoding, Iconv::IllegalSequence - if source_charset != "utf-8" - source_charset = "utf-8" - retry + if RUBY_VERSION.to_f >= 1.9 + begin + text.encode('utf-8') + rescue Encoding::UndefinedConversionError, Encoding::InvalidByteSequenceError + use_charset = source_charset + end + else + begin + text = Iconv.conv('utf-8', 'utf-8', text) + rescue Iconv::IllegalSequence + use_charset = source_charset + end end + text = MailHandler.get_attachment_text_one_file(part.content_type, text, use_charset) end end + # If text hasn't been converted, we sanitise it. + text = _sanitize_text(text) # Fix DOS style linefeeds to Unix style ones (or other later regexps won't work) - # Needed for e.g. http://www.whatdotheyknow.com/request/60/response/98 text = text.gsub(/\r\n/, "\n") # Compress extra spaces down to save space, and to stop regular expressions @@ -687,6 +501,51 @@ class IncomingMessage < ActiveRecord::Base return text end + + def _sanitize_text(text) + if RUBY_VERSION.to_f >= 1.9 + begin + # Test if it's good UTF-8 + text.encode('utf-8') + rescue Encoding::UndefinedConversionError, Encoding::InvalidByteSequenceError + source_charset = 'utf-8' if source_charset.nil? + # strip out anything that isn't UTF-8 + begin + text = text.encode("utf-8", :invalid => :replace, + :undef => :replace, + :replace => "") + + _("\n\n[ {{site_name}} note: The above text was badly encoded, and has had strange characters removed. ]", + :site_name => MySociety::Config.get('SITE_NAME', 'Alaveteli')) + rescue Encoding::UndefinedConversionError, Encoding::InvalidByteSequenceError + if source_charset != "utf-8" + source_charset = "utf-8" + retry + end + end + end + else + begin + # Test if it's good UTF-8 + text = Iconv.conv('utf-8', 'utf-8', text) + rescue Iconv::IllegalSequence + # Text looks like unlabelled nonsense, + # strip out anything that isn't UTF-8 + begin + source_charset = 'utf-8' if source_charset.nil? + text = Iconv.conv('utf-8//IGNORE', source_charset, text) + + _("\n\n[ {{site_name}} note: The above text was badly encoded, and has had strange characters removed. ]", + :site_name => Configuration::site_name) + rescue Iconv::InvalidEncoding, Iconv::IllegalSequence, Iconv::InvalidCharacter + if source_charset != "utf-8" + source_charset = "utf-8" + retry + end + end + end + end + text + end + # Returns part which contains main body text, or nil if there isn't one def get_main_body_text_part leaves = self.foi_attachments @@ -740,7 +599,7 @@ class IncomingMessage < ActiveRecord::Base filename = uu.match(/^begin\s+[0-9]+\s+(.*)$/)[1] calc_mime = AlaveteliFileTypes.filename_and_content_to_mimetype(filename, content) if calc_mime - calc_mime = normalise_content_type(calc_mime) + calc_mime = MailHandler.normalise_content_type(calc_mime) content_type = calc_mime else content_type = 'application/octet-stream' @@ -769,55 +628,15 @@ class IncomingMessage < ActiveRecord::Base end def extract_attachments! - leaves = get_attachment_leaves # XXX check where else this is called from - # XXX we have to call ensure_parts_counted after get_attachment_leaves - # which is really messy. - ensure_parts_counted + force = true + attachment_attributes = MailHandler.get_attachment_attributes(self.mail(force)) attachments = [] - for leaf in leaves - body = leaf.body - # As leaf.body causes MIME decoding which uses lots of RAM, do garbage collection here - # to prevent excess memory use. XXX not really sure if this helps reduce - # peak RAM use overall. Anyway, maybe there is something better to do than this. - GC.start - if leaf.within_rfc822_attachment - within_rfc822_subject = leaf.within_rfc822_attachment.subject - # Test to see if we are in the first part of the attached - # RFC822 message and it is text, if so add headers. - # XXX should probably use hunting algorithm to find main text part, rather than - # just expect it to be first. This will do for now though. - # Example request that needs this: - # http://www.whatdotheyknow.com/request/2923/response/7013/attach/2/Cycle%20Path%20Bank.txt - if leaf.within_rfc822_attachment == leaf && leaf.content_type == 'text/plain' - headers = "" - for header in [ 'Date', 'Subject', 'From', 'To', 'Cc' ] - if leaf.within_rfc822_attachment.header.include?(header.downcase) - header_value = leaf.within_rfc822_attachment.header[header.downcase] - # Example message which has a blank Date header: - # http://www.whatdotheyknow.com/request/30747/response/80253/attach/html/17/Common%20Purpose%20Advisory%20Group%20Meeting%20Tuesday%202nd%20March.txt.html - if !header_value.blank? - headers = headers + header + ": " + header_value.to_s + "\n" - end - end - end - # XXX call _convert_part_body_to_text here, but need to get charset somehow - # e.g. http://www.whatdotheyknow.com/request/1593/response/3088/attach/4/Freedom%20of%20Information%20request%20-%20car%20oval%20sticker:%20Article%2020,%20Convention%20on%20Road%20Traffic%201949.txt - body = headers + "\n" + body - - # This is quick way of getting all headers, but instead we only add some a) to - # make it more usable, b) as at least one authority accidentally leaked security - # information into a header. - #attachment.body = leaf.within_rfc822_attachment.port.to_s - end - end - hexdigest = Digest::MD5.hexdigest(body) - attachment = self.foi_attachments.find_or_create_by_hexdigest(:hexdigest => hexdigest) - attachment.update_attributes(:url_part_number => leaf.url_part_number, - :content_type => leaf.content_type, - :filename => _get_part_file_name(leaf), - :charset => leaf.charset, - :within_rfc822_subject => within_rfc822_subject, - :body => body) + attachment_attributes.each do |attrs| + attachment = self.foi_attachments.find_or_create_by_hexdigest(:hexdigest => attrs[:hexdigest]) + body = attrs.delete(:body) + attachment.update_attributes(attrs) + # Set the body separately as its handling can depend on the value of charset + attachment.body = body attachment.save! attachments << attachment.id end @@ -827,7 +646,7 @@ class IncomingMessage < ActiveRecord::Base # e.g. for https://secure.mysociety.org/admin/foi/request/show_raw_email/24550 if !main_part.nil? uudecoded_attachments = _uudecode_and_save_attachments(main_part.body) - c = @count_first_uudecode_count + c = self.mail.count_first_uudecode_count for uudecode_attachment in uudecoded_attachments c += 1 uudecode_attachment.url_part_number = c @@ -876,7 +695,7 @@ class IncomingMessage < ActiveRecord::Base text = text.gsub(/\n/, '<br>') text = text.gsub(/(?:<br>\s*){2,}/, '<br><br>') # remove excess linebreaks that unnecessarily space it out - return text + return text.html_safe end @@ -919,101 +738,15 @@ class IncomingMessage < ActiveRecord::Base return self.cached_attachment_text_clipped end - def IncomingMessage._get_attachment_text_internal_one_file(content_type, body, charset = 'utf-8') - # note re. charset: TMail always tries to convert email bodies - # to UTF8 by default, so normally it should already be that. - text = '' - # XXX - tell all these command line tools to return utf-8 - if content_type == 'text/plain' - text += body + "\n\n" - else - tempfile = Tempfile.new('foiextract') - tempfile.print body - tempfile.flush - if content_type == 'application/vnd.ms-word' - AlaveteliExternalCommand.run("wvText", tempfile.path, tempfile.path + ".txt") - # Try catdoc if we get into trouble (e.g. for InfoRequestEvent 2701) - if not File.exists?(tempfile.path + ".txt") - AlaveteliExternalCommand.run("catdoc", tempfile.path, :append_to => text) - else - text += File.read(tempfile.path + ".txt") + "\n\n" - File.unlink(tempfile.path + ".txt") - end - elsif content_type == 'application/rtf' - # catdoc on RTF prodcues less comments and extra bumf than --text option to unrtf - AlaveteliExternalCommand.run("catdoc", tempfile.path, :append_to => text) - elsif content_type == 'text/html' - # lynx wordwraps links in its output, which then don't - # get formatted properly by Alaveteli. We use elinks - # instead, which doesn't do that. - AlaveteliExternalCommand.run("elinks", "-eval", "set document.codepage.assume = \"#{charset}\"", "-eval", "set document.codepage.force_assumed = 1", "-dump-charset", "utf-8", "-force-html", "-dump", - tempfile.path, :append_to => text, :env => {"LANG" => "C"}) - elsif content_type == 'application/vnd.ms-excel' - # Bit crazy using /usr/bin/strings - but xls2csv, xlhtml and - # py_xls2txt only extract text from cells, not from floating - # notes. catdoc may be fooled by weird character sets, but will - # probably do for UK FOI requests. - AlaveteliExternalCommand.run("/usr/bin/strings", tempfile.path, :append_to => text) - elsif content_type == 'application/vnd.ms-powerpoint' - # ppthtml seems to catch more text, but only outputs HTML when - # we want text, so just use catppt for now - AlaveteliExternalCommand.run("catppt", tempfile.path, :append_to => text) - elsif content_type == 'application/pdf' - AlaveteliExternalCommand.run("pdftotext", tempfile.path, "-", :append_to => text) - elsif content_type == 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' - # This is Microsoft's XML office document format. - # Just pull out the main XML file, and strip it of text. - xml = AlaveteliExternalCommand.run("/usr/bin/unzip", "-qq", "-c", tempfile.path, "word/document.xml") - if !xml.nil? - doc = REXML::Document.new(xml) - text += doc.each_element( './/text()' ){}.join(" ") - end - elsif content_type == 'application/zip' - # recurse into zip files - begin - zip_file = Zip::ZipFile.open(tempfile.path) - text += IncomingMessage._get_attachment_text_from_zip_file(zip_file) - zip_file.close() - rescue - $stderr.puts("Error processing zip file: #{$!.inspect}") - end - end - tempfile.close - end - return text - end - def IncomingMessage._get_attachment_text_from_zip_file(zip_file) - text = "" - for entry in zip_file - if entry.file? - filename = entry.to_s - begin - body = entry.get_input_stream.read - rescue - # move to next attachment silently if there were problems - # XXX really should reduce this to specific exceptions? - # e.g. password protected - next - end - calc_mime = AlaveteliFileTypes.filename_to_mimetype(filename) - if calc_mime - content_type = calc_mime - else - content_type = 'application/octet-stream' - end - - text += _get_attachment_text_internal_one_file(content_type, body) - end - end - return text - end def _get_attachment_text_internal # Extract text from each attachment text = '' attachments = self.get_attachments_for_display for attachment in attachments - text += IncomingMessage._get_attachment_text_internal_one_file(attachment.content_type, attachment.body, attachment.charset) + text += MailHandler.get_attachment_text_one_file(attachment.content_type, + attachment.body, + attachment.charset) end # Remove any bad characters text = Iconv.conv('utf-8//IGNORE', 'utf-8', text) @@ -1081,66 +814,12 @@ class IncomingMessage < ActiveRecord::Base return AlaveteliFileTypes.all_extensions.join(" ") end - # Return false if for some reason this is a message that we shouldn't let them reply to - def valid_to_reply_to? - # check validity of email - if empty_from_field? - return false - end - email = self.from_email - if !MySociety::Validate.is_valid_email(email) - return false - end - - # reject postmaster - authorities seem to nearly always not respond to - # email to postmaster, and it tends to only happen after delivery failure. - # likewise Mailer-Daemon, Auto_Reply... - prefix = email - prefix =~ /^(.*)@/ - prefix = $1 - if !prefix.nil? && prefix.downcase.match(/^(postmaster|mailer-daemon|auto_reply|do.?not.?reply|no.reply)$/) - return false + def for_admin_column + self.class.content_columns.each do |column| + yield(column.human_name, self.send(column.name), column.type.to_s, column.name) end - if !self.mail['return-path'].nil? && self.mail['return-path'].addr == "<>" - return false - end - if !self.mail['auto-submitted'].nil? - return false - end - return true end - def normalise_content_type(content_type) - # e.g. http://www.whatdotheyknow.com/request/93/response/250 - if content_type == 'application/excel' or content_type == 'application/msexcel' or content_type == 'application/x-ms-excel' - content_type = 'application/vnd.ms-excel' - end - if content_type == 'application/mspowerpoint' or content_type == 'application/x-ms-powerpoint' - content_type = 'application/vnd.ms-powerpoint' - end - if content_type == 'application/msword' or content_type == 'application/x-ms-word' - content_type = 'application/vnd.ms-word' - end - if content_type == 'application/x-zip-compressed' - content_type = 'application/zip' - end - - # e.g. http://www.whatdotheyknow.com/request/copy_of_current_swessex_scr_opt#incoming-9928 - if content_type == 'application/acrobat' - content_type = 'application/pdf' - end - - return content_type - end - - def for_admin_column - self.class.content_columns.each do |column| - yield(column.human_name, self.send(column.name), column.type.to_s, column.name) - end - end - - private :normalise_content_type - end diff --git a/app/models/info_request.rb b/app/models/info_request.rb index e1885dee6..cee9eb959 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -138,7 +138,7 @@ class InfoRequest < ActiveRecord::Base if external_user_name.nil? fake_slug = "anonymous" else - fake_slug = external_user_name.parameterize + fake_slug = MySociety::Format.simplify_url_part(external_user_name, 'external_user', 32) end (public_body.url_name || "") + "_" + fake_slug else @@ -275,7 +275,7 @@ public return self.magic_email("request-") end def incoming_name_and_email - return TMail::Address.address_from_name_and_email(self.user_name, self.incoming_email).to_s + return MailHandler.address_from_name_and_email(self.user_name, self.incoming_email) end # Subject lines for emails about the request @@ -284,9 +284,9 @@ public # into some sort of separate jurisdiction dependent file if self.public_body.url_name == 'general_register_office' # without GQ in the subject, you just get an auto response - _('{{law_used_full}} request GQ - {{title}}',:law_used_full=>self.law_used_full,:title=>self.title) + _('{{law_used_full}} request GQ - {{title}}',:law_used_full=>self.law_used_full,:title=>self.title.html_safe) else - _('{{law_used_full}} request - {{title}}',:law_used_full=>self.law_used_full,:title=>self.title) + _('{{law_used_full}} request - {{title}}',:law_used_full=>self.law_used_full,:title=>self.title.html_safe) end end def email_subject_followup(incoming_message = nil) @@ -434,11 +434,11 @@ public elsif self.allow_new_responses_from == 'anybody' allow = true elsif self.allow_new_responses_from == 'authority_only' - if email.from_addrs.nil? || email.from_addrs.size == 0 + sender_email = MailHandler.get_from_address(email) + if sender_email.nil? allow = false reason = _('Only the authority can reply to this request, but there is no "From" address to check against') else - sender_email = email.from_addrs[0].spec sender_domain = PublicBody.extract_domain_from_email(sender_email) reason = _("Only the authority can reply to this request, and I don't recognise the address this reply was sent from") allow = false @@ -707,11 +707,11 @@ public return self.public_body.is_followupable? end def recipient_name_and_email - return TMail::Address.address_from_name_and_email( + return MailHandler.address_from_name_and_email( _("{{law_used}} requests at {{public_body}}", :law_used => self.law_used_short, :public_body => self.public_body.short_or_long_name), - self.recipient_email).to_s + self.recipient_email) end # History of some things that have happened @@ -1124,7 +1124,11 @@ public } if deep - ret[:user] = self.user.json_for_api + if self.user + ret[:user] = self.user.json_for_api + else + ret[:user_name] = self.user_name + end ret[:public_body] = self.public_body.json_for_api ret[:info_request_events] = self.info_request_events.map { |e| e.json_for_api(false) } end diff --git a/app/models/info_request_event.rb b/app/models/info_request_event.rb index 5a8e3416f..09eba31ab 100644 --- a/app/models/info_request_event.rb +++ b/app/models/info_request_event.rb @@ -384,7 +384,7 @@ class InfoRequestEvent < ActiveRecord::Base if prev_addr.nil? || curr_addr.nil? return false end - return TMail::Address.parse(prev_addr).address == TMail::Address.parse(curr_addr).address + return MailHandler.address_from_string(prev_addr) == MailHandler.address_from_string(curr_addr) end def json_for_api(deep, snippet_highlight_proc = nil) diff --git a/app/models/outgoing_mailer.rb b/app/models/outgoing_mailer.rb index 277794c69..503166b8a 100644 --- a/app/models/outgoing_mailer.rb +++ b/app/models/outgoing_mailer.rb @@ -47,7 +47,8 @@ class OutgoingMailer < ApplicationMailer return info_request.recipient_name_and_email else # calling safe_mail_from from so censor rules are run - return TMail::Address.address_from_name_and_email(incoming_message_followup.safe_mail_from, incoming_message_followup.from_email).to_s + return MailHandler.address_from_name_and_email(incoming_message_followup.safe_mail_from, + incoming_message_followup.from_email) end end # Used in the preview of followup diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb index 2e98e1021..c75894e6a 100644 --- a/app/models/outgoing_message.rb +++ b/app/models/outgoing_message.rb @@ -30,6 +30,7 @@ class OutgoingMessage < ActiveRecord::Base validates_inclusion_of :status, :in => ['ready', 'sent', 'failed'] validates_inclusion_of :message_type, :in => ['initial_request', 'followup' ] #, 'complaint'] + validate :format_of_body belongs_to :incoming_message_followup, :foreign_key => 'incoming_message_followup_id', :class_name => 'IncomingMessage' @@ -136,32 +137,6 @@ class OutgoingMessage < ActiveRecord::Base end end - # Check have edited letter - def validate - if self.body.empty? || self.body =~ /\A#{get_salutation}\s+#{get_signoff}/ || self.body =~ /#{get_internal_review_insert_here_note}/ - if self.message_type == 'followup' - if self.what_doing == 'internal_review' - errors.add(:body, _("Please give details explaining why you want a review")) - else - errors.add(:body, _("Please enter your follow up message")) - end - elsif - errors.add(:body, _("Please enter your letter requesting information")) - else - raise "Message id #{self.id} has type '#{self.message_type}' which validate can't handle" - end - end - if self.body =~ /#{get_signoff}\s*\Z/m - errors.add(:body, _("Please sign at the bottom with your name, or alter the \"%{signoff}\" signature" % { :signoff => get_signoff })) - end - if !MySociety::Validate.uses_mixed_capitals(self.body) - errors.add(:body, _('Please write your message using a mixture of capital and lower case letters. This makes it easier for others to read.')) - end - if self.what_doing.nil? || !['new_information', 'internal_review', 'normal_sort'].include?(self.what_doing) - errors.add(:what_doing_dummy, _('Please choose what sort of reply you are making.')) - end - end - # Deliver outgoing message # Note: You can test this from script/console with, say: # InfoRequest.find(1).outgoing_messages[0].send_message @@ -252,7 +227,7 @@ class OutgoingMessage < ActiveRecord::Base text = MySociety::Format.make_clickable(text, :contract => 1) text.gsub!(/\[(email address|mobile number)\]/, '[<a href="/help/officers#mobiles">\1</a>]') text = text.gsub(/\n/, '<br>') - return text + return text.html_safe end def fully_destroy @@ -275,6 +250,33 @@ class OutgoingMessage < ActiveRecord::Base yield(column.human_name, self.send(column.name), column.type.to_s, column.name) end end + + private + + def format_of_body + if self.body.empty? || self.body =~ /\A#{get_salutation}\s+#{get_signoff}/ || self.body =~ /#{get_internal_review_insert_here_note}/ + if self.message_type == 'followup' + if self.what_doing == 'internal_review' + errors.add(:body, _("Please give details explaining why you want a review")) + else + errors.add(:body, _("Please enter your follow up message")) + end + elsif + errors.add(:body, _("Please enter your letter requesting information")) + else + raise "Message id #{self.id} has type '#{self.message_type}' which validate can't handle" + end + end + if self.body =~ /#{get_signoff}\s*\Z/m + errors.add(:body, _("Please sign at the bottom with your name, or alter the \"%{signoff}\" signature" % { :signoff => get_signoff })) + end + if !MySociety::Validate.uses_mixed_capitals(self.body) + errors.add(:body, _('Please write your message using a mixture of capital and lower case letters. This makes it easier for others to read.')) + end + if self.what_doing.nil? || !['new_information', 'internal_review', 'normal_sort'].include?(self.what_doing) + errors.add(:what_doing_dummy, _('Please choose what sort of reply you are making.')) + end + end end diff --git a/app/models/profile_photo.rb b/app/models/profile_photo.rb index 6e605651d..73d7ca12b 100644 --- a/app/models/profile_photo.rb +++ b/app/models/profile_photo.rb @@ -23,6 +23,8 @@ class ProfilePhoto < ActiveRecord::Base belongs_to :user + validate :data_and_draft_checks + # deliberately don't strip_attributes, so keeps raw photo properly attr_accessor :x, :y, :w, :h @@ -81,7 +83,9 @@ class ProfilePhoto < ActiveRecord::Base end end - def validate + private + + def data_and_draft_checks if self.data.nil? errors.add(:data, N_("Please choose a file containing your photo.")) return diff --git a/app/models/public_body.rb b/app/models/public_body.rb index 57fe27767..168b9f4c7 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -35,6 +35,8 @@ class PublicBody < ActiveRecord::Base validates_uniqueness_of :short_name, :message => N_("Short name is already taken"), :if => Proc.new { |pb| pb.short_name != "" } validates_uniqueness_of :name, :message => N_("Name is already taken") + validate :request_email_if_requestable + has_many :info_requests, :order => 'created_at desc' has_many :track_things, :order => 'created_at desc' has_many :censor_rules, :order => 'created_at desc' @@ -135,15 +137,6 @@ class PublicBody < ActiveRecord::Base self.first_letter = self.name.scan(/./mu)[0].upcase end - def validate - # Request_email can be blank, meaning we don't have details - if self.is_requestable? - unless MySociety::Validate.is_valid_email(self.request_email) - errors.add(:request_email, "Request email doesn't look like a valid email address") - end - end - end - # If tagged "not_apply", then FOI/EIR no longer applies to authority at all def not_apply? return self.has_tag?('not_apply') @@ -301,7 +294,7 @@ class PublicBody < ActiveRecord::Base ret = ret + " and " end ret = ret + types[-1] - return ret + return ret.html_safe else return _("A public authority") end @@ -520,6 +513,8 @@ class PublicBody < ActiveRecord::Base 'Version', ] public_bodies.each do |public_body| + # Skip bodies we use only for site admin + next if public_body.has_tag?('site_administration') csv << [ public_body.name, public_body.short_name, @@ -640,4 +635,12 @@ class PublicBody < ActiveRecord::Base end end + def request_email_if_requestable + # Request_email can be blank, meaning we don't have details + if self.is_requestable? + unless MySociety::Validate.is_valid_email(self.request_email) + errors.add(:request_email, "Request email doesn't look like a valid email address") + end + end + end end diff --git a/app/models/request_mailer.rb b/app/models/request_mailer.rb index 90c4c6b53..6f8d88a1a 100644 --- a/app/models/request_mailer.rb +++ b/app/models/request_mailer.rb @@ -5,7 +5,11 @@ # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ require 'alaveteli_file_types' - +if Rails.env == 'test' && RUBY_VERSION.to_f >= 1.9 + # Avoid spec/script/mailin_spec.rb running script/runner as a test suite + # http://stackoverflow.com/questions/1899009/why-are-tests-running-in-production-mode-and-causing-my-script-runners-to-fail + Test::Unit.run = true +end class RequestMailer < ApplicationMailer @@ -67,8 +71,8 @@ class RequestMailer < ApplicationMailer @from = user.name_and_email @recipients = contact_from_name_and_email @subject = _("FOI response requires admin ({{reason}}) - {{title}}", :reason => info_request.described_state, :title => info_request.title) - url = main_url(request_url(info_request)) - admin_url = request_admin_url(info_request) + url = request_url(info_request) + admin_url = admin_request_show_url(info_request) @body = {:reported_by => user, :info_request => info_request, :url => url, :admin_url => admin_url } end @@ -76,14 +80,14 @@ class RequestMailer < ApplicationMailer def new_response(info_request, incoming_message) # Don't use login link here, just send actual URL. This is # because people tend to forward these emails amongst themselves. - url = main_url(incoming_message_url(incoming_message)) + url = incoming_message_url(incoming_message) @from = contact_from_name_and_email headers 'Return-Path' => blackhole_email, 'Reply-To' => @from, # not much we can do if the user's email is broken 'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834 'X-Auto-Response-Suppress' => 'OOF' @recipients = info_request.user.name_and_email - @subject = _("New response to your FOI request - ") + info_request.title + @subject = (_("New response to your FOI request - ") + info_request.title).html_safe @body = { :incoming_message => incoming_message, :info_request => info_request, :url => url } end @@ -102,7 +106,7 @@ class RequestMailer < ApplicationMailer 'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834 'X-Auto-Response-Suppress' => 'OOF' @recipients = user.name_and_email - @subject = _("Delayed response to your FOI request - ") + info_request.title + @subject = (_("Delayed response to your FOI request - ") + info_request.title).html_safe @body = { :info_request => info_request, :url => url } end @@ -121,7 +125,7 @@ class RequestMailer < ApplicationMailer 'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834 'X-Auto-Response-Suppress' => 'OOF' @recipients = user.name_and_email - @subject = _("You're long overdue a response to your FOI request - ") + info_request.title + @subject = (_("You're long overdue a response to your FOI request - ") + info_request.title).html_safe @body = { :info_request => info_request, :url => url } end @@ -131,7 +135,7 @@ class RequestMailer < ApplicationMailer # Make a link going to the form to describe state, and which logs the # user in. post_redirect = PostRedirect.new( - :uri => main_url(request_url(info_request)) + "#describe_state_form_1", + :uri => request_url(info_request) + "#describe_state_form_1", :user_id => info_request.user.id) post_redirect.save! url = confirm_url(:email_token => post_redirect.email_token) @@ -153,7 +157,7 @@ class RequestMailer < ApplicationMailer 'X-Auto-Response-Suppress' => 'OOF' @recipients = info_request.user.name_and_email @subject = _("Someone has updated the status of your request") - url = main_url(request_url(info_request)) + url = request_url(info_request) @body = {:info_request => info_request, :url => url} end @@ -173,7 +177,7 @@ class RequestMailer < ApplicationMailer 'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834 'X-Auto-Response-Suppress' => 'OOF' @recipients = info_request.user.name_and_email - @subject = _("Clarify your FOI request - ") + info_request.title + @subject = (_("Clarify your FOI request - ") + info_request.title).html_safe @body = { :incoming_message => incoming_message, :info_request => info_request, :url => url } end @@ -184,8 +188,8 @@ class RequestMailer < ApplicationMailer 'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834 'X-Auto-Response-Suppress' => 'OOF' @recipients = info_request.user.name_and_email - @subject = _("Somebody added a note to your FOI request - ") + info_request.title - @body = { :comment => comment, :info_request => info_request, :url => main_url(comment_url(comment)) } + @subject = (_("Somebody added a note to your FOI request - ") + info_request.title).html_safe + @body = { :comment => comment, :info_request => info_request, :url => comment_url(comment) } end def comment_on_alert_plural(info_request, count, earliest_unalerted_comment) @from = contact_from_name_and_email @@ -193,8 +197,8 @@ class RequestMailer < ApplicationMailer 'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834 'X-Auto-Response-Suppress' => 'OOF' @recipients = info_request.user.name_and_email - @subject = _("Some notes have been added to your FOI request - ") + info_request.title - @body = { :count => count, :info_request => info_request, :url => main_url(comment_url(earliest_unalerted_comment)) } + @subject = (_("Some notes have been added to your FOI request - ") + info_request.title).html_safe + @body = { :count => count, :info_request => info_request, :url => comment_url(earliest_unalerted_comment) } end # Class function, called by script/mailin with all incoming responses. @@ -204,15 +208,14 @@ class RequestMailer < ApplicationMailer # # That is because we want to be sure we properly record the actual message # received in its raw form - so any information won't be lost in a round - # trip via TMail, or by bugs in it, and so we can use something other than - # TMail at a later date. And so we can offer an option to download the + # trip via the mail handler, or by bugs in it, and so we can use something + # other than TMail at a later date. And so we can offer an option to download the # actual original mail sent by the authority in the admin interface (so # can check that attachment decoding failures are problems in the message, # not in our code). ] def self.receive(raw_email) logger.info "Received mail:\n #{raw_email}" unless logger.nil? - mail = TMail::Mail.parse(raw_email) - mail.base64_decode + mail = MailHandler.mail_from_raw_email(raw_email) new.receive(mail, raw_email) end diff --git a/app/models/track_mailer.rb b/app/models/track_mailer.rb index 7dfa87f52..51440e4d7 100644 --- a/app/models/track_mailer.rb +++ b/app/models/track_mailer.rb @@ -7,7 +7,7 @@ class TrackMailer < ApplicationMailer def event_digest(user, email_about_things) post_redirect = PostRedirect.new( - :uri => main_url(user_url(user)) + "#email_subscriptions", + :uri => user_url(user) + "#email_subscriptions", :user_id => user.id) post_redirect.save! unsubscribe_url = confirm_url(:email_token => post_redirect.email_token) diff --git a/app/models/track_thing.rb b/app/models/track_thing.rb index 2a61eb858..a0c74bdb6 100644 --- a/app/models/track_thing.rb +++ b/app/models/track_thing.rb @@ -23,6 +23,8 @@ require 'set' +# TODO: TrackThing looks like a good candidate for single table inheritance + class TrackThing < ActiveRecord::Base belongs_to :tracking_user, :class_name => 'User' validates_presence_of :track_query @@ -199,11 +201,12 @@ class TrackThing < ActiveRecord::Base if self.track_type == 'request_updates' @params = { # Website - :list_description => _("'{{link_to_request}}', a request", :link_to_request => "<a href=\"/request/" + CGI.escapeHTML(self.info_request.url_title) + "\">" + CGI.escapeHTML(self.info_request.title) + "</a>"), # XXX yeuch, sometimes I just want to call view helpers from the model, sorry! can't work out how + :list_description => _("'{{link_to_request}}', a request", + :link_to_request => ("<a href=\"/request/" + CGI.escapeHTML(self.info_request.url_title) + "\">" + CGI.escapeHTML(self.info_request.title) + "</a>").html_safe), # XXX yeuch, sometimes I just want to call view helpers from the model, sorry! can't work out how :verb_on_page => _("Follow this request"), :verb_on_page_already => _("You are already following this request"), # Email - :title_in_email => _("New updates for the request '{{request_title}}'", :request_title => self.info_request.title), + :title_in_email => _("New updates for the request '{{request_title}}'", :request_title => self.info_request.title.html_safe), :title_in_rss => _("New updates for the request '{{request_title}}'", :request_title => self.info_request.title), # Authentication :web => _("To follow the request '{{request_title}}'", :request_title => CGI.escapeHTML(self.info_request.title)), @@ -250,7 +253,7 @@ class TrackThing < ActiveRecord::Base elsif self.track_type == 'public_body_updates' @params = { # Website - :list_description => _("'{{link_to_authority}}', a public authority", :link_to_authority => "<a href=\"/body/" + CGI.escapeHTML(self.public_body.url_name) + "\">" + CGI.escapeHTML(self.public_body.name) + "</a>"), # XXX yeuch, sometimes I just want to call view helpers from the model, sorry! can't work out how + :list_description => _("'{{link_to_authority}}', a public authority", :link_to_authority => ("<a href=\"/body/" + CGI.escapeHTML(self.public_body.url_name) + "\">" + CGI.escapeHTML(self.public_body.name) + "</a>").html_safe), # XXX yeuch, sometimes I just want to call view helpers from the model, sorry! can't work out how :verb_on_page => _("Follow requests to {{public_body_name}}",:public_body_name=>CGI.escapeHTML(self.public_body.name)), :verb_on_page_already => _("You are already following requests to {{public_body_name}}", :public_body_name=>CGI.escapeHTML(self.public_body.name)), # Email @@ -266,11 +269,11 @@ class TrackThing < ActiveRecord::Base elsif self.track_type == 'user_updates' @params = { # Website - :list_description => _("'{{link_to_user}}', a person", :link_to_user => "<a href=\"/user/" + CGI.escapeHTML(self.tracked_user.url_name) + "\">" + CGI.escapeHTML(self.tracked_user.name) + "</a>"), # XXX yeuch, sometimes I just want to call view helpers from the model, sorry! can't work out how + :list_description => _("'{{link_to_user}}', a person", :link_to_user => ("<a href=\"/user/" + CGI.escapeHTML(self.tracked_user.url_name) + "\">" + CGI.escapeHTML(self.tracked_user.name) + "</a>").html_safe), # XXX yeuch, sometimes I just want to call view helpers from the model, sorry! can't work out how :verb_on_page => _("Follow this person"), :verb_on_page_already => _("You are already following this person"), # Email - :title_in_email => _("FOI requests by '{{user_name}}'", :user_name=>self.tracked_user.name), + :title_in_email => _("FOI requests by '{{user_name}}'", :user_name=>self.tracked_user.name.html_safe), :title_in_rss => _("FOI requests by '{{user_name}}'", :user_name=>self.tracked_user.name), # Authentication :web => _("To follow requests by '{{user_name}}'", :user_name=>CGI.escapeHTML(self.tracked_user.name)), @@ -282,7 +285,7 @@ class TrackThing < ActiveRecord::Base elsif self.track_type == 'search_query' @params = { # Website - :list_description => "<a href=\"/search/" + CGI.escapeHTML(self.track_query) + "/newest/advanced\">" + self.track_query_description + "</a>", # XXX yeuch, sometimes I just want to call view helpers from the model, sorry! can't work out how + :list_description => ("<a href=\"/search/" + CGI.escapeHTML(self.track_query) + "/newest/advanced\">" + CGI.escapeHTML(self.track_query_description) + "</a>").html_safe, # XXX yeuch, sometimes I just want to call view helpers from the model, sorry! can't work out how :verb_on_page => _("Follow things matching this search"), :verb_on_page_already => _("You are already following things matching this search"), # Email diff --git a/app/models/user.rb b/app/models/user.rb index 70386f7e4..e6c666e47 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -50,6 +50,8 @@ class User < ActiveRecord::Base 'super', ], :message => N_('Admin level is not included in list') + validate :email_and_name_are_valid + acts_as_xapian :texts => [ :name, :about_me ], :values => [ [ :created_at_numeric, 1, "created_at", :number ] # for sorting @@ -108,15 +110,6 @@ class User < ActiveRecord::Base self.comments.find(:all, :conditions => 'visible') end - def validate - if self.email != "" && !MySociety::Validate.is_valid_email(self.email) - errors.add(:email, _("Please enter a valid email address")) - end - if MySociety::Validate.is_valid_email(self.name) - errors.add(:name, _("Please enter your name, not your email address, in the name field.")) - end - end - # Don't display any leading/trailing spaces # XXX we have strip_attributes! now, so perhaps this can be removed (might # be still needed for existing cases) @@ -203,7 +196,7 @@ class User < ActiveRecord::Base # For use in to/from in email messages def name_and_email - return TMail::Address.address_from_name_and_email(self.name, self.email).to_s + return MailHandler.address_from_name_and_email(self.name, self.email) end # The "internal admin" is a special user for internal use. @@ -361,12 +354,13 @@ class User < ActiveRecord::Base end # Return about me text for display as HTML + # TODO: Move this to a view helper def get_about_me_for_html_display text = self.about_me.strip text = CGI.escapeHTML(text) text = MySociety::Format.make_clickable(text, :contract => 1) text = text.gsub(/\n/, '<br>') - return text + return text.html_safe end def json_for_api @@ -413,6 +407,15 @@ class User < ActiveRecord::Base self.salt = self.object_id.to_s + rand.to_s end + def email_and_name_are_valid + if self.email != "" && !MySociety::Validate.is_valid_email(self.email) + errors.add(:email, _("Please enter a valid email address")) + end + if MySociety::Validate.is_valid_email(self.name) + errors.add(:name, _("Please enter your name, not your email address, in the name field.")) + end + end + ## Class methods def User.encrypted_password(password, salt) string_to_hash = password + salt # XXX need to add a secret here too? |