Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RTM] Fix incompatibilities with PCRE 8.32 in the BackportedTranslator class #10

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

plorenz-etes
Copy link

Due to a regression in PCRE 8.32 there is a problem using the translator on systems using this version of the library. Some regular expressions can't be parsed which breaks functionality. As a result, e.g. using MetaModels isn't possible because every translation returned has the value 'Array'.

In Contao 4.5, the Translator class was introduced and ran into this problem: contao/contao#886
Since the BackportedTranslator class in this project uses parts of the code from the original Contao Translator class, I think the changes made here to Translator.php should also be made to the BackportedTranslator class: contao/contao#916

Changing these two lines in a test installation environment fixes the problem instantly for me.

@baumannsven
Copy link
Member

@plorenz-etes Can you post the translation to use? So we can wrote a test for this problem.

@plorenz-etes
Copy link
Author

@baumannsven It basically happens with any translation. The following code will be executed when looking up a translation (BackportedTranslator.php lines 128/129):

preg_match_all('/(?:\\\\[\\\\.]|[^.])++/', $id, $matches);
$parts = preg_replace('/\\\\([\\\\.])/', '$1', $matches[0]);

Since those regular expressions aren't correct for PCRE 8.32, they can't be compiled and thus, the function getFromGlobals() doesn't return a string but rather the complete language array $_GLOBALS['TL_LANG'].

@discordier
Copy link
Member

IMO the lines from Contao core can be used along the corresponding unit tests, see commit contao/contao@75c5d8c

@baumannsven
Copy link
Member

@discordier Thanks.

@plorenz-etes
Copy link
Author

Hi, sorry to ask but since we've got a customer running into this problem - can you tell whether this PR will make it into the next release?

@discordier
Copy link
Member

It will as long as the tests are ported as well and the build runs successfully.

Sadly no one of our team has currently the time to port them over, so it would be nice if you could update your PR.

As soon all is green, we will be happy to merge.

@zonky2 zonky2 added this to the 2.1.5 milestone Dec 20, 2019
@zonky2
Copy link
Contributor

zonky2 commented Dec 20, 2019

@plorenz-etes please add your name and email as author - see check in travis...

@plorenz-etes
Copy link
Author

@zonky2 Thank you for the reminder - I have added the requested author line.

@zonky2
Copy link
Contributor

zonky2 commented Dec 20, 2019

thx!

@zonky2 zonky2 changed the title Fix incompatibilities with PCRE 8.32 in the BackportedTranslator class [RTM] Fix incompatibilities with PCRE 8.32 in the BackportedTranslator class Dec 20, 2019
@discordier
Copy link
Member

The tests are still missing?

Copy link
Member

@discordier discordier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests still missing, therefore still a NACK from me.

@baumannsven baumannsven modified the milestones: 2.1.5, 2.2.0 Apr 11, 2021
@zonky2 zonky2 removed this from the 2.2.0 milestone Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants