Skip to content

Commit

Permalink
fix: avoid access internal socket state (#920)
Browse files Browse the repository at this point in the history
  • Loading branch information
ronag authored Aug 3, 2021
1 parent 82b1371 commit 1d560c4
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 21 deletions.
34 changes: 16 additions & 18 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -523,8 +523,18 @@ class Parser {
}

this.paused = false
this.socket.resume()
this.execute(EMPTY_BUF) // Flush parser.
this.execute(this.socket.read() || EMPTY_BUF) // Flush parser.
this.readMore()
}

readMore () {
while (!this.paused && this.ptr) {
const chunk = this.socket.read()
if (chunk === null) {
break
}
this.execute(chunk)
}
}

execute (data) {
Expand Down Expand Up @@ -569,7 +579,6 @@ class Parser {
this.onUpgrade(data.slice(offset))
} else if (ret === constants.ERROR.PAUSED) {
this.paused = true
socket.pause()
socket.unshift(data.slice(offset))
} else if (ret !== constants.ERROR.OK) {
const ptr = llhttp.exports.llhttp_get_error_reason(this.ptr)
Expand Down Expand Up @@ -709,7 +718,6 @@ class Parser {

assert(!socket.destroyed)
assert(socket === client[kSocket])
assert(!socket.isPaused())
assert(!this.paused)
assert(request.upgrade || request.method === 'CONNECT')

Expand All @@ -720,16 +728,6 @@ class Parser {
this.headers = []
this.headersSize = 0

// _readableState.flowing might be `true` if the socket has been
// explicitly `resume()`:d even if we never registered a 'data'
// listener.

// We need to stop unshift from emitting 'data'. However, we cannot
// call pause() as that will stop socket from automatically resuming
// when 'data' listener is registered.

// Reset socket state to non flowing:
socket._readableState.flowing = null
socket.unshift(head)

socket[kParser].destroy()
Expand All @@ -739,7 +737,7 @@ class Parser {
socket[kError] = null
socket
.removeListener('error', onSocketError)
.removeListener('data', onSocketData)
.removeListener('readable', onSocketReadable)
.removeListener('end', onSocketEnd)
.removeListener('close', onSocketClose)

Expand Down Expand Up @@ -995,9 +993,9 @@ function onParserTimeout (parser) {
}
}

function onSocketData (data) {
function onSocketReadable (data) {
const { [kParser]: parser } = this
parser.execute(data)
parser.readMore()
}

function onSocketError (err) {
Expand Down Expand Up @@ -1138,7 +1136,7 @@ function connect (client) {
socket[kClient] = client
socket
.on('error', onSocketError)
.on('data', onSocketData)
.on('readable', onSocketReadable)
.on('end', onSocketEnd)
.on('close', onSocketClose)

Expand Down
4 changes: 1 addition & 3 deletions test/client-upgrade.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ test('upgrade invalid opts', (t) => {
})

test('basic upgrade2', (t) => {
t.plan(4)
t.plan(3)

const server = http.createServer()
server.on('upgrade', (req, c, head) => {
Expand All @@ -200,8 +200,6 @@ test('basic upgrade2', (t) => {

const { headers, socket } = data

t.equal(socket._readableState.flowing, null)

let recvData = ''
data.socket.on('data', (d) => {
recvData += d
Expand Down

0 comments on commit 1d560c4

Please sign in to comment.