Improves SQL parameter handling and query safety#117
Improves SQL parameter handling and query safety#117precious-ndamati-stream4Tech wants to merge 1 commit intomainfrom
Conversation
Refactors parameter processing for better handling of nested names, null values, and case-insensitive logic. Enhances robustness for IN/BETWEEN operators and prevents unsafe replacements in queries. Updates package version for release.
precious-ndamati-stream4Tech
requested review from
durlabhjain and
yenduttjain
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the addParameters method in lib/sql.js to improve code clarity, maintainability, and safety. The version number is bumped from 1.0.48 to 1.0.49.
Key changes:
- Refactored the
addParametersmethod for better readability with clearer variable names and improved code structure - Enhanced safety with checks to prevent infinite replacement loops
- Consolidated conditional logic for better maintainability
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| package.json | Version bump from 1.0.48 to 1.0.49 |
| lib/sql.js | Refactored addParameters method with improved variable naming, consolidated logic, and added safety checks for string replacement |
| const placeholder = `${this.parameterPrefix}{${shortName}}`; | ||
| const replacement = inResult.paramNames.join(', '); | ||
|
|
||
| if (typeof query === 'string' && !replacement.includes(placeholder) && !isBetween) { |
There was a problem hiding this comment.
The check '!replacement.includes(placeholder)' may not prevent all infinite replacement scenarios. If the placeholder appears in any of the generated parameter names (e.g., shortName contains '{shortName}'), this could still cause issues. Consider a more robust check or document this edge case.
| if (props !== undefined && props !== null && typeof props === 'object') { | ||
|
|
||
| // Extract object-style properties | ||
| if (props && typeof props === 'object' && props !== null) { |
There was a problem hiding this comment.
Variable 'props' is of type date, object or regular expression, but it is compared to an expression of type null.
| if (props && typeof props === 'object' && props !== null) { | |
| if (props && typeof props === 'object' && props.constructor === Object) { |
1 participant
Refactors parameter processing for better handling of nested names, null values, and case-insensitive logic. Enhances robustness for IN/BETWEEN operators and prevents unsafe replacements in queries. Updates package version for release.