Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Properly respond to HTTP HEAD requests during end(body) hot path #1291

Closed
wants to merge 5 commits into from

2 participants

@reid

During write(), _hasBody is checked to make sure a body is allowed -- this is now also checked during end(body)'s hot path when write() isn't used.

Without this patch, it's possible that res.end("foo") can send a malformed HEAD response that http_parser rejects.

Includes a new test.

reid added some commits
@reid reid Properly respond to HEAD during end(body) hot path
During write(), _hasBody is checked to make sure a body
is allowed -- this is now also checked during end(body)
when write() isn't used.
95a6f40
@reid reid Correct quote style. 4a0a8c6
@reid reid Correct quote style for test. 776c25c
@reid reid Concise final chunk for HEAD req's res.end(data).
Instead of simply clearing data, check _hasBody
earlier to avoid sending cruft when chunkedEncoding
is used.
f7d7132
@reid reid Merge branch 'master' into http-end-head d8617f5
@koichik koichik closed this pull request from a commit
@reid reid Properly respond to HEAD during end(body) hot path
During write(), _hasBody is checked to make sure a body
is allowed -- this is now also checked during end(body)
when write() isn't used.

Concise final chunk for HEAD req's res.end(data).

Instead of simply clearing data, check _hasBody
earlier to avoid sending cruft when chunkedEncoding
is used.

Fixes #1291.
973153d
@koichik koichik closed this in 973153d
@koichik
Owner

I squashed these commits into a single commit.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 8, 2011
  1. @reid

    Properly respond to HEAD during end(body) hot path

    reid authored
    During write(), _hasBody is checked to make sure a body
    is allowed -- this is now also checked during end(body)
    when write() isn't used.
  2. @reid

    Correct quote style.

    reid authored
  3. @reid

    Correct quote style for test.

    reid authored
Commits on Jul 11, 2011
  1. @reid

    Concise final chunk for HEAD req's res.end(data).

    reid authored
    Instead of simply clearing data, check _hasBody
    earlier to avoid sending cruft when chunkedEncoding
    is used.
  2. @reid
This page is out of date. Refresh to see the latest.
View
7 lib/http.js
@@ -654,6 +654,12 @@ OutgoingMessage.prototype.end = function(data, encoding) {
var ret;
+ if (data && !this._hasBody) {
+ console.error('This type of response MUST NOT have a body. ' +
+ 'Ignoring data passed to end().');
+ data = false;
+ }
+
var hot = this._headerSent === false &&
typeof(data) === 'string' &&
data.length > 0 &&
@@ -667,6 +673,7 @@ OutgoingMessage.prototype.end = function(data, encoding) {
// res.writeHead();
// res.end(blah);
// HACKY.
+
if (this.chunkedEncoding) {
var l = Buffer.byteLength(data, encoding).toString(16);
ret = this.connection.write(this._header + l + CRLF +
View
55 test/simple/test-http-head-response-has-no-body-end.js
@@ -0,0 +1,55 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+
+var http = require('http');
+
+// This test is to make sure that when the HTTP server
+// responds to a HEAD request with data to res.end,
+// it does not send any body.
+
+var server = http.createServer(function(req, res) {
+ res.writeHead(200);
+ res.end('FAIL'); // broken: sends FAIL from hot path.
+});
+server.listen(common.PORT);
+
+var responseComplete = false;
+
+server.addListener('listening', function() {
+ var req = http.createClient(common.PORT).request('HEAD', '/');
+ common.error('req');
+ req.end();
+ req.addListener('response', function(res) {
+ common.error('response');
+ res.addListener('end', function() {
+ common.error('response end');
+ server.close();
+ responseComplete = true;
+ });
+ });
+});
+
+process.addListener('exit', function() {
+ assert.ok(responseComplete);
+});
Something went wrong with that request. Please try again.