Skip to content

Commit d2788ce

Browse files
aknousclaude
andcommitted
feat(debug-console): fix invalid websocket host and let clients choose network
The debug session URL was built as wss://Invalid Value:65479/debug/join on any processor without a control subnet. DebugWebsocketSink.Url hard-coded ethernet adapter id 1 as "the CS LAN adapter" and guarded it with IsNullOrEmpty, but CrestronEthernetHelper.GetEthernetParameter does not return null or empty for an absent adapter - it returns the literal string "Invalid Value". The guard passed, the bad host won, and the fallback URL that would have rescued it was never built because GetAdapterdIdForSpecifiedAdapterType threw instead. Add ProcessorEthernetInfo, which resolves addresses by adapter type rather than guessed index and returns null for the SDK's sentinel, and use it wherever the processor's own addresses are read. The session handlers now return every network the websocket is reachable on so a client on either side of the processor can pick, since a tech may be on the LAN or on the control subnet: - networks[] lists each reachable address as {id, label, host, url} - ?network=lan|cslan|current selects one; an unavailable id falls back to auto-selection rather than failing - without an explicit choice, the request's Host header picks the side the client is already on - port and path are returned so a client that already knows the processor's address can build the URL itself - the debug app was served from the processor, so window.location.hostname is by definition an address that reaches it url and fallbackUrl keep their existing meaning, so the current dev tools app works untouched. Also: - CreateCert validated no parameters, so a CN=Invalid Value.Invalid Value cert was possible; it now falls back to the IP address and adds the control subnet IP and bare hostname to the SAN list - DebugSessionRequestHandler's outer catch logged and returned nothing, leaving the browser on a hung request; it now sends a 500 - RoutingFeedbackSessionRequestHandler carried the same duplicated URL logic and gets the same treatment Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CshF5gZ4qiGbEWZWB9Mi2U
1 parent 325ac4d commit d2788ce

8 files changed

Lines changed: 611 additions & 125 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
using FluentAssertions;
2+
using Xunit;
3+
4+
namespace PepperDash.Core.Tests;
5+
6+
/// <summary>
7+
/// Tests for <see cref="ProcessorEthernetInfo"/>'s parameter validation. The adapter lookups themselves
8+
/// need the Crestron SDK, but the sentinel handling that produced URLs such as
9+
/// <c>wss://Invalid Value:65479/debug/join</c> is pure and testable.
10+
/// </summary>
11+
public class ProcessorEthernetInfoTests
12+
{
13+
[Theory]
14+
[InlineData("Invalid Value")]
15+
[InlineData("invalid value")]
16+
[InlineData(" Invalid Value ")]
17+
public void NullIfInvalid_RejectsSdkSentinel(string value)
18+
{
19+
ProcessorEthernetInfo.NullIfInvalid(value).Should().BeNull();
20+
}
21+
22+
[Theory]
23+
[InlineData(null)]
24+
[InlineData("")]
25+
[InlineData(" ")]
26+
public void NullIfInvalid_RejectsBlankValues(string? value)
27+
{
28+
ProcessorEthernetInfo.NullIfInvalid(value!).Should().BeNull();
29+
}
30+
31+
[Fact]
32+
public void NullIfInvalid_ReturnsTrimmedAddress()
33+
{
34+
ProcessorEthernetInfo.NullIfInvalid(" 10.0.0.5 ").Should().Be("10.0.0.5");
35+
}
36+
}

‎src/PepperDash.Core/Logging/DebugWebsocketSink.cs‎

Lines changed: 69 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -52,24 +52,56 @@ public int Port
5252
/// <summary>
5353
/// Gets the WebSocket URL for the current server instance.
5454
/// </summary>
55-
/// <remarks>The URL is dynamically constructed based on the server's current IP address, port,
56-
/// and WebSocket path.</remarks>
55+
/// <remarks>
56+
/// The URL is dynamically constructed from the processor's LAN IP address, the port and the
57+
/// WebSocket path. The control subnet address is only used when there is no usable LAN address,
58+
/// since the debug app is normally reached over the LAN. Returns an empty string when the server
59+
/// is not listening or no usable address can be read.
60+
/// </remarks>
5761
public string Url
5862
{
5963
get
6064
{
61-
if (_httpsServer == null || !_httpsServer.IsListening) return "";
62-
var service = _httpsServer.WebSocketServices[_path];
63-
if (service == null) return "";
65+
var host = ProcessorEthernetInfo.GetLanIpAddress() ?? ProcessorEthernetInfo.GetCsLanIpAddress();
6466

65-
// Use CSLAN IP if available, otherwise fallback to primary IP. This ensures we provide a reachable URL in dual-stack environments.
66-
if (!string.IsNullOrEmpty(CrestronEthernetHelper.GetEthernetParameter(CrestronEthernetHelper.ETHERNET_PARAMETER_TO_GET.GET_CURRENT_IP_ADDRESS, 1)))
67-
return $"wss://{CrestronEthernetHelper.GetEthernetParameter(CrestronEthernetHelper.ETHERNET_PARAMETER_TO_GET.GET_CURRENT_IP_ADDRESS, 1)}:{_httpsServer.Port}{service.Path}";
68-
else
69-
return $"wss://{CrestronEthernetHelper.GetEthernetParameter(CrestronEthernetHelper.ETHERNET_PARAMETER_TO_GET.GET_CURRENT_IP_ADDRESS, 0)}:{_httpsServer.Port}{service.Path}";
67+
return GetUrlForHost(host);
68+
}
69+
}
70+
71+
/// <summary>
72+
/// Gets the WebSocket path clients connect to, e.g. <c>/debug/join</c>. Exposed so a client that
73+
/// already knows the processor's address — a browser on a page the processor served, for instance —
74+
/// can build the URL itself from its own location.
75+
/// </summary>
76+
public string ServicePath
77+
{
78+
get
79+
{
80+
var service = _httpsServer?.WebSocketServices[_path];
81+
82+
return service?.Path ?? _path.TrimEnd('/');
7083
}
7184
}
7285

86+
/// <summary>
87+
/// Builds the WebSocket URL for this server using the supplied host, which lets callers hand back
88+
/// the address the client actually used to reach the processor.
89+
/// </summary>
90+
/// <param name="host">Host name or IP address, without scheme or port. IPv6 literals must already be bracketed.</param>
91+
/// <returns>The <c>wss://</c> URL, or an empty string when the server is not listening or <paramref name="host"/> is unusable.</returns>
92+
public string GetUrlForHost(string host)
93+
{
94+
if (_httpsServer == null || !_httpsServer.IsListening) return "";
95+
96+
var service = _httpsServer.WebSocketServices[_path];
97+
if (service == null) return "";
98+
99+
host = ProcessorEthernetInfo.NullIfInvalid(host);
100+
if (host == null) return "";
101+
102+
return $"wss://{host}:{_httpsServer.Port}{service.Path}";
103+
}
104+
73105
/// <summary>
74106
/// Gets a value indicating whether the HTTPS server is currently listening for incoming connections.
75107
/// </summary>
@@ -131,14 +163,29 @@ private static void CreateCert()
131163
// CrestronConsole.PrintLine only, to avoid a NullReferenceException that would poison the Debug type.
132164
try
133165
{
134-
var ipAddress = CrestronEthernetHelper.GetEthernetParameter(CrestronEthernetHelper.ETHERNET_PARAMETER_TO_GET.GET_CURRENT_IP_ADDRESS, 0);
135-
var hostName = CrestronEthernetHelper.GetEthernetParameter(CrestronEthernetHelper.ETHERNET_PARAMETER_TO_GET.GET_HOSTNAME, 0);
136-
var domainName = CrestronEthernetHelper.GetEthernetParameter(CrestronEthernetHelper.ETHERNET_PARAMETER_TO_GET.GET_DOMAIN_NAME, 0);
137-
138-
CrestronConsole.PrintLine(string.Format("CreateCert: DomainName: {0} | HostName: {1} | {1}.{0}@{2}", domainName, hostName, ipAddress));
166+
// GetEthernetParameter returns the literal string "Invalid Value" rather than throwing when a
167+
// parameter cannot be read, so every value has to be validated before it goes into the cert.
168+
var ipAddress = ProcessorEthernetInfo.GetLanIpAddress();
169+
var csIpAddress = ProcessorEthernetInfo.GetCsLanIpAddress();
170+
var hostName = ProcessorEthernetInfo.GetParameter(CrestronEthernetHelper.ETHERNET_PARAMETER_TO_GET.GET_HOSTNAME, 0);
171+
var domainName = ProcessorEthernetInfo.GetParameter(CrestronEthernetHelper.ETHERNET_PARAMETER_TO_GET.GET_DOMAIN_NAME, 0);
172+
173+
CrestronConsole.PrintLine(string.Format("CreateCert: DomainName: {0} | HostName: {1} | IP: {2} | CS IP: {3}",
174+
domainName ?? "<none>", hostName ?? "<none>", ipAddress ?? "<none>", csIpAddress ?? "<none>"));
175+
176+
// Fall back to the IP address when there is no usable host name, so the subject is never
177+
// built out of unreadable parameters.
178+
var fqdn = hostName == null
179+
? ipAddress
180+
: domainName == null ? hostName : string.Format("{0}.{1}", hostName, domainName);
181+
182+
if (string.IsNullOrEmpty(fqdn))
183+
{
184+
CrestronConsole.PrintLine("CreateCert: No usable host name or IP address; aborting certificate creation");
185+
return;
186+
}
139187

140-
var subjectName = string.Format("CN={0}.{1}", hostName, domainName);
141-
var fqdn = string.Format("{0}.{1}", hostName, domainName);
188+
var subjectName = string.Format("CN={0}", fqdn);
142189

143190
using var rsa = RSA.Create(2048);
144191

@@ -162,11 +209,15 @@ private static void CreateCert()
162209
},
163210
false));
164211

165-
// Subject Alternative Names: DNS + IP
212+
// Subject Alternative Names: DNS + every address the browser could reach the processor on
166213
var sanBuilder = new SubjectAlternativeNameBuilder();
167214
sanBuilder.AddDnsName(fqdn);
215+
if (hostName != null && hostName != fqdn)
216+
sanBuilder.AddDnsName(hostName);
168217
if (System.Net.IPAddress.TryParse(ipAddress, out var ip))
169218
sanBuilder.AddIpAddress(ip);
219+
if (System.Net.IPAddress.TryParse(csIpAddress, out var csIp))
220+
sanBuilder.AddIpAddress(csIp);
170221
request.CertificateExtensions.Add(sanBuilder.Build());
171222

172223
var notBefore = DateTimeOffset.UtcNow;
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
using System;
2+
using Crestron.SimplSharp;
3+
4+
namespace PepperDash.Core;
5+
6+
/// <summary>
7+
/// Helpers for reading the processor's own network addresses.
8+
/// </summary>
9+
/// <remarks>
10+
/// <c>CrestronEthernetHelper.GetEthernetParameter</c> does not throw when it is asked for a parameter
11+
/// belonging to an adapter that the processor does not have — it returns the literal string
12+
/// <c>"Invalid Value"</c>. Callers that only null/empty-check the result end up building URLs such as
13+
/// <c>wss://Invalid Value:65479/debug/join</c>. These helpers return <c>null</c> in that case so callers
14+
/// can fall back cleanly.
15+
/// </remarks>
16+
public static class ProcessorEthernetInfo
17+
{
18+
/// <summary>
19+
/// The sentinel string the Crestron SDK returns for a parameter that cannot be read.
20+
/// </summary>
21+
public const string InvalidValue = "Invalid Value";
22+
23+
/// <summary>
24+
/// Returns the supplied network parameter, or <c>null</c> when it is blank or the SDK's
25+
/// <c>"Invalid Value"</c> sentinel.
26+
/// </summary>
27+
/// <param name="value">The raw value returned by <c>GetEthernetParameter</c>.</param>
28+
/// <returns>The trimmed value, or <c>null</c> when it is not usable.</returns>
29+
public static string NullIfInvalid(string value)
30+
{
31+
if (string.IsNullOrWhiteSpace(value))
32+
return null;
33+
34+
var trimmed = value.Trim();
35+
36+
return trimmed.Equals(InvalidValue, StringComparison.OrdinalIgnoreCase) ? null : trimmed;
37+
}
38+
39+
/// <summary>
40+
/// Gets the current IP address of the processor's LAN adapter, or <c>null</c> when it cannot be read.
41+
/// </summary>
42+
/// <remarks>
43+
/// Falls back to adapter id 0 on platforms (such as Virtual Control) where the adapter type lookup
44+
/// is not supported.
45+
/// </remarks>
46+
public static string GetLanIpAddress() =>
47+
GetIpAddressForAdapterType(EthernetAdapterType.EthernetLANAdapter) ?? GetIpAddressForAdapterId(0);
48+
49+
/// <summary>
50+
/// Gets the current IP address of the processor's control subnet (CS LAN) adapter, or <c>null</c>
51+
/// when the processor has no control subnet.
52+
/// </summary>
53+
public static string GetCsLanIpAddress() =>
54+
GetIpAddressForAdapterType(EthernetAdapterType.EthernetCSAdapter);
55+
56+
/// <summary>
57+
/// Gets the current IP address for the specified adapter type, or <c>null</c> when that adapter is
58+
/// not present on this processor.
59+
/// </summary>
60+
/// <param name="adapterType">The adapter type to look up.</param>
61+
public static string GetIpAddressForAdapterType(EthernetAdapterType adapterType)
62+
{
63+
try
64+
{
65+
var adapterId = CrestronEthernetHelper.GetAdapterdIdForSpecifiedAdapterType(adapterType);
66+
67+
return adapterId < 0 ? null : GetIpAddressForAdapterId(adapterId);
68+
}
69+
catch (ArgumentException)
70+
{
71+
// This processor does not have an adapter of the requested type.
72+
return null;
73+
}
74+
catch (Exception)
75+
{
76+
return null;
77+
}
78+
}
79+
80+
/// <summary>
81+
/// Gets the current IP address for the specified adapter id, or <c>null</c> when it cannot be read.
82+
/// </summary>
83+
/// <param name="adapterId">The Crestron ethernet adapter id.</param>
84+
public static string GetIpAddressForAdapterId(short adapterId)
85+
{
86+
try
87+
{
88+
return NullIfInvalid(CrestronEthernetHelper.GetEthernetParameter(
89+
CrestronEthernetHelper.ETHERNET_PARAMETER_TO_GET.GET_CURRENT_IP_ADDRESS, adapterId));
90+
}
91+
catch (Exception)
92+
{
93+
return null;
94+
}
95+
}
96+
97+
/// <summary>
98+
/// Gets the specified parameter for the given adapter id, or <c>null</c> when it cannot be read.
99+
/// </summary>
100+
/// <param name="parameter">The parameter to read.</param>
101+
/// <param name="adapterId">The Crestron ethernet adapter id.</param>
102+
public static string GetParameter(CrestronEthernetHelper.ETHERNET_PARAMETER_TO_GET parameter, short adapterId)
103+
{
104+
try
105+
{
106+
return NullIfInvalid(CrestronEthernetHelper.GetEthernetParameter(parameter, adapterId));
107+
}
108+
catch (Exception)
109+
{
110+
return null;
111+
}
112+
}
113+
}

0 commit comments

Comments
 (0)