Bug 38523 - Unexpected characters in ILL standard form JS can break translations
Summary: Unexpected characters in ILL standard form JS can break translations
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 36221 38340
Blocks:
  Show dependency treegraph
 
Reported: 2024-11-23 03:28 UTC by Victor Grousset/tuxayo
Modified: 2025-08-18 00:45 UTC (History)
4 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Victor Grousset/tuxayo 2024-11-23 03:28:08 UTC
Follow-up of Bug 36221.

In Koha/ILL/Backend/shared-includes/shared.js

'" name="custom_key" placeholder="'+__('key')+'">' +
' ' +
'<input type="text" id="custom-value" name="custom_value" class="' +
( opac ? 'form-control input-fluid custom-field-input' : '') +'" placeholder="'+__('value')+'"> ' +
'<button type="button" class="btn btn-danger btn-sm ' +
'delete-new-field">' +
'<span class="fa fa-trash-can"></span> ' +
__('Delete') +
[...]
$('#custom-warning').text(__("The name '%s' is not permitted").format(val)).show();

-------

This will need to be tested once bug 38340 has patches.
For now stuff is not translatable but once it is, it's expected the JS can break if the translated string contains a simple quote.

-------

xt/single_quotes.t doesn't catch it because the test only searches in
koha-tmpl/opac-tmpl/bootstrap/en/
koha-tmpl/intranet-tmpl/prog/en/
Comment 1 David Cook 2025-08-17 23:58:34 UTC
It looks like this has been moved into these files:
./koha-tmpl/opac-tmpl/bootstrap/en/includes/ill/backends/Standard/shared/shared.js
./koha-tmpl/intranet-tmpl/prog/en/includes/ill/backends/Standard/shared/shared.js

And one day we really need to forbid creating HTML from Javascript strings.
Comment 2 David Cook 2025-08-18 00:05:16 UTC
That said, what's the expected problem here?

A single quote shouldn't be an issue since the HTML is encased in double quotes, but a double quote could be since that would create invalid HTML. 

Unless the problem is with __() specifically? It's been a little while since I looked at that specifically.
Comment 3 David Cook 2025-08-18 00:39:12 UTC
Admittedly, I'm not super familiar with the entire translation process in Koha start to finish. 

Here's some steps I took:
sudo apt-get install koha-l10n --reinstall
./debian/scripts/koha-translate -i fr-FR -d kohadev
vi koha-tmpl/intranet-tmpl/prog/fr-FR/js/locale_data.js

The "Delete" translation probably wouldn't cause syntax errors, but it could be used to inject XSS, although in theory the Translation Manager should catch that?

It looks like "key" and "value" aren't currently translated... but I can test it anyway by manually editing locale_data.js. We'll inject this string into locale_data.js:

"key":[null,"\"cle\""],

After we install fr-FR, change to French, enable ILLModule, and go to http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard we click on "Nouvelle demande de PEB", and we click on "Ajouter un nouveau champ".

Et voilà... no placeholder text because we've accidentally generated 
<input type="text" class="custom-name " name="custom_key" placeholder="" cle""="">
Comment 4 David Cook 2025-08-18 00:44:02 UTC
Now this is easy to avoid by using Javascript like document.createElement() to create the HTML Element Javascript object. 

Then to set the placeholder we can just do this:

custom_name.placeholder = __("key");

By using the browser web APIs and avoiding Javascript string manipulation, we set the placeholder more securely and more robustly. 

--

We also don't even need jQuery to do this. Javascript has advanced so much that this can easily be done in regular Javascript, which is easier to maintain in the long-run as well.