Script broken by minification (mistaken regex segment for comment)

  • It doesn’t happen when I try the affected code snippet directly on https://www.minifier.org/ but in the context of our site’s js, the resultant file gets one line prematurely truncated because the second regex is being mistaken for a comment, even though it’s perfectly legal JS:

    label = input.val().replace(/\\/g, '/').replace(/.*\//, '');

    (the minified line ended in .replace(/.*\)

    I don’t know if the problem is upstream with the PHP minifier or something your plugin is doing, but it broke our site because all JS execution on the concatenated file stopped.

    The work-around was to disable JS minification (but I still allowed concatenation).

    As a side note, I always disable CSS minification as it seems to drop fallbacks like when display: block; and display: flex; exist in the same ruleset. I’d disable it by default if I were you as it’s not obvious in a modern browser that older browsers have lost support.

Viewing 15 replies - 1 through 15 (of 16 total)
  • Plugin Author launchinteractive

    (@launchinteractive)

    Hi Roy,

    Does the MMR logs say that the minifier is trying to use Closure or PHP Minify?

    Thread Starter Roy Orbitson

    (@lev0)

    It was just minify, I believe.

    Sorry, I cannot assist further as it was happening on a production site so I just had to remove the plugin.

    Plugin Author launchinteractive

    (@launchinteractive)

    Ok, well I’d say it was PHP minify that was having the issue. I’ve found closure to perform better and have less issues but thats not always the case. I’ll close this now as its no longer being used.

    Thread Starter Roy Orbitson

    (@lev0)

    I activated the plugin again and it happened again. Redacted log:

    2018-07-23T06:36:58+00:00 – COMPRESSING WITH MINIFY (PHP exec not available or java not found)
    2018-07-23T06:36:58+00:00 – COMPRESSION COMPLETE – 27.83K saved

    If I put the same code into https://compressorrater.thruhere.net/ (only JSMin checked) or other online minifiers, it’s fine. It looks like something else in your code is naively stripping single line comments from the minified files, which fails when it runs into a regexp literal that ends with a /.

    Plugin Author launchinteractive

    (@launchinteractive)

    Nice find. Are you able to supply your regex so that I can fix and test this?

    Thread Starter Roy Orbitson

    (@lev0)

    I mistakenly thought you were using JSMin.

    It didn’t break on current https://www.minifier.org/ either. Looks like your packaged version is 7 Months old, therefore probably missing the fix for this issue and many others.

    Thread Starter Roy Orbitson

    (@lev0)

    Similar issue.

    Plugin Author launchinteractive

    (@launchinteractive)

    Hey Roy, I’ve just updated Minify and Closure. Hopefully this will fix your issue. Please let me know so I can mark this as resolved.

    Thread Starter Roy Orbitson

    (@lev0)

    The minified code is now fine, thank you. Only problem is that the corrected resulting files have the same names, so they haven’t busted cache. Could you add your plugin version to the hash() input?

    Thread Starter Roy Orbitson

    (@lev0)

    Also, Adler himself said Adler 32’s not a hash function. You could use SHA1 but put the modified date in the input so it doesn’t further increase the length of the filename (better privacy too).

    • This reply was modified 6 years, 8 months ago by Roy Orbitson.
    Thread Starter Roy Orbitson

    (@lev0)

    I don’t mean to hassle you, I’d just send pull requests if it was on GitHub or something. It’s a great plugin.

    Thread Starter Roy Orbitson

    (@lev0)

    Does remove_cssjs_ver() remove that param from external and/or ignored URLs too?

    Plugin Author launchinteractive

    (@launchinteractive)

    I’m not sure adding the version to the hash is a good idea because it will mean every update of MMR will force a cache flush which isn’t necessary most of the time.

    Thats interesting about Adler. The reason we used it is purely for speed. I’ve just done this speed test on my machine: https://php.net/manual/en/function.hash.php#89574 and md4 wins out.. maybe we should go with md4. What do you think?

    Yes, remove_cssjs_ver() removes params from external urls.

    Thread Starter Roy Orbitson

    (@lev0)

    Perhaps add the version of the bundled minifier instead. If *that* changes, which probably means output changes, then the cache is busted.

    I’m sure Adler is quick, it’s just more prone to collisions, which would be a site-breaking situation, I doubt the difference between calculating a few md4s and the same number of sha1s will make a noticeable difference.

    That’s not ideal, you never know what external scripts will be added. Couldn’t it be left in, and ignored when reading the minifiable files? The versioned URLs get dequeued anyway, right?

    Thread Starter Roy Orbitson

    (@lev0)

    There may be some better hashes like Murmur or CityHash, but they’re not built into PHP.

Viewing 15 replies - 1 through 15 (of 16 total)
  • The topic ‘Script broken by minification (mistaken regex segment for comment)’ is closed to new replies.