Make it a little harder to slip in use-after-free bugs by nulling out
references to the parser object after handing it off to freeParser().
Reviewed-by: Trevor Norris <trev.norris@gmail.com>
Avoid sending unsent data and destroying otherwise legitimate sockets
for requests that are aborted while still in the agent queue. This
reduces stress on upstream systems who will likely respond to the
request but client app already knows that it will be dropped on the
floor and also helps avoid killing keep-alive connections.
Include the "expected protocol" in the Error message
string, which evaluates to "http:" for the `http`
core module, and "https:" for the `https` module.
Closes#7355.
Default to the `defaultAgent.protocol` when comparing the
user-specified `options.protocol` string. This is so that
`http.Agent` instances do not need to specify their own
`protocol` field, since we have the relevant information
already from the `defaultAgent`.
Note that the test case could be separately cherry-picked
to the `v0.10` branch, since it already passes correctly.
Fixes#7349.
Fixes the regression described in: http://git.io/2ds-WQ
For the `request()` and `get()` functions. I could never
really understand why these two functions go through agent
first... Especially since the user could be passing `agent: false`
or a different Agent instance completely, in which `globalAgent`
will be completely bypassed.
Moved the relevant logic from `Agent#request()` into the
`ClientRequest` constructor.
Incidentally, this commit fixes#7012 (which was the original
intent of this commit).
This is a slightly modified revert of bc39bdd.
Getting domains to use AsyncListeners became too much of a challenge
with many edge cases. While this is still a goal, it will have to be
deferred for now until more test coverage can be provided.
HTTP Parser instance was freed twice, leading to the reusal of it
in several different requests simultaneously.
The flow:
`socketCloseListener` is firing, which calls `socket.read()` to flush
any queued data, `socket.buffer` has data which emits and fires
`socketOnData` in sync, this triggers a parser error which frees the
parser, `socketCloseListener` resumes execution only to have the wrong
parser associated with the socket.
The fix is to only cache the parser after the flushing from the socket,
and to assert in `socketOnData` that `socket === parser.socket`
fix#6451
The domain module has been switched over to use the domain module API as
much as currently possible. There are still some hooks in the
EventEmitter, but hopefully we can remove those in the future.
When the socket closes, the client's http incoming message object was
emitting an 'aborted' event if it had not yet been ended.
However, it's possible, when a response is being repeatedly paused and
resumed (eg, if piped to a slow FS write stream), that there will be a
final chunk remaining in the js-land buffer when the socket is torn
down.
When that happens, the socketCloseListener function detects that we have
not yet reached the end of the response message data, and treats this as
an abrupt abort, immediately (and forcibly) ending the incoming message
data stream, and discarding that final chunk of data.
The result is that, for example, npm will have problems because tarballs
are missing a few bytes off the end, every time.
Closes GH-6402
In cases where the Agent has maxSockets=Infinity, and
keepAlive=false, there's no case where we won't immediately close the
connection after the response is completed.
Since we're going to close it anyway, send a `connection:close` header
rather than a `connection:keep-alive` header. Still send the
`connection:keep-alive` if the agent will actually reuse the socket,
however.
Closes#5838
Make http.request() and friends escape unsafe characters in the request
path. That is, a request for '/foo bar' is now escaped as '/foo%20bar'.
Before this commit, the path was used as-is in the request status line,
creating an invalid HTTP request ("GET /foo bar HTTP/1.1").
Fixes#4381.
We were assuming that any string can be concatenated safely to
CRLF. However, for hex, base64, or binary encoded writes, this
is not the case, and results in sending the incorrect response.
An unusual edge case, but certainly a bug.
If an http response has an 'end' handler that throws, then the socket
will never be released back into the pool.
Granted, we do NOT guarantee that throwing will never have adverse
effects on Node internal state. Such a guarantee cannot be reasonably
made in a shared-global mutable-state side-effecty language like
JavaScript. However, in this case, it's a rather trivial patch to
increase our resilience a little bit, so it seems like a win.
There is no semantic change in this case, except that some event
listeners are removed, and the `'free'` event is emitted on nextTick, so
that you can schedule another request which will re-use the same socket.
From the user's point of view, there should be no detectable difference.
Closes#5107
The benefits of the hot-path optimization below start to fall off when
the buffer size gets up near 128KB, because the cost of the copy is more
than the cost of the extra write() call. Switch to the write/end method
at that point.
Heuristics and magic numbers are awful, but slow http responses are
worse.
Fix#4975
This solves the problem of calling `readable.pipe(writable)` after the
readable stream has already emitted 'end', as often is the case when
writing simple HTTP proxies.
The spirit of streams2 is that things will work properly, even if you
don't set them up right away on the first tick.
This approach breaks down, however, because pipe()ing from an ended
readable will just do nothing. No more data will ever arrive, and the
writable will hang open forever never being ended.
However, that does not solve the case of adding a `on('end')` listener
after the stream has received the EOF chunk, if it was the first chunk
received (and thus, length was 0, and 'end' got emitted). So, with
this, we defer the 'end' event emission until the read() function is
called.
Also, in pipe(), if the source has emitted 'end' already, we call the
cleanup/onend function on nextTick. Piping from an already-ended stream
is thus the same as piping from a stream that is in the process of
ending.
Updates many tests that were relying on 'end' coming immediately, even
though they never read() from the req.
Fix#4942
Fix#4948
This adds a check before setting the incoming parser
to null. Under certain circumstances it'll already be set to
null by freeParser().
Otherwise this will cause node to crash as it tries to set
null on something that is already null.
Fix#4948
This adds a check before setting the incoming parser
to null. Under certain circumstances it'll already be set to
null by freeParser().
Otherwise this will cause node to crash as it tries to set
null on something that is already null.
This adds the following to HTTP:
* server.setTimeout(msecs, callback)
Sets all new connections to time out after the specified time, at
which point it emits 'timeout' on the server, passing the socket as an
argument.
In this way, timeouts can be handled in one place consistently.
* req.setTimeout(), res.setTimeout()
Essentially an alias to req/res.socket.setTimeout(), but without
having to delve into a "buried" object. Adds a listener on the
req/res object, but not on the socket.
* server.timeout
Number of milliseconds before incoming connections time out.
(Default=1000*60*2, as before.)
Furthermore, if the user sets up their own timeout listener on either
the server, the request, or the response, then the default behavior
(destroying the socket) is suppressed.
Fix#3460
Ability to return just the length of listeners for a given type, using
EventEmitter.listenerCount(emitter, event). This will be a lot cheaper
than creating a copy of the listeners array just to check its length.
Register the 'close' event listener with .once(), not .on().
It doesn't matter in the grand scheme of things because the listener
doesn't keep references to any heavy-weight objects but using .once()
for a oneshot listener is something of a best practice.
This makes it so that `stream.push(chunk)` is the only way to signal the
end of reading, removing the confusing disparity between the
callback-style _read method, and the fact that most real-world streams
do not have a 1:1 corollation between the "please give me data" event,
and the actual arrival of a chunk of data.
It is still possible, of course, to implement a `CallbackReadable` on
top of this. Simply provide a method like this as the callback:
function readCallback(er, chunk) {
if (er)
stream.emit('error', er);
else
stream.push(chunk);
}
However, *only* fs streams actually would behave in this way, so it
makes not a lot of sense to make TCP, TLS, HTTP, and all the rest have
to bend into this uncomfortable paradigm.
This appears to fix#4673. That bug is very hard to reproduce, so it's
hard to tell for certain, but this approach is more correct anyway.
Hat-tip: @dougwilson
Ben Noordhuis
Brian White
Nick Muerdter
Timothy J Fontaine
Kevin Decker
Yuki KAN
Nathan Rajlich
Trevor Norris
Lalit Kapoor
Fedor Indutny
isaacs
Koichi Kobayashi
hc
Eugene Girshov