Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/html body #29

Open
wants to merge 9 commits into
base: primary
Choose a base branch
from
Open

Conversation

jmacdone
Copy link

@jmacdone jmacdone commented Jul 2, 2024

I was having trouble getting this to convert the msg files released by Microsoft related to the disclosure of the Midnight Blizzard attack. So, I went down a troubleshooting rabbit hole. The core issue was overlooking HTML bodies.

  • reorganized as a package (perhaps PyPI someday?)
  • add a handler for HTML bodied messages (property tag 0x1013)
  • filtered \x00 values being returned from UNICODE.value (not sure if bug?)
  • various linting and formatting tune-ups (hopefully not too opinionated)
  • updated README to reflect changes
  • no unit tests 😳

James Macdonell added 9 commits July 2, 2024 10:53
Not happy about the on-call import for EMBEDDED_MESSAGE
As per comment, I'm unsure if this is just how UNICODE values are
stored, or if there is a off-by-one error (off by two since it's utf16?)
somewhere.
@JoshData
Copy link
Owner

JoshData commented Jul 3, 2024

Thanks. I like the ideas.

But I can't accept changes that i can't easily review. The reorganization shows up on GitHub as large deletes and large insertions, so I can't see if anything changed.

I'd be happy to review if the reorganization is removed and left for another time.

@jmacdone jmacdone mentioned this pull request Jul 3, 2024
@jmacdone
Copy link
Author

jmacdone commented Jul 3, 2024

I did break them into individual commits so you don't have to try to evaluate the whole PR as one change. But, I respect that it's hard to approve changes without unit tests to give confidence.

I made an alternative PR #30 if this is too overwhelming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants