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

[4.4.1] Language constants of some extensions not translated #42416

Closed
toivo opened this issue Nov 29, 2023 · 34 comments
Closed

[4.4.1] Language constants of some extensions not translated #42416

toivo opened this issue Nov 29, 2023 · 34 comments

Comments

@toivo
Copy link
Contributor

toivo commented Nov 29, 2023

Steps to reproduce the issue

Download the Helix Ultimate v2.0.17 plugin and template from https://www.joomshaper.com/downloads/template/helixultimate
Install these extension in Joomla 4.4.0.
Update the website to Joomla 4.4.1.

Expected result

The language constants are translated.

Actual result

The language constants are not translated.

System information (as much as possible)

N/A

Additional comments

Reported in Joomla forum topics https://forum.joomla.org/viewtopic.php?f=815&t=1005723 and https://forum.joomla.org/viewtopic.php?f=815&t=1005719.

The same happened to my own backend component under development in Joomla 4.4.1 and 5.0.1. Only the language constants used in the manifest file and defined in the mycomponent.ini file got translated.

Revert the two files LanguageHelper.php and Text.php in the folder libraries/src/Language to the previous versions to fix the translation issue.

Question: Do the changes in the Language folder change the way the items in the manifest file have to be presented or something else? The change in 4.4.1 and 5.0.1 will no doubt break many third party extensions, even if they have followed the Joomla documentation to the letter, unless the documentaion is updated or some changes deprecated or reverted.

@toivo toivo changed the title [4.0.1] Language constants of some extensions not translated [4.4.1] Language constants of some extensions not translated Nov 29, 2023
@shur
Copy link
Contributor

shur commented Nov 29, 2023

Confirmed, same problem with all modules by RAXO
https://extensions.joomla.org/extension/news-display/articles-display/raxo-all-mode-pro/

After updating to Joomla 4.4.1, all language constants in the module control panel are gone.
Users are already starting to pour into the support team about this issue. So the problem is present and this is not an isolated case.

@brianteeman
Copy link
Contributor

I have replicated the issue with the helix template and can confirm that with the security changes the issue is present. However the root cause of the issue is the language files themself. Both the site and the admin language files for the template are invalid.

Both of them have a faulty string for COM_FINDER_ADVANCED_TIPS

image

As can be seen there is one invalid string. Instead of it being on one line ending with " it is spread across multiple lines.

As soon as you remove the hard returns in the string everything works.

This is not a core bug ut a problem in the language file that is now exposed.

@brianteeman
Copy link
Contributor

@HLeithner @SniperSister Please confirm my findings.

Note: 5.0.0 with the language debug did not report an error in the language file but 5.0.1 does

@shur
Copy link
Contributor

shur commented Nov 29, 2023

@brianteeman

As can be seen there is one invalid string. Instead of it being on one line ending with " it is spread across multiple lines.
As soon as you remove the hard returns in the string everything works.

Yes, the problem is also in one language constant, which contains a long description of the module and therefore takes up several lines. As soon as you remove the line breaks the problem disappears.

Brian, thank you for your quick and accurate response.

@SniperSister
Copy link
Contributor

The problem is indeed related to the security-fix. We had to change the parsing mode of PHP's ini methods to fix a security vector. The NORMAL mode used so far allowed linebreaks within multiline i18n strings, where the now used RAW mode does not. We tested the change on various testing and production sites before release, however the specific issue was overlooked as none of the test sites used language files with linebreaks within strings and the documentation of the method on php.net does not mention that behavior.

Reverting the mode change is not an option as the attack vector would be re-opened again, so the proper way forward is indeed to update the line breaks.

@brianteeman
Copy link
Contributor

Thanks for confirming my findings.

Maybe you should document this

@shur
Copy link
Contributor

shur commented Nov 29, 2023

I'll just leave this thought here.
Is there any way to make it so that if there is a multiline problem in one language constant, the whole localization for the extension doesn't fail? So that the problem in one constant remains a problem in one constant.

@SniperSister
Copy link
Contributor

Both PHP core methods, parse_ini_file and parse_ini_string, consume the complete i18n file as an input and either fail or succeed completely. There is no option to skip a single faulty string.

@toivo
Copy link
Contributor Author

toivo commented Nov 29, 2023

No problem. The JED Checker extension chould be updated to report linebreaks and warn to extension developers. I will create a feature request asap.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42416.

@toivo
Copy link
Contributor Author

toivo commented Nov 29, 2023

JED Checker feature request: JED Checker to report linebreaks in language strings


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42416.

@BrainforgeUK
Copy link
Contributor

Changing the JED checker does not solve the problem!

In the meantime v4.4.1 is not useable!

Added \ to escape the new line does not appear to work.
What is the workaround for this?

Removing the newlines from a large .ini file is not acceptable.
Does my extension have to load the language file by itself?

Could we add an option to the extension manifest option to turn raw mode off?
Quick fix undo the change for 4.4.2 and then add this new manifest option for 4.4.3.
Will that satisfy the security issue?

@SniperSister
Copy link
Contributor

What is the workaround for this?

Removing the linebreak is the workaround.

Could we add an option to the extension manifest option to turn raw mode off?

No because, disabling the RAW mode for any installed extensions would re-introduce the vulnerability.

@brianteeman
Copy link
Contributor

it will take you less time to fix your language file than it took you to write your post

@brianteeman
Copy link
Contributor

Note that the documentation for language files states

Each line consists of a key-value pair separated by an equals sign
https://docs.joomla.org/Creating_a_language_definition_file

@BrainforgeUK
Copy link
Contributor

How to add a multi-line translateable hint for a textarea form field?

@SniperSister
Copy link
Contributor

That's a interesting usecase @BrainforgeUK! Are you using the hint in context of a JForm XML or is it a manually rendered HTML tag?

@brianteeman
Copy link
Contributor

brianteeman commented Nov 30, 2023

This works as a multiline hint in a text area
JMULTILINE="Line1\nLine2\nLine3"

image

@BrainforgeUK
Copy link
Contributor

JForm
I use Joomla standard form types everywhere - or derivatives of.
https://docs.joomla.org/Standard_form_field_types

@BrainforgeUK
Copy link
Contributor

So what is this security vector this change is fixing?

Would it not be better to replace parse_ini_file() with custom Joomla code in the language helper file?
At least the logic would then be in the Joomla world when dealing with any future issues.

A quick trawl of parse_ini_file() security issues (see below) mention some hosts disallow it.
How widespread is that?
Another reason to get stop using parse_ini_file()?

Banning newlines in language strings makes maintaining .ini language files with long descriptive text very difficult.
Consider the long strings one can put together, with embedded HTML, when using the note form field type.
https://docs.joomla.org/Note_form_field_type
Ok - one could put such text in the form XML using CDATA, but then that throws translatable notes out of the window.
Or even extend the note class ... but that is dealing with a language translation problem which should not be there.

https://stackoverflow.com/questions/35067087/is-the-php-function-parse-ini-file-really-so-dangerous

https://bugs.php.net/bug.php?id=34949

dweeves/magmi-git#349

https://www.webhostingtalk.com/showthread.php?t=867864

@brianteeman
Copy link
Contributor

Did you actually try the string I suggested? Worked perfectly for me as shown in the screenshot. If it didnt work for you please let me know

@kulbabskyy
Copy link

i have the same issue in the PayPlans component.
i have removed all \n strings but with no result

@BrainforgeUK
Copy link
Contributor

Did you actually try the string I suggested? Worked perfectly for me as shown in the screenshot. If it didnt work for you please let me know

... Thanks \n worked for the textarea I looked at - its the other long descriptive text with embedded HTML.
Stringing it all together without linebreaks makes maintenance difficult ... linewraps in the IDE helps but the text is still a big jumble.

@brianteeman
Copy link
Contributor

brianteeman commented Nov 30, 2023

if its embedded html then in that case you can use <br> as we do in core already

@brianteeman
Copy link
Contributor

@kulbabskyy

The only multiline string I can see in payplans is

COM_PAYPLANS_PLAN_GRID_CAN_NOT_DELETE_PLAN_SUBSCRIPTION_EXISTS="Deletion of a plan is possible only when you delete all corresponding subscriptions. Till then, you can only disable the selected plan. <br> <br>
If you are still unable to delete a plan then make sure you have set <strong>Delete Incomplete Orders</strong> under <strong>Backend Settings >> System</strong>, so this will delete the incomplete order on the cron job as per specified time set in the setting."

Change it to

COM_PAYPLANS_PLAN_GRID_CAN_NOT_DELETE_PLAN_SUBSCRIPTION_EXISTS="Deletion of a plan is possible only when you delete all corresponding subscriptions. Till then, you can only disable the selected plan. <br> <br>If you are still unable to delete a plan then make sure you have set <strong>Delete Incomplete Orders</strong> under <strong>Backend Settings >> System</strong>, so this will delete the incomplete order on the cron job as per specified time set in the setting."

@kulbabskyy
Copy link

kulbabskyy commented Nov 30, 2023

@brianteeman Thank you, it solved my problem!
i have submited your solution to the PayPlans developers

@BrainforgeUK
Copy link
Contributor

if its embedded html then in that case you can use <br> as we do in core already

Thanks, that does not solve my issue.
In a small way the COM_PAYPLANS_PLAN_GRID_CAN_NOT_DELETE_PLAN_SUBSCRIPTION_EXISTS example illustrates the problm.
Removing the newline works but having one long unformatted string to maintain is what I find unacceptable.
The newlines are not apparent on the website but easier to maintain in the IDE.

I see 2 possible solutions...

(a) Keep the newlines in the source - as seen in the IDE / stored on github.
Modify the tool I use which packages my extensions into zip files for distribution so it removes the newlines.

(b) Modify the Joomla language code so instead of calling parse_ini_file() it does this ...
Read the file into a string
Remove the embedded newlines (no other parsing, content changes)
Call parse_ini_string()
Generic solution for everybody.

Having the vision for (b) when I have the time I can submit a pull request for that and it can be argued over separately.
The JED rules would need to be changed back to allowing newlines.

@bembelimen
Copy link
Contributor

Pre-Parse every language file which is loaded will for sure have a performance impact.

(c) set up your IDE, that it auto wraps long lines (ALT + Z in Visual Studio Code or View => Word Wrap)

@ghost

This comment was marked as abuse.

@SniperSister
Copy link
Contributor

Thanks, Joomla, for giving me a very hearty laugh this week.

Happy to hear! :) Have a great weekend!

@jschmi102
Copy link

Great! Because of this problem, developers are motivated to work: checking, debugging, changing code of extensions, asking users for patience ....

@BrainforgeUK
Copy link
Contributor

Hearty laugh comment - I did wonder if this was all a waste of time and maybe even related to issue 42416 and rumours of hosts which for security issues block parse_ini_file() e.g. the links I added to an earlier comment.

Anyway I have put in a pull request which I hope might keep everyone happy (me in particular)!
#42441

@BrainforgeUK
Copy link
Contributor

Just been informed of this issue on a very old extension of mine someone is using on Joomla 4 and upgraded to 4.1.1

Told them to use language overrides, I don't want to look at that old code ... don't know what it might uncover. If it works on J4 just keep quiet ... J5/6 rediness?

Anyway it raises the point that unless this is fixed (such as my pull request or reverting to J4.1.0 behaviour) this issue will keep rearing its head for sometime to come - and from the initial comments it won't just be issues related my extensions.

@BrainforgeUK
Copy link
Contributor

Looks like Joomla documentation is wrong.
Also there is no need for Joomla language handling to use parse_ini_file().
See pull request #42441

@alikon
Copy link
Contributor

alikon commented Dec 3, 2023

closed in favour of #42441

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants