Commit ecff45cf authored by Kyle Keating's avatar Kyle Keating

Add coercion map for specific host-config values

add `type-coercion.js` util with 4 methods. These are used on incoming the host-config values `openSidebar` and `requestConfigFromFrame` which may have an incompatible type upstream (via3) from what the client expects.

- toInteger
- toObject
- toString
- toBoolean

These methods coerces specific values into types so that downstream (client) methods can make assumptions. This is part of the prep work to support via3 which will eventually only export strings values in the config.
parent b96a23dc
import { toBoolean, toInteger, toObject, toString } from '../type-coercions';
describe('shared/type-coercions', () => {
describe('toBoolean', () => {
[
{
value: true,
result: true,
},
{
value: 'true',
result: true,
},
{
value: 'any',
result: true,
},
{
value: 'false',
result: false,
},
{
value: ' false',
result: false,
},
{
value: 'False',
result: false,
},
{
value: 'FALSE',
result: false,
},
{
value: '',
result: false,
},
{
value: '1',
result: true,
},
{
value: '0',
result: false,
},
{
value: 1,
result: true,
},
{
value: null,
result: false,
},
{
value: undefined,
result: false,
},
].forEach(test => {
it('coerces the values appropriately', () => {
assert.equal(toBoolean(test.value), test.result);
});
});
});
describe('toInteger', () => {
[
{
value: '1',
result: 1,
},
{
value: '0',
result: 0,
},
{
value: 1,
result: 1,
},
{
value: 1.1,
result: 1,
},
{
value: 'a',
result: NaN,
},
].forEach(test => {
it('coerces the values appropriately', () => {
assert.deepEqual(toInteger(test.value), test.result);
});
});
});
describe('toObject', () => {
[
{
value: { a: 'a', b: { c: ['c'] } },
result: { a: 'a', b: { c: ['c'] } },
},
{
value: 1,
result: {},
},
{
value: null,
result: {},
},
{
value: undefined,
result: {},
},
].forEach(test => {
it('coerces the values appropriately', () => {
assert.deepEqual(toObject(test.value), test.result);
});
});
});
describe('toString', () => {
[
{
value: 'a',
result: 'a',
},
{
value: 1,
result: '1',
},
{
value: null,
result: '',
},
{
value: undefined,
result: '',
},
{
value: {
// In a rare case where its an object with custom
// toString value that is not a function.
toString: false,
},
result: '',
},
].forEach(test => {
it('coerces the values appropriately', () => {
assert.equal(toString(test.value), test.result);
});
});
});
});
/**
* Type conversion methods that coerce incoming configuration values to an
* expected type or format that other parts of the UI may make assumptions
* on. This is needed for incoming configuration values that are otherwise
* not sanitized.
*
* Note that if the values passed are plain javascript values (such as ones
* produced from JSON.parse), then these methods do not throw errors.
*/
/**
* Returns a boolean
*
* @param {any} value - initial value
* @return {boolean}
*/
export function toBoolean(value) {
if (typeof value === 'string') {
if (value.trim().toLocaleLowerCase() === 'false') {
// "false", "False", " false", "FALSE" all return false
return false;
}
}
const numericalVal = Number(value);
if (!isNaN(numericalVal)) {
return Boolean(numericalVal);
}
// Any non numerical or falsely string should return true, otherwise return false
return typeof value === 'string';
}
/**
* Returns either an integer or NaN
*
* @param {any} value - initial value
* @return {number}
*/
export function toInteger(value) {
// Acts as a simple wrapper
return parseInt(value);
}
/**
* Returns either the value if its an object or an empty object
*
* @param {any} value - initial value
* @return {object}
*/
export function toObject(value) {
if (typeof value === 'object' && value !== null) {
return value;
}
// Don't attempt to fix the values, just ensure type correctness
return {};
}
/**
* Returns the value as a string or an empty string if the
* value undefined, null or otherwise falsely.
*
* @param {any} value - initial value
* @return {string}
*/
export function toString(value) {
if (value && typeof value.toString === 'function') {
return value.toString();
}
return '';
}
import * as queryString from 'query-string'; import * as queryString from 'query-string';
import {
toBoolean,
toInteger,
toObject,
toString,
} from '../shared/type-coercions';
/** /**
* Return the app configuration specified by the frame embedding the Hypothesis * Return the app configuration specified by the frame embedding the Hypothesis
...@@ -50,15 +56,54 @@ export default function hostPageConfig(window) { ...@@ -50,15 +56,54 @@ export default function hostPageConfig(window) {
'usernameUrl', 'usernameUrl',
]; ];
// We need to coerce incoming values from the host config for 2 reasons:
//
// 1. New versions of via may no longer support passing any type other than
// string and our client is set up to expect values that are in fact not a
// string in some cases. This will help cast these values to the expected
// type if they can be.
//
// 2. A publisher of our sidebar could accidentally pass un-sanitized values
// into the config and this ensures they safely work downstream even if they
// are incorrect.
//
// Currently we are only handling the following config values do to the fact
// that via3 will soon discontinue passing boolean types or integer types.
// - requestConfigFromFrame
// - openSidebar
//
// It is assumed we should expand this list and coerce and eventually
// even validate all such config values.
// See https://github.com/hypothesis/client/issues/1968
const coercions = {
openSidebar: toBoolean,
requestConfigFromFrame: value => {
if (typeof value === 'string') {
// Legacy `requestConfigFromFrame` value which holds only the origin.
return value;
}
const objectVal = toObject(value);
return {
origin: toString(objectVal.origin),
ancestorLevel: toInteger(objectVal.ancestorLevel),
};
},
};
return Object.keys(config).reduce(function(result, key) { return Object.keys(config).reduce(function(result, key) {
if (paramWhiteList.indexOf(key) !== -1) { if (paramWhiteList.indexOf(key) !== -1) {
// Ignore `null` values as these indicate a default value. // Ignore `null` values as these indicate a default value.
// In this case the config value set in the sidebar app HTML config is // In this case the config value set in the sidebar app HTML config is
// used. // used.
if (config[key] !== null) { if (config[key] !== null) {
if (coercions[key]) {
// If a coercion method exists, pass it through
result[key] = coercions[key](config[key]);
} else {
result[key] = config[key]; result[key] = config[key];
} }
} }
}
return result; return result;
}, {}); }, {});
} }
...@@ -39,6 +39,24 @@ describe('hostPageConfig', function() { ...@@ -39,6 +39,24 @@ describe('hostPageConfig', function() {
}); });
}); });
it('coerces `requestConfigFromFrame` and `openSidebar` values', function() {
const window_ = fakeWindow({
openSidebar: 'false',
requestConfigFromFrame: {
origin: 'origin',
ancestorLevel: '2',
},
});
assert.deepEqual(hostPageConfig(window_), {
openSidebar: false,
requestConfigFromFrame: {
origin: 'origin',
ancestorLevel: 2,
},
});
});
it('ignores non-whitelisted config params', function() { it('ignores non-whitelisted config params', function() {
const window_ = fakeWindow({ const window_ = fakeWindow({
apiUrl: 'https://not-the-hypothesis/api/', apiUrl: 'https://not-the-hypothesis/api/',
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment