Skip to content

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

Merged
tedivm merged 1 commit into
tedious:masterfrom
bubasuma:fix_regex_detection
Jul 28, 2025
Merged

Fix regex detection when a character class contains unescaped forward slash#148
tedivm merged 1 commit into
tedious:masterfrom
bubasuma:fix_regex_detection

Conversation

@bubasuma

@bubasuma bubasuma commented Jul 14, 2025

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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

tedivm commented Jul 28, 2025

Copy link
Copy Markdown
Member

Thanks!

@bubasuma

Copy link
Copy Markdown
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