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

Move this command out from translation #173

Closed
martonmiklos opened this issue Mar 20, 2023 · 6 comments
Closed

Move this command out from translation #173

martonmiklos opened this issue Mar 20, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@martonmiklos
Copy link
Contributor

Hi folks,

I came across this string during translation:

//% "for i in $(ssu lr | fgrep chum | cut -f 3 -d ' '); do ssu rr $i; done\n"

I would recommend to make the command untranslatable by adding a placeholder to it.
For security reasons and to ease the translations.
What do you think?

@Olf0 Olf0 added the enhancement New feature or request label Mar 21, 2023
@Olf0
Copy link
Collaborator

Olf0 commented Mar 21, 2023

Yes, I also wondered once, if there is better way to handle this code line embedded in a text string.

For security reasons and to ease the translations.

Can you please explain this a bit, specifically the security aspect. I perceived it just as a small nuisance ("a bit ugly"), hence I did not mind to research and address this.

I would recommend to make the command untranslatable by adding a placeholder to it.

I know there is a notranslate tag, but have no idea, if this is the right thing to use and how.

As always, PRs are welcome, then I might also understand better what solution you think of and how that looks like in practice.
Side note: I am merely little experienced with Qt's l10n infrastructure.

@martonmiklos
Copy link
Contributor Author

martonmiklos commented Mar 21, 2023

Can you please explain this a bit, specifically the security aspect.

Well if I 'mis-translate' this string to end with an rm -rf / there could be an issue if someone copies it over and execute it.

I know there is a notranslate tag, but have no idea, if this is the right thing to use and how.

I am not an expert on the localization, but I would use the qsTr somehow like this:

qsTr("The SailfishOS:Chum GUI application failed to manage the SailfishOS:Chum repository! "
                    "You probably have multiple SailfishOS:Chum repositories defined for SSU or disabled a "
                    "SailfishOS:Chum repository.\n\n"
                    "Please remove all SailfishOS:Chum repositories by executing this command line as root user:\n"
                    "%s\n"
                    "This SailfishOS:Chum GUI application will add any missing SailfishOS:Chum repository when started again.").arg("for i in $(ssu lr | fgrep chum | cut -f 3 -d ' '); do ssu rr $i; done");

I do no have SFOS SDK at hand at the moment, but I will submit a PR once get back to my dev box.

@Olf0
Copy link
Collaborator

Olf0 commented Apr 6, 2023

Can you please explain this a bit, specifically the security aspect.

Well, if one "mis-translates" this string to end with an rm -rf / there could be an issue if someone copies it over and executes it.

So this is about safety (not security), nevertheless the reasoning is valid. 👍

@Olf0
Copy link
Collaborator

Olf0 commented Oct 15, 2023

@martonmiklos, as your idea for rendering the shell code as immutable for translators is not ID-based, your implementation suggestion has to be transformed to suite ID-based, translatable Qt strings, first.

Olf0 added a commit that referenced this issue Oct 15, 2023
@Olf0
Copy link
Collaborator

Olf0 commented Oct 15, 2023

Should be fixed by PR #215.

Olf0 added a commit that referenced this issue Oct 15, 2023
* [chum.cpp] Fix issue #173

* [sailfishos-chum-gui.ts] Adapting TS file to altered source string
@Olf0
Copy link
Collaborator

Olf0 commented Nov 10, 2023

PR #215 seems to work fine, but by looking at this text I realise to have never seen it before. Thus I played bit with it and lastly created PR #253.

Before

Screenshot_20231110-1-v Screenshot_20231110-2-v

Screenshot_20231110-2-h
Screenshot_20231110-1-h

After

<Screenshots are missing, as one wold have to misconfigure SSU's SailfishOS:Chum repository entry for that. If someone wants to provide corresponding screenshots, please post them here.>

@Olf0 Olf0 closed this as completed Dec 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants