phpList / phplist3

Fully functional Open Source email marketing manager for creating, sending, integrating, and analysing email campaigns and newsletters.
https://www.phplist.org
GNU Affero General Public License v3.0
752 stars 269 forks source link

Missing commit for release 3.3 #142

Closed bramley closed 7 years ago

bramley commented 7 years ago

The release here https://github.com/phpList/phplist3/releases/tag/phplist-3.3.0 doesn't include the further fix I provided for replacing the '-4' sequences in the UUIDs. Currently links in text emails will be broken due to replacing one character by two. This is the fix I submitted to branch dev33 to replace the single statement at line 630

                    $masked = $linkUUID . $cached[$messageid]['uuid'] . $userdata['uuid'];
                    $uuidLength = strlen($linkUUID);
                    $masked[14] = substr(bin2hex(random_bytes(1)), 0, 1);
                    $masked[$uuidLength + 14] = substr(bin2hex(random_bytes(1)), 0, 1);
                    $masked[$uuidLength * 2 + 14] = substr(bin2hex(random_bytes(1)), 0, 1);
oliverklee commented 7 years ago

Do we need a PR for this? Or is it just missing from the release notes?

michield commented 7 years ago

Duncan, I must have missed your commit. The branch is gone. Can you re-push it?

bramley commented 7 years ago

@michield Can you just copy those lines (or amend the existing code to do something equivalent)? This is the existing code that replaces one character by two (bin2hex generates two characters from one random byte). The following hex2bin fails due to an odd length input.

                    $masked = substr($linkUUID,0,14).bin2hex(random_bytes(1)).substr($linkUUID,15).
                        substr($cached[$messageid]['uuid'],0,14).bin2hex(random_bytes(1)).substr($cached[$messageid]['uuid'],15).
                        substr($userdata['uuid'],0,14).bin2hex(random_bytes(1)).substr($userdata['uuid'],15);
michield commented 7 years ago

OK, I committed https://github.com/phpList/phplist3/commit/a139f35bece20ef08a22333f3c30d052e7cf7828

but it works for me either way. I'm not sure why. I think your version is safer though, as it forces the bin2hex into one character. Maybe it's not always the same on all systems.

bramley commented 7 years ago

With the original code, when sending a test message there was this error reported for each url image

and the tid parameter in links in the text format email was empty Here is a link http://strontian/lists/lt.php?tid=&hm=5af08ebfceb4eb0da430799e349afe38482f5c727cc547dd21a3c146d5e32f5f

michield commented 7 years ago

strange. That didn't happen for me. I guess some system differences,,,,