Fix setting inheritance

[dougwilson]

Looking for feedback on this, but a recent issue was brought up with "trust proxy" and got me reading through a bunch of old issues on here.

Personally I like having apps as isolated islands, but sometimes it's nice/makes sense to inherit some settings from the parent app when using sub apps. There was a suggestion back in #905 (comment) to add "inherit ___" settings where you would pick and choose settings to inherit. This seems kludgy to me, but I guess works?

Really, right now the way mounted sub apps work with settings is half-assed :) They pretend like they will inherit settings, unless they are special magical default Express settings. You can't have it both ways, Express!

So, what should we do for 5.0:

1. Remove setting inheritance and make it opt-in only
   a. per setting
   b. any setting
2. Fix setting inheritance to differentiate "default settings" from Express itself vs user-set settings. When sub apps are mounted, all non-user-set settings will thus be inherited (which means default Express settings would now be inherited)

[lennym]

I just came here to raise almost this exact issue. I recently mounted our app as a sub app and discovered that the x-powered-by header re-appeared despite having run app.disable('x-powered-by') in the top-level app.

I'd assumed that disabling this setting at the top level would be sufficient for it to be disabled all the way down the stack, but it looks from https://github.com/strongloop/express/blob/master/lib/middleware/init.js#L13 that the header will be added if it's not disabled at every level, which frankly sucks.

I would be strongly in favour of allowing child mounted apps to inherit parent settings, or at least make it explicit what is and what isn't inherited (since I assume from your reference to "special magical default Express settings" that some things will be inherited?).

[revington]

@lennym The problem is here https://github.com/strongloop/express/blob/master/lib/application.js#L69-L98

When the express application is created, there are some defaults set.
Then, if that application gets mounted, its settings are inherit from its parent. But it is too late because those settings are now set by default.

// Example here
var express = require('express');
var parent = express();
parent.disable('x-powered-by');
console.log(parent.set('x-powered-by')) // false, as expected
var child = express();
console.log(child.set('x-powered-by')) // true, because x-powered-by is set to true by default
parent.use(child); // mount
console.log(child.set('x-powered-by')) // true, we are using child setting
delete child.settings['x-powered-by']; 
console.log(child.set('x-powered-by')) // false, now, child does not define this setting and uses his parent one

A simple and dirty hack can be

child.on('mount', function () {             
    delete this.settings['x-powered-by'];
});
parent.use(child);

[lennym]

@dougwilson Further feedback/thoughts on settings inheritance, some of it doubtless caused by my own misapplication and misunderstanding of intent...

I've just been bitten slightly by the 'views' setting being defaulted at the instantiation of an app, where I had assumed that if not set then it would fall back to the parent app's setting.

With hindsight, the solution was to export a router instead of a full app, but because of the way that express defaulted the views directory I spent quite a lot of time debugging because the default for the child was the same path as the "grandparent" app.

I guess I'm saying that my feeling is that settings should be inherited by default - undefined settings call up to the parent, and only if no parent exists, resort to default values. So app.get effectively looks up the "app tree" to find its settings, in a not dissimilar way to how node's require searches up the file system for node_modules.

You could add app.set('inherit', false) to prevent this if required (which would of course be inherited by child apps (-;), but I would certainly say that my expectation has continually proven to be assumed inheritance.

[dougwilson]

@lennym then it sounds like what I plan to change with Express 5.0 will make sub apps and inheritance work exactly as you were expecting.

[lennym]

Excellent. Let me know if I can be of any help.

[hypesystem]

👍 on this 😄