Skip to content

sqlite: throw on oversized string values - #66209

Open
araujogui wants to merge 1 commit into
nodejs:mainfrom
araujogui:sqlite-throw-on-oversized-strings
Open

araujogui wants to merge 1 commit into
nodejs:mainfrom
araujogui:sqlite-throw-on-oversized-strings

Conversation

@araujogui

Copy link
Copy Markdown
Member

SQLite serves TEXT up to SQLITE_MAX_LENGTH (1e9 by default), past what
V8 can represent as a string. String::NewFromUtf8() returns an empty
handle without throwing, and the user-defined function path then called
SetIgnoreNextSQLiteError(true) as if a JavaScript exception were
pending, so the SQLite error was discarded too.

db.exec('INSERT INTO t VALUES (myfn(hex(zeroblob(300000000))))');
// returned normally; myfn never ran; nothing was inserted

db.prepare('SELECT hex(zeroblob(300000000))').get();
// undefined - indistinguishable from "no row"

Utf8StringMaybeOneByte() now throws ERR_STRING_TOO_LONG when the
value exceeds String::kMaxLength, which covers both the UDF arguments
and the column reads that share it. The suppression in
THROW_ERR_SQLITE_ERROR() is gated on an actually pending exception so
it can no longer turn a failed statement into a success.

SQLite serves TEXT up to SQLITE_MAX_LENGTH, past what V8 can represent
as a string. V8 returns an empty handle without throwing, and the
user-defined function path then suppressed the SQLite error as if a
JavaScript exception were pending. exec() reported success for a
statement that never ran, and get() returned undefined.

Throw ERR_STRING_TOO_LONG when the value cannot be converted, and
suppress a SQLite error only when an exception is actually pending.

Assisted-by: Claude Code
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite

Copilot AI lite review requested due to automatic review settings September 22, 2026 16:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. sqlite Issues and PRs related to the SQLite subsystem. labels Sep 22, 2026
@codecov

codecov Bot commented Sep 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.27%. Comparing base (bfb84d3) to head (1a27d49).
⚠️ Report is 31 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66209      +/-   ##
==========================================
- Coverage   90.28%   90.27%   -0.01%     
==========================================
  Files         790      790              
  Lines      272043   272888     +845     
  Branches    51938    52108     +170     
==========================================
+ Hits       245611   246356     +745     
- Misses      16930    16989      +59     
- Partials     9502     9543      +41     
Files with missing lines Coverage Δ
src/node_sqlite.cc 82.02% <100.00%> (-0.10%) ⬇️

... and 40 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. sqlite Issues and PRs related to the SQLite subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants