Split on an exact \r\n\r\n sequence if it exists #501

Merged
merged 1 commit into from Feb 12, 2013

Conversation

Projects
None yet
3 participants
@ConradIrwin
Collaborator

ConradIrwin commented Jan 28, 2013

While the optional whitespace thing is kind of nice, that shouldn't cause us to ignore the spec sometimes.

@jeremy

This comment has been minimized.

Show comment Hide comment
@jeremy

jeremy Feb 10, 2013

Collaborator

👍 to this. Using an example mail is a pretty big hammer though. Could you do a more precise spec in message_spec?

Collaborator

jeremy commented Feb 10, 2013

👍 to this. Using an example mail is a pretty big hammer though. Could you do a more precise spec in message_spec?

@ConradIrwin

This comment has been minimized.

Show comment Hide comment
@ConradIrwin

ConradIrwin Feb 10, 2013

Collaborator

I've updated the branch with some smaller specs that cover a range of splitting behaviour. Is that better?

Collaborator

ConradIrwin commented Feb 10, 2013

I've updated the branch with some smaller specs that cover a range of splitting behaviour. Is that better?

jeremy added a commit that referenced this pull request Feb 12, 2013

Merge pull request #501 from ConradIrwin/split-on-crlfcrlf
Split on an exact \r\n\r\n sequence if it exists

@jeremy jeremy merged commit 1d515f7 into mikel:master Feb 12, 2013

jeremy added a commit that referenced this pull request Feb 12, 2013

@leifbladt

This comment has been minimized.

Show comment Hide comment
@leifbladt

leifbladt Mar 19, 2013

@jeremy Any chance to see this in a new release soon, since Rails 3.2.13 requires mail-0.5.3?

@jeremy Any chance to see this in a new release soon, since Rails 3.2.13 requires mail-0.5.3?

@jeremy

This comment has been minimized.

Show comment Hide comment
@jeremy

jeremy Mar 22, 2013

Collaborator
Collaborator

jeremy commented Mar 22, 2013

@jeremy

This comment has been minimized.

Show comment Hide comment
@jeremy

jeremy May 29, 2014

Collaborator

This means we'll split headers/body on certain pathological folded header values:

diff --git a/spec/mail/message_spec.rb b/spec/mail/message_spec.rb
index 259482d..444f4ae 100644
--- a/spec/mail/message_spec.rb
+++ b/spec/mail/message_spec.rb
@@ -216,6 +216,11 @@ describe Mail::Message do
         message.decoded.should == " Hello there\n"
       end

+      it 'should not split the body at an empty folded header value' do
+        message = Mail::Message.new("To: foo\r\n \r\nFrom: bar\r\n\r\nbody\r\n")
+        message.decoded.should == "body\n"
+      end
+
       it 'should split only once if there are "\r\n\r\n"s in the body' do
         message = Mail::Message.new("To: Example <example@cirw.in>\r\n\r\nHello\r\n\r\nthere\r\n")
         message.decoded.should == "Hello\n\nthere\n"
  1) Mail::Message initialization splitting should not split the body at an empty folded header value
     Failure/Error: message.decoded.should == "body\n"
       expected: "body\n"
            got: "From: bar\n\nbody\n" (using ==)
       Diff:
       @@ -1,2 +1,4 @@
       +From: bar
       +
        body
Collaborator

jeremy commented on spec/mail/message_spec.rb in a2a4559 May 29, 2014

This means we'll split headers/body on certain pathological folded header values:

diff --git a/spec/mail/message_spec.rb b/spec/mail/message_spec.rb
index 259482d..444f4ae 100644
--- a/spec/mail/message_spec.rb
+++ b/spec/mail/message_spec.rb
@@ -216,6 +216,11 @@ describe Mail::Message do
         message.decoded.should == " Hello there\n"
       end

+      it 'should not split the body at an empty folded header value' do
+        message = Mail::Message.new("To: foo\r\n \r\nFrom: bar\r\n\r\nbody\r\n")
+        message.decoded.should == "body\n"
+      end
+
       it 'should split only once if there are "\r\n\r\n"s in the body' do
         message = Mail::Message.new("To: Example <example@cirw.in>\r\n\r\nHello\r\n\r\nthere\r\n")
         message.decoded.should == "Hello\n\nthere\n"
  1) Mail::Message initialization splitting should not split the body at an empty folded header value
     Failure/Error: message.decoded.should == "body\n"
       expected: "body\n"
            got: "From: bar\n\nbody\n" (using ==)
       Diff:
       @@ -1,2 +1,4 @@
       +From: bar
       +
        body

This comment has been minimized.

Show comment Hide comment
@ConradIrwin

ConradIrwin May 29, 2014

Collaborator

This was preserving previous behaviour of the mail gem. I'd be happy to drop it, but wanted to err on the side of minimal change.

Collaborator

ConradIrwin replied May 29, 2014

This was preserving previous behaviour of the mail gem. I'd be happy to drop it, but wanted to err on the side of minimal change.

This comment has been minimized.

Show comment Hide comment
@jeremy

jeremy May 29, 2014

Collaborator

Hmm yeah. Hard to tell whether the previous behavior was accidental or intentional.

Collaborator

jeremy replied May 29, 2014

Hmm yeah. Hard to tell whether the previous behavior was accidental or intentional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment