Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support HEAD #37

Closed
tj opened this issue Jul 13, 2009 · 10 comments
Closed

Support HEAD #37

tj opened this issue Jul 13, 2009 · 10 comments

Comments

@tj
Copy link
Member

tj commented Jul 13, 2009

No description provided.

@aviflax
Copy link

aviflax commented May 31, 2011

Why was this closed? I'd think this would be a sensible feature. I'd like to see it supported — would even consider contributing it myself, if that'd be desirable.

@tj
Copy link
Member Author

tj commented May 31, 2011

it's added where possible, however we can't easily support it everywhere, it (unfortunately) has to be part of application logic as well. For example when streaming a static file you wouldn't want to read the file into memory when you can just stat() on HEAD for the content-length etc

@tj
Copy link
Member Author

tj commented May 31, 2011

it's been added to res.send() and some other places

@aviflax
Copy link

aviflax commented May 31, 2011

That sounds promising, but it's pretty confusing that HTTPServer and HTTPSServer have get() and options() etc. but not head(). It's also just downright odd that the variable RFC2616 in router/methods.js is missing HEAD.

@tj
Copy link
Member Author

tj commented May 31, 2011

yeah the issue in my mind is that you wouldn't want to reproduce all the GET logic just for HEAD, so HEAD in the router becomes a GET. I dont see a way without hacking core node to support HEAD natively, and even then in many cases you would perform more computing than necessary. Not an easy issue for node

@tj
Copy link
Member Author

tj commented May 31, 2011

if you have any ideas let me know :D I just think this solution is better than an app.head() everywhere with duplicate logic

@aviflax
Copy link

aviflax commented May 31, 2011

TJ, I'm just getting to know Express so I suspect I'm just missing something here, I'm just bringing to the table my understanding of how I'd expect a web app framework to work — to me, sure, HEAD is generally the same as GET (except you don't send the body of course) — but sometimes when implementing an app you do want to programmatically handle HEAD differently than GET. For example if you have a more efficient way to generate the headers than generating the entire response body — which, granted, is rare.

Anyway, I'm also not sure what you mean about hacking core node to support HEAD natively. I would think it'd just work out of the box, and indeed I've coded up a quick example that seems to work correctly:

require('http').createServer(function(req, res) {
    var body = 'hello world';

    var headers = {'Content-Type': 'text/plain', 'Content-Length': body.length};

    if (req.method !== 'HEAD' && req.method !== 'OPTIONS') {
        res.writeHead(200, headers);
        res.write(body);
    } else {
        res.writeHead(204, headers);
    }

    res.end();
}).listen(1337, "127.0.0.1");
$ curl -i -X GET http://127.0.0.1:1337/
HTTP/1.1 200 OK
Content-Type: text/plain
Content-Length: 11
Connection: keep-alive

hello world
$ curl -i -X HEAD http://127.0.0.1:1337/
HTTP/1.1 204 No Content
Content-Type: text/plain
Content-Length: 11
Connection: keep-alive


I did notice that sending a 200 in response to HEAD caused the connection to stay open, I guess curl saw the Connection: keep-alive header and was waiting for the response body. Seems like a bug in curl, it should know that no body is coming back in response to HEAD. Anyway, that problem can be fixed/bypassed by either sending 204 or Connection: close — I tested both.

@tj
Copy link
Member Author

tj commented May 31, 2011

The one thing we could do, is possibly support both techniques I mentioned. So we would have app.head(), falling back on to app.get() if nothing is defined via app.head(), because the app.get() calls may be identical but simply ignore the body on HEAD, for example res.render() and res.send() handle this automatically so if you are rendering a template it will just work as-is without defining app.head()

@aviflax
Copy link

aviflax commented Jun 1, 2011

Sounds great!

Would you like me to see if I can put together a patch for that?

@tj
Copy link
Member Author

tj commented Jun 1, 2011

yeah sure if you want to

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants