Space in url not handled well

[vdavid]

Bug Description

If there is a space at the beginning or end of the URL of a link href, Mautic shows a 404 error page when a user clicks on the link in the email.

Note: Adding a space to the beginning or end of the URL is a user error. But users of the Mautic front end is not only technical people, and simple users tend not to care too much about whitespaces. Mautic could be prepared for this scenario.

Here is an example email: https://www.screencast.com/t/J0QAXqvwjm
Here is the source of the example email: https://www.screencast.com/t/07jeK3kY9
Here is the code in Mautic 2.16.0 that handles this URL: https://www.screencast.com/t/MTyAd2jhBq

This code uses PHP's filter_var() function, namely: filter_var($url, FILTER_VALIDATE_URL) in PublicController::redirectAction. But filter_var() will return false for any URL that has a space at its end. :(

Suggestion: apply a trim() on the $url before the filter_var() call.

Q	A
Mautic version	2.16.0
PHP version	7.2.16
Browser	latest Chrome

Steps to reproduce

1. Create an email in Mautic which includes a link. In the URL part, add e.g. http://google.com (note the space at the end.)
2. Send the email with the tracking settings on. It will correctly replace the URL and add Mautic's tracking URL, so the mistake will be accepted silently.
3. Receive the email and click on the link in it. You'll see a 404 page.

Log errors

(No errors in the log.)

Thanks a lot for your work btw, we love you guys.

[RCheesley]

Thanks for reporting this @vdavid, would you be up for submitting a PR to implement the suggestions you've made? If so, please ensure you mention that it resolves this issue so that the two get linked up :)

[vdavid]

Thanks for your response @RCheesley!
I'd be happy to do that, but I don't have the tools (PHP) at hand to test my changes.
Also, while I was a PHP dev for years, my PHP is rusty. Considering this, would it be fine and helpful to submit an untested PR as well? Because that I'd be happy to do.

[RCheesley]

@vdavid any PR requires two community testers before it is merged, so if you can submit something which you believe resolves the issue, we'll handle the testing :)

[kuzmany]

This should fixed it #8345
Can you test it?

[vdavid]

Thanks a lot for your PR @kuzmany! I had on my roadmap to respond with a PR, but I kept on pushing it to later. Unfortunately, as ai mentioned before, I don't have a PHP environment now so I can't really test it. I hope someone can review soon and it can be merged.

[kuzmany]

@vdavid you can test it by Mautibox

http://m3.mautibox.com/8345

Emails are fake and you can find it http://m3.mautibox.com/mail after queue command triggering

[vdavid]

Oh I see. Thanks. :) Will take a look at it then.

…

On Tue, May 12, 2020 at 8:43 AM Zdeno Kuzmany ***@***.***> wrote: @vdavid <https://github.com/vdavid> you can test it by Mautibox http://m3.mautibox.com/8345 Emails are fake and you can find it http://m3.mautibox.com/mail after queue command triggering — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8597 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AATPDSJV3HYGIAPEVL473TDRRDVYVANCNFSM4LUI7F2Q> .

[dennisameling]

@vdavid I can't seem to reproduce this issue on Mautic 3.

I added http://google.com (including the space at the end) to an email, enabled tracking, and the URL is replaced by the tracking URL correctly.

I'm getting redirected to Google without problems:

Can you please try to reproduce the issue on Mautic 3?

[kuzmany]

@dennisameling which PR do you comment? this is issue

[dennisameling]

@kuzmany no PR, just trying to reproduce the issue that's described in the issue description :) can't seem to reproduce the issue on M3

[npracht]

@kuzmany brought several enhancement on that in 3.x. We can close