diff --git a/README.md b/README.md index b74c0a2b..551f4039 100644 --- a/README.md +++ b/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]`. + diff --git a/docs/backlog/issues.md b/docs/backlog/issues.md index 40bdde3f..ea201861 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,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. + --- --- diff --git a/src/Perpetuum.Bootstrapper/PerpetuumBootstrapper.cs b/src/Perpetuum.Bootstrapper/PerpetuumBootstrapper.cs index ff5e749a..92432c1b 100644 --- a/src/Perpetuum.Bootstrapper/PerpetuumBootstrapper.cs +++ b/src/Perpetuum.Bootstrapper/PerpetuumBootstrapper.cs @@ -139,6 +139,23 @@ 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 + // 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) + { + 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}"); 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(); + } + } +}