Fix issues found during the connection-reuse review

.github/README.md

@@ -134,6 +134,7 @@ var client = new IPDataClient("API_KEY");
 
 var threatInfo = await client.Threat("69.78.70.144");
 Console.WriteLine($"Threat is Tor: {threatInfo.IsTor}");
+Console.WriteLine($"Threat is VPN: {threatInfo.IsVpn}, trust score: {threatInfo.Scores?.TrustScore}");
 ```
 
 ### Response fields
@@ -147,6 +148,8 @@ Console.WriteLine($"Threat is Tor: {threatInfo.IsTor}");
 | `threat.blocklists[].first_seen` | `BlocklistInfo.FirstSeen` | First day the address was seen on that blocklist, a `YYYY-MM-DD` (UTC) string; `null` when the feed does not report it. |
 | `threat.blocklists[].last_seen` | `BlocklistInfo.LastSeen` | Last day the address was seen on that blocklist, a `YYYY-MM-DD` (UTC) string; `null` when the feed does not report it. |
 | `threat.blocklists[].days_seen` | `BlocklistInfo.DaysSeen` | Number of days the address was seen on that blocklist (`int?`); `null` when the feed does not report it. |
+| `threat.is_vpn` | `Threat.IsVpn` | `true` when the address is a VPN endpoint (`bool?`); `null` when the response does not include the key. |
+| `threat.scores` | `Threat.Scores` | A `ThreatScores` object with `VpnScore`, `ProxyScore`, `ThreatScore` and `TrustScore` (`int?`, from `vpn_score`, `proxy_score`, `threat_score` and `trust_score`). `Scores` is `null` when the response does not include the object, and a score is `null` when the object does not include it. |
 
 Both flags can be requested on their own or as part of a field selection:
 
@@ -163,6 +166,7 @@ An `IPDataClient` keeps a persistent HTTPS connection pool for as long as it liv
 - **Shared default pool.** Clients constructed without an `HttpClient` (`new IPDataClient("API_KEY")`, with or without a base URL) share one process-wide `HttpClient`. It lives for the lifetime of the process and is never disposed, so there is nothing to close. On .NET 8+ it keeps idle connections for 60 seconds, recycles every connection after 5 minutes and sends HTTP/2 keep-alive pings every 30 seconds.
 - **HTTP/2 on .NET 8+.** The `net8.0` build asks for HTTP/2 on every request (`HttpVersionPolicy.RequestVersionOrLower`) and falls back to HTTP/1.1 automatically when the server or a proxy does not negotiate it. The `netstandard2.0` build, used by .NET Framework and other .NET Standard consumers, never asks for HTTP/2 and relies on HTTP/1.1 keep-alive.
 - **Bring your own `HttpClient`.** Pass an `HttpClient` to send every request through it, for example one managed by `IHttpClientFactory` (see [Dependency Injection](#dependency-injection)). It can be combined with a custom base URL through the `IPDataClient(string apiKey, HttpClient httpClient, Uri baseUrl)` constructor. You own that client's lifetime; the library never disposes it. The same applies to a custom `IHttpClient` implementation.
+- **Request headers.** Every request carries `User-Agent: ipdata-dotnet/<version>` (the package version) and `Accept: application/json`.
 
 ```csharp
 // One client for the whole application; reuse it for every lookup.

IPData.sln

@@ -17,9 +17,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "IPData.Tests", "test\Unit\I
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".github", ".github", "{3B7FCB46-B3B6-41EF-9495-3FA4D027E3FC}"
 	ProjectSection(SolutionItems) = preProject
-		.github\CODE_OF_CONDUCT.md = .github\CODE_OF_CONDUCT.md
 		.github\CONTRIBUTING.md = .github\CONTRIBUTING.md
-		.github\PULL_REQUEST_TEMPLATE.md = .github\PULL_REQUEST_TEMPLATE.md
 		.github\README.md = .github\README.md
 	EndProjectSection
 EndProject

samples/IPData.Lookup/Program.cs

@@ -45,6 +45,7 @@ public static async Task Main()
             // Get threat info from IP
             var threatInfo = await client.Threat("69.78.70.144");
             Console.WriteLine($"Threat is Tor: {threatInfo.IsTor}");
+            Console.WriteLine($"Threat is VPN: {threatInfo.IsVpn}, trust score: {threatInfo.Scores?.TrustScore}");
 
             // Get IP data for multiple IPs
             var ipInfoList = await client.Lookup(new[] { "1.1.1.1", "2.2.2.2", "3.3.3.3" });

src/IPData/Exceptions/ApiException.cs

@@ -10,6 +10,18 @@ namespace IPData.Exceptions
     [Serializable]
     public class ApiException : Exception
     {
+#if NET8_0_OR_GREATER
+        /// <summary>The message of the obsoletion that .NET 8 and later put on formatter-based serialization.</summary>
+        internal const string SerializationObsoleteMessage =
+            "This API supports obsolete formatter-based serialization. It should not be called or extended by application code.";
+
+        /// <summary>The diagnostic id of that obsoletion.</summary>
+        internal const string SerializationDiagnosticId = "SYSLIB0051";
+
+        /// <summary>The documentation link of that obsoletion.</summary>
+        internal const string SerializationUrlFormat = "https://aka.ms/dotnet-warnings/{0}";
+#endif
+
         private static readonly ISerializer _serializer = new JsonSerializer();
 
         [NonSerialized]
@@ -45,7 +57,15 @@ protected ApiException(HttpStatusCode statusCode, ApiError apiError, Exception i
             ApiError = apiError ?? throw new ArgumentNullException(nameof(apiError), "ApiError can't be null");
         }
 
+        /// <summary>Initializes a new instance of the <see cref="ApiException"/> class with serialized data.</summary>
+        /// <remarks>
+        /// Formatter-based serialization is obsolete on .NET 8 and later (SYSLIB0051). The constructor is kept on
+        /// every target so the type has the same shape everywhere, and carries that obsoletion on .NET 8 and later.
+        /// </remarks>
         [ExcludeFromCodeCoverage]
+#if NET8_0_OR_GREATER
+        [Obsolete(SerializationObsoleteMessage, DiagnosticId = SerializationDiagnosticId, UrlFormat = SerializationUrlFormat)]
+#endif
         protected ApiException(SerializationInfo serializationInfo, StreamingContext streamingContext)
             : base(serializationInfo, streamingContext)
         {

src/IPData/Exceptions/BadRequestException.cs

@@ -23,7 +23,15 @@ public BadRequestException(string responseContent, Exception innerException)
         {
         }
 
+        /// <summary>Initializes a new instance of the <see cref="BadRequestException"/> class with serialized data.</summary>
+        /// <remarks>
+        /// Formatter-based serialization is obsolete on .NET 8 and later (SYSLIB0051). The constructor is kept on
+        /// every target so the type has the same shape everywhere, and carries that obsoletion on .NET 8 and later.
+        /// </remarks>
         [ExcludeFromCodeCoverage]
+#if NET8_0_OR_GREATER
+        [Obsolete(SerializationObsoleteMessage, DiagnosticId = SerializationDiagnosticId, UrlFormat = SerializationUrlFormat)]
+#endif
         protected BadRequestException(SerializationInfo serializationInfo, StreamingContext streamingContext)
             : base(serializationInfo, streamingContext)
         {

src/IPData/Exceptions/ForbiddenException.cs

@@ -23,7 +23,15 @@ public ForbiddenException(string responseContent, Exception innerException)
         {
         }
 
+        /// <summary>Initializes a new instance of the <see cref="ForbiddenException"/> class with serialized data.</summary>
+        /// <remarks>
+        /// Formatter-based serialization is obsolete on .NET 8 and later (SYSLIB0051). The constructor is kept on
+        /// every target so the type has the same shape everywhere, and carries that obsoletion on .NET 8 and later.
+        /// </remarks>
         [ExcludeFromCodeCoverage]
+#if NET8_0_OR_GREATER
+        [Obsolete(SerializationObsoleteMessage, DiagnosticId = SerializationDiagnosticId, UrlFormat = SerializationUrlFormat)]
+#endif
         protected ForbiddenException(SerializationInfo serializationInfo, StreamingContext streamingContext)
             : base(serializationInfo, streamingContext)
         {

src/IPData/Exceptions/UnauthorizedException.cs

@@ -23,7 +23,15 @@ public UnauthorizedException(string message, Exception innerException)
         {
         }
 
+        /// <summary>Initializes a new instance of the <see cref="UnauthorizedException"/> class with serialized data.</summary>
+        /// <remarks>
+        /// Formatter-based serialization is obsolete on .NET 8 and later (SYSLIB0051). The constructor is kept on
+        /// every target so the type has the same shape everywhere, and carries that obsoletion on .NET 8 and later.
+        /// </remarks>
         [ExcludeFromCodeCoverage]
+#if NET8_0_OR_GREATER
+        [Obsolete(SerializationObsoleteMessage, DiagnosticId = SerializationDiagnosticId, UrlFormat = SerializationUrlFormat)]
+#endif
         protected UnauthorizedException(SerializationInfo serializationInfo, StreamingContext streamingContext)
             : base(serializationInfo, streamingContext)
         {

src/IPData/IPData.csproj

@@ -12,17 +12,19 @@
     <RepositoryType>git</RepositoryType>
     <PackageRequireLicenseAcceptance>false</PackageRequireLicenseAcceptance>
     <PackageLicenseExpression>MIT</PackageLicenseExpression>
-    <PackageIconUrl>https://image.ibb.co/iDQdUS/ipdatalogo.png</PackageIconUrl>
-    <ApplicationIcon>ipdata.ico</ApplicationIcon>
+    <PackageIcon>icon.png</PackageIcon>
     <Copyright>IPData Community</Copyright>
-    <Win32Resource />
     <PackageReleaseNotes>Release notes are at https://github.com/ipdata/dotnet/releases</PackageReleaseNotes>
     <SignAssembly>false</SignAssembly>
     <DelaySign>false</DelaySign>
     <AllowedOutputExtensionsInPackageBuildOutputFolder>$(AllowedOutputExtensionsInPackageBuildOutputFolder);.pdb</AllowedOutputExtensionsInPackageBuildOutputFolder>
     <VersionPrefix>3.1.0</VersionPrefix>
   </PropertyGroup>
 
+  <ItemGroup>
+    <None Include="icon.png" Pack="true" PackagePath="\" />
+  </ItemGroup>
+
   <!-- System.Text.Json ships in the shared framework on net8.0; only the netstandard2.0 asset needs the package. -->
   <ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
     <PackageReference Include="System.Text.Json" Version="4.7.2" />

src/IPData/IPDataClient.cs

@@ -3,6 +3,8 @@
 using System.Globalization;
 using System.Linq.Expressions;
 using System.Net.Http;
+using System.Net.Http.Headers;
+using System.Reflection;
 using System.Runtime.CompilerServices;
 using System.Text;
 using System.Threading.Tasks;
@@ -23,7 +25,8 @@ namespace IPData
     /// client that lives as long as the process does and is never disposed; an <see cref="HttpClient"/> or
     /// <see cref="IHttpClient"/> passed in stays owned by the caller. The class holds no per-request state
     /// and is safe to use from several threads at once. On .NET 5 and later every request asks for HTTP/2
-    /// and falls back to HTTP/1.1 when the server does not negotiate it.
+    /// and falls back to HTTP/1.1 when the server does not negotiate it. Every request carries a
+    /// <c>User-Agent</c> header naming the library and its version and an <c>Accept: application/json</c> header.
     /// </remarks>
     public class IPDataClient : IIPDataClient
     {
@@ -32,6 +35,15 @@ public class IPDataClient : IIPDataClient
         /// <summary>The exception factory</summary>
         private static readonly IApiExceptionFactory _exceptionFactory = new ApiExceptionFactory();
 
+        /// <summary>The product token sent in the <c>User-Agent</c> header of every request.</summary>
+        internal const string UserAgentProduct = "ipdata-dotnet";
+
+        /// <summary>
+        /// The <c>User-Agent</c> value sent with every request, <c>ipdata-dotnet/&lt;version&gt;</c>, where the
+        /// version is the package version of this assembly. It is read once, when the type is initialized.
+        /// </summary>
+        internal static readonly ProductInfoHeaderValue UserAgent = CreateUserAgent();
+
         /// <summary>The HTTP client</summary>
         private readonly IHttpClient _httpClient;
 
@@ -215,6 +227,7 @@ public async Task<Threat> Threat(string ip)
 
         /// <summary>Creates the request message for an API call.</summary>
         /// <remarks>
+        /// Every request carries the library's <c>User-Agent</c> and an <c>Accept: application/json</c> header.
         /// On .NET 5 and later the request asks for HTTP/2 and lets the handler fall back to HTTP/1.1
         /// (HttpVersionPolicy.RequestVersionOrLower). The .NET Standard 2.0 build leaves the default version
         /// alone, because .NET Framework rejects requests that ask for HTTP/2.
@@ -225,13 +238,43 @@ public async Task<Threat> Threat(string ip)
         private static HttpRequestMessage CreateRequest(HttpMethod method, Uri url)
         {
             var request = new HttpRequestMessage(method, url);
+            request.Headers.UserAgent.Add(UserAgent);
+            request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
 #if NET5_0_OR_GREATER
             request.Version = System.Net.HttpVersion.Version20;
             request.VersionPolicy = HttpVersionPolicy.RequestVersionOrLower;
 #endif
             return request;
         }
 
+        /// <summary>
+        /// Builds the <c>User-Agent</c> value from the assembly's informational version, which is the package
+        /// version without the <c>+commit</c> build metadata the SDK appends to it. Falls back to the assembly
+        /// version when the informational version is missing or is not a valid header token.
+        /// </summary>
+        /// <returns>The <c>ipdata-dotnet/&lt;version&gt;</c> product token.</returns>
+        private static ProductInfoHeaderValue CreateUserAgent()
+        {
+            var assembly = typeof(IPDataClient).Assembly;
+            var version = assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()?.InformationalVersion;
+
+            if (!string.IsNullOrWhiteSpace(version))
+            {
+                var metadataStart = version.IndexOf('+');
+                if (metadataStart >= 0)
+                {
+                    version = version.Substring(0, metadataStart);
+                }
+
+                if (ProductInfoHeaderValue.TryParse($"{UserAgentProduct}/{version}", out var userAgent))
+                {
+                    return userAgent;
+                }
+            }
+
+            return new ProductInfoHeaderValue(UserAgentProduct, assembly.GetName().Version?.ToString(3) ?? "0.0.0");
+        }
+
         /// <summary>Sends the request asynchronous and disposes the request and the response once the body has been read.</summary>
         /// <param name="httpClient">The HTTP client.</param>
         /// <param name="request">The request.</param>

src/IPData/Models/IPLookupResult.cs

@@ -78,8 +78,12 @@ public class IPLookupResult
         [JsonPropertyName("emoji_unicode")]
         public string EmojiUnicode { get; set; }
 
+        /// <summary>
+        /// The languages spoken where the address is located. The setter is public because
+        /// System.Text.Json does not populate collection properties that have a non-public setter.
+        /// </summary>
         [JsonPropertyName("languages")]
-        public List<Language> Languages { get; private set; } = new List<Language>();
+        public List<Language> Languages { get; set; } = new List<Language>();
 
         [JsonPropertyName("currency")]
         public Currency Currency { get; set; }

src/IPData/Models/Threat.cs

@@ -8,6 +8,12 @@ public class Threat
         [JsonPropertyName("is_tor")]
         public bool? IsTor { get; set; }
 
+        /// <summary>
+        /// Whether the address is a VPN endpoint, or <c>null</c> when the response does not include the key.
+        /// </summary>
+        [JsonPropertyName("is_vpn")]
+        public bool? IsVpn { get; set; }
+
         [JsonPropertyName("is_proxy")]
         public bool? IsProxy { get; set; }
 
@@ -38,5 +44,12 @@ public class Threat
         /// </summary>
         [JsonPropertyName("blocklists")]
         public List<BlocklistInfo> Blocklists { get; set; } = new List<BlocklistInfo>();
+
+        /// <summary>
+        /// The scores the API reports for the address (<c>vpn_score</c>, <c>proxy_score</c>, <c>threat_score</c>
+        /// and <c>trust_score</c>), or <c>null</c> when the response does not include the <c>scores</c> object.
+        /// </summary>
+        [JsonPropertyName("scores")]
+        public ThreatScores Scores { get; set; }
     }
 }

src/IPData/Models/ThreatScores.cs

@@ -0,0 +1,23 @@
+using System.Text.Json.Serialization;
+
+namespace IPData.Models
+{
+    /// <summary>
+    /// The <c>scores</c> object of a threat response. Each score is <c>null</c> when the response does not
+    /// include it.
+    /// </summary>
+    public class ThreatScores
+    {
+        [JsonPropertyName("vpn_score")]
+        public int? VpnScore { get; set; }
+
+        [JsonPropertyName("proxy_score")]
+        public int? ProxyScore { get; set; }
+
+        [JsonPropertyName("threat_score")]
+        public int? ThreatScore { get; set; }
+
+        [JsonPropertyName("trust_score")]
+        public int? TrustScore { get; set; }
+    }
+}

test/Unit/IPData.Tests/Exceptions/ApiExceptionTests.cs

@@ -1,4 +1,8 @@
-using System.Globalization;
+using System;
+using System.Collections.Generic;
+using System.Globalization;
+using System.Reflection;
+using System.Runtime.Serialization;
 using FluentAssertions;
 using IPData.Exceptions;
 using Xunit;
@@ -78,5 +82,32 @@ public void ApiExceptionException_WhenCreateWithValidJsonContent_ShouldBeDeseria
             // Assert
             sut.Message.Should().Be(message);
         }
+
+        public static IEnumerable<object[]> ExceptionTypes()
+        {
+            yield return new object[] { typeof(ApiException) };
+            yield return new object[] { typeof(BadRequestException) };
+            yield return new object[] { typeof(ForbiddenException) };
+            yield return new object[] { typeof(UnauthorizedException) };
+        }
+
+        [Theory]
+        [MemberData(nameof(ExceptionTypes))]
+        public void Exception_ShouldKeepTheProtectedSerializationConstructorAndMarkItObsolete(Type exceptionType)
+        {
+            // Act
+            var constructor = exceptionType.GetConstructor(
+                BindingFlags.Instance | BindingFlags.NonPublic,
+                null,
+                new[] { typeof(SerializationInfo), typeof(StreamingContext) },
+                null);
+
+            // Assert
+            constructor.Should().NotBeNull();
+            constructor.IsFamily.Should().BeTrue();
+            var obsolete = constructor.GetCustomAttribute<ObsoleteAttribute>();
+            obsolete.Should().NotBeNull();
+            obsolete.DiagnosticId.Should().Be("SYSLIB0051");
+        }
     }
 }