Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

HTTP POST / PUT race condition when using it with Expect: 100-continue #2636

Closed
SaltwaterC opened this Issue · 6 comments

4 participants

@SaltwaterC

I managed to create a script that reproduces this. However, I didn't manage to create a self contained one (maybe due to laziness), but here it goes:

var http = require('http')
var qs = require('querystring')
var reqBody = qs.stringify({foo: 'bar'})
var opt = {
        method: 'POST',
        host: '127.0.0.1',
        path: '/index.php',
        headers: {
                expect: '100-continue',
                'content-length': reqBody.length,
                'content-type': 'application/x-www-form-urlencoded'
        }
}
var req = http.request(opt, function (res) {
        res.on('data', function (data) {
                console.log(data.toString())
        })
        res.on('end', function () {
                console.log('end')
        })
})
req.on('error', function (err) {
        console.error(err)
})
req.write(reqBody)
req.end()

The remote script is a simple:

<?php
var_dump($_REQUEST);

Here's what's logged into the shell:

$ node php.js 

http.js:1147
      var ret = parser.execute(d, start, end - start);
                       ^
TypeError: Cannot call method 'execute' of null
    at Socket.ondata (http.js:1147:24)
    at TCP.onread (net.js:354:27)
$ node php.js 

http.js:1147
      var ret = parser.execute(d, start, end - start);
                       ^
TypeError: Cannot call method 'execute' of null
    at Socket.ondata (http.js:1147:24)
    at TCP.onread (net.js:354:27)
$ node php.js 
array(1) {
  ["foo"]=>
  string(3) "bar"
}

end
$ node -v
v0.6.9

Couple of failures and a success. For the same script. For a larger HTTP client implementation, it fails all the time. Here's the Wireshark capture: https://gist.github.com/1700091

Without Expect: 100-continue everything is peachy.

@bnoordhuis

Confirmed.

@bnoordhuis

@koichik: Can you take a look at this? It seems to be a regression that was introduced in 35fe3eb.

@jnak

+1

@koichik koichik referenced this issue from a commit in koichik/node
@koichik koichik http: fix free http-parser too early
when the response code is 100 (Continue).

Fixes #2636.
f36b0af
@koichik

@SaltwaterC - Thanks for the report.

@bnoordhuis - Can you review koichik/node@f36b0af?

@koichik koichik closed this issue from a commit
@koichik koichik http: fix free http-parser too early
when the status code is 100 (Continue).

Fixes #2636.
3fd13c6
@koichik koichik closed this in 3fd13c6
@koichik

@bnoordhuis - Thanks, merged in 3fd13c6.

@isaacs isaacs referenced this issue from a commit
@isaacs isaacs 2012.02.01, Version 0.7.2 (unstable)
* Update V8 to 3.8.9

* Support for sharing streams across Isolates (Igor Zinkovsky)

* #2636 - Fix case where http_parsers are freed too early (koichik)

* url: Support for IPv6 addresses in URLs (Łukasz Walukiewicz)

* child_process: Add disconnect() method to child processes (Andreas Madsen)

* fs: add O_EXCL support, exclusive open file (Ben Noordhuis)

* fs: more specific error messages (Tj Holowaychuk)

* tty: emit 'unknown' key event if key sequence not found (Dan VerWeire, Nathan Rajlich)

* build: compile release build too if BUILDTYPE=Debug (Ben Noordhuis)

* module: fix --debug-brk on symlinked scripts (Fedor Indutny)

* zlib: fix `Failed to set dictionary` issue (Fedor Indutny)

* waf: predict target arch for OS X (Fedor Indutny)
a3efcd2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.