forked from hashicorp/consul
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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)
- Loading branch information
John Cowen
committed
Jul 5, 2018
1 parent
6a407a0
commit b29546e
Showing
7 changed files
with
130 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; | ||
}, {}); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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: '��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); | ||
}); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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'); |