Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for setting browser :config with objects #594

Merged
merged 2 commits into from
Jun 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
29 changes: 27 additions & 2 deletions src/shared/modules/commands/commandsDuck.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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) => {
Expand All @@ -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
Expand Down
35 changes: 25 additions & 10 deletions src/shared/modules/commands/helpers/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@
*/

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'
import { isValidURL } from 'shared/modules/commands/helpers/http'
import jsonic from 'jsonic'

export function handleGetConfigCommand (action, cmdchar, store) {
const settingsState = getSettings(store.getState())
Expand All @@ -36,25 +37,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 = jsonic(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 = jsonic(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
Expand Down
185 changes: 185 additions & 0 deletions src/shared/modules/commands/helpers/config.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
/*
* 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 <http://www.gnu.org/licenses/>.
*/

/* 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('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' }
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: 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('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('handles :config {x: 1, y: 2} and calls the replace action creator', () => {
// 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 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
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())
})
})
4 changes: 4 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down