Skip to content

Commit

Permalink
http: improves expect header handling
Browse files Browse the repository at this point in the history
Now returns a 417 error status or allows for an event listener on
the `checkExpectation` event. Before we were ignoring requests that
had misspelled `100-continue` values for expect headers.

This is a quick port of the work done here:
nodejs/node-v0.x-archive#7132 by alFReD-NSH
with surrounding discussion here:
nodejs/node-v0.x-archive#4651

Also updates all the instances of the deprecated
EventEmitter.listenerCount to the current self.listenerCount. Most
of these were in the new code ported over but there was another
legacy instance.

Refs: #2403
PR-URL: #4501
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
designfrontier authored and evanlucas committed Jan 18, 2016
1 parent 2879395 commit c171294
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 9 deletions.
11 changes: 11 additions & 0 deletions doc/api/http.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,17 @@ The request implements the [Writable Stream][] interface. This is an
Emitted when the request has been aborted by the client. This event is only
emitted on the first call to `abort()`.

### Event: 'checkExpectation'

`function (request, response) { }`

Emitted each time a request with an http Expect header is received, where the
value is not 100-continue. If this event isn't listened for, the server will
automatically respond with a 417 Expectation Failed as appropriate.

Note that when this event is emitted and handled, the `request` event will
not be emitted.

### Event: 'connect'

`function (response, socket, head) { }`
Expand Down
26 changes: 17 additions & 9 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

const util = require('util');
const net = require('net');
const EventEmitter = require('events');
const HTTPParser = process.binding('http_parser').HTTPParser;
const assert = require('assert').ok;
const common = require('_http_common');
Expand Down Expand Up @@ -391,7 +390,7 @@ function connectionListener(socket) {
parser = null;

var eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade';
if (EventEmitter.listenerCount(self, eventName) > 0) {
if (self.listenerCount(eventName) > 0) {
debug('SERVER have listener for %s', eventName);
var bodyHead = d.slice(bytesParsed, d.length);

Expand Down Expand Up @@ -516,14 +515,23 @@ function connectionListener(socket) {
}

if (req.headers.expect !== undefined &&
(req.httpVersionMajor == 1 && req.httpVersionMinor == 1) &&
continueExpression.test(req.headers['expect'])) {
res._expect_continue = true;
if (EventEmitter.listenerCount(self, 'checkContinue') > 0) {
self.emit('checkContinue', req, res);
(req.httpVersionMajor == 1 && req.httpVersionMinor == 1)) {
if (continueExpression.test(req.headers.expect)) {
res._expect_continue = true;

if (self.listenerCount('checkContinue') > 0) {
self.emit('checkContinue', req, res);
} else {
res.writeContinue();
self.emit('request', req, res);
}
} else {
res.writeContinue();
self.emit('request', req, res);
if (self.listenerCount('checkExpectation') > 0) {
self.emit('checkExpectation', req, res);
} else {
res.writeHead(417);
res.end();
}
}
} else {
self.emit('request', req, res);
Expand Down
55 changes: 55 additions & 0 deletions test/parallel/test-http-expect-handling.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Spec documentation http://httpwg.github.io/specs/rfc7231.html#header.expect
'use strict';
const common = require('../common');
const assert = require('assert');
const http = require('http');

const tests = [417, 417];

let testsComplete = 0;
let testIdx = 0;

const s = http.createServer(function(req, res) {
throw new Error('this should never be executed');
});

s.listen(common.PORT, nextTest);

function nextTest() {
const options = {
port: common.PORT,
headers: { 'Expect': 'meoww' }
};

if (testIdx === tests.length) {
return s.close();
}

const test = tests[testIdx];

if (testIdx > 0) {
s.on('checkExpectation', common.mustCall((req, res) => {
res.statusCode = 417;
res.end();
}));
}

http.get(options, function(response) {
console.log('client: expected status: ' + test);
console.log('client: statusCode: ' + response.statusCode);
assert.equal(response.statusCode, test);
assert.equal(response.statusMessage, 'Expectation Failed');

response.on('end', function() {
testsComplete++;
testIdx++;
nextTest();
});
response.resume();
});
}


process.on('exit', function() {
assert.equal(2, testsComplete);
});

0 comments on commit c171294

Please sign in to comment.