Skip to content

Fix regex detection when a character class contains unescaped forward slash #148

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

Merged
merged 1 commit into from
Jul 28, 2025

Conversation

bubasuma
Copy link
Contributor

@bubasuma bubasuma commented Jul 14, 2025

Description

The current logic is failing to properly detect the following regular expression (see):

return string.replace( /([\\!"#$%&'()*+,./:;<=>?@\[\]^`{|}~])/g, "\\$1" );

resulting in

.....
return this.errors().filter(selector);},escapeCssMeta:function(string){if(string===undefined){return"";}
return string.replace(/([\\!"#$%&'()*+,./:;<=>?@\[\]^`{|}~])/g, "\\$1" );
            },

            idOrName: function( element ) {
                return this.groups[ element.name ] || ( this.checkable( element ) ? element.name : element.id || element.name );
            },

            validationTargetFor: function( element ) {

                // If radio/checkbox, validate first element in group instead
                if ( this.checkable( element ) ) {
                    element = this.findByName( element.name );
                }

                // Always apply ignore filter
                return $( element ).not( this.settings.ignore )[ 0 ];
            },

            checkable: function( element ) {
                return ( /radio|checkbox/i ).test( element.type );
            },
...

Root Cause

This is because the parser is stopping at the first unescaped forward slash which is located in the character class. However a character class can contain unescaped forward slash (see) and is a valid regular expression in Javascript.
Next it's parsing the rest of the regular expression as String starting from the string delimiter ` resulting in the next few lines not being minimized.

Solution

Added logic to ignore unescaped forward slash inside a character class

Issues

Related Pull Requests

Comment

I understand there had been an attempt to fix this issue but was reverted later due to regression issues caused by the fix. As I understand the regression issues were found in Magento according to this issue. I tested the solution with magento when JS minification is enabled and it worked fine.

 bin/magento config:set dev/js/minify_files 1
 bin/magento cache:flush
 bin/magento deploy:mode:set production

The same sequence of commands failed with the previously proposed solution.

@bubasuma
Copy link
Contributor Author

Hi @tedivm

Could you please review this fix? I believe it resolves the issue. Thank you for maintaining this library!

@bubasuma
Copy link
Contributor Author

Hi @tedivm,
Could you please take a look at this fix when you have a moment? Thank you!

@tedivm tedivm merged commit 29ee510 into tedious:master Jul 28, 2025
5 checks passed
@tedivm
Copy link
Member

tedivm commented Jul 28, 2025

Thanks!

@bubasuma
Copy link
Contributor Author

Thank You, @tedivm. I appreciate your prompt attention and support.

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

Successfully merging this pull request may close these issues.

jquery/jquery.validate.min.js file isn't getting minified properly.
2 participants