From dd2aa5eae1a3eca06e7bfd819924d6df1ab67089 Mon Sep 17 00:00:00 2001 From: Oskar Hane Date: Tue, 20 Jun 2017 14:06:25 +0200 Subject: [PATCH 1/2] Add support for setting browser :config with objects Add ability to accept setting browser configs with a JSON string, like: `:config {"maxFrames":50, "theme": "outline"}` --- .../modules/commands/commandsDuck.test.js | 29 ++- src/shared/modules/commands/helpers/config.js | 34 ++-- .../modules/commands/helpers/config.test.js | 169 ++++++++++++++++++ 3 files changed, 220 insertions(+), 12 deletions(-) create mode 100644 src/shared/modules/commands/helpers/config.test.js diff --git a/src/shared/modules/commands/commandsDuck.test.js b/src/shared/modules/commands/commandsDuck.test.js index 74aeacaff18..9393ba9a8f6 100644 --- a/src/shared/modules/commands/commandsDuck.test.js +++ b/src/shared/modules/commands/commandsDuck.test.js @@ -32,7 +32,7 @@ import { send } from 'shared/modules/requests/requestsDuck' import * as frames from 'shared/modules/stream/streamDuck' import { disconnectAction } from 'shared/modules/connections/connectionsDuck' import { merge, set } from 'shared/modules/params/paramsDuck' -import { update as updateSettings } from 'shared/modules/settings/settingsDuck' +import { update as updateSettings, replace as replaceSettings } from 'shared/modules/settings/settingsDuck' import { cleanCommand, getInterpreter } from 'services/commandUtils' const bus = createBus() @@ -184,7 +184,7 @@ describe('commandsDuck', () => { test('does the right thing for :config x: 2', (done) => { // Given const cmd = store.getState().settings.cmdchar + 'config' - const cmdString = cmd + ' x: 2' + const cmdString = cmd + ' "x": 2' const id = 1 const action = commands.executeCommand(cmdString, id) bus.take('NOOP', (currentAction) => { @@ -206,6 +206,31 @@ describe('commandsDuck', () => { // Then // See above }) + test('does the right thing for :config {"x": 2, "y":3}', (done) => { + // Given + const cmd = store.getState().settings.cmdchar + 'config' + const cmdString = cmd + ' {"x": 2, "y":3}' + const id = 1 + const action = commands.executeCommand(cmdString, id) + bus.take('NOOP', (currentAction) => { + // Then + expect(store.getActions()).toEqual([ + action, + addHistory(cmdString, maxHistory), + { type: commands.KNOWN_COMMAND }, + replaceSettings({x: 2, y: 3}), + frames.add({...action, type: 'pre', result: JSON.stringify({cmdchar: ':', maxHistory: 20}, null, 2)}), + { type: 'NOOP' } + ]) + done() + }) + + // When + store.dispatch(action) + + // Then + // See above + }) test('does the right thing for :config', (done) => { // Given diff --git a/src/shared/modules/commands/helpers/config.js b/src/shared/modules/commands/helpers/config.js index a1991894788..51a52d1a754 100644 --- a/src/shared/modules/commands/helpers/config.js +++ b/src/shared/modules/commands/helpers/config.js @@ -19,7 +19,7 @@ */ import { getSettings, update, replace } from 'shared/modules/settings/settingsDuck' -import { extractCommandParameters, splitStringOnFirst } from 'services/commandUtils' +import { splitStringOnFirst } from 'services/commandUtils' import { getRemoteContentHostnameWhitelist } from 'shared/modules/dbMeta/dbMetaDuck' import { hostIsAllowed } from 'services/utils' import { getJSON } from 'services/remote' @@ -36,25 +36,39 @@ export function handleUpdateConfigCommand (action, cmdchar, put, store) { const parts = splitStringOnFirst(strippedCmd, ' ') const p = new Promise((resolve, reject) => { if (parts[1] === undefined || parts[1] === '') return resolve(true) // Nothing to do - if (!isValidURL(parts[1].trim())) { // Not an URL. Parse as command line params - const params = extractCommandParameters(`config`, strippedCmd) - if (!params) return reject(new Error('Could not parse input. Usage: `:config x: 2`.')) - put(update(params)) - return resolve(params) + const param = parts[1].trim() + if (!isValidURL(param)) { // Not an URL. Parse as command line params + if (/^"?\{[^}]*\}"?$/.test(param)) { // JSON object string {"x": 2, "y":"string"} + try { + const res = JSON.parse(param.replace(/^"/, '').replace(/"$/, '')) // Remove any surrounding quotes + put(replace(res)) + return resolve(res) + } catch (e) { + return reject(new Error('Could not parse input. Usage: `:config {"x":1,"y":"string"}`. ' + e)) + } + } else { // Single param + try { + const json = '{' + param + '}' + const res = JSON.parse(json) + put(update(res)) + return resolve(res) + } catch (e) { + return reject(new Error('Could not parse input. Usage: `:config "x": 2`. ' + e)) + } + } } // It's an URL - const url = parts[1].trim() const whitelist = getRemoteContentHostnameWhitelist(store.getState()) - if (!hostIsAllowed(url, whitelist)) { // Make sure we're allowed to request entities on this host + if (!hostIsAllowed(param, whitelist)) { // Make sure we're allowed to request entities on this host return reject(new Error('Hostname is not allowed according to server whitelist')) } - getJSON(url) + getJSON(param) .then((res) => { put(replace(res)) resolve(res) }) .catch((e) => { - reject(new Error(e)) + reject(e) }) }) return p diff --git a/src/shared/modules/commands/helpers/config.test.js b/src/shared/modules/commands/helpers/config.test.js new file mode 100644 index 00000000000..a3e7d5e6df5 --- /dev/null +++ b/src/shared/modules/commands/helpers/config.test.js @@ -0,0 +1,169 @@ +/* + * Copyright (c) 2002-2017 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Neo4j is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +/* global jest, describe, test, expect, afterEach */ + +import nock from 'nock' +import * as config from './config' +import { update, replace } from 'shared/modules/settings/settingsDuck' + +function FetchError (message) { + this.name = 'FetchError' + this.message = message +} +FetchError.prototype = Object.create(Error.prototype) +FetchError.prototype.constructor = FetchError + +describe('commandsDuck config helper', () => { + const store = { + getState: () => { + return { + meta: { + settings: { + 'browser.remote_content_hostname_whitelist': 'okurl.com' + } + } + } + } + } + afterEach(() => { + nock.cleanAll() + }) + test('handles :config "x": 2 and calls the update action creator', () => { + // Given + const action = { cmd: ':config "x": 2' } + const cmdchar = ':' + const put = jest.fn() + + // When + const p = config.handleUpdateConfigCommand(action, cmdchar, put, store) + + // Then + return p.then((res) => { + expect(res).toEqual({x: 2}) + expect(put).toHaveBeenCalledWith(update({x: 2})) + }) + }) + test('handles :config "x y": 2 and calls the update action creator', () => { + // Given + const action = { cmd: ':config "x y": 2' } + const cmdchar = ':' + const put = jest.fn() + + // When + const p = config.handleUpdateConfigCommand(action, cmdchar, put, store) + + // Then + return p.then((res) => { + expect(res).toEqual({'x y': 2}) + expect(put).toHaveBeenCalledWith(update({'x y': 2})) + }) + }) + test('hints that :config x: 2 is the wrong format', () => { + // Given + const action = { cmd: ':config x: 2' } + const cmdchar = ':' + const put = jest.fn() + + // When + const p = config.handleUpdateConfigCommand(action, cmdchar, put, store) + + // Then + return expect(p) + .rejects.toEqual(new Error('Could not parse input. Usage: `:config "x": 2`. ' + 'SyntaxError: Unexpected token x in JSON at position 1')) + .then(() => expect(put).not.toHaveBeenCalled()) + }) + test('handles :config {"hej": "ho", "let\'s": "go"} and calls the replace action creator', () => { + // Given + const action = { cmd: ':config {"hej": "ho", "let\'s": "go"}' } + const cmdchar = ':' + const put = jest.fn() + + // When + const p = config.handleUpdateConfigCommand(action, cmdchar, put, store) + + // Then + return p.then((res) => { + expect(res).toEqual({hej: 'ho', "let's": 'go'}) + expect(put).toHaveBeenCalledWith(replace({hej: 'ho', "let's": 'go'})) + }) + }) + test('hints that :config {x: 1, y: 2} is the wrong format', () => { + // Given + const action = { cmd: ':config {x: 1, y: 2}' } + const cmdchar = ':' + const put = jest.fn() + + // When + const p = config.handleUpdateConfigCommand(action, cmdchar, put, store) + + // Then + return expect(p) + .rejects.toEqual(new Error('Could not parse input. Usage: `:config {"x":1,"y":"string"}`. ' + 'SyntaxError: Unexpected token x in JSON at position 1')) + .then(() => expect(put).not.toHaveBeenCalled()) + }) + test('rejects hostnames not in whitelist', () => { + // Given + const action = { cmd: ':config https://bad.com/cnf.json' } + const cmdchar = ':' + const put = jest.fn() + + // When + const p = config.handleUpdateConfigCommand(action, cmdchar, put, store) + + // Then + return expect(p) + .rejects.toEqual(new Error('Hostname is not allowed according to server whitelist')) + .then(() => expect(put).not.toHaveBeenCalled()) + }) + test('handles :config https://okurl.com/cnf.json and calls the replace action creator', () => { + // Given + const json = JSON.stringify({x: 1, y: 'hello'}) + nock('https://okurl.com').get('/cnf.json').reply(200, json) + const action = { cmd: ':config https://okurl.com/cnf.json' } + const cmdchar = ':' + const put = jest.fn() + + // When + const p = config.handleUpdateConfigCommand(action, cmdchar, put, store) + + // Then + return p.then((res) => { + expect(res).toEqual(JSON.parse(json)) + expect(put).toHaveBeenCalledWith(replace(JSON.parse(json))) + }) + }) + test('indicates error parsing remote content', () => { + // Given + const json = 'no json' + nock('https://okurl.com').get('/cnf.json').reply(200, json) + const action = { cmd: ':config https://okurl.com/cnf.json' } + const cmdchar = ':' + const put = jest.fn() + + // When + const p = config.handleUpdateConfigCommand(action, cmdchar, put, store) + + // Then + return expect(p) + .rejects.toEqual(new Error(new FetchError('invalid json response body at https://okurl.com/cnf.json reason: Unexpected token o in JSON at position 1'))) + .then(() => expect(put).not.toHaveBeenCalled()) + }) +}) From 82549a17352caddaa82bda79bfa6639576231f3b Mon Sep 17 00:00:00 2001 From: Oskar Hane Date: Tue, 20 Jun 2017 14:48:32 +0200 Subject: [PATCH 2/2] Use jsonic instead of JSON to be more relaxed on the format --- package.json | 1 + src/shared/modules/commands/helpers/config.js | 5 ++- .../modules/commands/helpers/config.test.js | 42 +++++++++++++------ yarn.lock | 4 ++ 4 files changed, 37 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index 28d47f2528e..83007547fd1 100644 --- a/package.json +++ b/package.json @@ -126,6 +126,7 @@ "file-saver": "^1.3.3", "firebase": "^3.6.5", "isomorphic-fetch": "^2.2.1", + "jsonic": "^0.3.0", "lodash.debounce": "^4.0.8", "neo4j-driver": "~1.2.0", "node-noop": "^1.0.0", diff --git a/src/shared/modules/commands/helpers/config.js b/src/shared/modules/commands/helpers/config.js index 51a52d1a754..9612ac7edcc 100644 --- a/src/shared/modules/commands/helpers/config.js +++ b/src/shared/modules/commands/helpers/config.js @@ -24,6 +24,7 @@ import { getRemoteContentHostnameWhitelist } from 'shared/modules/dbMeta/dbMetaD import { hostIsAllowed } from 'services/utils' import { getJSON } from 'services/remote' import { isValidURL } from 'shared/modules/commands/helpers/http' +import jsonic from 'jsonic' export function handleGetConfigCommand (action, cmdchar, store) { const settingsState = getSettings(store.getState()) @@ -40,7 +41,7 @@ export function handleUpdateConfigCommand (action, cmdchar, put, store) { if (!isValidURL(param)) { // Not an URL. Parse as command line params if (/^"?\{[^}]*\}"?$/.test(param)) { // JSON object string {"x": 2, "y":"string"} try { - const res = JSON.parse(param.replace(/^"/, '').replace(/"$/, '')) // Remove any surrounding quotes + const res = jsonic(param.replace(/^"/, '').replace(/"$/, '')) // Remove any surrounding quotes put(replace(res)) return resolve(res) } catch (e) { @@ -49,7 +50,7 @@ export function handleUpdateConfigCommand (action, cmdchar, put, store) { } else { // Single param try { const json = '{' + param + '}' - const res = JSON.parse(json) + const res = jsonic(json) put(update(res)) return resolve(res) } catch (e) { diff --git a/src/shared/modules/commands/helpers/config.test.js b/src/shared/modules/commands/helpers/config.test.js index a3e7d5e6df5..2c013af5673 100644 --- a/src/shared/modules/commands/helpers/config.test.js +++ b/src/shared/modules/commands/helpers/config.test.js @@ -46,6 +46,20 @@ describe('commandsDuck config helper', () => { afterEach(() => { nock.cleanAll() }) + test('fails on :config x x x and shows error hint', () => { + // Given + const action = { cmd: ':config x x x: 2' } + const cmdchar = ':' + const put = jest.fn() + + // When + const p = config.handleUpdateConfigCommand(action, cmdchar, put, store) + + // Then + return expect(p) + .rejects.toEqual(new Error('Could not parse input. Usage: `:config "x": 2`. SyntaxError: Expected ":" but "x" found.')) + .then(() => expect(put).not.toHaveBeenCalled()) + }) test('handles :config "x": 2 and calls the update action creator', () => { // Given const action = { cmd: ':config "x": 2' } @@ -61,9 +75,9 @@ describe('commandsDuck config helper', () => { expect(put).toHaveBeenCalledWith(update({x: 2})) }) }) - test('handles :config "x y": 2 and calls the update action creator', () => { + test('handles :config x: 2 and calls the update action creator', () => { // Given - const action = { cmd: ':config "x y": 2' } + const action = { cmd: ':config x: 2' } const cmdchar = ':' const put = jest.fn() @@ -72,13 +86,13 @@ describe('commandsDuck config helper', () => { // Then return p.then((res) => { - expect(res).toEqual({'x y': 2}) - expect(put).toHaveBeenCalledWith(update({'x y': 2})) + expect(res).toEqual({x: 2}) + expect(put).toHaveBeenCalledWith(update({x: 2})) }) }) - test('hints that :config x: 2 is the wrong format', () => { + test('handles :config "x y": 2 and calls the update action creator', () => { // Given - const action = { cmd: ':config x: 2' } + const action = { cmd: ':config "x y": 2' } const cmdchar = ':' const put = jest.fn() @@ -86,9 +100,10 @@ describe('commandsDuck config helper', () => { const p = config.handleUpdateConfigCommand(action, cmdchar, put, store) // Then - return expect(p) - .rejects.toEqual(new Error('Could not parse input. Usage: `:config "x": 2`. ' + 'SyntaxError: Unexpected token x in JSON at position 1')) - .then(() => expect(put).not.toHaveBeenCalled()) + return p.then((res) => { + expect(res).toEqual({'x y': 2}) + expect(put).toHaveBeenCalledWith(update({'x y': 2})) + }) }) test('handles :config {"hej": "ho", "let\'s": "go"} and calls the replace action creator', () => { // Given @@ -105,7 +120,7 @@ describe('commandsDuck config helper', () => { expect(put).toHaveBeenCalledWith(replace({hej: 'ho', "let's": 'go'})) }) }) - test('hints that :config {x: 1, y: 2} is the wrong format', () => { + test('handles :config {x: 1, y: 2} and calls the replace action creator', () => { // Given const action = { cmd: ':config {x: 1, y: 2}' } const cmdchar = ':' @@ -115,9 +130,10 @@ describe('commandsDuck config helper', () => { const p = config.handleUpdateConfigCommand(action, cmdchar, put, store) // Then - return expect(p) - .rejects.toEqual(new Error('Could not parse input. Usage: `:config {"x":1,"y":"string"}`. ' + 'SyntaxError: Unexpected token x in JSON at position 1')) - .then(() => expect(put).not.toHaveBeenCalled()) + return p.then((res) => { + expect(res).toEqual({x: 1, y: 2}) + expect(put).toHaveBeenCalledWith(replace({x: 1, y: 2})) + }) }) test('rejects hostnames not in whitelist', () => { // Given diff --git a/yarn.lock b/yarn.lock index 4457577ab16..cd671bd3a84 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3785,6 +3785,10 @@ jsonfile@^2.1.0: optionalDependencies: graceful-fs "^4.1.6" +jsonic@^0.3.0: + version "0.3.0" + resolved "https://neo.jfrog.io/neo/api/npm/npm/jsonic/-/jsonic-0.3.0.tgz#b545da95f54392e58b3dda05f5f2e377a6c9d1bf" + jsonify@~0.0.0: version "0.0.0" resolved "https://neo.jfrog.io/neo/api/npm/npm/jsonify/-/jsonify-0.0.0.tgz#2c74b6ee41d93ca51b7b5aaee8f503631d252a73"