expressjs / response-time

Response time header for node.js
MIT License
472 stars 73 forks source link

Expose X-Response-Time header by default #10

Closed mujz closed 7 years ago

mujz commented 7 years ago

This is definitely debatable, but in my opinion, X-Response-Time header should be added to the Access-Control-Expose-Headers header. My rationale for this is that if somebody is using this package, they are explicitly asking you to expose the X-Response-Time header, therefore, you need to explicitly tell the browser that this header is safe. This can be done with one line of code:

res.header('Access-Control-Expose-Headers', 'X-Response-Time');
dougwilson commented 7 years ago

I agree, but it doesn't belong in this module. There are two problems with that code above in general:

  1. It will stop over whatever someone has already set for 'Access-Control-Expose-Headers' which means it will cause a lot of headaches.
  2. Unconditionally sending that header on every response is a violation of the CORS spec, so in order to get it right, all the cors logic would have to be built directly into this module.

I think it is reasonable that anyone who uses a cors module would realize they want to expose this header and configure their cors module to do so.

mujz commented 7 years ago

Great response! Thanks a lot for the explanation!