carlos8f / buffet

Performance-oriented static file server
http://carlos8f.github.com/node-buffet
190 stars 12 forks source link

Better maxAge handling, allows zero as value #16

Closed raiden-dev closed 10 years ago

raiden-dev commented 10 years ago

This pull request needs https://github.com/carlos8f/node-dish/pull/2

Problem is the same -- unable to achive header "Cache-Control: max-age=0". Checking for null here is because of commander's option predefined type. It will work not only with 'false' option, but anything's that not a Number.

carlos8f commented 10 years ago

already fixed with https://github.com/carlos8f/node-buffet/commit/ccf0b6db617df58101c1a5ec86194043dd2aaf55

published dish v0.1.7 and buffet v0.6.3

Cheers, Carlos

raiden-dev commented 10 years ago
typeof options.maxAge === 'undefined'

is not a correct check, actually.

In bin/buffet.js you have

.option('--maxAge <seconds>', 'value for max-age Cache-Control header (default: 300)', Number, 300)

and that option will never be undefined, because of Number here. If option not passed it will be 300 (default value), but if you pass wrong typed value, for example false (meaning not set "cache-control" header at all), option will be null (commander works this way). So options.maxAge will never be undefined.

I think it will be more clear do

 if (options.maxAge === null) options.maxAge = 300;

UPD. Nope, sorry, it will not work. Now maxAge will be 300 when false passed. The correct way to check it still in my pull request.

UPD2. Never mind, checking for undefined is bit of tricky and unclear but it will be working well. Thanks for attention.