refactor: improve code formatting and add escapeRegex method for safer regex usage#1575
refactor: improve code formatting and add escapeRegex method for safer regex usage#1575
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the MongoDB data access object formatting and introduces an escapeRegex helper to sanitize user-provided search inputs before constructing RegExp queries, aiming to make regex-based searching safer.
Changes:
- Reformatted
data-access-object-mongodb.ts(indentation/structure). - Added
escapeRegex()helper and applied it to autocomplete/search/filter regex construction. - Updated several
RegExp(...)call sites to use escaped input.
Comments suppressed due to low confidence (1)
shared-code/src/data-access-layer/data-access-objects/data-access-object-mongodb.ts:656
- The doc comment on
escapeRegexsays it prevents ReDoS attacks. Escaping metacharacters primarily prevents regex injection and avoids invalid patterns; it doesn’t fully address ReDoS risks in general. Consider adjusting the comment to describe the actual protection being provided (escaping user input so it’s treated as a literal match).
private escapeRegex(str: string): string {
return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const query = fields.reduce((acc, field) => { | ||
| acc[field] = new RegExp(this.escapeRegex(String(value)), 'i'); | ||
| return acc; | ||
| }, {}); |
There was a problem hiding this comment.
autocompleteFields.value appears to use '*' as a special “match all” value in other DAOs (e.g., SQL-based implementations skip adding LIKE conditions when value is '*'). Here, escaping turns '*' into a literal match, so autocomplete will likely return no rows instead of “all”, and it no longer surfaces the previous invalid-regexp error. Consider handling value === '*' explicitly by skipping regex conditions (empty query) to match the behavior of the other data access objects.
| const query = fields.reduce((acc, field) => { | |
| acc[field] = new RegExp(this.escapeRegex(String(value)), 'i'); | |
| return acc; | |
| }, {}); | |
| // In other DAOs, autocomplete `value === '*'` is treated as "match all" | |
| // by skipping LIKE/regex conditions. Mirror that behavior here by using | |
| // an empty MongoDB query ({}) instead of a literal '*' regex match. | |
| let query: Record<string, RegExp> | {} = {}; | |
| if (value !== '*') { | |
| query = fields.reduce((acc: Record<string, RegExp>, field) => { | |
| acc[field] = new RegExp(this.escapeRegex(String(value)), 'i'); | |
| return acc; | |
| }, {} as Record<string, RegExp>); | |
| } |
| private createObjectIdFromSting(id: string): ObjectId { | ||
| try { | ||
| return new ObjectId(id); |
There was a problem hiding this comment.
The private helper is named createObjectIdFromSting (typo). Since it’s private to this class, renaming it to createObjectIdFromString and updating call sites in this file would improve readability and reduce confusion.
No description provided.