Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Must/should headers, conditional range request handling, clean up #380

Closed
wants to merge 9 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

ryanrolds commented Sep 26, 2011

Hope I did this this right. The merge was kinda annoying.

Fixed a couple issue with important headers not being sent, may be going overboard now.
Date wasn't not being sent with range requests
Date header is now used to calculate age
Added Content-Length to range requests
Conditional requests with a range should return 304 if the conditions are false (RFC 2616 14.35.2)
Static cache no longer handle range requests, before it would handle them with a 200.
Added a test for conditional ranges
Fixed what appeared to be a problems with some of the range tests
Removed a console.log I forgot to remove earlier
Fixed some typos

@tj tj commented on the diff Sep 26, 2011

lib/middleware/staticCache.js
@@ -105,15 +106,16 @@ module.exports = function staticCache(options){
});
});
- // cache hit
- if (hit && hit.complete) {
+ // cache hit, doesnt support range yet
+ if (hit && hit.complete && !ranges) {
@tj

tj Sep 26, 2011

Member

i wouldn't worry too much about cached range support personally, if people are storing videos in-memory, they're crazy

@ryanrolds

ryanrolds Sep 26, 2011

Contributor

Agree

Member

tj commented Sep 30, 2011

hmm this doesn't want to apply against master

Contributor

ryanrolds commented Sep 30, 2011

Any particular message? I will take a look at this evening.

Contributor

ryanrolds commented Oct 2, 2011

Redoing the pull request. Will split it in to a requests for changes to static and changes to staticCache.

@ryanrolds ryanrolds closed this Oct 2, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment