Skip to content

Fix generating lingua.selector url to other language on HTTPS#67

Open
Mark-H wants to merge 1 commit into
goldsky:masterfrom
Mark-H:patch-1
Open

Fix generating lingua.selector url to other language on HTTPS#67
Mark-H wants to merge 1 commit into
goldsky:masterfrom
Mark-H:patch-1

Conversation

@Mark-H

@Mark-H Mark-H commented Mar 11, 2021

Copy link
Copy Markdown

Line 208 of the lingua.selector snippet has the following preg_replace to generate the other language url:

$pageURL = preg_replace('/^' . preg_quote($parseUrl['scheme'] . '://' . $parseUrl['host'] . '/' . $requestUri, '/') . '/i'
       , $parseUrl['scheme'] . '://' . $parseUrl['host'] . '/' . $baseUrl . $itemUri
       , $originPageUrl);

That's trying to find $parseUrl['scheme'] . '://' . $parseUrl['host'] . '/' . $requestUri in $originPageUrl, to replace it with the (correct) $parseUrl['scheme'] . '://' . $parseUrl['host'] . '/' . $baseUrl . $itemUri.

Because HTTPS is (transparently) served on port 443, the port is injected into $pageURL here in line 55 and therefore around line 175 into the subject of this replace, $originPageUrl.

Because $parseUrl['host'] does not have the port, the preg_replace fails and the user is given the current link with ?lang=foo added. While that works to switch the language, the user wants to see the proper localised url and not the current one.

By not adding the port if it's 443, the preg_replace matches as expected and the right URL is returned.

@JoshuaLuckers

Copy link
Copy Markdown

@Mark-H

Mark-H commented Mar 12, 2021

Copy link
Copy Markdown
Author

On line 50 of the snippet, sure. Would make it work better on reverse proxy setups too. It still needs this fix to make sure the preg_replace works correctly, though.

It might also make sense to re-evaluate this entire snippet as it's all very confusing to me. It seems that most of the complexity in the snippet stems from wanting to keep query parameters for the new URL, which I'd imagine can be done much simpler (and less prone to issues like this) by just using $_GET to create the new URL.... but as I'm not familiar with the full history of the project and only have this one client using Lingua that has asked me to look into some issues, I'm most likely overlooking something important that led to it being done this way in the first place.

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