ISSUE-035: accept perpetuum.ini files carrying obsolete connection string keywords

README.md

@@ -4,3 +4,78 @@
 
 # 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 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
+   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]`.
+

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,52 @@ 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, 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.
+
+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.
+
+**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:
+
+| State | Result |
+|---|---|
+| `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.
+
+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
+
+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.
+
 ---
 
 ---

src/Perpetuum.Bootstrapper/PerpetuumBootstrapper.cs

@@ -139,6 +139,23 @@ public void Init(string gameRoot)
             GlobalConfiguration config = _container.Resolve<GlobalConfiguration>();
             _container.Resolve<IHostStateService>().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
+            // 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<string> unsupported = ConnectionStringSupport.FindUnsupportedKeywords(config.ConnectionString);
+            if (unsupported.Count > 0)
+            {
+                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}");
             Logger.Info($"GC isServerGC: {GCSettings.IsServerGC}");

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;";
+
+        /// <summary>
+        /// 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.
+        /// </summary>
+        [Fact]
+        public void The_driver_really_does_reject_the_installer_string()
+        {
+            NotSupportedException ex = Assert.Throws<NotSupportedException>(() => new SqlConnection(Installer));
+
+            Assert.Contains("Connection Reset", ex.Message, StringComparison.OrdinalIgnoreCase);
+        }
+
+        [Fact]
+        public void The_keyword_the_installer_writes_is_reported()
+        {
+            IReadOnlyList<string> 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));
+        }
+
+        /// <summary>
+        /// 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.
+        /// </summary>
+        [Fact]
+        public void Every_rejected_keyword_is_reported_in_one_pass()
+        {
+            IReadOnlyList<string> 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));
+        }
+
+        /// <summary>
+        /// 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.
+        /// </summary>
+        [Theory]
+        [InlineData("Network Library=dbmssocn;")]
+        [InlineData("Context Connection=True;")]
+        public void Keywords_that_still_carry_meaning_are_reported_too(string setting)
+        {
+            IReadOnlyList<string> unsupported = ConnectionStringSupport.FindUnsupportedKeywords(Supported + setting);
+
+            Assert.Single(unsupported);
+        }
+
+        /// <summary>
+        /// No hardcoded list to fall out of date: anything the driver refuses is reported, including
+        /// a keyword nobody anticipated.
+        /// </summary>
+        [Fact]
+        public void A_keyword_no_list_could_have_predicted_is_reported()
+        {
+            IReadOnlyList<string> 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));
+        }
+
+        /// <summary>
+        /// 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.
+        /// </summary>
+        [Fact]
+        public void An_unparseable_string_reports_nothing()
+        {
+            Assert.Empty(ConnectionStringSupport.FindUnsupportedKeywords("=;=;"));
+        }
+
+        /// <summary>
+        /// A value may legally contain the separator when quoted. Splitting on ';' by hand would
+        /// invent keywords that are not there.
+        /// </summary>
+        [Fact]
+        public void A_quoted_value_containing_the_separator_does_not_invent_a_keyword()
+        {
+            IReadOnlyList<string> unsupported = ConnectionStringSupport.FindUnsupportedKeywords(
+                @"Server=localhost;Database=perpetuumsa;Password=""a;b"";Trusted_Connection=True;");
+
+            Assert.Empty(unsupported);
+        }
+
+        /// <summary>
+        /// 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.
+        /// </summary>
+        [Fact]
+        public void A_reported_keyword_can_be_found_in_the_original_string()
+        {
+            IReadOnlyList<string> unsupported = ConnectionStringSupport.FindUnsupportedKeywords(Installer);
+
+            Assert.Contains(unsupported[0], Installer, StringComparison.OrdinalIgnoreCase);
+        }
+    }
+}