Skip to content

Commit b2b1af6

Browse files
kumar303kumar303
authored
Tell the browser to vary its cache by DNT (Do Not Track) (#2817)
1 parent fae0a73 commit b2b1af6

File tree

10 files changed

+206
-73
lines changed

10 files changed

+206
-73
lines changed

‎config/default.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,4 +242,10 @@ module.exports = {
242242
// The amount of time (in seconds) that an auth token lives for. This is
243243
// currently only used in AMO.
244244
authTokenValidFor: null,
245+
246+
// The number of seconds the client should cache all responses for.
247+
// If null, no caching headers will be sent.
248+
// This setting is intended to simulate the Discopane's nginx cache
249+
// header for development purposes.
250+
cacheAllResponsesFor: null,
245251
};

‎config/test.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Put any test configuration overrides here.
22
module.exports = {
3+
appName: 'test-app-set-from-config',
34
apiHost: 'https://localhost',
45
allowErrorSimulation: true,
56
enableClientConsole: true,
7+
isDeployed: false,
68
};

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@
223223
"stylelint": "^8.0.0",
224224
"stylelint-config-standard": "^17.0.0",
225225
"stylelint-config-suitcss": "^10.0.0",
226+
"supertest": "^3.0.0",
226227
"svg-url-loader": "^2.0.2",
227228
"tmp": "^0.0.31",
228229
"tosource": "^1.0.0",

‎src/core/middleware/prefixMiddleware.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,12 @@ export function prefixMiddleware(req, res, next, { _config = config } = {}) {
109109
// Collect vary headers to apply to the redirect
110110
// so we can make it cacheable.
111111
// TODO: Make the redirects cacheable by adding expires headers.
112-
const varyHeaders = [];
113112
if (isLangFromHeader) {
114-
varyHeaders.push('accept-language');
113+
res.vary('accept-language');
115114
}
116115
if (isApplicationFromHeader) {
117-
varyHeaders.push('user-agent');
116+
res.vary('user-agent');
118117
}
119-
res.set('vary', varyHeaders);
120118
return res.redirect(302, newURL);
121119
}
122120

‎src/core/server/base.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,22 @@ function baseServer(routes, createStore, { appInstanceName = appName } = {}) {
199199
webpackIsomorphicTools.refresh();
200200
}
201201

202+
const cacheAllResponsesFor = config.get('cacheAllResponsesFor');
203+
if (cacheAllResponsesFor) {
204+
if (!isDevelopment) {
205+
throw new Error(oneLine`You cannot simulate the cache with
206+
the cacheAllResponsesFor config value when isDevelopment is false.
207+
In other words, we already do caching in hosted environments
208+
(via nginx) so this would be confusing!`);
209+
}
210+
log.warn(oneLine`Sending a Cache-Control header so that the client caches
211+
all requests for ${cacheAllResponsesFor} seconds`);
212+
res.set('Cache-Control', `public, max-age=${cacheAllResponsesFor}`);
213+
}
214+
215+
// Vary the cache on Do Not Track headers.
216+
res.vary('DNT');
217+
202218
match({ location: req.url, routes }, (
203219
matchError, redirectLocation, renderProps
204220
) => {

‎tests/unit/core/containers/TestServerHtml.js‎

Lines changed: 9 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,59 +1,31 @@
11
import Helmet from 'react-helmet';
22
import React from 'react';
3-
import PropTypes from 'prop-types';
43
import { findDOMNode } from 'react-dom';
54
import {
65
findRenderedDOMComponentWithTag,
76
renderIntoDocument,
87
} from 'react-addons-test-utils';
98

109
import ServerHtml from 'core/containers/ServerHtml';
10+
import FakeApp, {
11+
fakeAssets, fakeSRIData,
12+
} from 'tests/unit/core/server/fakeApp';
1113

1214
describe('<ServerHtml />', () => {
15+
const _helmentCanUseDOM = Helmet.canUseDOM;
16+
1317
beforeEach(() => {
1418
Helmet.canUseDOM = false;
1519
});
1620

21+
afterEach(() => {
22+
Helmet.canUseDOM = _helmentCanUseDOM;
23+
});
24+
1725
const fakeStore = {
1826
getState: () => ({ foo: 'bar' }),
1927
};
2028

21-
const fakeAssets = {
22-
styles: {
23-
disco: '/bar/disco-blah.css',
24-
search: '/search-blah.css',
25-
},
26-
javascript: {
27-
disco: '/foo/disco-blah.js',
28-
search: '/search-blah.js',
29-
},
30-
};
31-
32-
const fakeSRIData = {
33-
'disco-blah.css': 'sha512-disco-css',
34-
'search-blah.css': 'sha512-search-css',
35-
'disco-blah.js': 'sha512-disco-js',
36-
'search-blah.js': 'sha512-search-js',
37-
};
38-
39-
class FakeApp extends React.Component {
40-
static propTypes = {
41-
children: PropTypes.node,
42-
}
43-
44-
render() {
45-
const { children } = this.props;
46-
return (
47-
<div>
48-
<Helmet defaultTitle="test title">
49-
<meta name="description" content="test meta" />
50-
</Helmet>
51-
{children}
52-
</div>
53-
);
54-
}
55-
}
56-
5729
function render(opts = {}) {
5830
const pageProps = {
5931
appName: 'disco',

‎tests/unit/core/middleware/test_prefixMiddleware.js‎

Lines changed: 33 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ describe('Prefix Middleware', () => {
1313
redirect: sinon.stub(),
1414
set: sinon.stub(),
1515
status: () => ({ end: sinon.stub() }),
16+
vary: sinon.stub(),
1617
};
1718
fakeConfig = new Map();
1819
fakeConfig.set('validClientApplications', ['firefox', 'android']);
@@ -27,8 +28,8 @@ describe('Prefix Middleware', () => {
2728
headers: {},
2829
};
2930
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
30-
expect(fakeRes.redirect.firstCall.args).toEqual([302, '/en-US/firefox']);
31-
expect(fakeNext.called).toBeFalsy();
31+
sinon.assert.calledWith(fakeRes.redirect, 302, '/en-US/firefox');
32+
sinon.assert.notCalled(fakeNext);
3233
});
3334

3435
it('should call res.redirect if handed a locale insted of a lang', () => {
@@ -37,8 +38,8 @@ describe('Prefix Middleware', () => {
3738
headers: {},
3839
};
3940
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
40-
expect(fakeRes.redirect.firstCall.args).toEqual([302, '/en-US/firefox']);
41-
expect(fakeNext.called).toBeFalsy();
41+
sinon.assert.calledWith(fakeRes.redirect, 302, '/en-US/firefox');
42+
sinon.assert.notCalled(fakeNext);
4243
});
4344

4445
it('should add an application when missing', () => {
@@ -47,7 +48,7 @@ describe('Prefix Middleware', () => {
4748
headers: {},
4849
};
4950
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
50-
expect(fakeRes.redirect.firstCall.args).toEqual([302, '/en-US/firefox/whatever/']);
51+
sinon.assert.calledWith(fakeRes.redirect, 302, '/en-US/firefox/whatever/');
5152
});
5253

5354
it('should prepend a lang when missing but leave a valid app intact', () => {
@@ -56,8 +57,8 @@ describe('Prefix Middleware', () => {
5657
headers: {},
5758
};
5859
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
59-
expect(fakeRes.redirect.firstCall.args).toEqual([302, '/en-US/firefox/whatever']);
60-
expect(fakeRes.set.firstCall.args).toEqual(['vary', []]);
60+
sinon.assert.calledWith(fakeRes.redirect, 302, '/en-US/firefox/whatever');
61+
sinon.assert.notCalled(fakeRes.vary);
6162
});
6263

6364
it('should prepend lang when missing but keep clientAppUrlException', () => {
@@ -66,8 +67,8 @@ describe('Prefix Middleware', () => {
6667
headers: {},
6768
};
6869
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
69-
expect(fakeRes.redirect.firstCall.args).toEqual([302, '/en-US/validprefix/whatever']);
70-
expect(fakeRes.set.firstCall.args).toEqual(['vary', []]);
70+
sinon.assert.calledWith(fakeRes.redirect, 302, '/en-US/validprefix/whatever');
71+
sinon.assert.notCalled(fakeRes.vary);
7172
});
7273

7374
it('should render 404 when a localeURL exception exists', () => {
@@ -77,7 +78,7 @@ describe('Prefix Middleware', () => {
7778
};
7879
const statusSpy = sinon.spy(fakeRes, 'status');
7980
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
80-
expect(statusSpy.firstCall.args).toEqual([404]);
81+
sinon.assert.calledWith(statusSpy, 404);
8182
});
8283

8384
it('should redirect a locale exception at the root', () => {
@@ -87,9 +88,9 @@ describe('Prefix Middleware', () => {
8788
};
8889
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
8990

90-
expect(fakeRes.redirect.firstCall.args).toEqual([302,
91-
'/firefox/downloads/file/224748/my-addon-4.9.21-fx%2Bsm.xpi']);
92-
expect(fakeRes.set.firstCall.args).toEqual(['vary', ['user-agent']]);
91+
sinon.assert.calledWith(fakeRes.redirect, 302,
92+
'/firefox/downloads/file/224748/my-addon-4.9.21-fx%2Bsm.xpi');
93+
sinon.assert.calledWith(fakeRes.vary, 'user-agent');
9394
});
9495

9596
it('should redirect a locale exception nested in a valid locale', () => {
@@ -99,9 +100,9 @@ describe('Prefix Middleware', () => {
99100
};
100101
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
101102

102-
expect(fakeRes.redirect.firstCall.args).toEqual([302,
103-
'/firefox/downloads/file/224748/my-addon-4.9.21-fx%2Bsm.xpi']);
104-
expect(fakeRes.set.firstCall.args).toEqual(['vary', ['user-agent']]);
103+
sinon.assert.calledWith(fakeRes.redirect, 302,
104+
'/firefox/downloads/file/224748/my-addon-4.9.21-fx%2Bsm.xpi');
105+
sinon.assert.calledWith(fakeRes.vary, 'user-agent');
105106
});
106107

107108
it('should render a 404 when a clientApp URL exception is found', () => {
@@ -111,7 +112,7 @@ describe('Prefix Middleware', () => {
111112
};
112113
const statusSpy = sinon.spy(fakeRes, 'status');
113114
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
114-
expect(statusSpy.firstCall.args).toEqual([404]);
115+
sinon.assert.calledWith(statusSpy, 404);
115116
});
116117

117118
it('should set lang when invalid, preserving clientApp URL exception', () => {
@@ -120,8 +121,8 @@ describe('Prefix Middleware', () => {
120121
headers: {},
121122
};
122123
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
123-
expect(fakeRes.redirect.firstCall.args).toEqual([302, '/en-US/developers/']);
124-
expect(fakeRes.set.firstCall.args).toEqual(['vary', []]);
124+
sinon.assert.calledWith(fakeRes.redirect, 302, '/en-US/developers/');
125+
sinon.assert.notCalled(fakeRes.vary);
125126
});
126127

127128
it('should render a 404 with clientApp exception', () => {
@@ -131,7 +132,7 @@ describe('Prefix Middleware', () => {
131132
};
132133
const statusSpy = sinon.spy(fakeRes, 'status');
133134
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
134-
expect(statusSpy.firstCall.args).toEqual([404]);
135+
sinon.assert.calledWith(statusSpy, 404);
135136
});
136137

137138
it('should fallback to and vary on accept-language headers', () => {
@@ -142,8 +143,8 @@ describe('Prefix Middleware', () => {
142143
},
143144
};
144145
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
145-
expect(fakeRes.redirect.firstCall.args).toEqual([302, '/pt-BR/firefox/whatever']);
146-
expect(fakeRes.set.firstCall.args[1]).toEqual(['accept-language']);
146+
sinon.assert.calledWith(fakeRes.redirect, 302, '/pt-BR/firefox/whatever');
147+
sinon.assert.calledWith(fakeRes.vary, 'accept-language');
147148
});
148149

149150
it('should map aliased langs', () => {
@@ -152,7 +153,7 @@ describe('Prefix Middleware', () => {
152153
headers: {},
153154
};
154155
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
155-
expect(fakeRes.redirect.firstCall.args).toEqual([302, '/pt-PT/firefox/whatever']);
156+
sinon.assert.calledWith(fakeRes.redirect, 302, '/pt-PT/firefox/whatever');
156157
});
157158

158159
it('should vary on accept-language and user-agent', () => {
@@ -163,8 +164,9 @@ describe('Prefix Middleware', () => {
163164
},
164165
};
165166
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
166-
expect(fakeRes.redirect.firstCall.args).toEqual([302, '/pt-BR/firefox/whatever']);
167-
expect(fakeRes.set.firstCall.args[1]).toEqual(['accept-language', 'user-agent']);
167+
sinon.assert.calledWith(fakeRes.redirect, 302, '/pt-BR/firefox/whatever');
168+
sinon.assert.calledWith(fakeRes.vary, 'accept-language');
169+
sinon.assert.calledWith(fakeRes.vary, 'user-agent');
168170
});
169171

170172
it('should find the app based on ua string', () => {
@@ -175,8 +177,8 @@ describe('Prefix Middleware', () => {
175177
},
176178
};
177179
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
178-
expect(fakeRes.redirect.firstCall.args).toEqual([302, '/en-US/android/whatever']);
179-
expect(fakeRes.set.firstCall.args[1]).toEqual(['user-agent']);
180+
sinon.assert.calledWith(fakeRes.redirect, 302, '/en-US/android/whatever');
181+
sinon.assert.calledWith(fakeRes.vary, 'user-agent');
180182
});
181183

182184
it('should populate res.locals for a valid request', () => {
@@ -187,7 +189,7 @@ describe('Prefix Middleware', () => {
187189
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
188190
expect(fakeRes.locals.lang).toEqual('en-US');
189191
expect(fakeRes.locals.clientApp).toEqual('firefox');
190-
expect(fakeRes.redirect.called).toBeFalsy();
192+
sinon.assert.notCalled(fakeRes.redirect);
191193
});
192194

193195
it('should not populate res.locals for a redirection', () => {
@@ -198,7 +200,7 @@ describe('Prefix Middleware', () => {
198200
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
199201
expect(fakeRes.locals.lang).toEqual(undefined);
200202
expect(fakeRes.locals.clientApp).toEqual(undefined);
201-
expect(fakeRes.redirect.called).toBeTruthy();
203+
sinon.assert.called(fakeRes.redirect);
202204
});
203205

204206
it('should not mangle a query string for a redirect', () => {
@@ -207,6 +209,7 @@ describe('Prefix Middleware', () => {
207209
headers: {},
208210
};
209211
prefixMiddleware(fakeReq, fakeRes, fakeNext, { _config: fakeConfig });
210-
expect(fakeRes.redirect.firstCall.args).toEqual([302, '/en-US/firefox/foo/bar?test=1&bar=2']);
212+
sinon.assert.calledWith(fakeRes.redirect, 302,
213+
'/en-US/firefox/foo/bar?test=1&bar=2');
211214
});
212215
});

‎tests/unit/core/server/fakeApp.js‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import PropTypes from 'prop-types';
2+
import React from 'react';
3+
import Helmet from 'react-helmet';
4+
5+
export const fakeAssets = {
6+
styles: {
7+
disco: '/bar/disco-blah.css',
8+
search: '/search-blah.css',
9+
},
10+
javascript: {
11+
disco: '/foo/disco-blah.js',
12+
search: '/search-blah.js',
13+
},
14+
};
15+
16+
export const fakeSRIData = {
17+
'disco-blah.css': 'sha512-disco-css',
18+
'search-blah.css': 'sha512-search-css',
19+
'disco-blah.js': 'sha512-disco-js',
20+
'search-blah.js': 'sha512-search-js',
21+
};
22+
23+
export default class FakeApp extends React.Component {
24+
static propTypes = {
25+
children: PropTypes.node,
26+
}
27+
28+
render() {
29+
const { children } = this.props;
30+
return (
31+
<div>
32+
<Helmet defaultTitle="test title">
33+
<meta name="description" content="test meta" />
34+
</Helmet>
35+
{children}
36+
</div>
37+
);
38+
}
39+
}

0 commit comments

Comments
 (0)