sqlite: reject statement-less SQLTagStore queries

[lazerg]

Fixes: #65149

A query that holds no statement, such as sql.run`-- comment`, segfaults every SQLTagStore method. sqlite3_prepare_v2() returns SQLITE_OK with a null sqlite3_stmt* for input that is only a comment or whitespace, and ResetAndBindStatement() passes that null pointer straight to sqlite3_clear_bindings().

StatementSync already models a null statement as finalized, so this checks for it and throws ERR_INVALID_STATE, the same error db.prepare('-- comment').run() gives today.

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (a876a12) to head (3ff24e1).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65152 +/- ##
=======================================
Coverage 90.32% 90.32% =======================================
Files 759 759 Lines 248325 248345 +20 Branches 46861 46875 +14 =======================================
+ Hits 224303 224322 +19 + Misses 15467 15462 -5 - Partials 8555 8561 +6 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	81.39% <100.00%> (+0.16%)	⬆️

... and 31 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Renegade334]

#65157 is the correct approach here, there's no point in allowing a null statement to be constructed and added to the tag store cache in the first place. We should reject these at the point of preparation.

That being said, this should also be the case for db.prepare()

[Renegade334]

Closing in favour of #65157.

I would recommend not opening PRs within seconds of an issue being opened. There is often discussion to take place, even if the issue seems straightforward as in this case.

[lazerg]

Fixes: #65149

A query that holds no statement, such as sql.run`-- comment`, segfaults every SQLTagStore method. sqlite3_prepare_v2() returns SQLITE_OK with a null sqlite3_stmt* for input that is only a comment or whitespace, and ResetAndBindStatement() passes that null pointer straight to sqlite3_clear_bindings().

StatementSync already models a null statement as finalized, so this checks for it and throws ERR_INVALID_STATE, the same error db.prepare('-- comment').run() gives today.

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (a876a12) to head (3ff24e1).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65152 +/- ##
=======================================
Coverage 90.32% 90.32% =======================================
Files 759 759 Lines 248325 248345 +20 Branches 46861 46875 +14 =======================================
+ Hits 224303 224322 +19 + Misses 15467 15462 -5 - Partials 8555 8561 +6 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	81.39% <100.00%> (+0.16%)	⬆️

... and 31 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Renegade334]

#65157 is the correct approach here, there's no point in allowing a null statement to be constructed and added to the tag store cache in the first place. We should reject these at the point of preparation.

That being said, this should also be the case for db.prepare()

[Renegade334]

Closing in favour of #65157.

I would recommend not opening PRs within seconds of an issue being opened. There is often discussion to take place, even if the issue seems straightforward as in this case.

[lazerg]

Fixes: #65149

A query that holds no statement, such as sql.run`-- comment`, segfaults every SQLTagStore method. sqlite3_prepare_v2() returns SQLITE_OK with a null sqlite3_stmt* for input that is only a comment or whitespace, and ResetAndBindStatement() passes that null pointer straight to sqlite3_clear_bindings().

StatementSync already models a null statement as finalized, so this checks for it and throws ERR_INVALID_STATE, the same error db.prepare('-- comment').run() gives today.

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (a876a12) to head (3ff24e1).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65152 +/- ##
=======================================
Coverage 90.32% 90.32% =======================================
Files 759 759 Lines 248325 248345 +20 Branches 46861 46875 +14 =======================================
+ Hits 224303 224322 +19 + Misses 15467 15462 -5 - Partials 8555 8561 +6 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	81.39% <100.00%> (+0.16%)	⬆️

... and 31 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Renegade334]

#65157 is the correct approach here, there's no point in allowing a null statement to be constructed and added to the tag store cache in the first place. We should reject these at the point of preparation.

That being said, this should also be the case for db.prepare()

[Renegade334]

Closing in favour of #65157.

I would recommend not opening PRs within seconds of an issue being opened. There is often discussion to take place, even if the issue seems straightforward as in this case.

[lazerg]

Fixes: #65149

A query that holds no statement, such as sql.run`-- comment`, segfaults every SQLTagStore method. sqlite3_prepare_v2() returns SQLITE_OK with a null sqlite3_stmt* for input that is only a comment or whitespace, and ResetAndBindStatement() passes that null pointer straight to sqlite3_clear_bindings().

StatementSync already models a null statement as finalized, so this checks for it and throws ERR_INVALID_STATE, the same error db.prepare('-- comment').run() gives today.

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (a876a12) to head (3ff24e1).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65152 +/- ##
=======================================
Coverage 90.32% 90.32% =======================================
Files 759 759 Lines 248325 248345 +20 Branches 46861 46875 +14 =======================================
+ Hits 224303 224322 +19 + Misses 15467 15462 -5 - Partials 8555 8561 +6 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	81.39% <100.00%> (+0.16%)	⬆️

... and 31 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Renegade334]

#65157 is the correct approach here, there's no point in allowing a null statement to be constructed and added to the tag store cache in the first place. We should reject these at the point of preparation.

That being said, this should also be the case for db.prepare()

[Renegade334]

Closing in favour of #65157.

I would recommend not opening PRs within seconds of an issue being opened. There is often discussion to take place, even if the issue seems straightforward as in this case.

[lazerg]

Fixes: #65149

A query that holds no statement, such as sql.run`-- comment`, segfaults every SQLTagStore method. sqlite3_prepare_v2() returns SQLITE_OK with a null sqlite3_stmt* for input that is only a comment or whitespace, and ResetAndBindStatement() passes that null pointer straight to sqlite3_clear_bindings().

StatementSync already models a null statement as finalized, so this checks for it and throws ERR_INVALID_STATE, the same error db.prepare('-- comment').run() gives today.

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (a876a12) to head (3ff24e1).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65152 +/- ##
=======================================
Coverage 90.32% 90.32% =======================================
Files 759 759 Lines 248325 248345 +20 Branches 46861 46875 +14 =======================================
+ Hits 224303 224322 +19 + Misses 15467 15462 -5 - Partials 8555 8561 +6 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	81.39% <100.00%> (+0.16%)	⬆️

... and 31 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Renegade334]

#65157 is the correct approach here, there's no point in allowing a null statement to be constructed and added to the tag store cache in the first place. We should reject these at the point of preparation.

That being said, this should also be the case for db.prepare()

[Renegade334]

Closing in favour of #65157.

I would recommend not opening PRs within seconds of an issue being opened. There is often discussion to take place, even if the issue seems straightforward as in this case.

[lazerg]

Fixes: #65149

A query that holds no statement, such as sql.run`-- comment`, segfaults every SQLTagStore method. sqlite3_prepare_v2() returns SQLITE_OK with a null sqlite3_stmt* for input that is only a comment or whitespace, and ResetAndBindStatement() passes that null pointer straight to sqlite3_clear_bindings().

StatementSync already models a null statement as finalized, so this checks for it and throws ERR_INVALID_STATE, the same error db.prepare('-- comment').run() gives today.

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (a876a12) to head (3ff24e1).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65152 +/- ##
=======================================
Coverage 90.32% 90.32% =======================================
Files 759 759 Lines 248325 248345 +20 Branches 46861 46875 +14 =======================================
+ Hits 224303 224322 +19 + Misses 15467 15462 -5 - Partials 8555 8561 +6 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	81.39% <100.00%> (+0.16%)	⬆️

... and 31 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Renegade334]

#65157 is the correct approach here, there's no point in allowing a null statement to be constructed and added to the tag store cache in the first place. We should reject these at the point of preparation.

That being said, this should also be the case for db.prepare()

[Renegade334]

Closing in favour of #65157.

I would recommend not opening PRs within seconds of an issue being opened. There is often discussion to take place, even if the issue seems straightforward as in this case.

[lazerg]

Fixes: #65149

A query that holds no statement, such as sql.run`-- comment`, segfaults every SQLTagStore method. sqlite3_prepare_v2() returns SQLITE_OK with a null sqlite3_stmt* for input that is only a comment or whitespace, and ResetAndBindStatement() passes that null pointer straight to sqlite3_clear_bindings().

StatementSync already models a null statement as finalized, so this checks for it and throws ERR_INVALID_STATE, the same error db.prepare('-- comment').run() gives today.

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (a876a12) to head (3ff24e1).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65152 +/- ##
=======================================
Coverage 90.32% 90.32% =======================================
Files 759 759 Lines 248325 248345 +20 Branches 46861 46875 +14 =======================================
+ Hits 224303 224322 +19 + Misses 15467 15462 -5 - Partials 8555 8561 +6 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	81.39% <100.00%> (+0.16%)	⬆️

... and 31 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Renegade334]

#65157 is the correct approach here, there's no point in allowing a null statement to be constructed and added to the tag store cache in the first place. We should reject these at the point of preparation.

That being said, this should also be the case for db.prepare()

[Renegade334]

Closing in favour of #65157.

I would recommend not opening PRs within seconds of an issue being opened. There is often discussion to take place, even if the issue seems straightforward as in this case.

[lazerg]

Fixes: #65149

A query that holds no statement, such as sql.run`-- comment`, segfaults every SQLTagStore method. sqlite3_prepare_v2() returns SQLITE_OK with a null sqlite3_stmt* for input that is only a comment or whitespace, and ResetAndBindStatement() passes that null pointer straight to sqlite3_clear_bindings().

StatementSync already models a null statement as finalized, so this checks for it and throws ERR_INVALID_STATE, the same error db.prepare('-- comment').run() gives today.

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (a876a12) to head (3ff24e1).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65152 +/- ##
=======================================
Coverage 90.32% 90.32% =======================================
Files 759 759 Lines 248325 248345 +20 Branches 46861 46875 +14 =======================================
+ Hits 224303 224322 +19 + Misses 15467 15462 -5 - Partials 8555 8561 +6 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	81.39% <100.00%> (+0.16%)	⬆️

... and 31 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Renegade334]

#65157 is the correct approach here, there's no point in allowing a null statement to be constructed and added to the tag store cache in the first place. We should reject these at the point of preparation.

That being said, this should also be the case for db.prepare()

[Renegade334]

Closing in favour of #65157.

I would recommend not opening PRs within seconds of an issue being opened. There is often discussion to take place, even if the issue seems straightforward as in this case.