From 76000d7d038f0b3cb830b630f3a07bd02099d07f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 9 Mar 2023 11:54:42 +0700 Subject: [PATCH 1/2] Fix PUTing documents with If-None-Match when non-existent --- lib/controllers/storage.js | 12 +++++++++--- lib/stores/file_tree.js | 10 ++++++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/lib/controllers/storage.js b/lib/controllers/storage.js index 6778e793..ebf7aaca 100644 --- a/lib/controllers/storage.js +++ b/lib/controllers/storage.js @@ -33,10 +33,12 @@ class Storage extends Controller { async get (head = false) { const version = this.getVersion(); + const matchVersion = !!this.request.headers['if-match']; + if (await this.checkToken('r')) { let numBytesWritten = 0; try { - var { item, versionMatch } = await this.server._store.get(this._username, this._path, version, head); + var { item, versionMatch } = await this.server._store.get(this._username, this._path, version, matchVersion, head); } catch (e) { getLogger().error(`Your storage backend does not behave correctly => ${e.message}`); this.response.writeHead(500, this._headers); @@ -59,7 +61,7 @@ class Storage extends Controller { } this.setVersion(item && item.ETag); - if (versionMatch) { + if (!matchVersion && versionMatch) { delete this._headers['Content-Type']; this.response.writeHead(304, this._headers); this.response.end(); @@ -86,10 +88,14 @@ class Storage extends Controller { return false; } const version = this.getVersion(); + const matchVersion = !!this.request.headers['if-match']; + let status, error, created, modified, conflict, isDir; if (await this.checkToken('w')) { try { - ({ created, modified, conflict, isDir } = await this.server._store.put(this._username, this._path, type, value, version)); + ({ created, modified, conflict, isDir } = await this.server._store.put( + this._username, this._path, type, value, version, matchVersion + )); status = conflict ? 412 : isDir diff --git a/lib/stores/file_tree.js b/lib/stores/file_tree.js index 13fac980..bb0e5ba0 100644 --- a/lib/stores/file_tree.js +++ b/lib/stores/file_tree.js @@ -198,7 +198,7 @@ class FileTree { } } - async put (username, pathname, type, value, version) { + async put (username, pathname, type, value, version, matchVersion) { const datapath = this.dataPath(username, pathname); const metapath = this._metaPath(username, pathname); const basename = decodeURI(path.basename(pathname)); @@ -206,7 +206,7 @@ class FileTree { const metadata = await this.readMeta(username, pathname); let created = false; - if (version) { + if (version && matchVersion) { if (version === '*' // check document existence when version '*' specified ? metadata.items && metadata.items[basename] @@ -217,6 +217,12 @@ class FileTree { await this._unlock(username); return { conflict: true, created }; } + } else if (version && !matchVersion) { + if (metadata.items && metadata.items[basename] && + version.replace(/"/g, '') === metadata.items[basename].ETag) { + await this._unlock(username); + return { conflict: true, created }; + } } if (metadata.items[`${basename}/`]) { From 198b871c38556499c5fc69f6b99c67150a7e18d1 Mon Sep 17 00:00:00 2001 From: "P. Douglas Reeder" Date: Thu, 9 Mar 2023 13:38:05 -0500 Subject: [PATCH 2/2] Store specs: adds tests for If-Match and If-None-Match --- spec/store_spec.js | 48 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/spec/store_spec.js b/spec/store_spec.js index 94408c7d..f90fdc97 100644 --- a/spec/store_spec.js +++ b/spec/store_spec.js @@ -148,6 +148,54 @@ sharedExamplesFor('Stores', (store) => { expect(!conflict).to.be.true; }); + it("doesn't set the value of an existing item, when If-Match [ETag] doesn't match", async () => { + // client 1 + const client1put1 = await store.put('boris', '/if-match/no-match', 'text/markdown', Buffer.from('Original paragraph'), null); + expect(client1put1.conflict).to.be.false; + expect(client1put1.created).to.be.true; + + // both clients + const { item: original } = await store.get('boris', '/if-match/no-match', null); + expect(original.ETag).to.exist; + + // client 2 + const client2put = await store.put('boris', '/if-match/no-match', 'text/markdown', Buffer.from('Changed paragraph'), original.ETag); + expect(client2put.conflict).to.be.false; + expect(client2put.created).to.be.false; + const { item: change1 } = await store.get('boris', '/if-match/no-match', null); + expect(change1.value.toString('utf8')).to.be.deep.equal('Changed paragraph'); + expect(change1.ETag).to.not.equal(original.ETag); + + // client 1 + const client1put2 = await store.put('boris', '/if-match/no-match', 'text/markdown', Buffer.from('Another change'), original.ETag); + expect(client1put2.conflict).to.be.true; + expect(client1put2.created).to.be.false; + + const { item: final } = await store.get('boris', '/if-match/no-match', null); + expect(final.value.toString('utf8')).to.be.deep.equal('Changed paragraph'); + expect(final.ETag).to.equal(change1.ETag); + }); + + it("doesn't set the value of an existing item, when If-None-Match * is used", async () => { + const response1 = await store.put('boris', '/if-none-match/star', 'text/markdown', Buffer.from('# Some Title'), null); + expect(response1.conflict).to.be.false; + expect(response1.created).to.be.true; + const response2 = await store.put('boris', '/if-none-match/star', 'text/markdown', Buffer.from('# Another Title'), '*'); + expect(response2.conflict).to.be.true; + expect(response2.created).to.be.false; + const { item } = await store.get('boris', '/if-none-match/star', null); + expect(item.value).to.be.deep.equal(Buffer.from('# Some Title')); + }); + + it("doesn't set the value of an existing item, when If-None-Match [ETag] matches", async () => { + await store.put('boris', '/if-none-match/etag', 'text/markdown', Buffer.from('* Original Item'), null); + const { item: original } = await store.get('boris', '/if-none-match/etag', null); + expect(original.ETag).to.exist; + await store.put('boris', '/if-none-match/etag', 'text/markdown', Buffer.from('* Changed Item'), original.ETag); + const { item: final } = await store.get('boris', '/if-none-match/etag', null); + expect(final.value.toString('utf8')).to.be.deep.equal('* Original Item'); + }); + describe('for a nested document', () => { it('created the parent directory', async () => { await store.put('boris', '/photos/foo/bar/qux', 'image/poster', Buffer.from('vertibo'), null);