Skip to content
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.

Sign up for GitHub

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

Fix: Add validation for invalid date query parameter values #3330

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix: Add validation for invalid date query parameter values #3330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7cc345d
Fix: Add validation for invalid date query parameter values
18rameez Oct 12, 2024
497ada0
fix: resolve linting issues
18rameez Oct 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/pg/lib/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ var prepareValue = function (val, seen) {
return buf.slice(val.byteOffset, val.byteOffset + val.byteLength) // Node.js v4 does not support those Buffer.from params
}
if (val instanceof Date) {
if (isNaN(val.getTime())) {
throw new Error('Query parameter value cannot be an invalid date.')
}
if (defaults.parseInputDatesAsUTC) {
return dateToStringUTC(val)
} else {
Expand Down
15 changes: 15 additions & 0 deletions packages/pg/test/integration/client/error-handling-tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,3 +257,18 @@ suite.test('cannot pass non-string values to query as text', (done) => {
})
})
})

if (!helper.args.native) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens in the 'native' case?

suite.test('when a query has an invalid date binding', function (done) {
var client = createErorrClient()
var calledDone = false

client.query(new pg.Query({ text: 'SELECT $1::timestamp', values: [new Date(undefined)] }), function (err, res) {
if (!calledDone) {
calledDone = true
assert.equal(err.message, 'Query parameter value cannot be an invalid date.')
client.end(done)
}
})
Comment on lines +264 to +272
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The query callback can only be called once (and if it weren’t, we wouldn’t want to silently ignore that):

Suggested change
var calledDone = false
client.query(new pg.Query({ text: 'SELECT $1::timestamp', values: [new Date(undefined)] }), function (err, res) {
if (!calledDone) {
calledDone = true
assert.equal(err.message, 'Query parameter value cannot be an invalid date.')
client.end(done)
}
})
client.query(new pg.Query({ text: 'SELECT $1::timestamp', values: [new Date(undefined)] }), function (err, res) {
assert.equal(err.message, 'Query parameter value cannot be an invalid date.')
client.end(done)
})

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will cause this error, as the connection also gets terminated, and the callback will be executed again.

Message: 'Connection terminated' == 'Query parameter value cannot be an invalid date.'
AssertionError [ERR_ASSERTION]: 'Connection terminated' == 'Query parameter value cannot be an invalid date.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s a serious bug, then.

})
}