Skip to content

Commit be875c4

Browse files
ferossfeross
committed
Fix serious client+server encoding error (fix webtorrent#32)
1 parent ddbd10e commit be875c4

File tree

4 files changed

+117
-87
lines changed

4 files changed

+117
-87
lines changed

‎client.js‎

Lines changed: 56 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ Tracker.prototype.start = function (opts) {
138138
opts.event = 'started'
139139

140140
debug('sent `start` to ' + self._announceUrl)
141-
self._request(opts)
141+
self._announce(opts)
142142
self.setInterval(self._intervalMs) // start announcing on intervals
143143
}
144144

@@ -148,7 +148,7 @@ Tracker.prototype.stop = function (opts) {
148148
opts.event = 'stopped'
149149

150150
debug('sent `stop` to ' + self._announceUrl)
151-
self._request(opts)
151+
self._announce(opts)
152152
self.setInterval(0) // stop announcing on intervals
153153
}
154154

@@ -159,18 +159,42 @@ Tracker.prototype.complete = function (opts) {
159159
opts.downloaded = opts.downloaded || self.torrentLength || 0
160160

161161
debug('sent `complete` to ' + self._announceUrl)
162-
self._request(opts)
162+
self._announce(opts)
163163
}
164164

165165
Tracker.prototype.update = function (opts) {
166166
var self = this
167167
opts = opts || {}
168168

169169
debug('sent `update` to ' + self._announceUrl)
170-
self._request(opts)
170+
self._announce(opts)
171171
}
172172

173-
Tracker.prototype.scrape = function (opts) {
173+
/**
174+
* Send an announce request to the tracker.
175+
* @param {Object} opts
176+
* @param {number=} opts.uploaded
177+
* @param {number=} opts.downloaded
178+
* @param {number=} opts.left (if not set, calculated automatically)
179+
*/
180+
Tracker.prototype._announce = function (opts) {
181+
var self = this
182+
opts = extend({
183+
uploaded: 0, // default, user should provide real value
184+
downloaded: 0 // default, user should provide real value
185+
}, opts)
186+
187+
if (self.client.torrentLength != null && opts.left == null) {
188+
opts.left = self.client.torrentLength - (opts.downloaded || 0)
189+
}
190+
191+
self._requestImpl(self._announceUrl, opts)
192+
}
193+
194+
/**
195+
* Send a scrape request to the tracker.
196+
*/
197+
Tracker.prototype.scrape = function () {
174198
var self = this
175199

176200
if (!self._scrapeUrl) {
@@ -189,12 +213,7 @@ Tracker.prototype.scrape = function (opts) {
189213
}
190214

191215
debug('sent `scrape` to ' + self._announceUrl)
192-
193-
opts = extend({
194-
info_hash: common.bytewiseEncodeURIComponent(self.client._infoHash)
195-
}, opts)
196-
197-
self._requestImpl(self._scrapeUrl, opts)
216+
self._requestImpl(self._scrapeUrl)
198217
}
199218

200219
Tracker.prototype.setInterval = function (intervalMs) {
@@ -207,35 +226,28 @@ Tracker.prototype.setInterval = function (intervalMs) {
207226
}
208227
}
209228

210-
/**
211-
* Send an announce request to the tracker
212-
*/
213-
Tracker.prototype._request = function (opts) {
229+
Tracker.prototype._requestHttp = function (requestUrl, opts) {
214230
var self = this
215-
opts = extend({
216-
info_hash: common.bytewiseEncodeURIComponent(self.client._infoHash),
217-
peer_id: common.bytewiseEncodeURIComponent(self.client._peerId),
218-
port: self.client._port,
219-
compact: 1,
220-
numwant: self.client._numWant,
221-
uploaded: 0, // default, user should provide real value
222-
downloaded: 0 // default, user should provide real value
223-
}, opts)
224231

225-
if (self.client.torrentLength !== undefined) {
226-
opts.left = self.client.torrentLength - (opts.downloaded || 0)
227-
}
228-
229-
if (self._trackerId) {
230-
opts.trackerid = self._trackerId
232+
if (isScrapeUrl(requestUrl)) {
233+
opts = extend({
234+
info_hash: self.client._infoHash.toString('binary')
235+
}, opts)
236+
} else {
237+
opts = extend({
238+
info_hash: self.client._infoHash.toString('binary'),
239+
peer_id: self.client._peerId.toString('binary'),
240+
port: self.client._port,
241+
compact: 1,
242+
numwant: self.client._numWant
243+
}, opts)
244+
245+
if (self._trackerId) {
246+
opts.trackerid = self._trackerId
247+
}
231248
}
232249

233-
self._requestImpl(self._announceUrl, opts)
234-
}
235-
236-
Tracker.prototype._requestHttp = function (requestUrl, opts) {
237-
var self = this
238-
var fullUrl = requestUrl + '?' + querystring.stringify(opts)
250+
var fullUrl = requestUrl + '?' + common.querystringStringify(opts)
239251

240252
var req = http.get(fullUrl, function (res) {
241253
if (res.statusCode !== 200) {
@@ -255,6 +267,7 @@ Tracker.prototype._requestHttp = function (requestUrl, opts) {
255267

256268
Tracker.prototype._requestUdp = function (requestUrl, opts) {
257269
var self = this
270+
opts = opts || {}
258271
var parsedUrl = url.parse(requestUrl)
259272
var socket = dgram.createSocket('udp4')
260273
var transactionId = new Buffer(hat(32), 'hex')
@@ -294,9 +307,8 @@ Tracker.prototype._requestUdp = function (requestUrl, opts) {
294307
return error('invalid udp handshake')
295308
}
296309

297-
var scrapeStr = 'scrape'
298-
if (requestUrl.substr(requestUrl.lastIndexOf('/') + 1, scrapeStr.length) === scrapeStr) {
299-
scrape(msg.slice(8, 16), opts)
310+
if (isScrapeUrl(requestUrl)) {
311+
scrape(msg.slice(8, 16))
300312
} else {
301313
announce(msg.slice(8, 16), opts)
302314
}
@@ -395,7 +407,7 @@ Tracker.prototype._requestUdp = function (requestUrl, opts) {
395407
]))
396408
}
397409

398-
function scrape (connectionId, opts) {
410+
function scrape (connectionId) {
399411
genTransactionId()
400412

401413
send(Buffer.concat([
@@ -477,10 +489,6 @@ Tracker.prototype._handleResponse = function (requestUrl, data) {
477489
}
478490
}
479491

480-
//
481-
// HELPERS
482-
//
483-
484492
function toUInt16 (n) {
485493
var buf = new Buffer(2)
486494
buf.writeUInt16BE(n, 0)
@@ -499,3 +507,7 @@ function toUInt64 (n) {
499507
}
500508
return Buffer.concat([common.toUInt32(0), common.toUInt32(n)])
501509
}
510+
511+
function isScrapeUrl (u) {
512+
return u.substr(u.lastIndexOf('/') + 1, 'scrape'.length) === 'scrape'
513+
}

‎lib/common.js‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,48 @@
11
/**
2-
* Functions and constants needed by both the tracker client and server.
2+
* Functions/constants needed by both the client and server.
33
*/
44

5+
var querystring = require('querystring')
6+
7+
exports.CONNECTION_ID = Buffer.concat([ toUInt32(0x417), toUInt32(0x27101980) ])
8+
exports.ACTIONS = { CONNECT: 0, ANNOUNCE: 1, SCRAPE: 2, ERROR: 3 }
9+
exports.EVENTS = { update: 0, completed: 1, started: 2, stopped: 3 }
10+
511
function toUInt32 (n) {
612
var buf = new Buffer(4)
713
buf.writeUInt32BE(n, 0)
814
return buf
915
}
1016
exports.toUInt32 = toUInt32
1117

12-
exports.CONNECTION_ID = Buffer.concat([ toUInt32(0x417), toUInt32(0x27101980) ])
13-
exports.ACTIONS = { CONNECT: 0, ANNOUNCE: 1, SCRAPE: 2, ERROR: 3 }
14-
exports.EVENTS = { update: 0, completed: 1, started: 2, stopped: 3 }
18+
exports.binaryToUtf8 = function (str) {
19+
return new Buffer(str, 'binary').toString('utf8')
20+
}
1521

16-
exports.bytewiseDecodeURIComponent = function (str) {
17-
return new Buffer(decodeURIComponent(str), 'binary')
22+
/**
23+
* `querystring.parse` using `unescape` instead of decodeURIComponent, since bittorrent
24+
* clients send non-UTF8 querystrings
25+
* @param {string} q
26+
* @return {Object}
27+
*/
28+
exports.querystringParse = function (q) {
29+
var saved = querystring.unescape
30+
querystring.unescape = unescape // global
31+
var ret = querystring.parse(q)
32+
querystring.unescape = saved
33+
return ret
1834
}
1935

20-
exports.bytewiseEncodeURIComponent = function (buf) {
21-
return encodeURIComponent(buf.toString('binary'))
36+
/**
37+
* `querystring.stringify` using `escape` instead of encodeURIComponent, since bittorrent
38+
* clients send non-UTF8 querystrings
39+
* @param {Object} obj
40+
* @return {string}
41+
*/
42+
exports.querystringStringify = function (obj) {
43+
var saved = querystring.escape
44+
querystring.escape = escape // global
45+
var ret = querystring.stringify(obj)
46+
querystring.escape = saved
47+
return ret
2248
}

‎server.js‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,14 +122,11 @@ Server.prototype._onHttpRequest = function (req, res) {
122122
var self = this
123123
var warning
124124
var s = req.url.split('?')
125-
var params = querystring.parse(s[1])
126-
125+
var params = common.querystringParse(s[1])
127126
if (s[0] === '/announce') {
128-
var infoHash = typeof params.info_hash === 'string' &&
129-
common.bytewiseDecodeURIComponent(params.info_hash).toString('binary')
127+
var infoHash = typeof params.info_hash === 'string' && params.info_hash
128+
var peerId = typeof params.peer_id === 'string' && common.binaryToUtf8(params.peer_id)
130129
var port = Number(params.port)
131-
var peerId = typeof params.peer_id === 'string' &&
132-
common.bytewiseDecodeURIComponent(params.peer_id).toString('utf8')
133130

134131
if (!infoHash) return error('invalid info_hash')
135132
if (infoHash.length !== 20) return error('invalid info_hash')
@@ -252,7 +249,6 @@ Server.prototype._onHttpRequest = function (req, res) {
252249
}
253250

254251
params.info_hash.some(function (infoHash) {
255-
infoHash = common.bytewiseDecodeURIComponent(infoHash).toString('binary')
256252
if (infoHash.length !== 20) {
257253
error('invalid info_hash')
258254
return true // early return

‎test/scrape.js‎

Lines changed: 24 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -10,23 +10,19 @@ var querystring = require('querystring')
1010
var Server = require('../').Server
1111
var test = require('tape')
1212

13+
function hexToBinary (str) {
14+
return new Buffer(str, 'hex').toString('binary')
15+
}
16+
1317
var infoHash1 = 'aaa67059ed6bd08362da625b3ae77f6f4a075aaa'
14-
var encodedInfoHash1 = common.bytewiseEncodeURIComponent(
15-
new Buffer(infoHash1, 'hex')
16-
)
17-
var binaryinfoHash1 = new Buffer(infoHash1, 'hex').toString('binary')
18+
var binaryInfoHash1 = hexToBinary(infoHash1)
1819
var infoHash2 = 'bbb67059ed6bd08362da625b3ae77f6f4a075bbb'
19-
var encodedInfoHash2 = common.bytewiseEncodeURIComponent(
20-
new Buffer(infoHash2, 'hex')
21-
)
22-
var binaryinfoHash2 = new Buffer(infoHash2, 'hex').toString('binary')
20+
var binaryInfoHash2 = hexToBinary(infoHash2)
2321

2422
var bitlove = fs.readFileSync(__dirname + '/torrents/bitlove-intro.torrent')
2523
var parsedBitlove = parseTorrent(bitlove)
26-
var encodedBitlove = common.bytewiseEncodeURIComponent(
27-
new Buffer(parsedBitlove.infoHash, 'hex')
28-
)
29-
var binaryBitlove = new Buffer(parsedBitlove.infoHash, 'hex').toString('binary')
24+
var binaryBitlove = hexToBinary(parsedBitlove.infoHash)
25+
3026
var peerId = new Buffer('01234567890123456789')
3127

3228
test('server: single info_hash scrape', function (t) {
@@ -44,19 +40,19 @@ test('server: single info_hash scrape', function (t) {
4440
var scrapeUrl = 'http://127.0.0.1:' + port + '/scrape'
4541

4642
server.once('listening', function () {
47-
var url = scrapeUrl + '?' + querystring.stringify({
48-
info_hash: encodedInfoHash1
43+
var url = scrapeUrl + '?' + common.querystringStringify({
44+
info_hash: binaryInfoHash1
4945
})
5046
http.get(url, function (res) {
5147
t.equal(res.statusCode, 200)
5248
res.pipe(concat(function (data) {
5349
data = bencode.decode(data)
5450
t.ok(data.files)
5551
t.equal(Object.keys(data.files).length, 1)
56-
t.ok(data.files[binaryinfoHash1])
57-
t.equal(typeof data.files[binaryinfoHash1].complete, 'number')
58-
t.equal(typeof data.files[binaryinfoHash1].incomplete, 'number')
59-
t.equal(typeof data.files[binaryinfoHash1].downloaded, 'number')
52+
t.ok(data.files[binaryInfoHash1])
53+
t.equal(typeof data.files[binaryInfoHash1].complete, 'number')
54+
t.equal(typeof data.files[binaryInfoHash1].incomplete, 'number')
55+
t.equal(typeof data.files[binaryInfoHash1].downloaded, 'number')
6056

6157
server.close(function () {
6258
t.end()
@@ -84,8 +80,8 @@ test('server: multiple info_hash scrape', function (t) {
8480
var scrapeUrl = 'http://127.0.0.1:' + port + '/scrape'
8581

8682
server.once('listening', function () {
87-
var url = scrapeUrl + '?' + querystring.stringify({
88-
info_hash: [ encodedInfoHash1, encodedInfoHash2 ]
83+
var url = scrapeUrl + '?' + common.querystringStringify({
84+
info_hash: [ binaryInfoHash1, binaryInfoHash2 ]
8985
})
9086
http.get(url, function (res) {
9187
t.equal(res.statusCode, 200)
@@ -94,15 +90,15 @@ test('server: multiple info_hash scrape', function (t) {
9490
t.ok(data.files)
9591
t.equal(Object.keys(data.files).length, 2)
9692

97-
t.ok(data.files[binaryinfoHash1])
98-
t.equal(typeof data.files[binaryinfoHash1].complete, 'number')
99-
t.equal(typeof data.files[binaryinfoHash1].incomplete, 'number')
100-
t.equal(typeof data.files[binaryinfoHash1].downloaded, 'number')
93+
t.ok(data.files[binaryInfoHash1])
94+
t.equal(typeof data.files[binaryInfoHash1].complete, 'number')
95+
t.equal(typeof data.files[binaryInfoHash1].incomplete, 'number')
96+
t.equal(typeof data.files[binaryInfoHash1].downloaded, 'number')
10197

102-
t.ok(data.files[binaryinfoHash2])
103-
t.equal(typeof data.files[binaryinfoHash2].complete, 'number')
104-
t.equal(typeof data.files[binaryinfoHash2].incomplete, 'number')
105-
t.equal(typeof data.files[binaryinfoHash2].downloaded, 'number')
98+
t.ok(data.files[binaryInfoHash2])
99+
t.equal(typeof data.files[binaryInfoHash2].complete, 'number')
100+
t.equal(typeof data.files[binaryInfoHash2].incomplete, 'number')
101+
t.equal(typeof data.files[binaryInfoHash2].downloaded, 'number')
106102

107103
server.close(function () {
108104
t.end()

0 commit comments

Comments
 (0)