sqlite: improve error for excess bound parameters - #65167

Closed
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:sqlite/excess-bind-params
Closed

sqlite: improve error for excess bound parameters#65167
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:sqlite/excess-bind-params

Conversation

@bitpshr

Copy link
Copy Markdown
Contributor

Fixes#65163. Passing more values than a prepared statement has parameters surfaced SQLite's own column index out of range error, which reads as a problem with the table's columns rather than with the call, and is inconsistent with the adjacent failure (a wrong-type value already throws a Node-authored error from the same function).

This detects the overflow before handing the index to sqlite3_bind_*(). The check sits after the scan for the next anonymous slot, so it also covers excess anonymous values passed alongside named parameters.

before: ERR_SQLITE_ERROR column index out of range
after: ERR_INVALID_ARG_VALUE Too many parameter values were provided. The statement accepts 1 parameter(s), which are already bound.

Three notes for reviewers, since I deviated from the issue's suggestions:

  • This needs semver-major treatment.test-sqlite-statement-sync.js had a test asserting the old ERR_SQLITE_ERROR / errcode: 25, so the change is a deliberate behavior change. That test is updated here and renamed, since the error no longer comes from SQLite.
  • ERR_INVALID_ARG_COUNT does not exist in node_errors.h or lib/internal/errors.js, so I used ERR_INVALID_ARG_VALUE. It is already a TypeError and is what the sibling binding failure a few lines away uses, which keeps the two consistent.
  • The message reports only the parameter count. I first included a "but N were provided" count as suggested, but when named and anonymous parameters are mixed that number counts only the anonymous values, so run({ $k: 1 }, 2) would have read "accepts 1, but 1 were provided". The current wording is accurate in every case.

Verified locally against a build: the full sqlite suite passes (18/18), and correct arity still binds normally. Happy to change the error code, the wording, or drop this entirely if you'd rather leave the behavior alone.

Fixes: #65163

Passing more values than a prepared statement has parameters surfaced
SQLite's own "column index out of range" error. That wording describes a
binding index, but reads to a JavaScript caller as a problem with the
table's columns rather than with the call, and it is inconsistent with
the adjacent failure: binding a value of the wrong type already throws a
Node-authored ERR_INVALID_ARG_VALUE from the same function.
Detect the overflow before handing the index to sqlite3_bind_*() and
throw an error that names the actual problem. The check runs after the
scan for the next anonymous slot, so it also covers excess anonymous
values passed alongside named parameters.
This changes the error thrown for an existing case, so it needs
semver-major treatment.
Fixes: nodejs#65163
Signed-off-by: Paul Bouchon <mail@bitpshr.net>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite

@nodejs-github-botnodejs-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 Aug 9, 2026
@bitpshr

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of #65164, which does the same thing and also covers the docs. I had this in flight while building and missed that it was already open, sorry for the noise.

@bitpshrbitpshr closed this Aug 9, 2026
Sign up for freeto 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-ciPRs that need a full CI run.sqliteIssues and PRs related to the SQLite subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sqlite: excess bound parameters produce an opaque "column index out of range" error

2 participants

@bitpshr@nodejs-github-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all \u003cpre\u003e\u003ccode\u003e blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks"); } } catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); } })(); (function(){ try { var __m = "github.com"; var __re = new RegExp('^' + "github\\.com" + '
Skip to content

sqlite: improve error for excess bound parameters - #65167

Closed
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:sqlite/excess-bind-params
Closed

sqlite: improve error for excess bound parameters#65167
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:sqlite/excess-bind-params

Conversation

@bitpshr

Copy link
Copy Markdown
Contributor

Fixes#65163. Passing more values than a prepared statement has parameters surfaced SQLite's own column index out of range error, which reads as a problem with the table's columns rather than with the call, and is inconsistent with the adjacent failure (a wrong-type value already throws a Node-authored error from the same function).

This detects the overflow before handing the index to sqlite3_bind_*(). The check sits after the scan for the next anonymous slot, so it also covers excess anonymous values passed alongside named parameters.

before: ERR_SQLITE_ERROR column index out of range
after: ERR_INVALID_ARG_VALUE Too many parameter values were provided. The statement accepts 1 parameter(s), which are already bound.

Three notes for reviewers, since I deviated from the issue's suggestions:

  • This needs semver-major treatment.test-sqlite-statement-sync.js had a test asserting the old ERR_SQLITE_ERROR / errcode: 25, so the change is a deliberate behavior change. That test is updated here and renamed, since the error no longer comes from SQLite.
  • ERR_INVALID_ARG_COUNT does not exist in node_errors.h or lib/internal/errors.js, so I used ERR_INVALID_ARG_VALUE. It is already a TypeError and is what the sibling binding failure a few lines away uses, which keeps the two consistent.
  • The message reports only the parameter count. I first included a "but N were provided" count as suggested, but when named and anonymous parameters are mixed that number counts only the anonymous values, so run({ $k: 1 }, 2) would have read "accepts 1, but 1 were provided". The current wording is accurate in every case.

Verified locally against a build: the full sqlite suite passes (18/18), and correct arity still binds normally. Happy to change the error code, the wording, or drop this entirely if you'd rather leave the behavior alone.

Fixes: #65163

Passing more values than a prepared statement has parameters surfaced
SQLite's own "column index out of range" error. That wording describes a
binding index, but reads to a JavaScript caller as a problem with the
table's columns rather than with the call, and it is inconsistent with
the adjacent failure: binding a value of the wrong type already throws a
Node-authored ERR_INVALID_ARG_VALUE from the same function.
Detect the overflow before handing the index to sqlite3_bind_*() and
throw an error that names the actual problem. The check runs after the
scan for the next anonymous slot, so it also covers excess anonymous
values passed alongside named parameters.
This changes the error thrown for an existing case, so it needs
semver-major treatment.
Fixes: nodejs#65163
Signed-off-by: Paul Bouchon <mail@bitpshr.net>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite

@nodejs-github-botnodejs-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 Aug 9, 2026
@bitpshr

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of #65164, which does the same thing and also covers the docs. I had this in flight while building and missed that it was already open, sorry for the noise.

@bitpshrbitpshr closed this Aug 9, 2026
Sign up for freeto 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-ciPRs that need a full CI run.sqliteIssues and PRs related to the SQLite subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sqlite: excess bound parameters produce an opaque "column index out of range" error

2 participants

@bitpshr@nodejs-github-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

sqlite: improve error for excess bound parameters - #65167

Closed
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:sqlite/excess-bind-params
Closed

sqlite: improve error for excess bound parameters#65167
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:sqlite/excess-bind-params

Conversation

@bitpshr

Copy link
Copy Markdown
Contributor

Fixes#65163. Passing more values than a prepared statement has parameters surfaced SQLite's own column index out of range error, which reads as a problem with the table's columns rather than with the call, and is inconsistent with the adjacent failure (a wrong-type value already throws a Node-authored error from the same function).

This detects the overflow before handing the index to sqlite3_bind_*(). The check sits after the scan for the next anonymous slot, so it also covers excess anonymous values passed alongside named parameters.

before: ERR_SQLITE_ERROR column index out of range
after: ERR_INVALID_ARG_VALUE Too many parameter values were provided. The statement accepts 1 parameter(s), which are already bound.

Three notes for reviewers, since I deviated from the issue's suggestions:

  • This needs semver-major treatment.test-sqlite-statement-sync.js had a test asserting the old ERR_SQLITE_ERROR / errcode: 25, so the change is a deliberate behavior change. That test is updated here and renamed, since the error no longer comes from SQLite.
  • ERR_INVALID_ARG_COUNT does not exist in node_errors.h or lib/internal/errors.js, so I used ERR_INVALID_ARG_VALUE. It is already a TypeError and is what the sibling binding failure a few lines away uses, which keeps the two consistent.
  • The message reports only the parameter count. I first included a "but N were provided" count as suggested, but when named and anonymous parameters are mixed that number counts only the anonymous values, so run({ $k: 1 }, 2) would have read "accepts 1, but 1 were provided". The current wording is accurate in every case.

Verified locally against a build: the full sqlite suite passes (18/18), and correct arity still binds normally. Happy to change the error code, the wording, or drop this entirely if you'd rather leave the behavior alone.

Fixes: #65163

Passing more values than a prepared statement has parameters surfaced
SQLite's own "column index out of range" error. That wording describes a
binding index, but reads to a JavaScript caller as a problem with the
table's columns rather than with the call, and it is inconsistent with
the adjacent failure: binding a value of the wrong type already throws a
Node-authored ERR_INVALID_ARG_VALUE from the same function.
Detect the overflow before handing the index to sqlite3_bind_*() and
throw an error that names the actual problem. The check runs after the
scan for the next anonymous slot, so it also covers excess anonymous
values passed alongside named parameters.
This changes the error thrown for an existing case, so it needs
semver-major treatment.
Fixes: nodejs#65163
Signed-off-by: Paul Bouchon <mail@bitpshr.net>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite

@nodejs-github-botnodejs-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 Aug 9, 2026
@bitpshr

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of #65164, which does the same thing and also covers the docs. I had this in flight while building and missed that it was already open, sorry for the noise.

@bitpshrbitpshr closed this Aug 9, 2026
Sign up for freeto 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-ciPRs that need a full CI run.sqliteIssues and PRs related to the SQLite subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sqlite: excess bound parameters produce an opaque "column index out of range" error

2 participants

@bitpshr@nodejs-github-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length \u003e 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

sqlite: improve error for excess bound parameters - #65167

Closed
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:sqlite/excess-bind-params
Closed

sqlite: improve error for excess bound parameters#65167
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:sqlite/excess-bind-params

Conversation

@bitpshr

Copy link
Copy Markdown
Contributor

Fixes#65163. Passing more values than a prepared statement has parameters surfaced SQLite's own column index out of range error, which reads as a problem with the table's columns rather than with the call, and is inconsistent with the adjacent failure (a wrong-type value already throws a Node-authored error from the same function).

This detects the overflow before handing the index to sqlite3_bind_*(). The check sits after the scan for the next anonymous slot, so it also covers excess anonymous values passed alongside named parameters.

before: ERR_SQLITE_ERROR column index out of range
after: ERR_INVALID_ARG_VALUE Too many parameter values were provided. The statement accepts 1 parameter(s), which are already bound.

Three notes for reviewers, since I deviated from the issue's suggestions:

  • This needs semver-major treatment.test-sqlite-statement-sync.js had a test asserting the old ERR_SQLITE_ERROR / errcode: 25, so the change is a deliberate behavior change. That test is updated here and renamed, since the error no longer comes from SQLite.
  • ERR_INVALID_ARG_COUNT does not exist in node_errors.h or lib/internal/errors.js, so I used ERR_INVALID_ARG_VALUE. It is already a TypeError and is what the sibling binding failure a few lines away uses, which keeps the two consistent.
  • The message reports only the parameter count. I first included a "but N were provided" count as suggested, but when named and anonymous parameters are mixed that number counts only the anonymous values, so run({ $k: 1 }, 2) would have read "accepts 1, but 1 were provided". The current wording is accurate in every case.

Verified locally against a build: the full sqlite suite passes (18/18), and correct arity still binds normally. Happy to change the error code, the wording, or drop this entirely if you'd rather leave the behavior alone.

Fixes: #65163

Passing more values than a prepared statement has parameters surfaced
SQLite's own "column index out of range" error. That wording describes a
binding index, but reads to a JavaScript caller as a problem with the
table's columns rather than with the call, and it is inconsistent with
the adjacent failure: binding a value of the wrong type already throws a
Node-authored ERR_INVALID_ARG_VALUE from the same function.
Detect the overflow before handing the index to sqlite3_bind_*() and
throw an error that names the actual problem. The check runs after the
scan for the next anonymous slot, so it also covers excess anonymous
values passed alongside named parameters.
This changes the error thrown for an existing case, so it needs
semver-major treatment.
Fixes: nodejs#65163
Signed-off-by: Paul Bouchon <mail@bitpshr.net>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite

@nodejs-github-botnodejs-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 Aug 9, 2026
@bitpshr

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of #65164, which does the same thing and also covers the docs. I had this in flight while building and missed that it was already open, sorry for the noise.

@bitpshrbitpshr closed this Aug 9, 2026
Sign up for freeto 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-ciPRs that need a full CI run.sqliteIssues and PRs related to the SQLite subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sqlite: excess bound parameters produce an opaque "column index out of range" error

2 participants

@bitpshr@nodejs-github-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

sqlite: improve error for excess bound parameters - #65167

Closed
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:sqlite/excess-bind-params
Closed

sqlite: improve error for excess bound parameters#65167
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:sqlite/excess-bind-params

Conversation

@bitpshr

Copy link
Copy Markdown
Contributor

Fixes#65163. Passing more values than a prepared statement has parameters surfaced SQLite's own column index out of range error, which reads as a problem with the table's columns rather than with the call, and is inconsistent with the adjacent failure (a wrong-type value already throws a Node-authored error from the same function).

This detects the overflow before handing the index to sqlite3_bind_*(). The check sits after the scan for the next anonymous slot, so it also covers excess anonymous values passed alongside named parameters.

before: ERR_SQLITE_ERROR column index out of range
after: ERR_INVALID_ARG_VALUE Too many parameter values were provided. The statement accepts 1 parameter(s), which are already bound.

Three notes for reviewers, since I deviated from the issue's suggestions:

  • This needs semver-major treatment.test-sqlite-statement-sync.js had a test asserting the old ERR_SQLITE_ERROR / errcode: 25, so the change is a deliberate behavior change. That test is updated here and renamed, since the error no longer comes from SQLite.
  • ERR_INVALID_ARG_COUNT does not exist in node_errors.h or lib/internal/errors.js, so I used ERR_INVALID_ARG_VALUE. It is already a TypeError and is what the sibling binding failure a few lines away uses, which keeps the two consistent.
  • The message reports only the parameter count. I first included a "but N were provided" count as suggested, but when named and anonymous parameters are mixed that number counts only the anonymous values, so run({ $k: 1 }, 2) would have read "accepts 1, but 1 were provided". The current wording is accurate in every case.

Verified locally against a build: the full sqlite suite passes (18/18), and correct arity still binds normally. Happy to change the error code, the wording, or drop this entirely if you'd rather leave the behavior alone.

Fixes: #65163

Passing more values than a prepared statement has parameters surfaced
SQLite's own "column index out of range" error. That wording describes a
binding index, but reads to a JavaScript caller as a problem with the
table's columns rather than with the call, and it is inconsistent with
the adjacent failure: binding a value of the wrong type already throws a
Node-authored ERR_INVALID_ARG_VALUE from the same function.
Detect the overflow before handing the index to sqlite3_bind_*() and
throw an error that names the actual problem. The check runs after the
scan for the next anonymous slot, so it also covers excess anonymous
values passed alongside named parameters.
This changes the error thrown for an existing case, so it needs
semver-major treatment.
Fixes: nodejs#65163
Signed-off-by: Paul Bouchon <mail@bitpshr.net>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite

@nodejs-github-botnodejs-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 Aug 9, 2026
@bitpshr

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of #65164, which does the same thing and also covers the docs. I had this in flight while building and missed that it was already open, sorry for the noise.

@bitpshrbitpshr closed this Aug 9, 2026
Sign up for freeto 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-ciPRs that need a full CI run.sqliteIssues and PRs related to the SQLite subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sqlite: excess bound parameters produce an opaque "column index out of range" error

2 participants

@bitpshr@nodejs-github-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

sqlite: improve error for excess bound parameters - #65167

Closed
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:sqlite/excess-bind-params
Closed

sqlite: improve error for excess bound parameters#65167
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:sqlite/excess-bind-params

Conversation

@bitpshr

Copy link
Copy Markdown
Contributor

Fixes#65163. Passing more values than a prepared statement has parameters surfaced SQLite's own column index out of range error, which reads as a problem with the table's columns rather than with the call, and is inconsistent with the adjacent failure (a wrong-type value already throws a Node-authored error from the same function).

This detects the overflow before handing the index to sqlite3_bind_*(). The check sits after the scan for the next anonymous slot, so it also covers excess anonymous values passed alongside named parameters.

before: ERR_SQLITE_ERROR column index out of range
after: ERR_INVALID_ARG_VALUE Too many parameter values were provided. The statement accepts 1 parameter(s), which are already bound.

Three notes for reviewers, since I deviated from the issue's suggestions:

  • This needs semver-major treatment.test-sqlite-statement-sync.js had a test asserting the old ERR_SQLITE_ERROR / errcode: 25, so the change is a deliberate behavior change. That test is updated here and renamed, since the error no longer comes from SQLite.
  • ERR_INVALID_ARG_COUNT does not exist in node_errors.h or lib/internal/errors.js, so I used ERR_INVALID_ARG_VALUE. It is already a TypeError and is what the sibling binding failure a few lines away uses, which keeps the two consistent.
  • The message reports only the parameter count. I first included a "but N were provided" count as suggested, but when named and anonymous parameters are mixed that number counts only the anonymous values, so run({ $k: 1 }, 2) would have read "accepts 1, but 1 were provided". The current wording is accurate in every case.

Verified locally against a build: the full sqlite suite passes (18/18), and correct arity still binds normally. Happy to change the error code, the wording, or drop this entirely if you'd rather leave the behavior alone.

Fixes: #65163

Passing more values than a prepared statement has parameters surfaced
SQLite's own "column index out of range" error. That wording describes a
binding index, but reads to a JavaScript caller as a problem with the
table's columns rather than with the call, and it is inconsistent with
the adjacent failure: binding a value of the wrong type already throws a
Node-authored ERR_INVALID_ARG_VALUE from the same function.
Detect the overflow before handing the index to sqlite3_bind_*() and
throw an error that names the actual problem. The check runs after the
scan for the next anonymous slot, so it also covers excess anonymous
values passed alongside named parameters.
This changes the error thrown for an existing case, so it needs
semver-major treatment.
Fixes: nodejs#65163
Signed-off-by: Paul Bouchon <mail@bitpshr.net>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite

@nodejs-github-botnodejs-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 Aug 9, 2026
@bitpshr

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of #65164, which does the same thing and also covers the docs. I had this in flight while building and missed that it was already open, sorry for the noise.

@bitpshrbitpshr closed this Aug 9, 2026
Sign up for freeto 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-ciPRs that need a full CI run.sqliteIssues and PRs related to the SQLite subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sqlite: excess bound parameters produce an opaque "column index out of range" error

2 participants

@bitpshr@nodejs-github-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

sqlite: improve error for excess bound parameters - #65167

Closed
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:sqlite/excess-bind-params
Closed

sqlite: improve error for excess bound parameters#65167
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:sqlite/excess-bind-params

Conversation

@bitpshr

Copy link
Copy Markdown
Contributor

Fixes#65163. Passing more values than a prepared statement has parameters surfaced SQLite's own column index out of range error, which reads as a problem with the table's columns rather than with the call, and is inconsistent with the adjacent failure (a wrong-type value already throws a Node-authored error from the same function).

This detects the overflow before handing the index to sqlite3_bind_*(). The check sits after the scan for the next anonymous slot, so it also covers excess anonymous values passed alongside named parameters.

before: ERR_SQLITE_ERROR column index out of range
after: ERR_INVALID_ARG_VALUE Too many parameter values were provided. The statement accepts 1 parameter(s), which are already bound.

Three notes for reviewers, since I deviated from the issue's suggestions:

  • This needs semver-major treatment.test-sqlite-statement-sync.js had a test asserting the old ERR_SQLITE_ERROR / errcode: 25, so the change is a deliberate behavior change. That test is updated here and renamed, since the error no longer comes from SQLite.
  • ERR_INVALID_ARG_COUNT does not exist in node_errors.h or lib/internal/errors.js, so I used ERR_INVALID_ARG_VALUE. It is already a TypeError and is what the sibling binding failure a few lines away uses, which keeps the two consistent.
  • The message reports only the parameter count. I first included a "but N were provided" count as suggested, but when named and anonymous parameters are mixed that number counts only the anonymous values, so run({ $k: 1 }, 2) would have read "accepts 1, but 1 were provided". The current wording is accurate in every case.

Verified locally against a build: the full sqlite suite passes (18/18), and correct arity still binds normally. Happy to change the error code, the wording, or drop this entirely if you'd rather leave the behavior alone.

Fixes: #65163

Passing more values than a prepared statement has parameters surfaced
SQLite's own "column index out of range" error. That wording describes a
binding index, but reads to a JavaScript caller as a problem with the
table's columns rather than with the call, and it is inconsistent with
the adjacent failure: binding a value of the wrong type already throws a
Node-authored ERR_INVALID_ARG_VALUE from the same function.
Detect the overflow before handing the index to sqlite3_bind_*() and
throw an error that names the actual problem. The check runs after the
scan for the next anonymous slot, so it also covers excess anonymous
values passed alongside named parameters.
This changes the error thrown for an existing case, so it needs
semver-major treatment.
Fixes: nodejs#65163
Signed-off-by: Paul Bouchon <mail@bitpshr.net>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite

@nodejs-github-botnodejs-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 Aug 9, 2026
@bitpshr

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of #65164, which does the same thing and also covers the docs. I had this in flight while building and missed that it was already open, sorry for the noise.

@bitpshrbitpshr closed this Aug 9, 2026
Sign up for freeto 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-ciPRs that need a full CI run.sqliteIssues and PRs related to the SQLite subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sqlite: excess bound parameters produce an opaque "column index out of range" error

2 participants

@bitpshr@nodejs-github-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

sqlite: improve error for excess bound parameters - #65167

Closed
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:sqlite/excess-bind-params
Closed

sqlite: improve error for excess bound parameters#65167
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:sqlite/excess-bind-params

Conversation

@bitpshr

Copy link
Copy Markdown
Contributor

Fixes#65163. Passing more values than a prepared statement has parameters surfaced SQLite's own column index out of range error, which reads as a problem with the table's columns rather than with the call, and is inconsistent with the adjacent failure (a wrong-type value already throws a Node-authored error from the same function).

This detects the overflow before handing the index to sqlite3_bind_*(). The check sits after the scan for the next anonymous slot, so it also covers excess anonymous values passed alongside named parameters.

before: ERR_SQLITE_ERROR column index out of range
after: ERR_INVALID_ARG_VALUE Too many parameter values were provided. The statement accepts 1 parameter(s), which are already bound.

Three notes for reviewers, since I deviated from the issue's suggestions:

  • This needs semver-major treatment.test-sqlite-statement-sync.js had a test asserting the old ERR_SQLITE_ERROR / errcode: 25, so the change is a deliberate behavior change. That test is updated here and renamed, since the error no longer comes from SQLite.
  • ERR_INVALID_ARG_COUNT does not exist in node_errors.h or lib/internal/errors.js, so I used ERR_INVALID_ARG_VALUE. It is already a TypeError and is what the sibling binding failure a few lines away uses, which keeps the two consistent.
  • The message reports only the parameter count. I first included a "but N were provided" count as suggested, but when named and anonymous parameters are mixed that number counts only the anonymous values, so run({ $k: 1 }, 2) would have read "accepts 1, but 1 were provided". The current wording is accurate in every case.

Verified locally against a build: the full sqlite suite passes (18/18), and correct arity still binds normally. Happy to change the error code, the wording, or drop this entirely if you'd rather leave the behavior alone.

Fixes: #65163

Passing more values than a prepared statement has parameters surfaced
SQLite's own "column index out of range" error. That wording describes a
binding index, but reads to a JavaScript caller as a problem with the
table's columns rather than with the call, and it is inconsistent with
the adjacent failure: binding a value of the wrong type already throws a
Node-authored ERR_INVALID_ARG_VALUE from the same function.
Detect the overflow before handing the index to sqlite3_bind_*() and
throw an error that names the actual problem. The check runs after the
scan for the next anonymous slot, so it also covers excess anonymous
values passed alongside named parameters.
This changes the error thrown for an existing case, so it needs
semver-major treatment.
Fixes: nodejs#65163
Signed-off-by: Paul Bouchon <mail@bitpshr.net>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite

@nodejs-github-botnodejs-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 Aug 9, 2026
@bitpshr

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of #65164, which does the same thing and also covers the docs. I had this in flight while building and missed that it was already open, sorry for the noise.

@bitpshrbitpshr closed this Aug 9, 2026
Sign up for freeto 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-ciPRs that need a full CI run.sqliteIssues and PRs related to the SQLite subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sqlite: excess bound parameters produce an opaque "column index out of range" error

2 participants

@bitpshr@nodejs-github-bot