From b27502e823ba2731c1b498dd488707e7cdafbed7 Mon Sep 17 00:00:00 2001 From: Alec Gibson <12036746+alecgibson@users.noreply.github.com> Date: Mon, 9 Nov 2026 17:45:09 +0000 Subject: [PATCH 1/2] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20Only=20read=20the=20re?= =?UTF-8?q?quested=20`fields`=20in=20`getSnapshot()`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes https://github.com/share/sharedb-postgres/issues/36 At the moment, `getSnapshot()` always selects the whole `data` column, and `fields ||= {}` can't tell "all fields" from "no fields". Since share/sharedb#770, every presence leave calls `getSnapshot()` with `fields: {}` just to read the version, so each one reads the whole document. This change stops reading `data` when `fields` is empty, so a live doc comes back with `data: {}`, as it does in [`sharedb-mongo`][1], and otherwise selects only the named keys. One consequence: a `json0` doc whose data isn't an object now projects to `{}` rather than `null` for empty `fields`, since checking its type would mean reading `data`. We leave `projectsSnapshots` false, so `Backend` still [re-projects][2] what we return, and the keyed projection passes non-object data through for it to turn into `null`. [1]: https://github.com/share/sharedb-mongo/blob/16c3fb60f69d9d5ddca6d7947e3bee9bf396ddbc/index.js#L1517-L1534 [2]: https://github.com/share/sharedb/blob/fb143054efe158337c56fc142716b1d43d91488c/lib/backend.js#L310-L312 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- index.js | 23 +++++++++-- index.spec.js | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index 0a093b1..260504c 100644 --- a/index.js +++ b/index.js @@ -114,13 +114,14 @@ RETURNING version`, // snapshot). A snapshot with a version of zero is returned if the docuemnt // has never been created in the database. PostgresDB.prototype.getSnapshot = async function(collection, id, fields, options, callback) { - fields ||= {}; options ||= {}; - const wantsMetadata = fields.$submit || options.metadata; + const wantsMetadata = fields?.$submit || options.metadata; + const params = [collection, id]; + const dataColumn = selectData(fields, params); try { const result = await this._pool.query( - 'SELECT version, data, doc_type, metadata FROM snapshots WHERE collection = $1 AND doc_id = $2 LIMIT 1', - [collection, id], + `SELECT version, ${dataColumn} AS data, doc_type, metadata FROM snapshots WHERE collection = $1 AND doc_id = $2 LIMIT 1`, + params, ); var row = result.rows[0] @@ -140,6 +141,20 @@ PostgresDB.prototype.getSnapshot = async function(collection, id, fields, option } }; +function selectData(fields, params) { + if (!fields || fields.$submit) return 'data'; + + const keys = Object.keys(fields); + if (!keys.length) return `CASE WHEN doc_type IS NULL THEN NULL ELSE '{}'::jsonb END`; + + params.push(keys); + const keysParam = `$${params.length}`; + return `CASE WHEN jsonb_typeof(data) = 'object' + THEN (SELECT coalesce(jsonb_object_agg(key, value), '{}') FROM jsonb_each(data) WHERE key = ANY(${keysParam}::text[])) + ELSE data + END`; +} + // Get operations between [from, to) noninclusively. (Ie, the range should // contain start but not end). // diff --git a/index.spec.js b/index.spec.js index b21888c..42a2cfd 100644 --- a/index.spec.js +++ b/index.spec.js @@ -1,6 +1,9 @@ const PostgresDB = require('.'); const {Pool} = require('pg'); const fs = require('node:fs'); +const {expect} = require('chai'); +const sinon = require('sinon'); +const Backend = require('sharedb'); const DB_NAME = 'sharedbtest'; @@ -9,6 +12,12 @@ function create(callback) { callback(null, db); }; +function promisifyCallback(fn) { + return (...args) => new Promise((resolve, reject) => { + fn(...args, (error, result) => error ? reject(error) : resolve(result)); + }); +} + describe('PostgresDB', function() { let pool; let client; @@ -34,4 +43,103 @@ describe('PostgresDB', function() { }); require('sharedb/test/db')({create: create}); + + describe('getSnapshot with fields', () => { + let db; + let commit; + let getSnapshot; + + beforeEach(async () => { + db = new PostgresDB({database: DB_NAME}); + commit = promisifyCallback(db.commit.bind(db)); + getSnapshot = promisifyCallback(db.getSnapshot.bind(db)); + const snapshot = {type: 'json0', v: 1, data: {x: 5, y: 6}, m: {test: 3}}; + await commit('testcollection', 'test', {v: 0, create: {}}, snapshot, null); + }); + + afterEach(async () => { + sinon.restore(); + await db.close(); + }); + + it('does not read data when fields is empty', async () => { + const query = sinon.spy(db._pool, 'query'); + const snapshot = await getSnapshot('testcollection', 'test', {}, null); + expect(snapshot).to.eql({id: 'test', v: 1, type: 'json0', data: {}, m: null}); + expect(query.lastCall.args[0]).not.to.match(/(? { + const deleted = {type: null, v: 2, data: undefined}; + await commit('testcollection', 'test', {v: 1, del: true}, deleted, null); + const snapshot = await getSnapshot('testcollection', 'test', {}, null); + expect(snapshot).to.eql({id: 'test', v: 2, type: null, data: undefined, m: null}); + }); + + it('returns no data for a deleted doc when fields has keys', async () => { + const deleted = {type: null, v: 2, data: undefined}; + await commit('testcollection', 'test', {v: 1, del: true}, deleted, null); + const snapshot = await getSnapshot('testcollection', 'test', {x: true}, null); + expect(snapshot.data).to.equal(undefined); + }); + + it('returns a v0 snapshot for a missing doc when fields is empty', async () => { + const snapshot = await getSnapshot('testcollection', 'missing', {}, null); + expect(snapshot).to.eql({id: 'missing', v: 0, type: null, data: undefined, m: null}); + }); + + it('returns only the requested keys', async () => { + const snapshot = await getSnapshot('testcollection', 'test', {y: true}, null); + expect(snapshot.data).to.eql({y: 6}); + }); + + it('returns empty data when no requested key exists', async () => { + const snapshot = await getSnapshot('testcollection', 'test', {z: true}, null); + expect(snapshot.data).to.eql({}); + }); + + it('returns non-object data untouched when fields has keys', async () => { + const snapshot = {type: 'json0', v: 1, data: [1, 2]}; + await commit('testcollection', 'array', {v: 0, create: {}}, snapshot, null); + const result = await getSnapshot('testcollection', 'array', {x: true}, null); + expect(result.data).to.eql([1, 2]); + }); + + it('returns empty data for non-object data when fields is empty', async () => { + const snapshot = {type: 'json0', v: 1, data: [1, 2]}; + await commit('testcollection', 'array', {v: 0, create: {}}, snapshot, null); + const result = await getSnapshot('testcollection', 'array', {}, null); + expect(result.data).to.eql({}); + }); + + it('returns all data and metadata when fields has $submit', async () => { + const snapshot = await getSnapshot('testcollection', 'test', {$submit: true, x: true}, null); + expect(snapshot.data).to.eql({x: 5, y: 6}); + expect(snapshot.m).to.eql({test: 3}); + }); + }); + + describe('through a Backend', function() { + beforeEach(function() { + this.backend = new Backend({db: new PostgresDB({database: DB_NAME})}); + }); + + afterEach(function(done) { + this.backend.close(done); + }); + + require('sharedb/test/client/projections')({}); + + it('projects non-object data to null', async function() { + this.backend.addProjection('summary', 'testcollection', {x: true}); + const connection = this.backend.connect(); + const doc = connection.get('testcollection', 'array'); + await promisifyCallback(doc.create.bind(doc))([1, 2]); + + const connection2 = this.backend.connect(); + const summary = connection2.get('summary', 'array'); + await promisifyCallback(summary.fetch.bind(summary))(); + expect(summary.data).to.equal(null); + }); + }); }); From 1d3044828b6f760038545cf58a25ee74e0dd8225 Mon Sep 17 00:00:00 2001 From: Alec Gibson <12036746+alecgibson@users.noreply.github.com> Date: Thu, 12 Nov 2026 08:34:09 +0000 Subject: [PATCH 2/2] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Make=20`getSnapshot()`?= =?UTF-8?q?=20query=20text=20constant?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs https://github.com/share/sharedb-postgres/pull/37#discussion_r4536002588 At the moment, `selectData()` assembles part of the `getSnapshot()` SQL at runtime, including the `$3` placeholder for the requested `fields` keys. The keys themselves are already bound, so this is safe, but checking that means tracing the helper. This change writes the three possible statements out as constants, so no runtime value can reach the SQL text, and adds a test with keys that would break the query, or leak other keys, if they were ever spliced in. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- index.js | 39 +++++++++++++++++++++++---------------- index.spec.js | 9 +++++++++ 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/index.js b/index.js index 260504c..b23a7e4 100644 --- a/index.js +++ b/index.js @@ -116,13 +116,9 @@ RETURNING version`, PostgresDB.prototype.getSnapshot = async function(collection, id, fields, options, callback) { options ||= {}; const wantsMetadata = fields?.$submit || options.metadata; - const params = [collection, id]; - const dataColumn = selectData(fields, params); + const {text, values} = snapshotQuery(collection, id, fields); try { - const result = await this._pool.query( - `SELECT version, ${dataColumn} AS data, doc_type, metadata FROM snapshots WHERE collection = $1 AND doc_id = $2 LIMIT 1`, - params, - ); + const result = await this._pool.query(text, values); var row = result.rows[0] const snapshot = { @@ -141,18 +137,29 @@ PostgresDB.prototype.getSnapshot = async function(collection, id, fields, option } }; -function selectData(fields, params) { - if (!fields || fields.$submit) return 'data'; +const SELECT_SNAPSHOT = + 'SELECT version, data, doc_type, metadata FROM snapshots WHERE collection = $1 AND doc_id = $2 LIMIT 1'; + +const SELECT_SNAPSHOT_WITHOUT_DATA = `SELECT version, + CASE WHEN doc_type IS NULL THEN NULL ELSE '{}'::jsonb END AS data, + doc_type, metadata + FROM snapshots WHERE collection = $1 AND doc_id = $2 LIMIT 1`; + +const SELECT_SNAPSHOT_KEYS = `SELECT version, + CASE WHEN jsonb_typeof(data) = 'object' + THEN (SELECT coalesce(jsonb_object_agg(key, value), '{}') FROM jsonb_each(data) WHERE key = ANY($3::text[])) + ELSE data + END AS data, + doc_type, metadata + FROM snapshots WHERE collection = $1 AND doc_id = $2 LIMIT 1`; + +function snapshotQuery(collection, id, fields) { + if (!fields || fields.$submit) return {text: SELECT_SNAPSHOT, values: [collection, id]}; const keys = Object.keys(fields); - if (!keys.length) return `CASE WHEN doc_type IS NULL THEN NULL ELSE '{}'::jsonb END`; - - params.push(keys); - const keysParam = `$${params.length}`; - return `CASE WHEN jsonb_typeof(data) = 'object' - THEN (SELECT coalesce(jsonb_object_agg(key, value), '{}') FROM jsonb_each(data) WHERE key = ANY(${keysParam}::text[])) - ELSE data - END`; + if (!keys.length) return {text: SELECT_SNAPSHOT_WITHOUT_DATA, values: [collection, id]}; + + return {text: SELECT_SNAPSHOT_KEYS, values: [collection, id, keys]}; } // Get operations between [from, to) noninclusively. (Ie, the range should diff --git a/index.spec.js b/index.spec.js index 42a2cfd..f626d44 100644 --- a/index.spec.js +++ b/index.spec.js @@ -93,6 +93,15 @@ describe('PostgresDB', function() { expect(snapshot.data).to.eql({y: 6}); }); + it('matches keys literally, whatever they contain', async () => { + const keys = [`x'::text[]); DROP TABLE snapshots; --`, 'a,secret', '{secret}', '"secret"', 'NULL']; + const requested = Object.fromEntries(keys.map((key) => [key, true])); + const snapshot = {type: 'json0', v: 1, data: {...requested, secret: true}}; + await commit('testcollection', 'hostile', {v: 0, create: {}}, snapshot, null); + const result = await getSnapshot('testcollection', 'hostile', requested, null); + expect(result.data).to.eql(requested); + }); + it('returns empty data when no requested key exists', async () => { const snapshot = await getSnapshot('testcollection', 'test', {z: true}, null); expect(snapshot.data).to.eql({});