$base with arrays is merged with (not overridden by) filtered value

[dpmott]

Consider this example code:

const Confidence = require('confidence');
const store = new Confidence.Store();

store.load({ cors: { $filter: 'env', $base: { origins: { $value: 'a' } }, $default: { origins: { $value: 'b' } }, dev: {}, } });
console.log('Expected: b, Actual: ', store.get('/cors/origins'));

store.load({ cors: { $filter: 'env', $base: { origins: { $value: { blah: 'a' } } }, $default: { origins: { $value: { blah: 'b' } } }, dev: {}, } });
console.log('Expected: { blah: \'b\' }, Actual: ', store.get('/cors/origins'));

store.load({ cors: { origins: { $filter: 'env', $base: { $value: ['a'] }, $default: { $value: ['b'] }, dev: {}, } } } );
console.log('Expected: [ \'b\' ], Actual: ', store.get('/cors/origins'));

store.load({ cors: { $filter: 'env', $base: { origins: { $value: ['a'] } }, $default: { origins: { $value: ['b'] } }, dev: {}, } });
console.log('Expected: [ \'b\' ], Actual: ', store.get('/cors/origins'));

store.load({ cors: { $filter: 'env', $base: { origins: ['a'] }, $default: { origins: ['b'] }, dev: {}, } });
console.log('Expected: [ \'b\' ], Actual: ', store.get('/cors/origins'));

Output:

Expected: b, Actual: b
Expected: { blah: 'b' }, Actual: { blah: 'b' }
Expected: [ 'b' ], Actual: [ 'b' ]
Expected: [ 'b' ], Actual: [ 'a', 'b' ]
Expected: [ 'b' ], Actual: [ 'a', 'b' ]

The last two examples don't work as expected.

Note: My current work-around is to move the 'origins' key up a layer, as shown in the third example.

[patrickkettner]

hey @dpmott!
That is actually the intended logic. The point is to not have to build repetitive arrays.

[dpmott]

@patrickkettner thanks for the reply & the link to the implementation. This behavior is definitely not made clear by the docs, nor are there any examples from which this behavior could be inferred.

As you might have guessed from the naming of my config fields, I was trying to put together a list of domains to use with CORS (Cross-Origin-Resource-Sharing). The unexpected combination of array items was unexpected, and might have introduced a security bug in my project.

Might you consider an update to the docs, to be more clear about how arrays are merged in this scenario, perhaps in the vicinity of here?

[patrickkettner]

hey @dpmott
Terribly sorry for the poor documentation on the matter. I am actually curious as to why you were using the $base value at all, as opposed to $default. Want to make sure I understand the misunderstanding before I go about trying to properly document it

[dpmott]

Well, I'm using both $base and $default.

$base provides the base set of values. $default provides (fields in addition to, and/or overriding existing, fields in $base) values when no other configuration matches the filter criteria.

So, I expected that if I specified a field in $default, it would override the field in $base. That merging behavior doesn't happen for strings or objects - just arrays - which makes it unexpected and perhaps difficult to reason about.

Here's a real-world example:

  crossOriginResourceSharing: {
    $filter: 'env',
    $base: {
      origins: {
        $value: (process.env.CORS || '').split(/\s*,\s*/),
      },
    },
    $default: {
      origins: {
        $value: (process.env.CORS || '*').split(/\s*,\s*/),
      },
    },
    dev: {},
    stage: {},
    prod: {},
  },

In this example: dev, stage, and prod configurations should (with CORS environment variable unset) configure crossOriginResourceSharing.origins to have [ '' ], and if the 'env' filter is unset (as it often is on a local dev machine), crossOriginResourceSharing.origins should be configured to have [ '*' ].

However, this code actually results in crossOriginResourceSharing.origins being configured as [ '', '*' ], which is (thankfully) an error for Hapi server, when used in its connection configuration:

{ routes: { cors: { origin: Config.get('/crossOriginResourceSharing/origins') } }, }

(See https://hapijs.com/api#route-options for docs on Hapi server's route options.)

When deployed, CORS would be defined in the environment to yield a valid string array of origins, while developers could run with no environment variable and have things "just work" on their local dev machines.

[augnin]

with #94 merged, guess we can close this issue now