From ee830342b95b9bd5781245a39088df8eca88118c Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Fri, 14 Aug 2026 22:24:00 -0300 Subject: [PATCH 1/2] fix: accept perpetuum.ini files carrying obsolete connection string keywords (ISSUE-035) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The perpetuum.ini written by the Perpetuum Dedicated Server installer does not start this server. It was written for the original server, which used System.Data.SqlClient; this one uses Microsoft.Data.SqlClient, which differs in two ways that both abort startup before any zone loads. Measured against Microsoft.Data.SqlClient 6.0.1 with neutral resources, so the messages below are the ones a maintainer sees rather than a translation: Connection Reset=True System.NotSupportedException: The keyword 'Connection Reset' is not supported on this platform. thrown while SqlConnection is being constructed, not on Open(). no Encrypt / TrustServerCertificate SqlException on Open(): A connection was successfully established with the server, but then an error occurred during the login process. (provider: SSL Provider, error: 0 - ...) Neither message names perpetuum.ini, which is what makes a fresh setup hard to diagnose. The first is now handled. LegacyConnectionString.RemoveObsoleteKeywords drops keywords that Microsoft.Data.SqlClient refuses AND that the framework had already stopped honouring, so removing them cannot change how the server connects. PerpetuumBootstrapper.Init calls it directly after resolving GlobalConfiguration — before anything constructs a SqlConnection — and logs a warning naming perpetuum.ini and the keyword. Keywords that still carry meaning are deliberately left alone. Network Library selects a protocol and Context Connection selects a SQLCLR connection; both need an operator decision, and the driver's own error already names the replacement. Two implementation notes, both measured rather than assumed: - Parsing goes through DbConnectionStringBuilder, the provider-agnostic parser. SqlConnectionStringBuilder throws on the same keyword, so it cannot be used to find it. Splitting on ';' by hand was rejected because it corrupts quoted values containing a separator, verified against Password="a;b=c". - The input is returned untouched when nothing is removed. Rebuilding through DbConnectionStringBuilder lower-cases every key and drops the trailing separator, and a connection string that reaches a log should be the one the operator wrote. The second failure is documentation only. Defaulting TrustServerCertificate in code would weaken authentication for every operator in order to fix a local development case. README.md gains a setup section instead: the requirement, a working connection string, and the caveat that TrustServerCertificate=True belongs on a local instance only. Verified against a real server run in three states: Connection Reset present, fix applied warning naming perpetuum.ini and the keyword, then Database: perpetuumsa Connection Reset present, fix reverted the NotSupportedException above, the process exits, no mention of the file keyword absent, fix applied no warning, log identical to before The third state is the one that shows valid configurations are unaffected. --- README.md | 69 ++++++++++++++++ docs/backlog/issues.md | 31 ++++++- .../PerpetuumBootstrapper.cs | 9 +++ src/Perpetuum/Data/LegacyConnectionString.cs | 80 +++++++++++++++++++ 4 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 src/Perpetuum/Data/LegacyConnectionString.cs diff --git a/README.md b/README.md index b74c0a2b..c92b3511 100644 --- a/README.md +++ b/README.md @@ -4,3 +4,72 @@ # The Open Perpetuum Server 2 +## Running a local server + +Windows and x64 only. The bootstrapper is annotated `[SupportedOSPlatform("windows")]` and the Admin +Tool is WPF. + +You need the .NET 8 SDK, a SQL Server instance, and the `perpetuumsa` database — see +[OPDB](https://github.com/OpenPerpetuum/OPDB) for restoring and patching it. + +### Build + +``` +dotnet build PerpetuumServer2.sln -c Release -p:Platform=x64 +``` + +### Configure + +The server reads `perpetuum.ini` from the game root directory. `ConnectionString` is the only setting +that must match your machine. + +**The `perpetuum.ini` written by the Perpetuum Dedicated Server installer will not start this server.** +It was written for the original server, which used `System.Data.SqlClient`; this one uses +`Microsoft.Data.SqlClient`, which differs in two ways that both abort startup before any zone loads: + +1. **`Connection Reset` was removed.** `Microsoft.Data.SqlClient` refuses the keyword while the + connection is being constructed: + + ``` + The keyword 'Connection Reset' is not supported on this platform. + ``` + + The server drops this keyword for you and logs a warning naming the file, because the framework + had already stopped honouring it — a pooled connection is always reset. Deleting it from + `perpetuum.ini` silences the warning. + +2. **`Encrypt` now defaults to `true`.** Since version 4.0 the driver encrypts by default and + validates the server certificate. A local SQL Server using a self-signed certificate fails logon + in the SSL provider: + + ``` + A connection was successfully established with the server, but then an error occurred during + the login process. (provider: SSL Provider, error: 0 - The certificate chain was issued by an + authority that is not trusted.) + ``` + + The trailing part of that message comes from Windows and appears in the system language, so match + on the condition rather than the exact text. **This one you must fix yourself** — the server + cannot decide for you whether skipping certificate validation is acceptable. + +A connection string that works against a local named instance with Windows authentication: + +``` +Server=localhost\PERPSQL;Database=perpetuumsa;Trusted_Connection=True;TrustServerCertificate=True;Pooling=True;Connection Timeout=30;Connection Lifetime=260;Min Pool Size=20;Max Pool Size=60; +``` + +`TrustServerCertificate=True` keeps the connection encrypted but skips validating the certificate. +**It is appropriate for a local development instance only.** Do not carry it into a deployment where +the connection leaves the machine — install a trusted certificate there instead. Setting +`Encrypt=False` also works locally and is strictly worse: it drops the encryption as well. + +### Run + +``` +cd src/Perpetuum.Server +dotnet run -- "C:\PerpetuumServer\data" +``` + +The server is up when the log reads `>>>> Perpetuum Server State : [Online]`. Ctrl+C shuts it down; +a clean shutdown ends at `State : [Off]`. + diff --git a/docs/backlog/issues.md b/docs/backlog/issues.md index 40bdde3f..138d3c9d 100644 --- a/docs/backlog/issues.md +++ b/docs/backlog/issues.md @@ -52,7 +52,7 @@ Reproduced on a local P36.8 database (`develop` at `f9ddac2`) with the ISSUE-036 ## ISSUE-035 - Server fails to start with the perpetuum.ini produced by the official installer -Status: TODO +Status: IN_PROGRESS Priority: HIGH Area: Configuration / Setup @@ -77,6 +77,35 @@ Reproduced against SQL Server 2022 Express, named instance, Windows integrated a `TrustServerCertificate=True` disables server certificate validation. It is appropriate for a local development instance only and must not be carried into a deployment where the connection leaves the machine. +### Progress + +Both failures re-measured against `Microsoft.Data.SqlClient` 6.0.1 with neutral resources, so the messages below are the ones a maintainer sees rather than a translation: + +- `Connection Reset=True` → `System.NotSupportedException: The keyword 'Connection Reset' is not supported on this platform.`, thrown while `SqlConnection` is being **constructed**, not on `Open()`. +- No `Encrypt` / `TrustServerCertificate` → `SqlException` on `Open()`: `A connection was successfully established with the server, but then an error occurred during the login process. (provider: SSL Provider, error: 0 - ...)`. The trailing clause comes from Win32 and stays in the system language. + +**Keyword 1 is now handled in code.** `LegacyConnectionString.RemoveObsoleteKeywords` drops keywords that `Microsoft.Data.SqlClient` refuses *and* that the framework had already stopped honouring, so removing them cannot change how the server connects. `PerpetuumBootstrapper.Init` calls it right after resolving `GlobalConfiguration` and logs a warning naming `perpetuum.ini` and the keyword. + +Keywords that still carry meaning are deliberately left alone — `Network Library` selects a protocol, `Context Connection` selects a SQLCLR connection. Both need an operator decision and the driver's own error already names the replacement. + +Parsing goes through `DbConnectionStringBuilder`, the provider-agnostic parser, because `SqlConnectionStringBuilder` throws on the same keyword and so cannot be used to find it. Splitting on `;` by hand was rejected: it corrupts any quoted value containing a separator, verified against `Password="a;b=c"`. + +**Keyword 2 is documentation only.** Defaulting `TrustServerCertificate` in code would weaken authentication for every operator to fix a local development case, so `README.md` now carries a setup section stating the requirement, the working connection string, and the caveat that `TrustServerCertificate=True` belongs on a local instance only. + +Verified in three states against a real server run: + +| State | Result | +|---|---| +| `Connection Reset` present, fix applied | Warning naming `perpetuum.ini` and the keyword, then `Database: perpetuumsa` — startup proceeds | +| `Connection Reset` present, fix reverted | `The keyword 'Connection Reset' is not supported on this platform.` and the process exits, with no mention of `perpetuum.ini` | +| Keyword absent, fix applied | No warning, log identical to before the change | + +The third state matters: the connection string is returned untouched when nothing is removed, because rebuilding it through `DbConnectionStringBuilder` lower-cases every key and drops the trailing separator. + +### Notes on the documentation location + +The Proposed Fix above suggested a setup page under `docs/codebase/`. It went into `README.md` instead: the repository had no setup documentation of any kind, `README.md` was a badge and a title, and it is where someone who just ran the installer looks first. `docs/codebase/` describes the codebase for contributors; this is an operator instruction. Happy to move it if the maintainers prefer. + --- --- diff --git a/src/Perpetuum.Bootstrapper/PerpetuumBootstrapper.cs b/src/Perpetuum.Bootstrapper/PerpetuumBootstrapper.cs index ff5e749a..c0d26d4d 100644 --- a/src/Perpetuum.Bootstrapper/PerpetuumBootstrapper.cs +++ b/src/Perpetuum.Bootstrapper/PerpetuumBootstrapper.cs @@ -139,6 +139,15 @@ public void Init(string gameRoot) GlobalConfiguration config = _container.Resolve(); _container.Resolve().State = HostState.Init; + // Before anything builds a SqlConnection from it, which happens further down at the + // DbConnectionFactory resolve. A perpetuum.ini written for the original server carries + // keywords Microsoft.Data.SqlClient refuses, and its error names the keyword but not + // the file. + config.ConnectionString = LegacyConnectionString.RemoveObsoleteKeywords(config.ConnectionString, out IReadOnlyList obsoleteKeywords); + foreach (string keyword in obsoleteKeywords) + { + Logger.Warning($"perpetuum.ini: ignoring the connection string keyword '{keyword}'. Microsoft.Data.SqlClient does not accept it and the framework had already stopped honouring it, so removing it changes nothing. Delete it from perpetuum.ini to silence this warning."); + } Logger.Info($"Game root: {config.GameRoot}"); Logger.Info($"GC isServerGC: {GCSettings.IsServerGC}"); diff --git a/src/Perpetuum/Data/LegacyConnectionString.cs b/src/Perpetuum/Data/LegacyConnectionString.cs new file mode 100644 index 00000000..e7576e51 --- /dev/null +++ b/src/Perpetuum/Data/LegacyConnectionString.cs @@ -0,0 +1,80 @@ +using System.Data.Common; + +namespace Perpetuum.Data +{ + /// + /// Microsoft.Data.SqlClient rejects connection string keywords that System.Data.SqlClient + /// accepted. A perpetuum.ini written for the original server carries them, and SqlConnection + /// throws while it is being constructed, before any query runs, with an error that names the + /// keyword but not the file it came from. + /// + /// Only keywords the framework had already stopped honouring are removed here, so dropping + /// them cannot change how the server connects. Keywords that still carry meaning are left in + /// place deliberately: Network Library selects a protocol and Context Connection selects a + /// SQLCLR connection, both need an operator decision, and the driver's own error already + /// names the keyword and its replacement. + /// + public static class LegacyConnectionString + { + private static readonly string[] ObsoleteKeywords = + { + // Ignored since .NET Framework 4.5 — a pooled connection is always reset. + "Connection Reset", + + // Ignored since .NET Framework 4.5 — asynchronous execution no longer opts in. + "Asynchronous Processing", + }; + + /// + /// Returns without the obsolete keywords, and reports + /// which ones were dropped. Returns the input untouched when it carries none of them, and + /// when it cannot be parsed at all. + /// + public static string RemoveObsoleteKeywords(string connectionString, out IReadOnlyList removed) + { + removed = Array.Empty(); + + if (string.IsNullOrWhiteSpace(connectionString)) + { + return connectionString; + } + + DbConnectionStringBuilder builder; + try + { + // DbConnectionStringBuilder is the provider-agnostic parser: it applies the + // ADO.NET quoting rules without validating keywords against any driver, so it + // reads strings that SqlConnection and SqlConnectionStringBuilder both refuse. + // Splitting on ';' by hand would corrupt any quoted value containing a separator. + builder = new DbConnectionStringBuilder { ConnectionString = connectionString }; + } + catch (ArgumentException) + { + // Malformed beyond the keywords this handles. Hand it back untouched so the + // connection attempt fails exactly as it does today. + return connectionString; + } + + List dropped = null; + foreach (string keyword in ObsoleteKeywords) + { + if (builder.Remove(keyword)) + { + (dropped ??= new List()).Add(keyword); + } + } + + if (dropped == null) + { + // Nothing to fix. Return the original rather than the rebuilt string, because + // rebuilding lower-cases every key and drops the trailing separator, and a + // connection string that appears in a log should be the one the operator wrote. + return connectionString; + } + + removed = dropped; + + return builder.ConnectionString; + } + } +} From b14cf40a54c1b3448c47832f71e9521135eadbe2 Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Sun, 16 Aug 2026 03:12:57 -0300 Subject: [PATCH 2/2] Report rejected connection string settings instead of removing them Maintainer direction, given on Discord on 2026-08-15: the installer already deploys a corrected perpetuum.ini, so this only bites when a fresh PP2 is pointed at an old PP1 data folder, and a clear diagnostic serves that case better than silently accepting the file. This is also what ISSUE-035's own Proposed Fix asked for: "an error that names perpetuum.ini and the offending key". LegacyConnectionString is replaced by ConnectionStringSupport, which reports and never rewrites. The connection string reaches the driver exactly as the operator wrote it. PerpetuumBootstrapper.Init now logs an error naming perpetuum.ini, the directory it sits in, and every offending setting, then throws so the server does not start on a connection that cannot open. There is no precedent for refusing to start in this file -- it holds no other throw -- so the choice of InvalidOperationException is stated in the pull request for the maintainers to overrule. The check keeps no keyword list. It parses with DbConnectionStringBuilder, then offers each setting to SqlConnectionStringBuilder and reports whatever that refuses. Two consequences: Network Library and Context Connection are now named, where removal had to leave them alone because dropping them would change how the server connects; and a setting nobody anticipated is reported the same way. Every offending setting is reported in one message. The installer's file carries more than one, so a first-failure-only report would cost the operator a restart per setting. The catch around the per-setting probe is deliberately broad. Measured against Microsoft.Data.SqlClient 6.0.1, the driver uses three different exception types for this -- NotSupportedException for Connection Reset and Network Library, ArgumentException for Asynchronous Processing, InvalidOperationException for Context Connection -- and a narrow filter that missed one would report the setting as supported and hand the operator the obscure failure this check exists to replace. Verified: - 15 unit tests in ConnectionStringSupportTests, 8 of them observed failing against a stub before the detection was written - full unit tier green, 73/73 - solution builds with 0 errors, and no warning comes from a project this touches - a game root carrying Connection Reset and Network Library produces one ERR line naming both and exits 1, before anything else in Init needs the game root - tools/smoke-test.ps1 green against the real database with a good file: [Online] after 80s, 6435 members spawned, graceful shutdown, exit 0 Co-Authored-By: Claude Opus 5 --- README.md | 12 +- docs/backlog/issues.md | 33 ++++- .../PerpetuumBootstrapper.cs | 18 ++- .../Unit/ConnectionStringSupportTests.cs | 138 ++++++++++++++++++ src/Perpetuum/Data/ConnectionStringSupport.cs | 73 +++++++++ src/Perpetuum/Data/LegacyConnectionString.cs | 80 ---------- 6 files changed, 258 insertions(+), 96 deletions(-) create mode 100644 src/Perpetuum.Tests/Unit/ConnectionStringSupportTests.cs create mode 100644 src/Perpetuum/Data/ConnectionStringSupport.cs delete mode 100644 src/Perpetuum/Data/LegacyConnectionString.cs diff --git a/README.md b/README.md index c92b3511..551f4039 100644 --- a/README.md +++ b/README.md @@ -34,9 +34,15 @@ It was written for the original server, which used `System.Data.SqlClient`; this The keyword 'Connection Reset' is not supported on this platform. ``` - The server drops this keyword for you and logs a warning naming the file, because the framework - had already stopped honouring it — a pooled connection is always reset. Deleting it from - `perpetuum.ini` silences the warning. + The server checks the connection string before it connects and refuses to start if the driver + would reject any of it, naming `perpetuum.ini`, the directory it is in, and **every** setting the + driver refused — so one restart is enough to clear them all rather than one restart per setting. + Delete them from `perpetuum.ini`. `Connection Reset` in particular is safe to delete outright: + the framework stopped honouring it long ago, because a pooled connection is always reset. + + The check keeps no list of its own — it asks the driver about each setting in turn, so a keyword + nobody anticipated is reported the same way. `Network Library` and `Context Connection` are also + refused and will be named if they are present. 2. **`Encrypt` now defaults to `true`.** Since version 4.0 the driver encrypts by default and validates the server certificate. A local SQL Server using a self-signed certificate fails logon diff --git a/docs/backlog/issues.md b/docs/backlog/issues.md index 138d3c9d..ea201861 100644 --- a/docs/backlog/issues.md +++ b/docs/backlog/issues.md @@ -84,23 +84,40 @@ Both failures re-measured against `Microsoft.Data.SqlClient` 6.0.1 with neutral - `Connection Reset=True` → `System.NotSupportedException: The keyword 'Connection Reset' is not supported on this platform.`, thrown while `SqlConnection` is being **constructed**, not on `Open()`. - No `Encrypt` / `TrustServerCertificate` → `SqlException` on `Open()`: `A connection was successfully established with the server, but then an error occurred during the login process. (provider: SSL Provider, error: 0 - ...)`. The trailing clause comes from Win32 and stays in the system language. -**Keyword 1 is now handled in code.** `LegacyConnectionString.RemoveObsoleteKeywords` drops keywords that `Microsoft.Data.SqlClient` refuses *and* that the framework had already stopped honouring, so removing them cannot change how the server connects. `PerpetuumBootstrapper.Init` calls it right after resolving `GlobalConfiguration` and logs a warning naming `perpetuum.ini` and the keyword. +**Keyword 1 is now handled in code, by reporting rather than repairing.** `ConnectionStringSupport.FindUnsupportedKeywords` returns every setting the driver will refuse. `PerpetuumBootstrapper.Init` calls it right after resolving `GlobalConfiguration`, and when the list is not empty it logs an error naming `perpetuum.ini`, the directory it sits in, and every offending setting, then throws so the server does not start. The connection string itself is never modified — the operator's file stays the only source of truth for how this server connects. -Keywords that still carry meaning are deliberately left alone — `Network Library` selects a protocol, `Context Connection` selects a SQLCLR connection. Both need an operator decision and the driver's own error already names the replacement. +This follows the Proposed Fix above, which already asked for "an error that names `perpetuum.ini` and the offending key". An earlier revision of this work removed the keywords instead; that was changed on maintainer direction. -Parsing goes through `DbConnectionStringBuilder`, the provider-agnostic parser, because `SqlConnectionStringBuilder` throws on the same keyword and so cannot be used to find it. Splitting on `;` by hand was rejected: it corrupts any quoted value containing a separator, verified against `Password="a;b=c"`. +**The check keeps no list of its own.** It parses with `DbConnectionStringBuilder` — the provider-agnostic parser, which applies the ADO.NET quoting rules without validating against any driver — and then offers each setting to `SqlConnectionStringBuilder` in turn. Whatever that refuses is reported. So `Network Library` and `Context Connection` are now named too, where the removal-based revision had to leave them alone because removing them would have changed how the server connects. A setting nobody anticipated is reported the same way, and nothing here falls out of date when the driver changes. + +Splitting on `;` by hand was rejected: it corrupts any quoted value containing a separator, verified against `Password="a;b=c"`. + +The catch around the per-setting probe is deliberately broad, because the driver does not use one exception type. Measured against `Microsoft.Data.SqlClient` 6.0.1, and `SqlConnectionStringBuilder` and `SqlConnection` agree on every one: + +| Setting | Exception | +|---|---| +| `Connection Reset` | `NotSupportedException` | +| `Network Library` | `NotSupportedException` | +| `Asynchronous Processing` | `ArgumentException` | +| `Context Connection` | `InvalidOperationException` | + +A narrow filter that missed a type would report the setting as supported and hand the operator exactly the obscure startup failure this check exists to replace. + +All offending settings are reported in one message rather than one at a time, because the installer's file carries more than one and a first-failure-only report would cost a restart per setting. **Keyword 2 is documentation only.** Defaulting `TrustServerCertificate` in code would weaken authentication for every operator to fix a local development case, so `README.md` now carries a setup section stating the requirement, the working connection string, and the caveat that `TrustServerCertificate=True` belongs on a local instance only. -Verified in three states against a real server run: +Verified in three states: | State | Result | |---|---| -| `Connection Reset` present, fix applied | Warning naming `perpetuum.ini` and the keyword, then `Database: perpetuumsa` — startup proceeds | -| `Connection Reset` present, fix reverted | `The keyword 'Connection Reset' is not supported on this platform.` and the process exits, with no mention of `perpetuum.ini` | -| Keyword absent, fix applied | No warning, log identical to before the change | +| `Connection Reset` **and** `Network Library` present, check applied | One `ERR` line naming the game root and both settings, then the process exits with code 1. The check runs before anything else in `Init` needs the game root | +| Neither present, check applied | `tools/smoke-test.ps1` green against the real database: `[Online]` after 80 s, 6435 members spawned, graceful shutdown in 29 s, exit 0. No new output on the startup path | +| Settings present, check absent | `System.NotSupportedException: The keyword 'Connection Reset' is not supported on this platform.`, naming the keyword but not the file. Held as an automated test rather than a manual observation — `ConnectionStringSupportTests.The_driver_really_does_reject_the_installer_string` fails the moment the driver stops rejecting it, which would make the whole check dead weight | + +Covered by 15 unit tests in `src/Perpetuum.Tests/Unit/ConnectionStringSupportTests.cs`. Eight of them were observed failing against a stub returning an empty list before the detection was written. -The third state matters: the connection string is returned untouched when nothing is removed, because rebuilding it through `DbConnectionStringBuilder` lower-cases every key and drops the trailing separator. +Reported settings carry the lower-cased spelling `DbConnectionStringBuilder` produces, not the operator's own capitalisation. Left as is: any case-insensitive search finds the line, and preserving the original spelling would mean re-scanning the raw string for cosmetics. ### Notes on the documentation location diff --git a/src/Perpetuum.Bootstrapper/PerpetuumBootstrapper.cs b/src/Perpetuum.Bootstrapper/PerpetuumBootstrapper.cs index c0d26d4d..92432c1b 100644 --- a/src/Perpetuum.Bootstrapper/PerpetuumBootstrapper.cs +++ b/src/Perpetuum.Bootstrapper/PerpetuumBootstrapper.cs @@ -141,12 +141,20 @@ public void Init(string gameRoot) // Before anything builds a SqlConnection from it, which happens further down at the // DbConnectionFactory resolve. A perpetuum.ini written for the original server carries - // keywords Microsoft.Data.SqlClient refuses, and its error names the keyword but not - // the file. - config.ConnectionString = LegacyConnectionString.RemoveObsoleteKeywords(config.ConnectionString, out IReadOnlyList obsoleteKeywords); - foreach (string keyword in obsoleteKeywords) + // settings Microsoft.Data.SqlClient refuses, and its error names the setting but not + // the file it came from, which is the part an operator needs. + IReadOnlyList unsupported = ConnectionStringSupport.FindUnsupportedKeywords(config.ConnectionString); + if (unsupported.Count > 0) { - Logger.Warning($"perpetuum.ini: ignoring the connection string keyword '{keyword}'. Microsoft.Data.SqlClient does not accept it and the framework had already stopped honouring it, so removing it changes nothing. Delete it from perpetuum.ini to silence this warning."); + string message = + $"perpetuum.ini in {config.GameRoot} has a connectionString carrying " + + $"{unsupported.Count} setting(s) Microsoft.Data.SqlClient does not accept: " + + $"{string.Join(", ", unsupported)}. Remove them and start the server again. " + + "The original server used System.Data.SqlClient, which accepted them."; + + Logger.Error(message); + + throw new InvalidOperationException(message); } Logger.Info($"Game root: {config.GameRoot}"); diff --git a/src/Perpetuum.Tests/Unit/ConnectionStringSupportTests.cs b/src/Perpetuum.Tests/Unit/ConnectionStringSupportTests.cs new file mode 100644 index 00000000..93f69273 --- /dev/null +++ b/src/Perpetuum.Tests/Unit/ConnectionStringSupportTests.cs @@ -0,0 +1,138 @@ +using Microsoft.Data.SqlClient; +using Perpetuum.Data; +using Xunit; + +namespace Perpetuum.Tests.Unit +{ + public class ConnectionStringSupportTests + { + private const string Installer = + @"Server=localhost\PERPSQL;Database=perpetuumsa;Trusted_Connection=True;Connection Reset=True;TrustServerCertificate=True;Pooling=True;"; + + private const string Supported = + @"Server=localhost\PERPSQL;Database=perpetuumsa;Trusted_Connection=True;TrustServerCertificate=True;Pooling=True;Connection Timeout=30;"; + + /// + /// The premise of the whole class. If Microsoft.Data.SqlClient ever starts accepting the + /// keyword again, this fails and the rest of the file becomes dead weight. + /// + [Fact] + public void The_driver_really_does_reject_the_installer_string() + { + NotSupportedException ex = Assert.Throws(() => new SqlConnection(Installer)); + + Assert.Contains("Connection Reset", ex.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void The_keyword_the_installer_writes_is_reported() + { + IReadOnlyList unsupported = ConnectionStringSupport.FindUnsupportedKeywords(Installer); + + Assert.Single(unsupported); + Assert.Contains("Connection Reset", unsupported[0], StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void A_string_the_driver_accepts_reports_nothing() + { + Assert.Empty(ConnectionStringSupport.FindUnsupportedKeywords(Supported)); + } + + /// + /// The installer's file carries more than one problem at a time. Reporting them one at a + /// time would cost the operator a restart per keyword. + /// + [Fact] + public void Every_rejected_keyword_is_reported_in_one_pass() + { + IReadOnlyList unsupported = ConnectionStringSupport.FindUnsupportedKeywords( + Supported + "Connection Reset=True;Asynchronous Processing=True;"); + + Assert.Equal(2, unsupported.Count); + Assert.Contains(unsupported, k => k.Contains("Connection Reset", StringComparison.OrdinalIgnoreCase)); + Assert.Contains(unsupported, k => k.Contains("Asynchronous Processing", StringComparison.OrdinalIgnoreCase)); + } + + /// + /// These two were deliberately left alone while the code removed keywords, because removing + /// them would have changed how the server connects. Reporting carries no such risk, so they + /// are in scope now. + /// + [Theory] + [InlineData("Network Library=dbmssocn;")] + [InlineData("Context Connection=True;")] + public void Keywords_that_still_carry_meaning_are_reported_too(string setting) + { + IReadOnlyList unsupported = ConnectionStringSupport.FindUnsupportedKeywords(Supported + setting); + + Assert.Single(unsupported); + } + + /// + /// No hardcoded list to fall out of date: anything the driver refuses is reported, including + /// a keyword nobody anticipated. + /// + [Fact] + public void A_keyword_no_list_could_have_predicted_is_reported() + { + IReadOnlyList unsupported = ConnectionStringSupport.FindUnsupportedKeywords( + Supported + "Totally Made Up Keyword=1;"); + + Assert.Single(unsupported); + Assert.Contains("Totally Made Up Keyword", unsupported[0], StringComparison.OrdinalIgnoreCase); + } + + [Theory] + [InlineData("connection reset=True;")] + [InlineData("CONNECTION RESET=True;")] + public void Keyword_matching_ignores_case(string spelling) + { + Assert.Single(ConnectionStringSupport.FindUnsupportedKeywords(Supported + spelling)); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void Absent_or_blank_input_reports_nothing(string input) + { + Assert.Empty(ConnectionStringSupport.FindUnsupportedKeywords(input)); + } + + /// + /// Malformed beyond anything this can explain. Report nothing and let the driver's own + /// error stand, rather than inventing a second explanation for the same failure. + /// + [Fact] + public void An_unparseable_string_reports_nothing() + { + Assert.Empty(ConnectionStringSupport.FindUnsupportedKeywords("=;=;")); + } + + /// + /// A value may legally contain the separator when quoted. Splitting on ';' by hand would + /// invent keywords that are not there. + /// + [Fact] + public void A_quoted_value_containing_the_separator_does_not_invent_a_keyword() + { + IReadOnlyList unsupported = ConnectionStringSupport.FindUnsupportedKeywords( + @"Server=localhost;Database=perpetuumsa;Password=""a;b"";Trusted_Connection=True;"); + + Assert.Empty(unsupported); + } + + /// + /// The operator has to find the keyword in perpetuum.ini after reading it in the log, so + /// what is reported has to match what is in the file. + /// + [Fact] + public void A_reported_keyword_can_be_found_in_the_original_string() + { + IReadOnlyList unsupported = ConnectionStringSupport.FindUnsupportedKeywords(Installer); + + Assert.Contains(unsupported[0], Installer, StringComparison.OrdinalIgnoreCase); + } + } +} diff --git a/src/Perpetuum/Data/ConnectionStringSupport.cs b/src/Perpetuum/Data/ConnectionStringSupport.cs new file mode 100644 index 00000000..d88911f7 --- /dev/null +++ b/src/Perpetuum/Data/ConnectionStringSupport.cs @@ -0,0 +1,73 @@ +using System.Data.Common; +using Microsoft.Data.SqlClient; + +namespace Perpetuum.Data +{ + /// + /// Microsoft.Data.SqlClient rejects connection string settings that System.Data.SqlClient + /// accepted, and a perpetuum.ini written for the original server carries them. SqlConnection + /// then throws while it is being constructed, before any query runs, with an error that names + /// the setting but not the file it came from. + /// + /// Nothing here changes the connection string. It reports what the driver will refuse so the + /// caller can say which file to edit — the operator's own file stays the only source of truth + /// for how this server connects. + /// + public static class ConnectionStringSupport + { + /// + /// Returns every setting in that + /// Microsoft.Data.SqlClient will not accept, in the order they appear. Returns an empty + /// list when the string is usable, absent, or malformed beyond this check — in the last + /// case the driver's own error is the better explanation and is left to stand. + /// + public static IReadOnlyList FindUnsupportedKeywords(string connectionString) + { + if (string.IsNullOrWhiteSpace(connectionString)) + { + return Array.Empty(); + } + + DbConnectionStringBuilder permissive; + try + { + // The provider-agnostic parser: it applies the ADO.NET quoting rules without + // validating anything against a driver, so it reads strings that SqlConnection and + // SqlConnectionStringBuilder both refuse. Splitting on ';' by hand would corrupt a + // quoted value containing a separator, and invent keywords that are not there. + permissive = new DbConnectionStringBuilder { ConnectionString = connectionString }; + } + catch (ArgumentException) + { + return Array.Empty(); + } + + List unsupported = null; + + foreach (string keyword in permissive.Keys.Cast()) + { + try + { + // The driver is the authority on what it accepts. Asking it per setting needs + // no list of our own, so nothing here can fall out of date when the driver + // changes, and a setting nobody anticipated is still reported. + _ = new SqlConnectionStringBuilder { [keyword] = permissive[keyword] }; + } + catch (Exception) + { + // Deliberately broad. The driver does not use one exception type for this: + // measured against Microsoft.Data.SqlClient 6.0.1, 'Connection Reset' and + // 'Network Library' throw NotSupportedException, 'Asynchronous Processing' + // throws ArgumentException, and 'Context Connection' throws + // InvalidOperationException. A narrow filter that misses a type would report + // the setting as supported and hand the operator the obscure startup failure + // this check exists to replace; a broad one can at worst name a setting that + // failed for some other reason, which still points at the right line. + (unsupported ??= new List()).Add(keyword); + } + } + + return unsupported ?? (IReadOnlyList)Array.Empty(); + } + } +} diff --git a/src/Perpetuum/Data/LegacyConnectionString.cs b/src/Perpetuum/Data/LegacyConnectionString.cs deleted file mode 100644 index e7576e51..00000000 --- a/src/Perpetuum/Data/LegacyConnectionString.cs +++ /dev/null @@ -1,80 +0,0 @@ -using System.Data.Common; - -namespace Perpetuum.Data -{ - /// - /// Microsoft.Data.SqlClient rejects connection string keywords that System.Data.SqlClient - /// accepted. A perpetuum.ini written for the original server carries them, and SqlConnection - /// throws while it is being constructed, before any query runs, with an error that names the - /// keyword but not the file it came from. - /// - /// Only keywords the framework had already stopped honouring are removed here, so dropping - /// them cannot change how the server connects. Keywords that still carry meaning are left in - /// place deliberately: Network Library selects a protocol and Context Connection selects a - /// SQLCLR connection, both need an operator decision, and the driver's own error already - /// names the keyword and its replacement. - /// - public static class LegacyConnectionString - { - private static readonly string[] ObsoleteKeywords = - { - // Ignored since .NET Framework 4.5 — a pooled connection is always reset. - "Connection Reset", - - // Ignored since .NET Framework 4.5 — asynchronous execution no longer opts in. - "Asynchronous Processing", - }; - - /// - /// Returns without the obsolete keywords, and reports - /// which ones were dropped. Returns the input untouched when it carries none of them, and - /// when it cannot be parsed at all. - /// - public static string RemoveObsoleteKeywords(string connectionString, out IReadOnlyList removed) - { - removed = Array.Empty(); - - if (string.IsNullOrWhiteSpace(connectionString)) - { - return connectionString; - } - - DbConnectionStringBuilder builder; - try - { - // DbConnectionStringBuilder is the provider-agnostic parser: it applies the - // ADO.NET quoting rules without validating keywords against any driver, so it - // reads strings that SqlConnection and SqlConnectionStringBuilder both refuse. - // Splitting on ';' by hand would corrupt any quoted value containing a separator. - builder = new DbConnectionStringBuilder { ConnectionString = connectionString }; - } - catch (ArgumentException) - { - // Malformed beyond the keywords this handles. Hand it back untouched so the - // connection attempt fails exactly as it does today. - return connectionString; - } - - List dropped = null; - foreach (string keyword in ObsoleteKeywords) - { - if (builder.Remove(keyword)) - { - (dropped ??= new List()).Add(keyword); - } - } - - if (dropped == null) - { - // Nothing to fix. Return the original rather than the rebuilt string, because - // rebuilding lower-cases every key and drops the trailing separator, and a - // connection string that appears in a log should be the one the operator wrote. - return connectionString; - } - - removed = dropped; - - return builder.ConnectionString; - } - } -}