Bug - Corrupted CommonJS in js-cache

[faf]

Expected behavior

All JS working as intended.

Actual behavior

Syntax error making JS dysfunctional.

How to reproduce

Steps to reproduce the behavior:

1. Open any Znuny web interface page (either agent, or customer) in browser after upgrade from OTRS 6.0.30.
2. Open devtools console and see errors starting with Uncaught SyntaxError: missing ) after argument list in CommonJS.

Environment

• OS: Linux (ALT Linux P9 x86_64)
• Browser: Chromium (84.0.4147.105)
• Znuny 6.0.31

Additional information

The problem occurs because of CKEditor, namely because of this line. Seems like during the generation of combined js-file everything after the double slashes and before the line break is treated as comment and is ignored.

The quick solution is to manually fix ckeditor.js by escaping double slashes, so indexOf("file://") becomes indexOf("file:\/\/"). But I'm not sure whether it's the correct way.

[hanneshal]

Hi @faf

we update the CKEditor right up to 4.16.0 because they released a new security release, yesterday sigh https://ckeditor.com/cke4/release/CKEditor-4.16.0 A new release is on the way.

For further reference: We did miss this in our environment because we do not have the JS cache enabled, to make it easier for the development team, when changes to JS files are needed.

From now on we changed this on the QA stage.

Thank you for your help ❤️ Спасибо за вашу помощь :)

[ymyasoedov]

I'm almost sure that it's a bug in JavaScript::Minifier. More over ckeditor.js is already minified out of the box. There is no sense to do it twicely. The simplest bug fix would be to ignore the minification stage. Also, it's would be nice to report about this bug to upstream repository: https://github.com/zoffixznet/JavaScript-Minifier BTW, there is an alternative package JavaScript::Minifier::XS, that could be used instead of that one. It has stronger syntax analyzer.

[ymyasoedov]

Opened a bug for JavaScript::Minifier package https://github.com/zoffixznet/JavaScript-Minifier/issues/10.

[hanneshal]

Thank your for your help. We really appreciate this. The updated release is available now: https://www.znuny.org/releases/znuny-6-0-32 We had to bump the CKE also due to an security issue on their side.

[zoffixznet]

Opened a bug for JavaScript::Minifier package

Just a note: I inherited this codebase to be sort of a janitor. I've no idea how the code works, so unfortunately, can't provide the fix. Can only apply and upload one if someone is kind enough to provide it.

Sorry.

[faf]

Seems like it's possible to fix this particular issue in minifier, but it looks like there could be more problems there. Maybe one should switch to JavaScript::Minifier::XS or some other minifier instead.

[zoffixznet]

I've merged the fix faf submitted (thank you) and pushed updated module to CPAN.

switch to JavaScript::Minifier::XS or some other minifier instead.

That's my recommendation as well. I believe JavaScript::Minifier::XS or something similar is used as an optional dependency by Mojolicious::Plugin::Webpack and Mojolicious::Plugin::AssetPack. I've used Mojolicious::Plugin::AssetPack with a ton of JS files and never had any issues, so whatever it uses under the hood to minify should be fairly solid.

[jepf]

Thank you all very much for your support.

Both solutions work fine for us (JavaScript::Minifier 1.15 and JavaScript::Minifier::XS).

The next release of Znuny will contain the updated version 1.15 of JavaScript::Minifier and will optionally automatically utilize JavaScript::Minifier::XS instead if it is installed.

Same will be the case btw. for CSS::Minifier and CSS::Minifier::XS.