fixing non-ASCII redirects from issue #42

ovidiul commented 3 years ago

This PR addressed the bug highlighted in issue #42

It adds:

the main fix includes replacing the wp_parse_url method with a helper method called mb_parse_url that properly handles non-ASCII chars encodings
a new Utils class to keep helper method mb_parse_url
a new tests class UtilsTest for the Utils class
it refactors the maybe_do_redirect method by adding a Lookup::get_redirect_data method for easier testing; this will also decrease its overall cyclomatic code complexity
it adds a few tests for the Lookup::get_redirect_data method in the LookupTest class

GaryJones commented 1 year ago

Other than the conflicts, having looked at this again with fresh eyes, I think there are a lot of parts that could and should be pulled out into fresh PRs as iterative steps to improve the testing / resilience of this plugin, culminating with a final PR that actually does the needed fix. Things like Normalize tests, Utils::mb_parse_url etc.

GaryJones commented 4 months ago

I started splitting this out, but the more I looked and tested locally, the more it made sense to just double-checked the PR worked as is, and it does!

Automattic / WPCOM-Legacy-Redirector

fixing non-ASCII redirects from issue #42 #102