Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix pipelined requests with content-length header. #1

Merged
merged 1 commit into from

2 participants

@sethml

When a request includes a Content-length header, it does not include a
terminating CRLF after the content - the next request begins immediately.
This change prevents us from misparsing the next request.

My brain's parser is not RFC-compliant enough to manage to extract this information from the RFC, but a quick test with each of Chrome 15, FireFox 8, and Safari 5 show this behavior, so I'm reasonably confident that it's correct.

@sethml sethml Fix pipelined requests with content-length header.
When a request includes a Content-length header, it does not include a
terminating CRLF after the content - the next request begins immediately.
This change prevents us from misparsing the next request.
e65957e
@sethml

For posterity, here's my little test script - save this in a .html file, then point the browser to it with a file: URL:

<!DOCTYPE html>
<html>
<head>
  <script src="http://code.jquery.com/jquery-latest.js"></script>
</head>
<body>
<script>
  for (var i = 0; i < 10; ++i) {
    $.post('http://localhost:8888/', '0123456789');
  }
</script>
</body>
</html>

Here's the node server side:

var http = require('http');
var debug = require('util').debug; 

var server = http.createServer();
var connectionCount = 0;
server.on('connection', function(socket) {
  var connectionNumber = connectionCount++;
  debug('connection ' + connectionNumber);
  socket.on('data', function(chunk) {
    debug(connectionNumber + ' Got chunk: ' + chunk.toString());
  });
  socket.on('end', function() {
    debug(connectionNumber + ' Socket end.');
  });
});
server.on('request', function(req, res) {
  res.end('response!');
});
server.listen(8888);
@substack substack merged commit e65957e into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 11, 2011
  1. @sethml

    Fix pipelined requests with content-length header.

    sethml authored
    When a request includes a Content-length header, it does not include a
    terminating CRLF after the content - the next request begins immediately.
    This change prevents us from misparsing the next request.
This page is out of date. Refresh to see the latest.
Showing with 3 additions and 3 deletions.
  1. +2 −1  lib/modes.js
  2. +1 −2  test/keep_alive_post.js
View
3  lib/modes.js
@@ -357,7 +357,8 @@ exports.readLen = function (buf, start, len) {
if (this._pendingBytes === 0) {
this.request.emit('end');
- this.mode = 'lastCRLF';
+ req.emit('rawEnd');
+ this.mode = this._shouldKeepAlive ? 'begin' : 'finished';
}
return start + slice.length;
View
3  test/keep_alive_post.js
@@ -73,8 +73,7 @@ test('keep-alive post', function (t) {
'Host: second.beep',
'Content-Length: 10',
'',
- '0123456789',
- 'POST /third HTTP/1.1',
+ '0123456789POST /third HTTP/1.1',
'Host: third.beep',
'Transfer-Encoding: chunked',
'',
Something went wrong with that request. Please try again.