[tracker] Ensure parameters are url encoded/decoded appropriately

Some tests were added to cover cases where different strings
contained characters which need escaping. These cover peerid,
key and trackerid.

src/MonoTorrent.Tests/Client/HttpTrackerTests.cs

@@ -49,19 +49,22 @@ public class HttpTrackerTests
         Uri AnnounceUrl => new Uri (ListeningPrefix + "announce");
         HTTPTracker tracker;
 
+        InfoHash infoHash;
+        string peerId;
+        string trackerId;
+
 
         readonly List<string> keys = new List<string> ();
 
         [OneTimeSetUp]
         public void FixtureSetup()
         {
-            server = new MonoTorrent.Tracker.Tracker();
-            server.AllowUnregisteredTorrents = true;
+            peerId = "my peer id &&=?!<>  ";
+            trackerId = "&=?!<>   ";
             listener = new HttpTrackerListener (ListeningPrefix);
             listener.AnnounceReceived += delegate (object o, MonoTorrent.Tracker.AnnounceParameters e) {
                 keys.Add(e.Key);
             };
-            server.RegisterListener(listener);
 
             listener.Start();
         }
@@ -70,15 +73,30 @@ public void FixtureSetup()
         public void Setup()
         {
             keys.Clear();
+
+            server = new MonoTorrent.Tracker.Tracker(trackerId);
+            server.AllowUnregisteredTorrents = true;
+            server.RegisterListener(listener);
+
             tracker = (HTTPTracker) TrackerFactory.Create(AnnounceUrl);
-
+
+            var infoHashBytes = new [] { ' ', '%', '&', '?', '&', '&', '?', '5', '1', '=' }
+                        .Select (t => (byte)t);
+
+            infoHash = new InfoHash (infoHashBytes.Concat (infoHashBytes).ToArray ());
             announceParams = new AnnounceParameters()
-                .WithPeerId ("id")
-                .WithInfoHash (new InfoHash (new byte[20]));
+                .WithPeerId (peerId)
+                .WithInfoHash (infoHash);
 
             scrapeParams = new ScrapeParameters (new InfoHash (new byte[20]));
         }
 
+        [TearDown]
+        public void Teardown ()
+        {
+            server.UnregisterListener (listener);
+        }
+
         [OneTimeTearDown]
         public void FixtureTeardown()
         {
@@ -120,6 +138,23 @@ public async Task Announce()
             Assert.IsTrue(StringComparer.OrdinalIgnoreCase.Equals (keys[0], tracker.Key), "#2");
         }
 
+        [Test]
+        public async Task Announce_ValidateParams()
+        {
+            var argsTask = new TaskCompletionSource<MonoTorrent.Tracker.AnnounceParameters> ();
+            listener.AnnounceReceived += (o, e) => argsTask.TrySetResult (e);
+
+            await tracker.AnnounceAsync(announceParams);
+            Assert.IsTrue (argsTask.Task.Wait (5000), "#1");
+
+            var args = argsTask.Task.Result;
+            Assert.AreEqual (peerId, announceParams.PeerId, "#1");
+            Assert.AreEqual (announceParams.PeerId, args.PeerId, "#2");
+
+            Assert.AreEqual (infoHash, args.InfoHash, "#3");
+            Assert.AreEqual (announceParams.InfoHash, args.InfoHash, "#3");
+        }
+
         [Test]
         public void Announce_Timeout ()
         {
@@ -138,15 +173,31 @@ public void Announce_Timeout ()
         [Test]
         public async Task KeyTest()
         {
-            tracker = (HTTPTracker) TrackerFactory.Create(new Uri(AnnounceUrl + "?key=value"));
+            // Set a key which uses characters which need escaping.
+            tracker = (HTTPTracker) TrackerFactory.Create(AnnounceUrl);
+            tracker.Key = peerId;
+
             await tracker.AnnounceAsync(announceParams);
-            Assert.AreEqual("value", keys[0], "#1");
+            Assert.AreEqual(peerId, keys[0], "#1");
         }
 
+        [Test]
+        public async Task NullKeyTest()
+        {
+            // Set a key which uses characters which need escaping.
+            tracker = (HTTPTracker) TrackerFactory.Create(AnnounceUrl);
+            tracker.Key = null;
+
+            await tracker.AnnounceAsync(announceParams);
+            Assert.AreEqual (null, keys[0], "#1");
+        }
+
+
         [Test]
         public async Task Scrape()
         {
-            var infoHash = new InfoHash (Enumerable.Repeat ((byte)1, 20).ToArray ());
+            // make sure it's a unique infohash as the listener isn't re-created for every test.
+            infoHash = new InfoHash (Enumerable.Repeat ((byte)1, 20).ToArray ());
             scrapeParams = new ScrapeParameters (infoHash);
 
             await tracker.ScrapeAsync(scrapeParams);
@@ -188,5 +239,23 @@ public void Scrape_Timeout()
                 tcs.SetResult(true);
             }
         }
+
+        [Test]
+        public async Task TrackerId ()
+        {
+            // Null until the server side tracker sends us the value
+            Assert.IsNull (tracker.TrackerId, "#1");
+            await tracker.AnnounceAsync (announceParams);
+
+            // Now we have the value, the next announce should contain it
+            Assert.AreEqual (trackerId, tracker.TrackerId, "#2");
+
+            var argsTask = new TaskCompletionSource<MonoTorrent.Tracker.AnnounceParameters> ();
+            listener.AnnounceReceived += (o, e) => argsTask.TrySetResult (e);
+
+            await tracker.AnnounceAsync (announceParams);
+            Assert.IsTrue (argsTask.Task.Wait (5000), "#3");
+            Assert.AreEqual (trackerId, argsTask.Task.Result.TrackerId, "#4");
+        }
     }
 }

src/MonoTorrent/MonoTorrent.Client.Tracker/HTTPTracker.cs

@@ -44,9 +44,9 @@ class HTTPTracker : Tracker
         static readonly Random random = new Random();
         static readonly TimeSpan DefaultRequestTimeout = TimeSpan.FromSeconds(10);
 
-        string TrackerId { get; set; }
+        internal string TrackerId { get; set; }
 
-        internal string Key { get; }
+        internal string Key { get; set; }
 
         internal TimeSpan RequestTimeout { get; set; } = DefaultRequestTimeout;
 
@@ -64,10 +64,9 @@ public HTTPTracker(Uri announceUrl)
             CanAnnounce = true;
             CanScrape = ScrapeUri != null;
 
-            byte[] passwordKey = new byte[8];
+            // Use a random integer prefixed by our identifier.
             lock (random)
-                random.NextBytes(passwordKey);
-            Key = UriHelper.UrlEncode(passwordKey);
+                Key = VersionInfo.ClientIdentifier + random.Next (1, int.MaxValue).ToString ();
         }
 
         protected override async Task<List<Peer>> DoAnnounceAsync(AnnounceParameters parameters)
@@ -130,7 +129,7 @@ Uri CreateAnnounceString (AnnounceParameters parameters)
         {
             UriQueryBuilder b = new UriQueryBuilder (Uri);
             b.Add ("info_hash", parameters.InfoHash.UrlEncode ())
-             .Add ("peer_id", parameters.PeerId)
+             .Add ("peer_id", parameters.PeerId.UrlEncode ())
              .Add ("port", parameters.Port)
              .Add ("uploaded", parameters.BytesUploaded)
              .Add ("downloaded", parameters.BytesDownloaded)
@@ -142,8 +141,8 @@ Uri CreateAnnounceString (AnnounceParameters parameters)
                 b.Add ("supportcrypto", 1);
             if (parameters.RequireEncryption)
                 b.Add ("requirecrypto", 1);
-            if (!b.Contains ("key"))
-                b.Add ("key", Key);
+            if (!b.Contains ("key") && Key != null)
+                b.Add ("key", Key.UrlEncode ());
             if (!string.IsNullOrEmpty (parameters.IPAddress))
                 b.Add ("ip", parameters.IPAddress);
 
@@ -158,7 +157,7 @@ Uri CreateAnnounceString (AnnounceParameters parameters)
                 b.Add ("event", parameters.ClientEvent.ToString ().ToLower ());
 
             if (!string.IsNullOrEmpty (TrackerId))
-                b.Add ("trackerid", TrackerId);
+                b.Add ("trackerid", TrackerId.UrlEncode ());
 
             return b.ToUri ();
         }

src/MonoTorrent/MonoTorrent.Client.Tracker/StringExtensions.cs

@@ -0,0 +1,44 @@
+//
+// StringExtensions.cs
+//
+// Authors:
+//   Alan McGovern <[email protected]>
+//
+// Copyright (C) 2019 Alan McGovern
+//
+// Permission is hereby granted, free of charge, to any person obtaining
+// a copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to
+// permit persons to whom the Software is furnished to do so, subject to
+// the following conditions:
+// 
+// The above copyright notice and this permission notice shall be
+// included in all copies or substantial portions of the Software.
+// 
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+// LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+//
+
+
+using System.Text;
+
+namespace MonoTorrent
+{
+    static class StringExtensions
+    {
+        static readonly Encoding UTF8 = Encoding.UTF8;
+
+        public static string UrlEncode (this string str)
+            => UriHelper.UrlEncode (UTF8.GetBytes (str));
+
+        public static string UrlDecode (this string str)
+            => UTF8.GetString (UriHelper.UrlDecode (str));
+    }
+}

src/MonoTorrent/MonoTorrent.Tracker.Listeners/HttpTrackerListener.cs

@@ -112,7 +112,7 @@ private void HandleRequest(HttpListenerContext context)
             context.Response.ContentType = "text/plain";
             context.Response.StatusCode = 200;
             context.Response.ContentLength64 = response.LongLength;
-            context.Response.OutputStream.Write(response, 0, response.Length);
+            context.Response.Close (response, true);
         }
 
         #endregion Methods

src/MonoTorrent/MonoTorrent.Tracker/AnnounceParameters.cs

@@ -93,7 +93,7 @@ public InfoHash InfoHash
 
         public string Key
         {
-            get { return Parameters["key"]; }
+            get { return Parameters["key"]?.UrlDecode (); }
         }
 
         public override bool IsValid
@@ -112,7 +112,7 @@ public int NumberWanted
 
         public string PeerId
         {
-            get { return Parameters["peer_id"]; } 
+            get { return Parameters["peer_id"].UrlDecode (); }
         }
 
         public int Port
@@ -122,7 +122,7 @@ public int Port
 
         public string TrackerId
         {
-            get { return Parameters["trackerid"]; }
+            get { return Parameters["trackerid"]?.UrlDecode (); }
         }
 
         public long Uploaded

src/MonoTorrent/MonoTorrent/VersionInfo.cs

@@ -37,16 +37,18 @@ public static class VersionInfo
         /// <summary>
         /// Protocol string for version 1.0 of Bittorrent Protocol
         /// </summary>
-        public static readonly string ProtocolStringV100 = "BitTorrent protocol";
+        internal static readonly string ProtocolStringV100 = "BitTorrent protocol";
+
+        public static readonly string ClientIdentifier = "MO";
 
         /// <summary>
         /// The current version of the client
         /// </summary>
-        public static readonly string ClientVersion;
+        internal static readonly string ClientVersion;
 
-        public static readonly string DhtClientVersion = "MO06";
+        internal static readonly string DhtClientVersion = $"{ClientIdentifier}06";
 
-        internal static readonly Version Version;
+        public static readonly Version Version;
 
         static VersionInfo ()
         {
@@ -61,7 +63,7 @@ static VersionInfo ()
                 version = version.Substring (0, 4);
             else
                 version = version.PadRight (4, '0');
-            ClientVersion = string.Format ("-MO{0}-", version);
+            ClientVersion = $"-{ClientIdentifier}{version}-";
         }
     }
 }