Handle parsing of LF-only body with separate parts

[mikel]

Original PR #1319

Conversion of body to CRLF does not help here because body is not ascii_only so does not get converted. Instead to search for CR?LF (optional CR) in which case conversion to CRLF is unnecessary.

[mikel]

Thanks @felipec and @toofar and @sebbASF for this, the specs are failing in JRuby 9.2, if you'd like to help work out why, that would be grand and I'll get this merged.

[sebbASF]

I tried to reproduce the error in jruby-9.2, but so far have failed.
I set up a copy of test.yml that only ran 9.2 and 9.3; both succeeded in one run.
In another run, 9.2 succeeded but 9.3 failed with the same error as above.
I then ran all the tests, and they all passed.

It looks like the failure is intermittent.

Perhaps you could try triggering another test run in this fork and see what happens?
(Note: it's useful to include workflow_dispatch in the run triggers)

[mikel]

Hey there @sebbASF how annoying, intermittent failures are the "best"! :)

Could you please go ahead and add the workflow_dispatch and I'm going to ping Charles and see if he has any ideas on this, because it's a core change to our parser, I want to try and get to the bottom of it.

[sebbASF]

Added manual trigger in PR#1518

I just tried setting up a workflow in my fork that runs the tests 4 times in succession for 9.3 and 9.2, and both failed with new errors.

9.3 failed with:

Failure/Error: ::YAML.safe_load(yaml, :permitted_classes => permitted_classes)

 NoMethodError:
   undefined method `_native_parse' for #<Psych::Parser:0x2805cf5d>

9.2 - Ruby installation failed.

Looks like some dependencies may have changed?

[headius]

In the current builds I can only see 9.2 errors. They range across several tests and I'm not sure if there's a common cause. If you can isolate them to specific reproductions the next step would be filing some JRuby issues.

If you can point me at the intermittent 9.3 tests I can have a look into that.

[sebbASF]

I don't think there is any point trying to solve this issue yet, given the problems with setting up tests that started recently.

See https://github.com/mikel/mail/actions for plenty of examples of tests which fail during setup of Ruby.

If anyone can solve that problem, it would help lots of projects!

[sebbASF]

Psych 5.0.0 needs libyaml-dev; AIUI this is to be expected.

However on JRuby, tests then fail because the method _native_parse cannot be found.

As a work-round, see #1520, which attempts to trigger test failures.

[headius]

However on JRuby, tests then fail because the method _native_parse cannot be found.

Aha! Ok so there's just some Psych 5 updates we need to make for JRuby.

[sebbASF]

#1518 now also fixes the Psych 5.0.0 error for JRuby.

However there is still the issue of occasional test failures.
See for example:
https://github.com/mikel/mail/actions/runs/3637107008 - JRuby-head fails
Versus
https://github.com/mikel/mail/actions/runs/3637126244 - all JRuby runs work

It looks like there might be an issue with the handling of == in the objects that are being compared.

[headius]

The _native_parse error is due to a bad release of the Psych 5.0 gem: ruby/psych#598

[headius]

I don't know of anything in JRuby that would cause == to work incorrectly. In this failure:

       expected: #<Mail::Message:156184, Multipart: false, Headers: <From: you@you.com>, <To: mikel@me.com>, <Subject: testing>>
            got: #<Mail::Message:156184, Multipart: false, Headers: <From: you@you.com>, <To: mikel@me.com>, <Subject: testing>>

My guess would be that the == method is expecting something to be identical, or else some of the strings are getting different encodings that make them look non-equal.

[sebbASF]

Note that I saw a similar error once in non-JRuby test.

I agree that this is highly unlikely to be an issue in the underlying Ruby implementation.
It is more likely to be an issue with how == is implemented in mail (or perhaps a dependency) for the objects that are being compared.

Unfortunately failures are quite rare, which makes it much harder to debug (and harder to prove that a change fixes it).

[headius]

Psych 5.0.1 has been released, fixing the _native_parse issue.

https://rubygems.org/gems/psych/versions/5.0.1-java

[sebbASF]

See #1527 - this looks to be the cause of the occasional errors.

It is a timing issue, which is why the errors are infrequent.

[mikel]

Awesome, thanks @sebbASF and @headius :)

[alexdean]

@mikel hi! i was looking for info about when i might see the change in the PR in an official mail release. looked for some info on the release process & didn't find anything.

context: i maintain the as2 gem. AS2 messages are MIME messages, so we use mail to parse out the message body & digital signature. starting in mail 2.7.1, all line endings in message bodies are being converted to \r\n, even when the raw content had only \n line endings. This breaks signature verification.

# before this PR...
require 'mail'
mail = Mail::Body.new("new\nbody")
mail.raw_source
# => "new\r\nbody"       <------- note additional "\r"

# after this PR...
require 'mail'
mail = Mail::Body.new("new\nbody")
mail.raw_source
# => "new\nbody" 

The change in this PR fixes the issue, so I'm hoping to find out when I can start using it in as2. If a release is some time away, can you suggest any way to patch this behavior in a relatively well-contained way?