Commits

Constantin Veretennicov committed 2d6d07e

Fixed bug: client was assumed to provide different peer id for each torrent.

In practice some clients (e.g. uTorrent) may use the same peer id for different torrents. This caused incorrect behaviour and DataStore contention over peer entities when client announced more than one torrent.

Warning: deploying this fix will effectively reset existing peer lists, because we change peer key encoding scheme. However, this should not be a problem as active peers will reannounce themselves soon.

Comments (0)

Files changed (5)

       n_bytes_left=n_bytes_left_to_download)
   elif event_type == 'stopped':
     # unregister peer, if there is one
-    p = TorrentPeer.get_by_peer_id(peer_id)
+    p = TorrentPeer.get_by_peer_id_and_torrent_info_hash(peer_id, info_hash)
     if p is None:
       return fail("Unfamiliar peer.")
     p.delete()
   elif event_type == 'completed':
     # mark peer as seeder
     t = Torrent.get_or_create_by_info_hash(info_hash)
-    p = TorrentPeer.get_by_peer_id(peer_id)
+    p = TorrentPeer.get_by_peer_id_and_torrent_info_hash(peer_id, info_hash)
     if p is None:
       return fail("Unfamiliar peer.")
     p.n_bytes_left = 0
   elif event_type is None:
     # update peer status
     t = Torrent.get_or_create_by_info_hash(info_hash)
-    p = TorrentPeer.get_by_peer_id(peer_id)
+    p = TorrentPeer.get_by_peer_id_and_torrent_info_hash(peer_id, info_hash)
     if p is None:
       p = TorrentPeer.create(
         peer_id,
 class TorrentPeer(db.Model):
   # key_name is peer_id string prefixed with '#'
 
-  model_version = db.IntegerProperty(required=True, default=3)
+  model_version = db.IntegerProperty(required=True, default=4)
   torrent = db.ReferenceProperty(
       Torrent, collection_name='peers', required=True)
   address = db.StringProperty(required=True)
 
   @property
   def peer_id(self):
-    return self.key().name()[1:].decode('base64')
+    return self.key().name()[1:].decode('base64')[:20]
 
   @classmethod
-  def create(cls, peer_id, **kwargs):
-    instance = cls(key_name=cls._encode_key_name(peer_id), **kwargs)
+  def create(cls, peer_id, torrent, **kwargs):
+    instance = cls(
+      key_name=cls._encode_key_name(peer_id, torrent.info_hash),
+      torrent=torrent,
+      **kwargs)
     instance.put()
     return instance
 
   @classmethod
-  def get_by_peer_id(cls, peer_id):
-    return cls.get_by_key_name(cls._encode_key_name(peer_id))
+  def get_by_peer_id_and_torrent_info_hash(cls, peer_id, torrent_info_hash):
+    return cls.get_by_key_name(cls._encode_key_name(peer_id, torrent_info_hash))
 
   @classmethod
-  def _encode_key_name(cls, peer_id):
-    return '#' + peer_id.encode('base64')
+  def _encode_key_name(cls, peer_id, torrent_info_hash):
+    return '#' + (peer_id + torrent_info_hash).encode('base64')

app/static/site.css

   color: gray;
 }
 
-#valid-html, #valid-css {
+#badges a {
   text-decoration: none;
 }
 

app/templates/base.html

     <meta http-equiv="content-type" content="text/html;charset=UTF-8">
     <title>{% block title %}Tracko - Public BitTorrent Tracker{% endblock %}</title>
     {% block styles %}
-      <link rel="stylesheet" type="text/css" href="/static/site.css">
+      <link rel="stylesheet" type="text/css" href="/static/site.css?v=0">
     {% endblock %}
     {% block prescripts %}{% endblock %}
   </head>
           </a>
         </span>
         <span id="badges">
+          <a id="engine-version"
+             href="http://bitbucket.org/kveretennicov/tracko/">Tracko 0.1</a>
+          +
           <a id="valid-html"
              href="http://validator.w3.org/check?uri=referer">HTML 4.01</a>
           +

tests/test_announce_basic.py

 def test_large_byte_counts_are_accepted():
 
   request_params = good_start_request_params.copy()
+  request_params['info_hash'] = 'h' * 20
   request_params['peer_id'] = 'x' * 20
   request_params['compact'] = '0'
   request_params['uploaded'] = str(2**1024 - 1)
   request_params['left'] = str(3**512 - 1)
   logic.process_announce_request(request_params, '1.2.3.4', settings)
 
-  p = models.TorrentPeer.get_by_peer_id('x' * 20)
+  p = models.TorrentPeer.get_by_peer_id_and_torrent_info_hash(
+    'x' * 20, 'h' * 20)
   assert_equals(p.n_bytes_left, 3**512 - 1)
+
+
+@with_setup(util.clear_datastore)
+def test_same_peer_id_for_two_different_torrents_is_tracked_separately():
+  # some clients, like uTorrent, use same peer id when requesting
+  # different torrents; others, like Transmission, use different peer ids.
+  # therefore, TorrentPeer should be identified by both torrent id and peer id
+
+  peer_id = 'p' * 20
+
+  request_params = good_start_request_params.copy()
+  request_params['info_hash'] = 'x' * 20
+  request_params['peer_id'] = peer_id
+  logic.process_announce_request(request_params, '1.2.3.4', settings)
+
+  request_params = good_start_request_params.copy()
+  request_params['info_hash'] = 'y' * 20
+  request_params['peer_id'] = peer_id
+  logic.process_announce_request(request_params, '1.2.3.4', settings)
+
+  assert_equals(models.TorrentPeer.all().count(), 2)