diff --git a/lib/data/schema.js b/lib/data/schema.js index 281566254..af6d7f4ae 100644 --- a/lib/data/schema.js +++ b/lib/data/schema.js @@ -21,7 +21,7 @@ const { always, equals, last, map } = require('ramda'); const ptr = last; // just rename to make it more relevant to our context. const hparser = require('htmlparser2'); const { parse } = require('csv-parse/sync'); -const { decodeXML } = require('entities'); +const { decodeXML, encodeXML } = require('entities'); const semverSatisfies = require('semver/functions/satisfies'); const Option = require('../util/option'); const Problem = require('../util/problem'); @@ -542,18 +542,10 @@ const _versionSplicer = (replace) => (xml, insert) => new Promise((pass, fail) = if ((stripNamespacesFromPath(name) === 'version') && (stack.length) === 4 && (stack[0] === 'html') && (stack[1] === 'head') && (stack[2] === 'model') && (stack[3] === 'instance')) { - // okay, we have found the thing we are looking for. - // idx points at the position of the closing " - // - // TODO: we cheat here and reference the hp2 internal tokenizer index to find - // out where the attribute actually is. the parser startIndex and endIndex point - // at the whitespace preceding the tag until the tag is closed. obviously this is - // pretty bad but i don't see a more robust solution right now. - const idx = parser.tokenizer.index; + const newValue = encodeXML(replace ? insert : `${value}${insert}`); + const result = `${xml.slice(0, parser.startIndex)}${name}="${newValue}"${xml.slice(parser.endIndex)}`; parser.reset(); - return replace - ? pass(`${xml.slice(0, idx - value.length)}${insert}${xml.slice(idx)}`) - : pass(`${xml.slice(0, idx)}${insert}${xml.slice(idx)}`); + return pass(result); } }, // n.b. opentag happens AFTER all the attributes for that tag have been emitted! @@ -565,7 +557,7 @@ const _versionSplicer = (replace) => (xml, insert) => new Promise((pass, fail) = const idx = _findSplicePoint(xml, parser.endIndex); parser.reset(); // stop parsing. - return pass(`${xml.slice(0, idx)} version="${insert}"${xml.slice(idx)}`); + return pass(`${xml.slice(0, idx)} version="${encodeXML(insert)}"${xml.slice(idx)}`); } }, onclosetag: () => { diff --git a/lib/model/frames/form.js b/lib/model/frames/form.js index 1b54c42b7..0d37b6e22 100644 --- a/lib/model/frames/form.js +++ b/lib/model/frames/form.js @@ -122,7 +122,7 @@ class Form extends Frame.define( const xmlFormId = idText.map(blankStringToNull) .orThrow(Problem.user.missingParameter({ field: 'formId' })); - const version = versionText.orElse(''); + const version = versionText.map(decodeXML).orElse(''); const name = nameText.map(decodeXML).orNull(); const key = pubKey.map((k) => new Key({ public: k })); return new Form.Partial({ xmlFormId, name }, { def: new Form.Def({ version, name }), key }); diff --git a/test/integration/api/forms/draft.js b/test/integration/api/forms/draft.js index da05757f2..7b8f0cb6d 100644 --- a/test/integration/api/forms/draft.js +++ b/test/integration/api/forms/draft.js @@ -1936,6 +1936,17 @@ describe('api: /projects/:id/forms (drafts)', () => { }); }))))); + it('should update version even if version provided in query parameter has special chars', testService((service) => + service.login('alice', (asAlice) => + asAlice.post('/v1/projects/1/forms/simple/draft') + .expect(200) + .then(() => asAlice.post('/v1/projects/1/forms/simple/draft/publish?version=x" version%3D"y') + .expect(200) + .then(() => asAlice.get('/v1/projects/1/forms/simple') + .then(({ body }) => { + body.version.should.eql('x" version="y'); + })))))); + it('should log the correct def ids in the form audit log', testService(async (service, { Forms }) => { const asAlice = await service.login('alice'); diff --git a/test/integration/api/forms/forms.js b/test/integration/api/forms/forms.js index 6a99381a3..225ebe266 100644 --- a/test/integration/api/forms/forms.js +++ b/test/integration/api/forms/forms.js @@ -92,6 +92,19 @@ describe('api: /projects/:id/forms (create, read, update)', () => { body.hash.should.equal('07ed8a51cc3f6472b7dfdc14c2005861'); })))); + it('should decode html entities in the value of form version - getodk/central#1470', testService((service) => + service.login('alice', (asAlice) => + asAlice.post('/v1/projects/1/forms') + .send(testData.forms.simple2.replace('2.1', '<{}>')) + .set('Content-Type', 'application/xml') + .expect(200) + .then(({ body }) => { + body.should.be.a.Form(); + body.name.should.equal('Simple 2'); + body.version.should.equal('<{}>'); + body.hash.should.equal('232ffd287a8eaef0103ad2386dafa44e'); + })))); + it('should reject if form id ends in .xml', testService((service) => service.login('alice', (asAlice) => asAlice.post('/v1/projects/1/forms') diff --git a/test/unit/data/schema.js b/test/unit/data/schema.js index 1180dafbc..7fb402ec2 100644 --- a/test/unit/data/schema.js +++ b/test/unit/data/schema.js @@ -2020,6 +2020,53 @@ describe('form schema', () => { `))); + const arabicWord = 'صادق'; + [ + /* eslint-disable no-multi-spaces, key-spacing */ + { versionAttribute: '', suffix: 'test', expected: 'test' }, + { versionAttribute: 'version=""', suffix: 'test', expected: 'test' }, + { versionAttribute: 'version="first"', suffix: '', expected: 'first' }, + { versionAttribute: 'version=\'first\'', suffix: 'test', expected: 'firsttest' }, + { versionAttribute: 'version="2.1"', suffix: 'test', expected: '2.1test' }, + { versionAttribute: 'version = "final"', suffix: ' (working)', expected: 'final (working)' }, + { versionAttribute: 'version=1', suffix: '.0', expected: '1.0' }, + { versionAttribute: 'version=1.0', suffix: '.1', expected: '1.0.1' }, + { versionAttribute: 'version=1', suffix: '_final', expected: '1_final' }, + { versionAttribute: 'version="john\'s copy"', suffix: '_final', expected: 'john's copy_final' }, + { versionAttribute: 'version="final"', suffix: ' "working"', expected: 'final "working"' }, + { versionAttribute: 'version="emoji 🙃"', suffix: ' twice 🙃', expected: 'emoji 🙃 twice 🙃' }, + { versionAttribute: `version="${arabicWord}"`, suffix: arabicWord, expected: 'صادقصادق' }, + { + versionAttribute: 'version="!@#$%^&*()_+=-`~[]\\{}|":'<>?/.,"', + suffix: '!@#$%^&*()_+=-`~[]\\{}|":\'<>?/.,', + expected: '!@#$%^&*()_+=-`~[]\\{}|":'<>?/.,!@#$%^&*()_+=-`~[]\\{}|":'<>?/.,' + }, + /* eslint-enable no-multi-spaces, key-spacing */ + ].forEach(({ versionAttribute, suffix, expected }) => { + it(`should have ${expected} version`, () => + addVersionSuffix(testData.forms.simple2.replace('version="2.1"', versionAttribute), suffix) + .then(result => { + // we always return version with "" delimiter + result.match(/(version="(.*)")/)[2].should.be.equal(expected); + }) + ); + }); + + + it('should suffix an existing version attribute even if it uses single quote to delimit version text', () => + addVersionSuffix(testData.forms.simple2.replace('version="2.1"', "version='2.1\"final\"'"), 'testtest') + .then(result => { + result.includes('version="2.1"final"testtest"').should.be.true(); + }) + ); + + it('should suffix an existing version attribute even if it uses single quote to delimit version text', () => + addVersionSuffix(testData.forms.simple2.replace('version="2.1"', 'version=1.0'), '.1') + .then(result => { + result.includes('version="1.0.1"').should.be.true(); + }) + ); + it('should suffix an existing namespaced version attribute', () => addVersionSuffix(testData.forms.simple2.replace('version', 'orx:version'), 'testtest').then((result) => result.should.equal(` @@ -2156,6 +2203,44 @@ describe('form schema', () => { `))); + + const arabicWord = 'صادق'; + [ + /* eslint-disable no-multi-spaces, key-spacing */ + { versionAttribute: '', newVersion: 'test', expected: 'test' }, + { versionAttribute: 'version=""', newVersion: 'test', expected: 'test' }, + { versionAttribute: 'version="first"', newVersion: '', expected: '' }, + { versionAttribute: 'version=\'first\'', newVersion: 'test', expected: 'test' }, + { versionAttribute: 'version="2.1"', newVersion: 'test', expected: 'test' }, + { versionAttribute: 'version = "final"', newVersion: '(working)', expected: '(working)' }, + { versionAttribute: 'version=1', newVersion: '1.0', expected: '1.0' }, + { versionAttribute: 'version=1.0', newVersion: '2.0', expected: '2.0' }, + { versionAttribute: 'version=1', newVersion: 'final', expected: 'final' }, + { versionAttribute: 'version="john\'s copy"', newVersion: 'jane\'s copy', expected: 'jane's copy' }, + { versionAttribute: 'version="final"', newVersion: '"working"', expected: '"working"' }, + { versionAttribute: 'version="emoji 🙃"', newVersion: '🙃 🙃', expected: '🙃 🙃' }, + { versionAttribute: `version="sadiq"`, newVersion: arabicWord, expected: 'صادق' }, + { + // getodk/central#1470 + versionAttribute: 'version="!@#$%^&*()_+=-`~[]\\{}|":'<>?/.,"', + newVersion: 'clean', + expected: 'clean' + }, + { + versionAttribute: 'version="clean"', + newVersion: '!@#$%^&*()_+=-`~[]\\{}|":\'<>?/.,', + expected: '!@#$%^&*()_+=-`~[]\\{}|":'<>?/.,' + }, + /* eslint-enable no-multi-spaces, key-spacing */ + ].forEach(({ versionAttribute, newVersion, expected }) => { + it(`should have ${expected} version`, () => + setVersion(testData.forms.simple2.replace('version="2.1"', versionAttribute), newVersion) + .then(result => { + // we always return version with "" delimiter + result.match(/(version="(.*)")/)[2].should.be.equal(expected); + }) + ); + }); }); describe('updateEntityForm', () => {