search

bbottema / outlook-message-parser

A Java parser for Outlook messages (.msg files)
77 stars 35 forks source link

Bug: __nameid_ directory should not be parsed (and causing invalid HTML body) #23

Closed derrohrbach closed 4 years ago

derrohrbach commented 4 years ago

As discussed in PR #22, see also nameid-fix branch:

In a MSG file the _nameid... directory represents named properties, which are as of right now not supported by this library. It tries to parse the entries in this directory using the normal property parser though, which creates conflicts and is wrong. You can also refer to the Microsoft documentation for this: [MS-OXMSG].

I had several emails where an id in this directory collided with the property id for HTML-Content (10130102), which caused RTF-Emails to incorrctly report this invalid content as the HTML body, which prevented the RTF Conversion from being read.

grafik

derrohrbach commented 4 years ago

The fix in #22 could not be merged because of Tests failing! I can not reproduce this though, because my tests always fail with this error:

[ERROR] Tests run: 16, Failures: 0, Errors: 16, Skipped: 0, Time elapsed: 0.07 s <<< FAILURE! - in org.simplejavamail.outlookmessageparser.HighoverEmailsTest
[ERROR] testUnicodeMessage(org.simplejavamail.outlookmessageparser.HighoverEmailsTest)  Time elapsed: 0.009 s  <<< ERROR!
java.lang.Error:
Unresolved compilation problems:
        OutlookMessageAssert cannot be resolved
        OutlookMessageAssert cannot be resolved
        OutlookMessageAssert cannot be resolved
        OutlookMessageAssert cannot be resolved
        The method normalizeText(String) is undefined for the type HighoverEmailsTest

        at org.simplejavamail.outlookmessageparser.HighoverEmailsTest.testUnicodeMessage(HighoverEmailsTest.java:520)
bbottema commented 4 years ago

Ahh, those are classes generated by a maven plugin when you run mvn test

derrohrbach commented 4 years ago

Well it seems for me those are not being generated, because i run using mvn test

derrohrbach commented 4 years ago

Are there any news about these errors? I would really like to run the tests and fix the bugs ;)

bbottema commented 4 years ago

I really don't have a clue why mvn test wouldn't work for you. Except maybe that you're JDK is too new? I'm developing this library with JDK 1.8 (and Maven 3.6.0).

derrohrbach commented 4 years ago 
Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
Maven home: C:\Maven\bin\..
Java version: 1.8.0_222, vendor: AdoptOpenJDK, runtime: C:\Program Files\AdoptOpenJDK\jdk-8.0.222.10-hotspot\jre
Default locale: de_DE, platform encoding: Cp1252
OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
derrohrbach commented 4 years ago

I got something running by copying the class files from target/test-classes to target/classes... Now trying to look into the tests

bbottema commented 4 years ago

That's just bizar that you have to copy anything, I have no explanation for it.

derrohrbach commented 4 years ago

Well a second run of the tests does not work either and fails with an error about something being already instrumented. My current procedure for testing:

  1. mvn clean test
  2. copy files
  3. mvn test

I was able to verify though that it's an error in the tests and not in the fixed code. It always fails at getBodyHTML().isNotEmpty(), because before you had the invalid body content in there. Now it is null, which is correct, because there is no HTML body in these four mails.

To verify this just look at the supposed HTML content which the current code produces for the following mails:

You will see that the content is just garbled data. That is essentially the bug, which the patch fixed. I will open a new pull request with the tests fixed.

bbottema commented 4 years ago

Thanks for the research, much appreciated. Before I can merge the change, I would like to investigate why body HTML remains empty even if there is an RTF body. I'll be able to check that this weekend.

derrohrbach commented 4 years ago

Well thats a different issue though, isn't it? The behaviour with this bugfix is better than before, because you can at least see, that there is no HTML Body, instead of getting invalid data. Also how would you detect if it is a real html email or if it was converted from rtf, if the html attribute is always set?

bbottema commented 4 years ago

Indeed and I wasn't looking properly (it's been some time since I touched that part of the code): the converted HTML is there, but in msg.getConvertedBodyHTML() instead of msg.getBodyHTML(). so that's all hunky-dory. I'll prepare a release soon.

bbottema commented 4 years ago

Released in 1.7.1