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

Process 100, 102-199 status codes according to specs. #18033

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,37 @@ Emitted when the server sends a '100 Continue' HTTP response, usually because
the request contained 'Expect: 100-continue'. This is an instruction that
the client should send the request body.

### Event: 'information'
<!-- YAML
added: REPLACEME
-->

Emitted when the server sends a 1xx response (excluding 101 Upgrade). This
event is emitted with a callback containing an object with a status code.

```js
const http = require('http');

const options = {
hostname: '127.0.0.1',
port: 8080,
path: '/length_request'
};

// Make a request
const req = http.request(options);
req.end();

req.on('information', (res) => {
console.log('got information prior to main response: ' + res.statusCode);
});
```

101 Upgrade statuses do not fire this event due to their break from the
traditional HTTP request/response chain, such as web sockets, in-place TLS
upgrades, or HTTP 2.0. To be notified of 101 Upgrade notices, listen for the
[`'upgrade'`]() event instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change should also be added to the http2 compat API

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also explain why 101 Upgrade is not included in this

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good spot.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also an example illustrating what "This event is emitted with a object" means would be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick question: Should 101s actually fire the event too?

I excluded 101 Upgrade since this was typically used to make TLS upgrades (rare) or Web Socket connections (typical). I don't work on the Socket.IO project—for example—so I don't know the potential drawbacks. I suppose performance wouldn't suffer measurably just from firing an event with no listeners.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take that back. 101 Upgrade messages are handled so differently, they can break the request/response model dramatically. Web Sockets being a prime example but TLS as well. Leaving as is.

### Event: 'response'
<!-- YAML
added: v0.1.0
Expand Down Expand Up @@ -1382,6 +1413,14 @@ which has been transmitted are equal or not.
Attempting to set a header field name or value that contains invalid characters
will result in a [`TypeError`][] being thrown.

### response.writeProcessing()
<!-- YAML
added: REPLACEME
-->

Sends a HTTP/1.1 102 Processing message to the client, indicating that
the request body should be sent.

## Class: http.IncomingMessage
<!-- YAML
added: v0.1.17
Expand Down Expand Up @@ -1881,6 +1920,7 @@ const req = http.request(options, (res) => {
[`'checkContinue'`]: #http_event_checkcontinue
[`'request'`]: #http_event_request
[`'response'`]: #http_event_response
[`'upgrade'`]: #http_event_upgrade
[`Agent`]: #http_class_http_agent
[`Duplex`]: stream.html#stream_class_stream_duplex
[`EventEmitter`]: events.html#events_class_eventemitter
Expand Down
29 changes: 22 additions & 7 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -447,16 +447,25 @@ function socketOnData(d) {
socket.destroy();
}
} else if (parser.incoming && parser.incoming.complete &&
// When the status code is 100 (Continue), the server will
// send a final response after this client sends a request
// body. So, we must not free the parser.
parser.incoming.statusCode !== 100) {
// When the status code is informational (100, 102-199),
// the server will send a final response after this client
// sends a request body, so we must not free the parser.
// 101 (Switching Protocols) and all other status codes
// should be processed normally.
!statusIsInformational(parser.incoming.statusCode)) {
socket.removeListener('data', socketOnData);
socket.removeListener('end', socketOnEnd);
freeParser(parser, req, socket);
}
}

function statusIsInformational(status) {
// 100 (Continue) RFC7231 Section 6.2.1
// 102 (Processing) RFC2518
// 103 (Early Hints) RFC8297
// 104-199 (Unassigned)
return (status < 200 && status >= 100 && status !== 101);
}

// client
function parserOnIncomingClient(res, shouldKeepAlive) {
Expand Down Expand Up @@ -486,10 +495,16 @@ function parserOnIncomingClient(res, shouldKeepAlive) {
return 2; // Skip body and treat as Upgrade.
}

if (res.statusCode === 100) {
// restart the parser, as this is a continue message.
if (statusIsInformational(res.statusCode)) {
// Restart the parser, as this is a 1xx informational message.
req.res = null; // Clear res so that we don't hit double-responses.
req.emit('continue');
// Maintain compatibility by sending 100-specific events
if (res.statusCode === 100) {
req.emit('continue');
}
// Send information events to all 1xx responses except 101 Upgrade.
req.emit('information', { statusCode: res.statusCode });

return 1; // Skip body but don't treat as Upgrade.
}

Expand Down
4 changes: 4 additions & 0 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ ServerResponse.prototype.writeContinue = function writeContinue(cb) {
this._sent100 = true;
};

ServerResponse.prototype.writeProcessing = function writeProcessing(cb) {
this._writeRaw(`HTTP/1.1 102 Processing${CRLF}${CRLF}`, 'ascii', cb);
};

ServerResponse.prototype._implicitHeader = function _implicitHeader() {
this.writeHead(this.statusCode);
};
Expand Down
52 changes: 52 additions & 0 deletions test/parallel/test-http-information-processing.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const http = require('http');
const Countdown = require('../common/countdown');

const test_res_body = 'other stuff!\n';
const countdown = new Countdown(3, common.mustCall(() => server.close()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Countdown is already a mustCall the wrapping is therefore not necessary :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


const server = http.createServer((req, res) => {
console.error('Server sending informational message #1...');
res.writeProcessing();
console.error('Server sending informational message #2...');
res.writeProcessing();
console.error('Server sending full response...');
res.writeHead(200, {
'Content-Type': 'text/plain',
'ABCD': '1'
});
res.end(test_res_body);
});

server.listen(0, function() {
const req = http.request({
port: this.address().port,
path: '/world'
});
req.end();
console.error('Client sending request...');

let body = '';

req.on('information', function(res) {
console.error('Client got 102 Processing...');
countdown.dec();
});

req.on('response', function(res) {
assert.strictEqual(countdown.remaining, 1,
'Full response received before all 102 Processing');
assert.strictEqual(200, res.statusCode,
`Final status code was ${res.statusCode}, not 200.`);
res.setEncoding('utf8');
res.on('data', function(chunk) { body += chunk; });
res.on('end', function() {
console.error('Got full response.');
assert.strictEqual(body, test_res_body);
assert.ok('abcd' in res.headers);
countdown.dec();
});
});
});