From a353232e6222598dcc74be84f3761dccc6ad6b24 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sun, 27 Apr 2025 13:34:05 -0400 Subject: [PATCH] Do not pass TGS params when running without DMAPI --- .../Components/Engine/ByondInstallation.cs | 13 ++- .../Components/Engine/EngineExecutableLock.cs | 4 +- .../Engine/EngineInstallationBase.cs | 11 ++- .../Components/Engine/IEngineInstallation.cs | 8 +- .../Engine/OpenDreamInstallation.cs | 16 ++-- .../Session/SessionControllerFactory.cs | 18 ++-- tests/DMAPI/BasicOperation/Test.dm | 15 +-- .../Live/Instance/WatchdogTest.cs | 92 ++++++++++--------- 8 files changed, 101 insertions(+), 76 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Engine/ByondInstallation.cs b/src/Tgstation.Server.Host/Components/Engine/ByondInstallation.cs index 8ee095babc..524151784f 100644 --- a/src/Tgstation.Server.Host/Components/Engine/ByondInstallation.cs +++ b/src/Tgstation.Server.Host/Components/Engine/ByondInstallation.cs @@ -113,19 +113,24 @@ namespace Tgstation.Server.Host.Components.Engine /// public override string FormatServerArguments( IDmbProvider dmbProvider, - IReadOnlyDictionary parameters, + IReadOnlyDictionary? parameters, DreamDaemonLaunchParameters launchParameters, + string accessIdentifier, string? logFilePath) { ArgumentNullException.ThrowIfNull(dmbProvider); - ArgumentNullException.ThrowIfNull(parameters); ArgumentNullException.ThrowIfNull(launchParameters); + ArgumentNullException.ThrowIfNull(accessIdentifier); - var parametersString = EncodeParameters(parameters, launchParameters); + var encodedParameters = EncodeParameters(parameters, launchParameters); + var parametersString = !String.IsNullOrEmpty(encodedParameters) + ? $" -params \"{encodedParameters}\"" + : String.Empty; + // important to run on all ports to allow port changing var arguments = String.Format( CultureInfo.InvariantCulture, - "\"{0}\" -port {1} -ports 1-65535 {2}-close -verbose -{3} -{4}{5}{6}{7} -params \"{8}\"", + "\"{0}\" -port {1} -ports 1-65535 {2}-close -verbose -{3} -{4}{5}{6}{7}{8}", dmbProvider.DmbName, launchParameters.Port!.Value, launchParameters.AllowWebClient!.Value diff --git a/src/Tgstation.Server.Host/Components/Engine/EngineExecutableLock.cs b/src/Tgstation.Server.Host/Components/Engine/EngineExecutableLock.cs index 3589d6c4af..1c6e69d95b 100644 --- a/src/Tgstation.Server.Host/Components/Engine/EngineExecutableLock.cs +++ b/src/Tgstation.Server.Host/Components/Engine/EngineExecutableLock.cs @@ -45,13 +45,15 @@ namespace Tgstation.Server.Host.Components.Engine /// public string FormatServerArguments( IDmbProvider dmbProvider, - IReadOnlyDictionary parameters, + IReadOnlyDictionary? parameters, DreamDaemonLaunchParameters launchParameters, + string accessIdentifier, string? logFilePath) => Instance.FormatServerArguments( dmbProvider, parameters, launchParameters, + accessIdentifier, logFilePath); /// diff --git a/src/Tgstation.Server.Host/Components/Engine/EngineInstallationBase.cs b/src/Tgstation.Server.Host/Components/Engine/EngineInstallationBase.cs index 928f8e3006..e499ea9dfd 100644 --- a/src/Tgstation.Server.Host/Components/Engine/EngineInstallationBase.cs +++ b/src/Tgstation.Server.Host/Components/Engine/EngineInstallationBase.cs @@ -57,13 +57,15 @@ namespace Tgstation.Server.Host.Components.Engine /// The active . /// The formatted parameters . protected static string EncodeParameters( - IReadOnlyDictionary parameters, + IReadOnlyDictionary? parameters, DreamDaemonLaunchParameters launchParameters) { - var parametersString = String.Join('&', parameters.Select(kvp => $"{HttpUtility.UrlEncode(kvp.Key)}={HttpUtility.UrlEncode(kvp.Value)}")); + var parametersString = parameters != null + ? $"{String.Join('&', parameters.Select(kvp => $"{HttpUtility.UrlEncode(kvp.Key)}={HttpUtility.UrlEncode(kvp.Value)}"))}&" + : String.Empty; if (!String.IsNullOrEmpty(launchParameters.AdditionalParameters)) - parametersString = $"{parametersString}&{launchParameters.AdditionalParameters}"; + parametersString += launchParameters.AdditionalParameters; return parametersString; } @@ -83,8 +85,9 @@ namespace Tgstation.Server.Host.Components.Engine /// public abstract string FormatServerArguments( IDmbProvider dmbProvider, - IReadOnlyDictionary parameters, + IReadOnlyDictionary? parameters, DreamDaemonLaunchParameters launchParameters, + string accessIdentifier, string? logFilePath); /// diff --git a/src/Tgstation.Server.Host/Components/Engine/IEngineInstallation.cs b/src/Tgstation.Server.Host/Components/Engine/IEngineInstallation.cs index 1d467a2881..d5cb2136ae 100644 --- a/src/Tgstation.Server.Host/Components/Engine/IEngineInstallation.cs +++ b/src/Tgstation.Server.Host/Components/Engine/IEngineInstallation.cs @@ -60,14 +60,16 @@ namespace Tgstation.Server.Host.Components.Engine /// Return the command line arguments for launching with given . /// /// The . - /// The map of parameter s as a . MUST include . Should NOT include the of . + /// The optional map of parameter s as a . MUST include . Should NOT include the of . /// The . + /// The secure used to authenticate communication with the game server. /// The full path to the log file, if any. /// The formatted arguments . string FormatServerArguments( IDmbProvider dmbProvider, - IReadOnlyDictionary parameters, + IReadOnlyDictionary? parameters, DreamDaemonLaunchParameters launchParameters, + string accessIdentifier, string? logFilePath); /// @@ -83,7 +85,7 @@ namespace Tgstation.Server.Host.Components.Engine /// /// The to write to. /// The to be terminated. - /// The of the session. + /// The secure used to authenticate communication with the game server. /// The port the server is running on. /// The for the operation. /// A representing the running operation. diff --git a/src/Tgstation.Server.Host/Components/Engine/OpenDreamInstallation.cs b/src/Tgstation.Server.Host/Components/Engine/OpenDreamInstallation.cs index b8f5e7f877..ddc5a8265f 100644 --- a/src/Tgstation.Server.Host/Components/Engine/OpenDreamInstallation.cs +++ b/src/Tgstation.Server.Host/Components/Engine/OpenDreamInstallation.cs @@ -14,7 +14,6 @@ using Tgstation.Server.Api.Models; using Tgstation.Server.Api.Models.Internal; using Tgstation.Server.Common.Http; using Tgstation.Server.Host.Components.Deployment; -using Tgstation.Server.Host.Components.Interop; using Tgstation.Server.Host.IO; using Tgstation.Server.Host.System; using Tgstation.Server.Host.Utils; @@ -110,20 +109,21 @@ namespace Tgstation.Server.Host.Components.Engine /// public override string FormatServerArguments( IDmbProvider dmbProvider, - IReadOnlyDictionary parameters, + IReadOnlyDictionary? parameters, DreamDaemonLaunchParameters launchParameters, + string accessIdentifier, string? logFilePath) { ArgumentNullException.ThrowIfNull(dmbProvider); - ArgumentNullException.ThrowIfNull(parameters); ArgumentNullException.ThrowIfNull(launchParameters); + ArgumentNullException.ThrowIfNull(accessIdentifier); - if (!parameters.TryGetValue(DMApiConstants.ParamAccessIdentifier, out var accessIdentifier)) - throw new ArgumentException($"parameters must have \"{DMApiConstants.ParamAccessIdentifier}\" set!", nameof(parameters)); + var encodedParameters = EncodeParameters(parameters, launchParameters); + var parametersString = !String.IsNullOrEmpty(encodedParameters) + ? $" --cvar opendream.world_params=\"{encodedParameters}\"" + : String.Empty; - var parametersString = EncodeParameters(parameters, launchParameters); - - var arguments = $"{serverDllPath} --cvar {(logFilePath != null ? $"log.path=\"{InstallationIOManager.GetDirectoryName(logFilePath)}\" --cvar log.format=\"{InstallationIOManager.GetFileName(logFilePath)}\"" : "log.enabled=false")} --cvar watchdog.token={accessIdentifier} --cvar log.runtimelog=false --cvar net.port={launchParameters.Port!.Value} --cvar opendream.topic_port={launchParameters.OpenDreamTopicPort!.Value} --cvar opendream.world_params=\"{parametersString}\" --cvar opendream.json_path=\"./{dmbProvider.DmbName}\""; + var arguments = $"{serverDllPath} --cvar {(logFilePath != null ? $"log.path=\"{InstallationIOManager.GetDirectoryName(logFilePath)}\" --cvar log.format=\"{InstallationIOManager.GetFileName(logFilePath)}\"" : "log.enabled=false")} --cvar watchdog.token={accessIdentifier} --cvar log.runtimelog=false --cvar net.port={launchParameters.Port!.Value} --cvar opendream.topic_port={launchParameters.OpenDreamTopicPort!.Value}{parametersString} --cvar opendream.json_path=\"./{dmbProvider.DmbName}\""; return arguments; } diff --git a/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs b/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs index 5c602ca1e5..84f0f4b654 100644 --- a/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs +++ b/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs @@ -526,17 +526,23 @@ namespace Tgstation.Server.Host.Components.Session bool apiValidate, CancellationToken cancellationToken) { - // important to run on all ports to allow port changing - var environment = await engineLock.LoadEnv(logger, false, cancellationToken); - var arguments = engineLock.FormatServerArguments( - dmbProvider, - new Dictionary + var serverMayHaveDMApi = apiValidate || dmbProvider.CompileJob.DMApiVersion != null; + + var serverArguments = serverMayHaveDMApi + ? new Dictionary { { DMApiConstants.ParamApiVersion, DMApiConstants.InteropVersion.Semver().ToString() }, { DMApiConstants.ParamServerPort, serverPortProvider.HttpApiPort.ToString(CultureInfo.InvariantCulture) }, { DMApiConstants.ParamAccessIdentifier, accessIdentifier }, - }, + } + : null; + + var environment = await engineLock.LoadEnv(logger, false, cancellationToken); + var arguments = engineLock.FormatServerArguments( + dmbProvider, + serverArguments, launchParameters, + accessIdentifier, !engineLock.HasStandardOutput || engineLock.PreferFileLogging ? logFilePath : null); diff --git a/tests/DMAPI/BasicOperation/Test.dm b/tests/DMAPI/BasicOperation/Test.dm index 30b02a4bee..61edd05e23 100644 --- a/tests/DMAPI/BasicOperation/Test.dm +++ b/tests/DMAPI/BasicOperation/Test.dm @@ -39,15 +39,16 @@ fdel("test_event_output.txt") var/test_data = "nwfiuurhfu" world.TgsTriggerEvent("test_event", list(test_data), TRUE) - if(!fexists("test_event_output.txt")) - FailTest("Expected test_event_output.txt to exist here", "test_fail_reason.txt") + if(world.TgsAvailable()) + if(!fexists("test_event_output.txt")) + FailTest("Expected test_event_output.txt to exist here", "test_fail_reason.txt") - var/test_contents = copytext(file2text("test_event_output.txt"), 1, length(test_data) + 1) - if(test_contents != test_data) - FailTest("Expected test_event_output.txt to contain [test_data] here. Got [test_contents]", "test_fail_reason.txt") + var/test_contents = copytext(file2text("test_event_output.txt"), 1, length(test_data) + 1) + if(test_contents != test_data) + FailTest("Expected test_event_output.txt to contain [test_data] here. Got [test_contents]", "test_fail_reason.txt") - world.log << "file check 1" - fdel("test_event_output.txt") + world.log << "file check 1" + fdel("test_event_output.txt") var/start_time = world.timeofday world.TgsTriggerEvent("test_event", list("asdf"), FALSE) diff --git a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs index df2476444b..adeddfb9d6 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs @@ -554,53 +554,56 @@ namespace Tgstation.Server.Tests.Live.Instance Assert.AreEqual(1, dumpFiles.Length); File.Delete(dumpFiles.Single()); - JobResponse job; - while (true) + if (testVersion.Engine != EngineType.OpenDream) { - KillDD(true); - var jobTcs = new TaskCompletionSource(); - var killTaskStarted = new TaskCompletionSource(); - var killThread = new Thread(() => + JobResponse job; + while (true) { - killTaskStarted.SetResult(); - while (!jobTcs.Task.IsCompleted) - KillDD(false); - }) - { - Priority = ThreadPriority.AboveNormal - }; + KillDD(true); + var jobTcs = new TaskCompletionSource(); + var killTaskStarted = new TaskCompletionSource(); + var killThread = new Thread(() => + { + killTaskStarted.SetResult(); + while (!jobTcs.Task.IsCompleted) + KillDD(false); + }) + { + Priority = ThreadPriority.AboveNormal + }; - killThread.Start(); - try - { - await killTaskStarted.Task; - var dumpTask = instanceClient.DreamDaemon.CreateDump(cancellationToken); - job = await WaitForJob(await dumpTask, 20, true, null, cancellationToken); - } - finally - { - jobTcs.SetResult(); - killThread.Join(); + killThread.Start(); + try + { + await killTaskStarted.Task; + var dumpTask = instanceClient.DreamDaemon.CreateDump(cancellationToken); + job = await WaitForJob(await dumpTask, 20, true, null, cancellationToken); + } + finally + { + jobTcs.SetResult(); + killThread.Join(); + } + + // these can also happen + + if (!(new PlatformIdentifier().IsWindows + && (job.ExceptionDetails.Contains("Access is denied.") + || job.ExceptionDetails.Contains("The handle is invalid.") + || job.ExceptionDetails.Contains("Unknown error") + || job.ExceptionDetails.Contains("No process is associated with this object.") + || job.ExceptionDetails.Contains("The program issued a command but the command length is incorrect.") + || job.ExceptionDetails.Contains("Only part of a ReadProcessMemory or WriteProcessMemory request was completed.") + || job.ExceptionDetails.Contains("Unknown error")))) + break; + + var restartJob = await instanceClient.DreamDaemon.Restart(cancellationToken); + await WaitForJob(restartJob, 20, false, null, cancellationToken); } - // these can also happen - - if (!(new PlatformIdentifier().IsWindows - && (job.ExceptionDetails.Contains("Access is denied.") - || job.ExceptionDetails.Contains("The handle is invalid.") - || job.ExceptionDetails.Contains("Unknown error") - || job.ExceptionDetails.Contains("No process is associated with this object.") - || job.ExceptionDetails.Contains("The program issued a command but the command length is incorrect.") - || job.ExceptionDetails.Contains("Only part of a ReadProcessMemory or WriteProcessMemory request was completed.") - || job.ExceptionDetails.Contains("Unknown error")))) - break; - - var restartJob = await instanceClient.DreamDaemon.Restart(cancellationToken); - await WaitForJob(restartJob, 20, false, null, cancellationToken); + Assert.IsTrue(job.ErrorCode == ErrorCode.GameServerOffline || job.ErrorCode == ErrorCode.GCoreFailure, $"{job.ErrorCode}: {job.ExceptionDetails}"); } - Assert.IsTrue(job.ErrorCode == ErrorCode.GameServerOffline || job.ErrorCode == ErrorCode.GCoreFailure, $"{job.ErrorCode}: {job.ExceptionDetails}"); - var restartJob2 = await instanceClient.DreamDaemon.Restart(cancellationToken); await WaitForJob(restartJob2, 20, false, null, cancellationToken); } @@ -746,9 +749,12 @@ namespace Tgstation.Server.Tests.Live.Instance Assert.AreNotEqual(0, daemonStatus.ImmediateMemoryUsage.Value); if (skipApiValidation) + { Assert.IsFalse(daemonStatus.ClientCount.HasValue); - - await GracefulWatchdogShutdown(cancellationToken); + await instanceClient.DreamDaemon.Shutdown(cancellationToken); + } + else + await GracefulWatchdogShutdown(cancellationToken); daemonStatus = await instanceClient.DreamDaemon.Read(cancellationToken); Assert.AreEqual(WatchdogStatus.Offline, daemonStatus.Status.Value); @@ -756,7 +762,7 @@ namespace Tgstation.Server.Tests.Live.Instance Assert.IsFalse(daemonStatus.LaunchTime.HasValue); await ExpectGameDirectoryCount(1, cancellationToken); - await CheckDMApiFail(daemonStatus.ActiveCompileJob, cancellationToken, false, skipApiValidation); + await CheckDMApiFail(daemonStatus.ActiveCompileJob, cancellationToken, false, false); daemonStatus = await instanceClient.DreamDaemon.Update(new DreamDaemonRequest {