aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/controllers/request_controller.rb3
-rw-r--r--app/models/foi_attachment.rb33
-rw-r--r--app/models/incoming_message.rb86
-rw-r--r--app/models/request_mailer.rb1
-rw-r--r--db/migrate/106_add_hex_digest_to_foi_attachment.rb9
-rw-r--r--spec/controllers/request_controller_spec.rb8
-rw-r--r--spec/fixtures/foi_attachments.yml5
-rw-r--r--spec/models/incoming_message_spec.rb13
-rw-r--r--spec/models/info_request_event_spec.rb5
-rw-r--r--spec/models/info_request_spec.rb2
-rw-r--r--spec/models/xapian_spec.rb1
-rw-r--r--spec/spec_helper.rb5
12 files changed, 98 insertions, 73 deletions
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 1801648ca..f7d870b4f 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -690,7 +690,6 @@ class RequestController < ApplicationController
# check permissions
raise "internal error, pre-auth filter should have caught this" if !@info_request.user_can_view?(authenticated_user)
-
@attachment = IncomingMessage.get_attachment_by_url_part_number(@incoming_message.get_attachments_for_display, @part_number)
raise ActiveRecord::RecordNotFound.new("attachment not found part number " + @part_number.to_s + " incoming_message " + @incoming_message.id.to_s) if @attachment.nil?
@@ -713,6 +712,7 @@ class RequestController < ApplicationController
:email => _("Then you can upload an FOI response. "),
:email_subject => _("Confirm your account on {{site_name}}",:site_name=>site_name)
}
+
if !authenticated?(@reason_params)
return
end
@@ -754,6 +754,7 @@ class RequestController < ApplicationController
def search_typeahead
# Since acts_as_xapian doesn't support the Partial match flag, we work around it
# by making the last work a wildcard, which is quite the same
+
query = params[:q] + '*'
query = query.split(' ').join(' OR ') # XXX: HACK for OR instead of default AND!
diff --git a/app/models/foi_attachment.rb b/app/models/foi_attachment.rb
index a7bc690ea..057dcdb69 100644
--- a/app/models/foi_attachment.rb
+++ b/app/models/foi_attachment.rb
@@ -7,6 +7,8 @@
# Email: francis@mysociety.org; WWW: http://www.mysociety.org/
# This is the type which is used to send data about attachments to the view
+require 'digest'
+
class FoiAttachment < ActiveRecord::Base
belongs_to :incoming_message
validates_presence_of :content_type
@@ -14,41 +16,38 @@ class FoiAttachment < ActiveRecord::Base
validates_presence_of :display_size
before_validation :ensure_filename!, :only => [:filename]
+ before_destroy :delete_cached_file!
def directory
- if ENV["RAILS_ENV"] == "test"
- base_dir = File.join('cache', 'attachments_test')
- else
- base_dir = File.join('cache', 'attachments')
- end
- request_id = self.incoming_message.info_request.id.to_s
- return File.join(base_dir, request_id[0..2], request_id, self.incoming_message.id.to_s)
+ base_dir = File.join("cache", "attachments_#{ENV['RAILS_ENV']}")
+ return File.join(base_dir, self.hexdigest[0..2])
end
def filepath
- part_number = self.url_part_number.nil? ? "1" : self.url_part_number.to_s
- File.join(self.directory, part_number)
+ File.join(self.directory, self.hexdigest)
+ end
+
+ def delete_cached_file!
+ begin
+ File.delete(self.filepath)
+ rescue
+ end
end
def body=(d)
+ self.hexdigest = Digest::MD5.hexdigest(d)
if !File.exists?(self.directory)
FileUtils.mkdir_p self.directory
end
File.open(self.filepath, "wb") { |file|
file.write d
}
- self.update_display_size!
+ update_display_size!
end
def body
if @cached_body.nil?
- if !File.exists?(self.filepath)
- # For some reason, we've lost the cache; extract everything again
- self.incoming_message.extract_attachments!
- @cached_body = self.body
- else
- @cached_body = File.open(self.filepath, "rb" ).read
- end
+ @cached_body = File.open(self.filepath, "rb" ).read
end
return @cached_body
end
diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb
index 6fa08b261..d8e2891e5 100644
--- a/app/models/incoming_message.rb
+++ b/app/models/incoming_message.rb
@@ -386,7 +386,12 @@ class IncomingMessage < ActiveRecord::Base
if !prefix.nil? && prefix.downcase.match(/^(postmaster|mailer-daemon|auto_reply|donotreply|no.reply)$/)
return false
end
-
+ if !self.mail['return-path'].nil? && self.mail['return-path'].addr == "<>"
+ return false
+ end
+ if !self.mail['auto-submitted'].nil? && !self.mail['auto-submitted'].keys.empty?
+ return false
+ end
return true
end
@@ -1007,16 +1012,7 @@ class IncomingMessage < ActiveRecord::Base
return p
end
# Returns attachments that are uuencoded in main body part
- def get_main_body_text_uudecode_attachments
- # we don't use get_main_body_text_internal, as we want to avoid charset
- # conversions, since /usr/bin/uudecode needs to deal with those.
- # e.g. for https://secure.mysociety.org/admin/foi/request/show_raw_email/24550
- main_part = get_main_body_text_part
- if main_part.nil?
- return []
- end
- text = main_part.body
-
+ def _uudecode_and_save_attachments(text)
# Find any uudecoded things buried in it, yeuchly
uus = text.scan(/^begin.+^`\n^end\n/sm)
attachments = []
@@ -1039,11 +1035,16 @@ class IncomingMessage < ActiveRecord::Base
else
content_type = 'application/octet-stream'
end
- attachment = self.foi_attachments.create(:body => content,
- :filename => filename,
- :content_type => content_type)
+ hexdigest = Digest::MD5.hexdigest(content)
+ attachment = self.foi_attachments.find_or_create_by_hexdigest(:hexdigest => hexdigest)
+ attachment.update_attributes(:filename => filename,
+ :content_type => content_type,
+ :body => content,
+ :display_size => "0K")
+ attachment.save!
+ attachments << attachment
end
- return self.foi_attachments
+ return attachments
end
def get_attachments_for_display
@@ -1059,16 +1060,11 @@ class IncomingMessage < ActiveRecord::Base
return attachments
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
-
- self.foi_attachments.clear
-
attachments = []
for leaf in leaves
body = leaf.body
@@ -1106,24 +1102,36 @@ class IncomingMessage < ActiveRecord::Base
#attachment.body = leaf.within_rfc822_attachment.port.to_s
end
end
- self.foi_attachments.create(:content_type => leaf.content_type,
- :url_part_number => leaf.url_part_number,
- :filename => _get_part_file_name(leaf),
- :body => body,
- :charset => leaf.charset,
- :within_rfc822_attachment => within_rfc822_attachment)
-
- end
-
- uudecode_attachments = get_main_body_text_uudecode_attachments
- c = @count_first_uudecode_count
- for uudecode_attachment in uudecode_attachments
- c += 1
- uudecode_attachment.url_part_number = c
- uudecode_attachment.save!
- end
- return self.foi_attachments
- 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,
+ :display_size => "0K",
+ :body => body)
+ attachment.save!
+ attachments << attachment.id
+ end
+ main_part = get_main_body_text_part
+ # we don't use get_main_body_text_internal, as we want to avoid charset
+ # conversions, since /usr/bin/uudecode needs to deal with those.
+ # 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
+ for uudecode_attachment in uudecoded_attachments
+ c += 1
+ uudecode_attachment.url_part_number = c
+ uudecode_attachment.save!
+ attachments << uudecode_attachment.id
+ end
+ end
+
+ # now get rid of any attachments we no longer have
+ FoiAttachment.destroy_all("id NOT IN (#{attachments.join(',')}) AND incoming_message_id = #{self.id}")
+ end
# Returns body text as HTML with quotes flattened, and emails removed.
def get_body_for_html_display(collapse_quoted_sections = true)
@@ -1164,6 +1172,7 @@ class IncomingMessage < ActiveRecord::Base
return text
end
+
# Returns text of email for using in quoted section when replying
def get_body_for_quoting
# Get the body text with emails and quoted sections removed
@@ -1301,6 +1310,7 @@ class IncomingMessage < ActiveRecord::Base
return text
end
+
# Returns text for indexing
def get_text_for_indexing_full
return get_body_for_quoting + "\n\n" + get_attachment_text_full
diff --git a/app/models/request_mailer.rb b/app/models/request_mailer.rb
index 75dc58447..272f2ea83 100644
--- a/app/models/request_mailer.rb
+++ b/app/models/request_mailer.rb
@@ -10,6 +10,7 @@ require 'alaveteli_file_types'
class RequestMailer < ApplicationMailer
+
# Used when an FOI officer uploads a response from their web browser - this is
# the "fake" email used to store in the same format in the database as if they
# had emailed it.
diff --git a/db/migrate/106_add_hex_digest_to_foi_attachment.rb b/db/migrate/106_add_hex_digest_to_foi_attachment.rb
new file mode 100644
index 000000000..d9520a934
--- /dev/null
+++ b/db/migrate/106_add_hex_digest_to_foi_attachment.rb
@@ -0,0 +1,9 @@
+class AddHexDigestToFoiAttachment < ActiveRecord::Migration
+ def self.up
+ add_column :foi_attachments, :hexdigest, :string, :limit => 32
+ end
+
+ def self.down
+ remove_column :foi_attachments, :hexdigest
+ end
+end
diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index 459667b9d..9eddbc294 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -173,7 +173,7 @@ describe RequestController, "when showing one request" do
# re-parse...
lambda {
get :get_attachment_as_html, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 2, :file_name => ['hello.txt.baz.html'], :skip_cache => 1
- }.should raise_error(RuntimeError)
+ }.should raise_error(ActiveRecord::RecordNotFound)
attachment = IncomingMessage.get_attachment_by_url_part_number(ir.incoming_messages[1].get_attachments_for_display, 2)
attachment.body.should have_text(/Second hello/)
@@ -995,7 +995,6 @@ describe RequestController, "when sending a followup message" do
before(:each) do
load_raw_emails_data(raw_emails)
- info_requests(:fancy_dog_request).incoming_messages.each{|x| x.parse_raw_email!}
end
it "should require login" do
@@ -1346,9 +1345,10 @@ end
describe RequestController, "authority uploads a response from the web interface" do
+ integrate_views
fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things
- before(:all) do
+ before(:each) do
# domain after the @ is used for authentication of FOI officers, so to test it
# we need a user which isn't at localhost.
@normal_user = User.new(:name => "Mr. Normal", :email => "normal-user@flourish.org",
@@ -1402,7 +1402,7 @@ describe RequestController, "authority uploads a response from the web interface
# How do I test a file upload in rails?
# http://stackoverflow.com/questions/1178587/how-do-i-test-a-file-upload-in-rails
- it "should let the requester upload a file" do
+ it "should let the authority upload a file" do
@ir = info_requests(:fancy_dog_request)
incoming_before = @ir.incoming_messages.size
session[:user_id] = @foi_officer_user.id
diff --git a/spec/fixtures/foi_attachments.yml b/spec/fixtures/foi_attachments.yml
index 2a08e8a35..8b1378917 100644
--- a/spec/fixtures/foi_attachments.yml
+++ b/spec/fixtures/foi_attachments.yml
@@ -1,4 +1 @@
-useless_attachment:
- incoming_message_id: 1
- content_type: text/plain
- filename: foo.txt
+
diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb
index 4d64206e1..417a9b06c 100644
--- a/spec/models/incoming_message_spec.rb
+++ b/spec/models/incoming_message_spec.rb
@@ -336,16 +336,13 @@ describe IncomingMessage, " when uudecoding bad messages" do
mail_body = load_file_fixture('incoming-request-bad-uuencoding.email')
mail = TMail::Mail.parse(mail_body)
mail.base64_decode
-
im = incoming_messages(:useless_incoming_message)
im.stub!(:mail).and_return(mail)
-
-require 'ruby-debug'
-debugger
im.extract_attachments!
- attachments = im.get_main_body_text_uudecode_attachments
- attachments.size.should == 1
- attachments[0].filename.should == 'moo.txt'
+ attachments = im.foi_attachments
+ attachments.size.should == 2
+ attachments[1].filename.should == 'moo.txt'
+ im.get_attachments_for_display.size.should == 1
end
it "should apply censor rules" do
@@ -365,7 +362,7 @@ debugger
ir.censor_rules << @censor_rule
im.extract_attachments!
- attachments = im.get_main_body_text_uudecode_attachments
+ attachments = im.get_attachments_for_display
attachments.size.should == 1
attachments[0].display_filename.should == 'bah.txt'
end
diff --git a/spec/models/info_request_event_spec.rb b/spec/models/info_request_event_spec.rb
index 055965c23..3229284cc 100644
--- a/spec/models/info_request_event_spec.rb
+++ b/spec/models/info_request_event_spec.rb
@@ -54,6 +54,11 @@ describe InfoRequestEvent do
describe "doing search/index stuff" do
fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things
+ before(:each) do
+ load_raw_emails_data(raw_emails)
+ parse_all_incoming_messages
+ end
+
it 'should get search text for outgoing messages' do
event = info_request_events(:useless_outgoing_message_event)
message = outgoing_messages(:useless_outgoing_message).body
diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb
index 409d48ede..b1baa66a2 100644
--- a/spec/models/info_request_spec.rb
+++ b/spec/models/info_request_spec.rb
@@ -143,8 +143,8 @@ describe InfoRequest do
end
it "should cope with indexing after item is deleted" do
+ IncomingMessage.find(:all).each{|x| x.parse_raw_email!}
rebuild_xapian_index
-
# delete event from underneath indexing; shouldn't cause error
info_request_events(:useless_incoming_message_event).save!
info_request_events(:useless_incoming_message_event).destroy
diff --git a/spec/models/xapian_spec.rb b/spec/models/xapian_spec.rb
index cf9ea5fbd..ec11c944b 100644
--- a/spec/models/xapian_spec.rb
+++ b/spec/models/xapian_spec.rb
@@ -4,6 +4,7 @@ describe User, " when indexing users with Xapian" do
fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things
it "should search by name" do
+ parse_all_incoming_messages
rebuild_xapian_index
# def InfoRequest.full_search(models, query, order, ascending, collapse, per_page, page)
xapian_object = InfoRequest.full_search([User], "Silly", 'created_at', true, nil, 100, 1)
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index e5a42f1a9..5c5cd9a7f 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -78,6 +78,7 @@ def load_file_fixture(file_name)
end
def rebuild_xapian_index(terms = true, values = true, texts = true, dropfirst = true)
+ parse_all_incoming_messages
if dropfirst
begin
ActsAsXapian.readable_init
@@ -168,3 +169,7 @@ def load_raw_emails_data(raw_emails)
end
raw_email.data = load_file_fixture("useless_raw_email.email")
end
+
+def parse_all_incoming_messages
+ IncomingMessage.find(:all).each{|x| x.parse_raw_email!}
+end
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-25 05:11:47 +0000