search

laminas / laminas-mime

Create and parse MIME messages and parts
https://docs.laminas.dev/laminas-mime/
BSD 3-Clause "New" or "Revised" License
29 stars 23 forks source link

fix: binary files as part are corrupted when stripping carriage returns #26

Open pgmillon opened 2 years ago

pgmillon commented 2 years ago
Q A
Documentation no
Bugfix yes
BC Break no
New Feature no
RFC no
QA no

Description

When using a simple PNG binary as part of a message, the carriage return characters are ripped off, corrupting the binary. Not ripping the carriage return characters have an impact in the parts splitting that was updated to use preg_match + PREG_OFFSET_CAPTURE as a replacement to strpos. This was done the support either \r\n or \n with a regex rather than a more complex approach. Working with a binary file highlighted the line feed that is added while reading each part, this was removed too a/ to avoid binary corruption and b/ a mime with no line feed shouldn't magically have one when reading. A unit test was added to ensure further support of binaries.

Ocramius commented 2 years ago

@pgmillon it's possible that this, being a bugfix, should go to 2.9.x

pgmillon commented 2 years ago

Hi, I wasn't sure how you deal with bugfixes, sorry. If you pull bugfixes from "legacy" branches into the main branch or if you "backport" from main into legacy branches. I can update this PR for 2.9.x, as you prefer.

Ocramius commented 2 years ago

We merge up from oldstable branches to latest branches.

Since the last release is 2.9.1, I'd expect this to land in 2.9.2, therefore 2.9.x target branch.

We don't merge regular bugs into older branches, since they need to be security issues, in order to qualify for that :)

EDIT: I changed the base branch, but your commit will need to probably be cherry-picked on top of that, and then force-pushed here, so extraneous commits are gone.

pgmillon commented 2 years ago

Should be fine for 2.9.x now.

pgmillon commented 2 years ago

Wait, I just found another bug : if the message sender uses \r\n to generate the message rather than just \n, then the splitMime is not working correctly and leaving a lonely \r.

pgmillon commented 2 years ago

There we go : in my original fix I assumed sender's EOL was \n but when testing with a server that was using \r\n, I noticed the intend to prevent binaries corruption was not met because of a trailing \r. I added a sender EOL detection to fix the parts splitting which now works fine with binaries.

pgmillon commented 2 years ago

Hi, any chance to get that fix merged ? I'm waiting for it to cut a new release of a library I work on. Thanks.