Skip to content

Scrape implementation for websocket. Issue #116 #125

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.

Sign up for GitHub

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

Merged
feross merged 1 commit into from
Mar 16, 2016
Merged

Scrape implementation for websocket. Issue #116 #125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
55 changes: 54 additions & 1 deletion lib/client/websocket-tracker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ WebSocketTracker.prototype.announce = function (opts) {

self._generateOffers(numwant, function (offers) {
var params = extend(opts, {
action: 'announce',
numwant: numwant,
info_hash: self.client._infoHashBinary,
peer_id: self.client._peerIdBinary,
Expand All @@ -61,7 +62,21 @@ WebSocketTracker.prototype.announce = function (opts) {
WebSocketTracker.prototype.scrape = function (opts) {
var self = this
if (self.destroyed || self.reconnecting) return
self._onSocketError(new Error('scrape not supported ' + self.announceUrl))
if (!self.socket.connected) {
return self.socket.once('connect', self.scrape.bind(self, opts))
}

var infoHashes = (Array.isArray(opts.infoHash) && opts.infoHash.length > 0)
? opts.infoHash.map(function (infoHash) {
return infoHash.toString('binary')
})
: (opts.infoHash && opts.infoHash.toString('binary')) || self.client._infoHashBinary
var params = {
action: 'scrape',
info_hash: infoHashes
}

self._send(params)
}

WebSocketTracker.prototype.destroy = function (onclose) {
Expand Down Expand Up @@ -133,6 +148,18 @@ WebSocketTracker.prototype._onSocketData = function (data) {
return
}

if (data.action === common.ACTIONS.ANNOUNCE || data.offer || data.answer) {
Copy link
Member

@feross feross Mar 16, 2016

Choose a reason for hiding this comment

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

common.ACTIONS is defined in common-node.js, so it is excluded from browser builds by this line in package.json: https://github.com/feross/bittorrent-tracker/blob/7daf7ec2974f6a8de6b25ca01f75f8db82ce7f3e/package.json#L14 There should have been better code comments to explain this.

We should just use the string 'announce' because that's simpler and we're using JSON after all.

self._onAnnounceResponse(data)
} else if (data.action === common.ACTIONS.SCRAPE) {
self._onScrapeResponse(data)
} else {
throw new Error('invalid action in WS response: ' + data.action)
Copy link
Member

@feross feross Mar 16, 2016

Choose a reason for hiding this comment

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

Just noticed this: Throwing is a bad idea. That means that any tracker that sends back an invalid response will cause an uncaught exception in the client. This should just call self._onSocketError

}
}

WebSocketTracker.prototype._onAnnounceResponse = function (data) {
var self = this

if (data.info_hash !== self.client._infoHashBinary) {
debug(
'ignoring websocket data from %s for %s (looking for %s: reused socket)',
Expand Down Expand Up @@ -215,6 +242,32 @@ WebSocketTracker.prototype._onSocketData = function (data) {
}
}

WebSocketTracker.prototype._onScrapeResponse = function (data) {
var self = this
// NOTE: the unofficial spec says to use the 'files' key, 'host' has been
// seen in practice
data = data.files || data.host || {}
Copy link
Member

@feross feross Mar 16, 2016

Choose a reason for hiding this comment

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

We should just use files since this is the first implementation. We don't need to support hacks for bad tracker servers.


var keys = Object.keys(data)
if (keys.length === 0) {
self.client.emit('warning', new Error('invalid scrape response'))
return
}

keys.forEach(function (infoHash) {
var response = data[infoHash]
// TODO: optionally handle data.flags.min_request_interval
// (separate from announce interval)
self.client.emit('scrape', {
announce: self.announceUrl,
infoHash: common.binaryToHex(infoHash),
complete: response.complete,
incomplete: response.incomplete,
downloaded: response.downloaded
})
})
}

WebSocketTracker.prototype._onSocketClose = function () {
var self = this
if (self.destroyed) return
Expand Down
58 changes: 38 additions & 20 deletions lib/server/parse-websocket.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,51 @@ function parseWebSocketRequest (socket, opts, params) {
if (!opts) opts = {}
params = JSON.parse(params) // may throw

params.action = common.ACTIONS.ANNOUNCE
params.type = 'ws'
params.socket = socket
if (params.action === 'announce' || params.answer || params.offers) {
Copy link
Member

@feross feross Mar 16, 2016

Choose a reason for hiding this comment

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

nit pick: put offers first

params.action = common.ACTIONS.ANNOUNCE

if (typeof params.info_hash !== 'string' || params.info_hash.length !== 20) {
throw new Error('invalid info_hash')
}
params.info_hash = common.binaryToHex(params.info_hash)
if (typeof params.info_hash !== 'string' || params.info_hash.length !== 20) {
throw new Error('invalid info_hash')
}
params.info_hash = common.binaryToHex(params.info_hash)

if (typeof params.peer_id !== 'string' || params.peer_id.length !== 20) {
throw new Error('invalid peer_id')
}
params.peer_id = common.binaryToHex(params.peer_id)
if (typeof params.peer_id !== 'string' || params.peer_id.length !== 20) {
throw new Error('invalid peer_id')
}
params.peer_id = common.binaryToHex(params.peer_id)

if (params.answer) {
if (typeof params.to_peer_id !== 'string' || params.to_peer_id.length !== 20) {
throw new Error('invalid `to_peer_id` (required with `answer`)')
if (params.answer) {
if (typeof params.to_peer_id !== 'string' || params.to_peer_id.length !== 20) {
throw new Error('invalid `to_peer_id` (required with `answer`)')
}
params.to_peer_id = common.binaryToHex(params.to_peer_id)
}
params.to_peer_id = common.binaryToHex(params.to_peer_id)
}

params.left = Number(params.left) || Infinity
params.numwant = Math.min(
Number(params.offers && params.offers.length) || 0, // no default - explicit only
common.MAX_ANNOUNCE_PEERS
)
params.compact = -1 // return full peer objects (used for websocket responses)
params.left = Number(params.left) || Infinity
params.numwant = Math.min(
Number(params.offers && params.offers.length) || 0, // no default - explicit only
common.MAX_ANNOUNCE_PEERS
)
params.compact = -1 // return full peer objects (used for websocket responses)
} else if (params.action === 'scrape') {
params.action = common.ACTIONS.SCRAPE

if (typeof params.info_hash === 'string') params.info_hash = [ params.info_hash ]
if (Array.isArray(params.info_hash)) {
params.info_hash = params.info_hash.map(function (binaryInfoHash) {
if (typeof binaryInfoHash !== 'string' || binaryInfoHash.length !== 20) {
throw new Error('invalid info_hash')
}
return common.binaryToHex(binaryInfoHash)
})
} else {
params.info_hash = common.binaryToHex(params.info_hash)
Copy link
Member

@feross feross Mar 16, 2016

Choose a reason for hiding this comment

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

This else statement can be removed. It should never be true. If params.info_hash is a string it will be converted to an array with a single string in it on line 40.

}
} else {
throw new Error('invalid action in WS request: ' + params.action)
}

params.ip = opts.trustProxy
? socket.upgradeReq.headers['x-forwarded-for'] || socket.upgradeReq.connection.remoteAddress
Expand Down
12 changes: 9 additions & 3 deletions server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ Server.prototype._onWebSocketRequest = function (socket, opts, params) {
self._onRequest(params, function (err, response) {
if (err) {
socket.send(JSON.stringify({
action: params.action,
'failure reason': err.message,
info_hash: common.hexToBinary(params.info_hash)
}), socket.onSend)
Expand All @@ -336,9 +337,14 @@ Server.prototype._onWebSocketRequest = function (socket, opts, params) {
}
if (self.destroyed) return

if (socket.infoHashes.indexOf(params.info_hash) === -1) {
socket.infoHashes.push(params.info_hash)
}
var hashes
if (typeof params.info_hash === 'string') hashes = [ params.info_hash ]
else hashes = params.info_hash
Copy link
Member

@feross feross Mar 16, 2016

Choose a reason for hiding this comment

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

If params.info_hash is not an Array, then the server will crash on the next line. Better to check with Array.isArray.

Copy link
Member

@feross feross Mar 16, 2016

Choose a reason for hiding this comment

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

Also, this change shouldn't be required because socket.infoHashes is only used so we can make sure that the client leaves all the swarms that they were a part of. See: https://github.com/feross/bittorrent-tracker/blob/7daf7ec2974f6a8de6b25ca01f75f8db82ce7f3e/server.js#L596

So, the proper change to make here is to only do this logic for announces, and skip it for scrapes.

hashes.forEach(function (info_hash) {
if (socket.infoHashes.indexOf(info_hash) === -1) {
socket.infoHashes.push(info_hash)
}
})

var peers = response.peers
delete response.peers
Expand Down