Allow caching of CORS headers by setting Access-Control-Max-Age header #77

gnjack · 2017-07-14T09:25:00Z

Add maxAge config option. If set, an Access-Control-Max-Age header with this value (in seconds) will be added. This will allow the results of a preflight request to be cached by the browser, avoiding a preflight request being made for every request.
Defaults to not set.

Example: createServer({ maxAge: 600 })
Allow CORS preflight requests to be cached by the browser for 10 minutes.

…th this value (in seconds) will be added.

coveralls · 2017-07-14T09:27:00Z

Coverage remained the same at 100.0% when pulling 881392c on gnjack:maxAge into 70400ab on Rob--W:master.

Rob--W

Thanks for this PR! The functionality, documentation and tests look great.
If you address the few nits, then I can merge it.

Could you rename maxAge to corsMaxAge?

Rob--W · 2017-07-14T10:16:00Z

lib/cors-anywhere.js

@@ -50,8 +50,11 @@ function isValidHostName(hostname) {
 * @param headers {object} Response headers
 * @param request {ServerRequest}
 */
-function withCORS(headers, request) {
+function withCORS(headers, request, maxAge) {
  headers['access-control-allow-origin'] = '*';


Please use the following instead of passing an extra parameter to withCORS:
var maxAge = req.corsAnywhereRequestState.location;

Sure. To do this, I had to move the definition of req.corsAnywhereRequestState to before the call to withCORS in the getHandler function.

Rob--W · 2017-07-14T10:18:22Z

lib/cors-anywhere.js

@@ -234,6 +237,7 @@ function getHandler(options, proxy) {
    requireHeader: null,            // Require a header to be set?
    removeHeaders: [],              // Strip these request headers.
    setHeaders: {},                 // Set these request headers.
+    maxAge: null,                   // If set, an Access-Control-Max-Age header with this value (in seconds) will be added.


Default to 0 please, based on the following substeps from https://fetch.spec.whatwg.org/#cors-preflight-fetch

Let max-age be the result of extracting header list values given `Access-Control-Max-Age` and response’s header list.

If max-age is failure or null, then set max-age to zero.

Done. Also updated the tests and readme to note that the default behaviour is now an Access-Control-Max-Age header set to 0.

Rob--W · 2017-07-14T10:18:59Z

test/test.js

+      .get('/example.com')
+      .expect('Access-Control-Allow-Origin', '*')
+      .expect('Access-Control-Max-Age', '600')
+      .expect(200, 'Response from example.com', done);;


Replace the two ;; with one ;.

Rob--W · 2017-07-14T10:19:59Z

README.md

@@ -107,6 +107,8 @@ proxy requests. The following options are supported:
  Example: `["cookie"]`
 * dictionary of lowercase strings `setHeaders` - Set headers for the request (overwrites existing ones).  
  Example: `{"x-powered-by": "CORS Anywhere"}`
+* number `maxAge` - If set, an Access-Control-Max-Age header with this value (in seconds) will be added.


Add two spaces at the end, then markdown will show a forced line break.

And use "response header" instead of just "header".

Set corsAnywhereRequestState before calling withCORS and use the state instead of a parameter to get corsMaxAge.

gnjack · 2017-07-14T11:35:35Z

Thanks @Rob--W - I have now implemented all of the above suggested changes.

coveralls · 2017-07-14T11:35:37Z

Coverage remained the same at 100.0% when pulling 10df7c9 on gnjack:maxAge into 70400ab on Rob--W:master.

Rob--W · 2017-07-14T11:44:17Z

README.md

@@ -107,6 +107,9 @@ proxy requests. The following options are supported:
  Example: `["cookie"]`
 * dictionary of lowercase strings `setHeaders` - Set headers for the request (overwrites existing ones).  
  Example: `{"x-powered-by": "CORS Anywhere"}`
+* number `corsMaxAge` - If set, an Access-Control-Max-Age request header with this value (in seconds) will be added.
+  Defaults to 0.  


This line can be removed `"If set, ..." implies that the header is not set if not specified, which is already clear.

Rob--W · 2017-07-14T11:44:34Z

lib/cors-anywhere.js

@@ -52,6 +52,10 @@ function isValidHostName(hostname) {
 */
 function withCORS(headers, request) {
  headers['access-control-allow-origin'] = '*';
+  var corsMaxAge = request.corsAnywhereRequestState.corsMaxAge;
+  if (corsMaxAge != null) {


Change this to if (corsMaxAge) {.

Rob--W · 2017-07-14T11:45:10Z

test/test.js

@@ -50,6 +50,7 @@ describe('Basic functionality', function() {
      .get('/')
      .type('text/plain')
      .expect('Access-Control-Allow-Origin', '*')
+      .expect('Access-Control-Max-Age', '0')


The header should not be added by default. The user agent (browser) will assume 0 if not set.

Ah, sorry. I just misinterpreted your suggestion to set the default corsMaxAge to 0 instead of null to mean we should send an Access-Control-Max-Age header with the value of 0 by default.

Actually, the default corsMaxAge should be 0 which will mean no header is sent, correct?

Yes, that's correct.

coveralls · 2017-07-14T12:26:44Z

Coverage remained the same at 100.0% when pulling 49d429d on gnjack:maxAge into 70400ab on Rob--W:master.

Add maxAge config option. If set, an Access-Control-Max-Age header wi…

881392c

…th this value (in seconds) will be added.

Rob--W requested changes Jul 14, 2017

View reviewed changes

Rename maxAge to corsMaxAge and set the default corsMaxAge to 0.

10df7c9

Set corsAnywhereRequestState before calling withCORS and use the state instead of a parameter to get corsMaxAge.

Rob--W reviewed Jul 14, 2017

View reviewed changes

Do not send Access-Control-Max-Age header if corsMaxAge is 0 (default)

49d429d

Rob--W approved these changes Jul 16, 2017

View reviewed changes

Rob--W merged commit 143eff1 into Rob--W:master Jul 16, 2017

gnjack deleted the maxAge branch November 8, 2017 18:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow caching of CORS headers by setting Access-Control-Max-Age header #77

Allow caching of CORS headers by setting Access-Control-Max-Age header #77

gnjack commented Jul 14, 2017

coveralls commented Jul 14, 2017 •

edited

Loading

Rob--W left a comment

Rob--W Jul 14, 2017

gnjack Jul 14, 2017

Rob--W Jul 14, 2017

gnjack Jul 14, 2017

Rob--W Jul 14, 2017

Rob--W Jul 14, 2017

gnjack commented Jul 14, 2017

coveralls commented Jul 14, 2017 •

edited

Loading

Rob--W Jul 14, 2017

Rob--W Jul 14, 2017

Rob--W Jul 14, 2017

gnjack Jul 14, 2017

Rob--W Jul 14, 2017

coveralls commented Jul 14, 2017 •

edited

Loading

Allow caching of CORS headers by setting Access-Control-Max-Age header #77

Allow caching of CORS headers by setting Access-Control-Max-Age header #77

Conversation

gnjack commented Jul 14, 2017

coveralls commented Jul 14, 2017 • edited Loading

Rob--W left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnjack commented Jul 14, 2017

coveralls commented Jul 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jul 14, 2017 • edited Loading

coveralls commented Jul 14, 2017 •

edited

Loading

coveralls commented Jul 14, 2017 •

edited

Loading

coveralls commented Jul 14, 2017 •

edited

Loading