Status code not set to 304 about node-ecstatic HOT 8 CLOSED

Interesting! I don't 100% understand either, but I'm glad you found this clue. If you feel like digging into it, the node.js api docs for the http module might mention the more subtle differences between res.writeHead and res.statusCode.

from node-ecstatic.

That said, I would accept a pull request if everything seemed to be in (more or less) working order.

from node-ecstatic.

Ok, I'll try to take a look next week.

On 27 Jan 2012, at 17:41, Joshua [email protected] wrote:

Interesting! I don't 100% understand either, but I'm glad you found this clue. If you feel like digging into it, the node.js api docs for the http module might mention the more subtle differences between res.writeHead and res.statusCode.

---

Reply to this email directly or view it on GitHub:
#25 (comment)

from node-ecstatic.

It seems to be related to the way that the Union module handles responses. If you take a look at this file in union

https://github.com/flatiron/union/blob/master/lib/response-stream.js

You will see that to set a response it actually uses this.response.statusCode. And if I change the code in status-handlers.js to be

res.response.statusCode = 304;

that seems to work.

But I think I prefer the res.writeHead() approach as it reads better. I'll make some changes along those lines and submit a pull request.

Cheers,
Colin

from node-ecstatic.

Interesting. I wonder if you can define the setter for res.statusCode to proxy properly... I haven't tried to do anything quite so tricksy with javascript.

Either way, I approve of the writeHead approach.

from node-ecstatic.

I'm sure you're right and union could be amended to allow the use of re.statusCode directly. But anyway I have submitted a pull request on issue 27 to use writeHead. Thanks.

from node-ecstatic.

I merged your pull request afaik, and I made corresponding changes in the v0.2.x branch.

Still, this is a bug with union imo. I'll file it. Thanks!

from node-ecstatic.

Hi,

I'm using 0.1.6 via npm. I've first checked the code and found a different fix, then I found this issue. I'm not sure this is the same thing but in effect I would have created an issue with a similar subject line, so I rather comment on this one.

My test is: request a file, and request it again. Check the headers.

Expected:

• The Request Header contains the "if-modified-since" tag
• on renewed request the status code is 304

Actual Result

• no "if-modified-since"
• status code 200

My setup looks like this:

app.use(flatiron.plugins.http, {
    before: [
        function (req, res) {
            var found = app.router.dispatch(req, res);
            if (!found) {
                res.emit('next');
            }
        },
        ecstatic(__dirname + '/public', {
            autoIndex : false,
            cache: 60000 // or "60000, must-revalidate"
        })
    ],
    after: []
});

Wenn I change the ecstatic.js code (lines 86pp) to check the 'if-modified-since' header than the status code is returned as I would expect it.

    // Return a 304 if necessary
    if ( req.headers
      && ( (req.headers['if-none-match'] === etag(stat))
        || (Date.parse(req.headers['if-none-match']) >= stat.mtime )
        || (Date.parse(req.headers['if-modified-since']) <= stat.mtime)
      )
    ) {
    console.log("sending 304");
      status[304](res, next);
    }

When I comment the date comparison of 'if-modified-since' I get back 200 every time with Firefox, if I uncomment it, I get 304.

With Safari however, I always get 200: Safari does never send 'if-modified-since'. I controlled different pages and there this header is sent and 304 is returned, so in general it can work. I've tried adding the 'Expires' header but that did not help.

EDIT:
I've checked the 0.2 branch. The respective code is there in ecstatic/serve.js.
What is the reason for 'if-modified-since' not being checked when determining whether to return 304?

EDIT:
BTW: "if-none-match" can be a list of strings, as well. See http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html

Thanks,
Chantal

from node-ecstatic.