Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix for UnstructuredField which responds to Content-Field #470

Closed
wants to merge 1 commit into from

2 participants

@DavyCardinaal

Recently received an e-mail which had the following content-type for an attachment:

Content-Type: unknown/unknown;
    charset=iso-8859-1;
    name=IMSTP19.gif

After receiving the e-mail I perform a to string on the Mail object to generate a unique hash. Something like the following:

Mail::POP3.new(settings).find_and_delete(count: 25) do |msg| 
  receive(msg)
  process_email_key(Digest::SHA1.hexdigest(msg.to_s))
end

When I received the e-mail with the content-type I noticed above I got the following error

undefined method `string' for unknown/unknown; name="image.gif":Mail::UnstructuredField

The private method get_order_value() in lib/mail/parts_list.rb assumes that the Field is a Mail::ContentTypeField when the part responds_to content-type. But with the content-type used in this e-mail the field is a Mail::UnstructuredField.

The part[:content_type].string raises an error because there's no string method for an UnstructuredField.

@jeremy
Collaborator

:+1:

@jeremy jeremy commented on the diff
lib/mail/parts_list.rb
@@ -44,7 +44,7 @@ def sort!(order)
private
def get_order_value(part, order)
- if part.respond_to?(:content_type)
+ if part.respond_to?(:content_type) && part[:content_type].field.is_a?(Mail::ContentTypeField)
@jeremy Collaborator
jeremy added a note

Perhaps check for respond_to?(:string) rather than the specific field class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jeremy jeremy commented on the diff
spec/mail/parts_list_spec.rb
@@ -17,4 +17,13 @@
p << 'text/html'
p.sort!(order)
end
+
+ it "should not raise an error when the part is content_type and Mail::UnstructuredField" do
+ part = Mail::Part.new do
+ content_type 'unknown/unknown; name="image.gif"'
+ end
+
+ Mail::PartsList.new.send(:get_order_value, part, [])
@jeremy Collaborator
jeremy added a note

Can you test this without calling in to a private method? Spec something concrete, like checking the expected parts ordering, rather than an implicit spec that no exception is raised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jeremy jeremy closed this in ba06563
@DavyCardinaal

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 27, 2012
  1. @DavyCardinaal
This page is out of date. Refresh to see the latest.
Showing with 10 additions and 1 deletion.
  1. +1 −1  lib/mail/parts_list.rb
  2. +9 −0 spec/mail/parts_list_spec.rb
View
2  lib/mail/parts_list.rb
@@ -44,7 +44,7 @@ def sort!(order)
private
def get_order_value(part, order)
- if part.respond_to?(:content_type)
+ if part.respond_to?(:content_type) && part[:content_type].field.is_a?(Mail::ContentTypeField)
@jeremy Collaborator
jeremy added a note

Perhaps check for respond_to?(:string) rather than the specific field class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
order.index(part[:content_type].string.downcase) || 10000
else
10000
View
9 spec/mail/parts_list_spec.rb
@@ -17,4 +17,13 @@
p << 'text/html'
p.sort!(order)
end
+
+ it "should not raise an error when the part is content_type and Mail::UnstructuredField" do
+ part = Mail::Part.new do
+ content_type 'unknown/unknown; name="image.gif"'
+ end
+
+ Mail::PartsList.new.send(:get_order_value, part, [])
@jeremy Collaborator
jeremy added a note

Can you test this without calling in to a private method? Spec something concrete, like checking the expected parts ordering, rather than an implicit spec that no exception is raised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ end
+
end
Something went wrong with that request. Please try again.