From e8dea8e588045b43c7a5d4931cde692fd135a21d Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 18 May 2016 23:01:52 -0700 Subject: [PATCH 1/3] Fixes for PR #799 - Support multiple &xs= params in parallel - Fail on 'error' 'warning' events --- lib/torrent.js | 130 ++++++++++++++++++++++----------- test/node/download-metadata.js | 19 ++++- 2 files changed, 101 insertions(+), 48 deletions(-) diff --git a/lib/torrent.js b/lib/torrent.js index 431e2ee7..33eecc3a 100644 --- a/lib/torrent.js +++ b/lib/torrent.js @@ -114,7 +114,7 @@ function Torrent (torrentId, client, opts) { // for cleanup this._servers = [] - this._xsRequest = null + this._xsRequests = [] // TODO: remove this and expose a hook instead // optimization: don't recheck every file if it hasn't changed @@ -271,38 +271,6 @@ Torrent.prototype._onParsedTorrent = function (parsedTorrent) { self._onListening() }) } - - if (parsedTorrent.xs && !self.info) { - // TODO: Could try multiple in parallel here - var src = parsedTorrent.xs instanceof Array ? parsedTorrent.xs[0] : parsedTorrent.xs - var isHTTP = src.substring(0, 7) === 'http://' || src.substring(0, 8) === 'https://' - - if (isHTTP) { - var opts = { - url: src, - method: 'GET', - headers: { - 'user-agent': 'WebTorrent (http://webtorrent.io)' - } - } - this._xsRequest = get.concat(opts, function (err, res, torrent) { - if (err || res.statusCode !== 200 || !torrent.length) return - - var parsedTorrent - try { - parsedTorrent = parseTorrent(torrent) - } catch (err) {} - - if (parsedTorrent) { - if (parsedTorrent.infoHash !== self.infoHash) { - return - } - - self._onMetadata(parsedTorrent) - } - }) - } - } } Torrent.prototype._processParsedTorrent = function (parsedTorrent) { @@ -395,9 +363,81 @@ Torrent.prototype._onListening = function () { self.emit('warning', err) } - // if full metadata was included in initial torrent id, use it immediately. Otherwise, - // wait for torrent-discovery to find peers and ut_metadata to get the metadata. - if (self.info) self._onMetadata(self) + if (self.info) { + // if full metadata was included in initial torrent id, use it immediately. Otherwise, + // wait for torrent-discovery to find peers and ut_metadata to get the metadata. + self._onMetadata(self) + } else if (self.xs) { + self._getMetadataFromServer() + } +} + +Torrent.prototype._getMetadataFromServer = function () { + var self = this + var urls = Array.isArray(self.xs) ? self.xs : [ self.xs ] + + var tasks = urls.map(function (url) { + return function (cb) { + getMetadataFromURL(url, cb) + } + }) + parallel(tasks) + + function getMetadataFromURL (url, cb) { + if (url.indexOf('http://') !== 0 && url.indexOf('https://') !== 0) { + self._debug('skipping non-http xs param: %s', url) + return cb(null) + } + + var opts = { + url: url, + method: 'GET', + headers: { + 'user-agent': 'WebTorrent (https://webtorrent.io)' + } + } + var req + try { + req = get.concat(opts, onResponse) + } catch (err) { + self._debug('skipping invalid url xs param: %s', url) + return cb(null) + } + + self._xsRequests.push(req) + + function onResponse (err, res, torrent) { + if (self.destroyed) return cb(null) + if (self.metadata) return cb(null) + + if (err) { + self._debug('http error from xs param: %s', url) + return cb(null) + } + if (res.statusCode !== 200) { + self._debug('non-200 status code from xs param: %s', url) + return cb(null) + } + + var parsedTorrent + try { + parsedTorrent = parseTorrent(torrent) + } catch (err) {} + + if (!parsedTorrent) { + self._debug('got invalid torrent file from xs param: %s', url) + return cb(null) + } + + if (parsedTorrent.infoHash !== self.infoHash) { + self._debug('got torrent file with incorrect info hash from xs param: %s', url) + return cb(null) + } + + self._onMetadata(parsedTorrent) + cb(null) + } + } } /** @@ -405,13 +445,14 @@ Torrent.prototype._onListening = function () { */ Torrent.prototype._onMetadata = function (metadata) { var self = this - if (self._xsRequest) { - self._xsRequest.abort() - self._xsRequest = null - } if (self.metadata || self.destroyed) return self._debug('got metadata') + self._xsRequests.forEach(function (req) { + req.abort() + }) + self._xsRequests = [] + var parsedTorrent if (metadata && metadata.infoHash) { // `metadata` is a parsed torrent (from parse-torrent module) @@ -591,13 +632,14 @@ Torrent.prototype._destroy = function (err, cb) { self.destroyed = true self._debug('destroy') - if (self._xsRequest) { - self._xsRequest.abort() - } self.client._remove(self) clearInterval(self._rechokeIntervalId) + self._xsRequests.forEach(function (req) { + req.abort() + }) + if (self._rarityMap) { self._rarityMap.destroy() } @@ -653,7 +695,7 @@ Torrent.prototype._destroy = function (err, cb) { self._rarityMap = null self._peers = null self._servers = null - self._xsRequest = null + self._xsRequests = null } Torrent.prototype.addPeer = function (peer) { diff --git a/test/node/download-metadata.js b/test/node/download-metadata.js index 07b605a1..59936cb8 100644 --- a/test/node/download-metadata.js +++ b/test/node/download-metadata.js @@ -25,9 +25,12 @@ test('Download metadata for magnet URI with xs parameter', function (t) { var client = new WebTorrent({ dht: false, tracker: false }) + client.on('error', function (err) { t.fail(err) }) + client.on('warning', function (err) { t.fail(err) }) + createServer(fixtures.leaves.torrent, function (url, next) { client.add(fixtures.leaves.magnetURI + '&xs=' + encodeURIComponent(url), function (torrent) { - t.equal(torrent.name, 'Leaves of Grass by Walt Whitman.epub') + t.equal(torrent.files[0].name, 'Leaves of Grass by Walt Whitman.epub') client.destroy(function (err) { t.error(err, 'client destroyed') }) next() @@ -40,12 +43,15 @@ test('Download metadata for magnet URI with xs parameter', function (t) { var client = new WebTorrent({ dht: false, tracker: false }) + client.on('error', function (err) { t.fail(err) }) + client.on('warning', function (err) { t.fail(err) }) + createServer(fixtures.leaves.torrent, function (url, next) { var encoded = encodeURIComponent(url) var uri = fixtures.leaves.magnetURI + '&xs=' + encoded + '&xs=' + encoded + '2' client.add(uri, function (torrent) { - t.equal(torrent.name, 'Leaves of Grass by Walt Whitman.epub') + t.equal(torrent.files[0].name, 'Leaves of Grass by Walt Whitman.epub') client.destroy(function (err) { t.error(err, 'client destroyed') }) next() @@ -54,12 +60,17 @@ test('Download metadata for magnet URI with xs parameter', function (t) { }) test('Download metadata magnet URI with unsupported protocol in xs parameter', function (t) { - t.plan(2) + t.plan(1) var client = new WebTorrent({ dht: false, tracker: false }) + + client.on('error', function (err) { t.fail(err) }) + client.on('warning', function (err) { t.fail(err) }) + client.add(fixtures.leaves.magnetURI + '&xs=' + encodeURIComponent('invalidurl:example')) + setTimeout(function () { - t.ok(true, 'no crash') + // no crash by now client.destroy(function (err) { t.error(err, 'client destroyed') }) }, 100) }) From f0a035ae321dbf3ab8c6191b39017f7e0666ba55 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Thu, 19 May 2016 16:07:44 -0700 Subject: [PATCH 2/3] xs params: show status code in error log --- lib/torrent.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/torrent.js b/lib/torrent.js index 33eecc3a..6b109274 100644 --- a/lib/torrent.js +++ b/lib/torrent.js @@ -415,7 +415,7 @@ Torrent.prototype._getMetadataFromServer = function () { return cb(null) } if (res.statusCode !== 200) { - self._debug('non-200 status code from xs param: %s', url) + self._debug('non-200 status code %s from xs param: %s', res.statusCode, url) return cb(null) } From 722cd0ab2f46b7a0bdafb6069b5ee1530c34fd15 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Thu, 19 May 2016 16:08:18 -0700 Subject: [PATCH 3/3] add more metadata tests - test multiple &xs= params in magnet link - test param that 404s --- test/node/download-metadata.js | 83 +++++++++++++++++++++++++++------- 1 file changed, 67 insertions(+), 16 deletions(-) diff --git a/test/node/download-metadata.js b/test/node/download-metadata.js index 59936cb8..95d57956 100644 --- a/test/node/download-metadata.js +++ b/test/node/download-metadata.js @@ -5,56 +5,107 @@ var WebTorrent = require('../../') function createServer (data, cb) { var server = http.createServer(function (req, res) { + if (req.url !== '/') { + res.statusCode = 404 + res.end() + } res.end(data) - }).listen(function () { + }) + + server.on('listening', function () { var address = server.address() if (address.family === 'IPv6') { address.address = '[' + address.address + ']' } var url = 'http://' + address.address + ':' + address.port + '/' - cb(url, next) + cb(url, server) }) - function next () { - server.close() - } + server.listen() } test('Download metadata for magnet URI with xs parameter', function (t) { - t.plan(2) + t.plan(3) var client = new WebTorrent({ dht: false, tracker: false }) client.on('error', function (err) { t.fail(err) }) client.on('warning', function (err) { t.fail(err) }) - createServer(fixtures.leaves.torrent, function (url, next) { - client.add(fixtures.leaves.magnetURI + '&xs=' + encodeURIComponent(url), function (torrent) { + createServer(fixtures.leaves.torrent, function (url, server) { + var encodedUrl = encodeURIComponent(url) + client.add(fixtures.leaves.magnetURI + '&xs=' + encodedUrl, function (torrent) { t.equal(torrent.files[0].name, 'Leaves of Grass by Walt Whitman.epub') - client.destroy(function (err) { t.error(err, 'client destroyed') }) - next() + server.close(function () { t.pass('server closed') }) }) }) }) -test('Download metadata for magnet URI with xs parameter', function (t) { - t.plan(2) +test('Download metadata for magnet URI with 2 xs parameters', function (t) { + t.plan(4) var client = new WebTorrent({ dht: false, tracker: false }) client.on('error', function (err) { t.fail(err) }) client.on('warning', function (err) { t.fail(err) }) - createServer(fixtures.leaves.torrent, function (url, next) { - var encoded = encodeURIComponent(url) - var uri = fixtures.leaves.magnetURI + '&xs=' + encoded + '&xs=' + encoded + '2' + createServer(fixtures.leaves.torrent, function (url1, server1) { + var encodedUrl1 = encodeURIComponent(url1) + + createServer(fixtures.leaves.torrent, function (url2, server2) { + var encodedUrl2 = encodeURIComponent(url2) + + var uri = fixtures.leaves.magnetURI + '&xs=' + encodedUrl1 + '&xs=' + encodedUrl2 + + client.add(uri, function (torrent) { + t.equal(torrent.files[0].name, 'Leaves of Grass by Walt Whitman.epub') + client.destroy(function (err) { t.error(err, 'client destroyed') }) + server1.close(function () { t.pass('server closed') }) + server2.close(function () { t.pass('server closed') }) + }) + }) + }) +}) + +test('Download metadata for magnet URI with 2 xs parameters, with 1 invalid protocol', function (t) { + t.plan(3) + + var client = new WebTorrent({ dht: false, tracker: false }) + + client.on('error', function (err) { t.fail(err) }) + client.on('warning', function (err) { t.fail(err) }) + + createServer(fixtures.leaves.torrent, function (url, server) { + var encodedUrl1 = encodeURIComponent('invalidurl:example') + var encodedUrl2 = encodeURIComponent(url) + var uri = fixtures.leaves.magnetURI + '&xs=' + encodedUrl1 + '&xs=' + encodedUrl2 client.add(uri, function (torrent) { t.equal(torrent.files[0].name, 'Leaves of Grass by Walt Whitman.epub') + client.destroy(function (err) { t.error(err, 'client destroyed') }) + server.close(function () { t.pass('server closed') }) + }) + }) +}) +test('Download metadata for magnet URI with 2 xs parameters, with 1 404 URL', function (t) { + t.plan(3) + + var client = new WebTorrent({ dht: false, tracker: false }) + + client.on('error', function (err) { t.fail(err) }) + client.on('warning', function (err) { t.fail(err) }) + + createServer(fixtures.leaves.torrent, function (url, server) { + var encodedUrl1 = encodeURIComponent(url + 'blah_404') + var encodedUrl2 = encodeURIComponent(url) + var uri = fixtures.leaves.magnetURI + '&xs=' + encodedUrl1 + '&xs=' + encodedUrl2 + + client.add(uri, function (torrent) { + t.equal(torrent.files[0].name, 'Leaves of Grass by Walt Whitman.epub') client.destroy(function (err) { t.error(err, 'client destroyed') }) - next() + server.close(function () { t.pass('server closed') }) }) }) })