-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Update LanguageHelper.php to handle translated text with embedded new lines. #42456
Conversation
Emulate Joomla v4.4.0 and earlier behaviour when multi-line text strings present.
Corrected comment.
Can anyone show me an example of a real attack vector that you are trying to fix with these patches? I know all the insides of the ini parser in PHP (and even contributed to it), I know the insides of Joomla of course. But I absolutely don't see how the ini files can be manipulated from the outside. Overriding ini files is limited to admins only. As for third-party extensions, just like using constants in ini files, they can simply print any constant or environment variable. Are you going to handle every php file in a similar way??? And as a result of this "fix" we have a big performance hit. So, what are you still struggling with? You can answer me in Mattermost (@denis-ryabov). |
This patch is not attempting to fix any attack vector. An attack vector (real or imagained) was addressed in J4.1.1. I produced a PR to address this which was rejected because of the performance raminifications. Later, after looking at further changes made in J5 I rewrote it without any performance impact (except in those few instances which failed anyway in J4.1.1). |
Added lost function.
I thought since you're trying to fix it, you'd understand the exact reason for @bembelimen's patch (609cafc), and the problem is that I don't, and it looks like you're trying to fix the problem that shouldn't exist. |
The problem did not exist until J4.1.1 The security fix in J4.1.1 meant that some previously working language .ini files no longer worked. Without arguing about whether the security issue is real or imagined (in the language translation scenario) and reversing the change the quickest solution is to cater for those failing .ini files without impacting performance with the others... unless you argue that finding all the problematic .ini files (plus months of more come to light as people upgrade to J4.1.1+) is the correct solution - but then those problem .ini files I've encountered have been working for years without a problem. |
I will leave it to the JSST to respond specifically about the security issue but the explanation of why to use parse_ini_raw is well documented |
No point waiting for JSST to respond, I am happy with what has been done for J5.0 |
@BrainforgeUK thanks for the PR - could you fix the phpcs error? |
Improved comment.
Changed @SInCE to __DEPLOY_VERSION__
Anything else I need to do here? |
I did the Update branch (comes up if the current branch - 4.4- is updated with other PR's) - but there are still phpcs errors... |
Changed bracket position {
Thanks. |
Corrections for phpcs errors.
Another phpcd error fix.
Further phpcs error correction.
Another phpcs error correction.
Hello @BrainforgeUK we, the maintainer team decided, that the solution is not the way we want to go. Thank you very much for your contribution, we really appreciate your work and hope, this does not discourage you and we're looking forward to further ideas from you. |
Thanks, my concern is when will embedded newline capability be restored to .ini language files? |
Emulate Joomla v4.4.0 and earlier behaviour when multi-line text strings present.
Pull Request for Issue #42416 .
Summary of Changes
Allow embedded newlines in text (i.e. between quotes ").
Using them to structure very large areas of text makes maintenance easier.
In some cases using linewrap in IDE slightly helps, but one is left with trying to read / unravel a massive jumble of text.
Newlines in text worked up to v4.4.0
Security related change in v4.4.1 removed this functionality.
This change detects and removes the embedded newlines before using PHP ini parsing as before.
The security related change introduced in v4.4.1 is maintained.
Testing Instructions
Install this plugin.
plg_system_bflanguagetest.zip
Go to plugin admin page.
Actual result BEFORE applying this Pull Request
The plugin name and description not translated.
The language labels are displayed.
Expected result AFTER applying this Pull Request
Name and description translated.
As previous behavious in J4.1.0 and earlier.
Link to documentations
Please select:
Documentation link for docs.joomla.org:
No documentation changes for docs.joomla.org needed
Pull Request link for manual.joomla.org:
No documentation changes for manual.joomla.org needed
Line breaks in characters string have worked for a very long time.
Mention made in another conversation that somewhere in the documentation it says they are not allowed.
Is so the location where that appears needs to be found and corrected.
Also mention was made that the JED needs to be changed to prevent the use of such line breaks.
Such a change will no longer be necessary - assume the developer wants such included.