Looking into atob functionality, consequence of Value: null

The Consul API can pass through `Value: null` which does not get cast to
a string by ember-data. This snowballs into problems with `atob` which
then tried to decode `null`.

There are 2 problems here.

1. `Value` should never be `null`
  - I've added a removeNull function to shallowly loop though props and
  remove properties that are `null`, for the moment this is only on
  single KV JSON responses - therefore `Value` will never be `null`
  which is the root of the problem

2. `atob` doesn't quite follow the `window.atob` API in that the
`window.atob` API casts everything down to a string first, therefore it
will try to decode `null` > `'null'` > `crazy unicode thing`.
  - I've commented in a fix for this, but whilst this shouldn't be
  causing anymore problems in our UI (now that `Value` is never `null`),
  I'll uncomment it in another future release. Tests are already written
  for it which more closely follow `window.atob` but skipped for now
  (next commit)
This commit is contained in:
John Cowen 2018-07-05 13:35:06 +01:00
parent a0ea46ce28
commit 4ad691fa2f
7 changed files with 130 additions and 2 deletions

View File

@ -11,6 +11,7 @@ import { get } from '@ember/object';
import { inject as service } from '@ember/service';
import keyToArray from 'consul-ui/utils/keyToArray';
import removeNull from 'consul-ui/utils/remove-null';
import { PRIMARY_KEY, SLUG_KEY } from 'consul-ui/models/kv';
import { FOREIGN_KEY as DATACENTER_KEY } from 'consul-ui/models/dc';
@ -98,7 +99,7 @@ export default Adapter.extend({
break;
case this.isQueryRecord(url):
response = {
...response[0],
...removeNull(response[0]),
...{
[PRIMARY_KEY]: this.uidForURL(url),
},

View File

@ -1,6 +1,7 @@
import TextEncoderLite from 'npm:text-encoder-lite';
import base64js from 'npm:base64-js';
export default function(str, encoding = 'utf-8') {
// str = String(str).trim();
//decode
const bytes = base64js.toByteArray(str);
return new (TextDecoder || TextEncoderLite)(encoding).decode(bytes);

View File

@ -0,0 +1,9 @@
export default function(obj) {
// non-recursive for the moment
return Object.keys(obj).reduce(function(prev, item, i, arr) {
if (obj[item] !== null) {
return { ...prev, ...{ [item]: obj[item] } };
}
return prev;
}, {});
}

View File

@ -38,6 +38,7 @@ export default function(obj, stub) {
return _super;
},
});
// TODO: try/catch this?
const actual = cb();
Object.defineProperty(Object.getPrototypeOf(obj), '_super', {
set: function(val) {

View File

@ -46,7 +46,7 @@ module('Unit | Adapter | kv', function(hooks) {
const uid = {
uid: JSON.stringify([dc, expected]),
};
const actual = adapter.handleResponse(200, {}, uid, { url: url });
const actual = adapter.handleResponse(200, {}, [uid], { url: url });
assert.deepEqual(actual, uid);
});
});

View File

@ -0,0 +1,67 @@
import { module } from 'ember-qunit';
import test from 'ember-sinon-qunit/test-support/test';
import { skip } from 'qunit';
import atob from 'consul-ui/utils/atob';
module('Unit | Utils | atob', {});
skip('it decodes non-strings properly', function(assert) {
[
{
test: ' ',
expected: '',
},
{
test: new String(),
expected: '',
},
{
test: new String('MTIzNA=='),
expected: '1234',
},
{
test: [],
expected: '',
},
{
test: [' '],
expected: '',
},
{
test: new Array(),
expected: '',
},
{
test: ['MTIzNA=='],
expected: '1234',
},
{
test: null,
expected: '<27><>e',
},
].forEach(function(item) {
const actual = atob(item.test);
assert.equal(actual, item.expected);
});
});
test('it decodes strings properly', function(assert) {
[
{
test: '',
expected: '',
},
{
test: 'MTIzNA==',
expected: '1234',
},
].forEach(function(item) {
const actual = atob(item.test);
assert.equal(actual, item.expected);
});
});
test('throws when passed the wrong value', function(assert) {
[{}, ['MTIz', 'NA=='], new Number(), 'hi'].forEach(function(item) {
assert.throws(function() {
atob(item);
});
});
});

View File

@ -0,0 +1,49 @@
import removeNull from 'consul-ui/utils/remove-null';
import { skip } from 'qunit';
import { module, test } from 'qunit';
module('Unit | Utility | remove null');
test('it removes null valued properties shallowly', function(assert) {
[
{
test: {
Value: null,
},
expected: {},
},
{
test: {
Key: 'keyname',
Value: null,
},
expected: {
Key: 'keyname',
},
},
{
test: {
Key: 'keyname',
Value: '',
},
expected: {
Key: 'keyname',
Value: '',
},
},
{
test: {
Key: 'keyname',
Value: false,
},
expected: {
Key: 'keyname',
Value: false,
},
},
].forEach(function(item) {
const actual = removeNull(item.test);
assert.deepEqual(actual, item.expected);
});
});
skip('it removes null valued properties deeply');