remediate: replace eval cookie writes with safe JS helper + add tests (18.6 Track A)
Both SettingsCookieService and DarkModeCookieService now call window.DeepDrftSettings.setCookie (new Interop/settings/settings.ts) instead of eval. New tests cover SettingsServiceBase parse/format round-trip and the PreferenceAwareStreamingPlayerService invariant (Lossless skips probe; LowData inherits base).
This commit is contained in:
daniel-c-harvey
@@ -14,9 +14,7 @@ public class DarkModeCookieService(DarkModeSettings darkModeSetting, IJSRuntime
|
||||
|
||||
public async ValueTask SetDarkModeAsync(bool isDarkMode)
|
||||
{
|
||||
var expires = DateTime.UtcNow.AddDays(EXPIRY_DAYS).ToString("R");
|
||||
await js.InvokeVoidAsync("eval",
|
||||
$"document.cookie = '{COOKIE_NAME}={isDarkMode.ToString().ToLower()}; expires={expires}; path=/; SameSite=Lax'");
|
||||
await js.InvokeVoidAsync("DeepDrftSettings.setCookie", COOKIE_NAME, isDarkMode.ToString().ToLower(), EXPIRY_DAYS);
|
||||
darkModeSetting.IsDarkMode = isDarkMode;
|
||||
}
|
||||
}
|
||||
@@ -27,8 +27,6 @@ public class SettingsCookieService(PublicSiteSettings settings, IJSRuntime js) :
|
||||
|
||||
private async ValueTask WriteCookieAsync(string name, string value)
|
||||
{
|
||||
var expires = DateTime.UtcNow.AddDays(ExpiryDays).ToString("R");
|
||||
await js.InvokeVoidAsync("eval",
|
||||
$"document.cookie = '{name}={value}; expires={expires}; path=/; SameSite=Lax'");
|
||||
await js.InvokeVoidAsync("DeepDrftSettings.setCookie", name, value, ExpiryDays);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -24,6 +24,7 @@
|
||||
<script src="_framework/blazor.web.js"></script>
|
||||
<script src=@Assets["_content/MudBlazor/MudBlazor.min.js"]></script>
|
||||
<script type="module">
|
||||
import('./js/settings/settings.js');
|
||||
import('./js/audio/index.js');
|
||||
import('./js/telemetry/beacon.js');
|
||||
import('./js/telemetry/anonid.js');
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
/**
|
||||
* Listener-settings interop (Phase 18 wave 18.6). A safe, eval-free cookie helper for persisting
|
||||
* public-site preferences (streaming quality, and any future setting added under PublicSiteSettings).
|
||||
* The 365-day durable-truth seam dark mode uses — same mechanism, no eval.
|
||||
*
|
||||
* Exposed on window.DeepDrftSettings; imported once in App.razor.
|
||||
*/
|
||||
|
||||
const DeepDrftSettings = {
|
||||
/**
|
||||
* Write a cookie with the given name, value, and lifetime. Equivalent to the browser's
|
||||
* document.cookie assignment but without building JS via string interpolation or eval.
|
||||
* Path is always "/"; SameSite is always "Lax" — matches the dark-mode cookie semantics.
|
||||
*/
|
||||
setCookie: (name: string, value: string, days: number): void => {
|
||||
const expires = new Date();
|
||||
expires.setTime(expires.getTime() + days * 24 * 60 * 60 * 1000);
|
||||
document.cookie =
|
||||
`${encodeURIComponent(name)}=${encodeURIComponent(value)}` +
|
||||
`; expires=${expires.toUTCString()}` +
|
||||
`; path=/; SameSite=Lax`;
|
||||
},
|
||||
};
|
||||
|
||||
declare global {
|
||||
interface Window {
|
||||
DeepDrftSettings: typeof DeepDrftSettings;
|
||||
}
|
||||
}
|
||||
|
||||
window.DeepDrftSettings = DeepDrftSettings;
|
||||
|
||||
export { DeepDrftSettings };
|
||||
@@ -0,0 +1,209 @@
|
||||
using System.Net;
|
||||
using DeepDrftModels.Enums;
|
||||
using DeepDrftPublic.Client.Clients;
|
||||
using DeepDrftPublic.Client.Common;
|
||||
using DeepDrftPublic.Client.Services;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Microsoft.JSInterop;
|
||||
|
||||
namespace DeepDrftTests;
|
||||
|
||||
/// <summary>
|
||||
/// Unit tests for the critical invariant on
|
||||
/// <see cref="PreferenceAwareStreamingPlayerService.ResolveStreamFormatAsync"/> (Phase 18 wave 18.6):
|
||||
/// <list type="bullet">
|
||||
/// <item>A <see cref="StreamQuality.Lossless"/> preference returns <see cref="AudioFormat.Lossless"/>
|
||||
/// immediately — without probing Opus capability or fetching the sidecar.</item>
|
||||
/// <item>Any other preference (currently only <see cref="StreamQuality.LowData"/>) delegates to
|
||||
/// <c>base.ResolveStreamFormatAsync</c>, so the AC7 capability gate and the C2 sidecar-absent →
|
||||
/// lossless fallback from Phase 18.5 are inherited, not bypassed.</item>
|
||||
/// </list>
|
||||
/// The test infra (FakeJsRuntime, StubSidecarHandler, SingleClientFactory) mirrors
|
||||
/// <see cref="OpusFormatSelectionTests"/> exactly so both test classes exercise the same seam with
|
||||
/// the same fake boundary.
|
||||
/// </summary>
|
||||
[TestFixture]
|
||||
public class PreferenceAwareStreamingPlayerServiceTests
|
||||
{
|
||||
// Scriptable JS runtime: tracks whether the Opus-capability probe was called so tests can assert
|
||||
// the Lossless branch never touches it.
|
||||
private sealed class FakeJsRuntime : IJSRuntime
|
||||
{
|
||||
private readonly bool _canDecode;
|
||||
private readonly bool _sidecarParseSucceeds;
|
||||
|
||||
public FakeJsRuntime(bool canDecode = true, bool sidecarParseSucceeds = true)
|
||||
{
|
||||
_canDecode = canDecode;
|
||||
_sidecarParseSucceeds = sidecarParseSucceeds;
|
||||
}
|
||||
|
||||
public int CanDecodeCallCount { get; private set; }
|
||||
public int SetSidecarCallCount { get; private set; }
|
||||
|
||||
public ValueTask<TValue> InvokeAsync<TValue>(string identifier, object?[]? args)
|
||||
{
|
||||
if (identifier == "DeepDrftAudio.canDecodeOggOpus")
|
||||
{
|
||||
CanDecodeCallCount++;
|
||||
return ValueTask.FromResult((TValue)(object)_canDecode);
|
||||
}
|
||||
|
||||
if (identifier == "DeepDrftAudio.setOpusSidecar")
|
||||
{
|
||||
SetSidecarCallCount++;
|
||||
var result = new AudioOperationResult
|
||||
{
|
||||
Success = _sidecarParseSucceeds,
|
||||
Error = _sidecarParseSucceeds ? null : "Invalid Opus sidecar blob",
|
||||
};
|
||||
return ValueTask.FromResult((TValue)(object)result);
|
||||
}
|
||||
|
||||
return ValueTask.FromResult<TValue>(default!);
|
||||
}
|
||||
|
||||
public ValueTask<TValue> InvokeAsync<TValue>(string identifier, CancellationToken cancellationToken, object?[]? args)
|
||||
=> InvokeAsync<TValue>(identifier, args);
|
||||
}
|
||||
|
||||
// Returns a configured status (with an optional body) for sidecar requests; anything else 404s.
|
||||
private sealed class StubSidecarHandler : HttpMessageHandler
|
||||
{
|
||||
private readonly HttpStatusCode _status;
|
||||
private readonly byte[] _body;
|
||||
|
||||
public StubSidecarHandler(HttpStatusCode status, byte[]? body = null)
|
||||
{
|
||||
_status = status;
|
||||
_body = body ?? [];
|
||||
}
|
||||
|
||||
protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
|
||||
{
|
||||
var response = new HttpResponseMessage(_status);
|
||||
if (_status == HttpStatusCode.OK)
|
||||
response.Content = new ByteArrayContent(_body);
|
||||
return Task.FromResult(response);
|
||||
}
|
||||
}
|
||||
|
||||
private sealed class SingleClientFactory : IHttpClientFactory
|
||||
{
|
||||
private readonly HttpMessageHandler _handler;
|
||||
public SingleClientFactory(HttpMessageHandler handler) => _handler = handler;
|
||||
public HttpClient CreateClient(string name) =>
|
||||
new(_handler, disposeHandler: false) { BaseAddress = new Uri("https://content.test/") };
|
||||
}
|
||||
|
||||
// Exposes the protected seam for direct assertion.
|
||||
private sealed class TestablePreferencePlayer : PreferenceAwareStreamingPlayerService
|
||||
{
|
||||
public TestablePreferencePlayer(
|
||||
AudioInteropService interop,
|
||||
TrackMediaClient media,
|
||||
PublicSiteSettings settings)
|
||||
: base(interop, media, NullLogger<StreamingAudioPlayerService>.Instance, settings) { }
|
||||
|
||||
public Task<AudioFormat> ResolveFormatForTest(string entryKey) =>
|
||||
ResolveStreamFormatAsync(entryKey, CancellationToken.None);
|
||||
}
|
||||
|
||||
private static readonly byte[] SidecarBytes = "setup-header+seek-index"u8.ToArray();
|
||||
|
||||
private static (TestablePreferencePlayer player, FakeJsRuntime js) Build(
|
||||
StreamQuality quality,
|
||||
bool canDecode = true,
|
||||
bool sidecarParseSucceeds = true,
|
||||
HttpStatusCode sidecarStatus = HttpStatusCode.OK,
|
||||
byte[]? sidecarBody = null)
|
||||
{
|
||||
sidecarBody ??= SidecarBytes;
|
||||
var js = new FakeJsRuntime(canDecode, sidecarParseSucceeds);
|
||||
var interop = new AudioInteropService(js);
|
||||
var media = new TrackMediaClient(new SingleClientFactory(new StubSidecarHandler(sidecarStatus, sidecarBody)));
|
||||
var settings = new PublicSiteSettings { StreamQuality = quality };
|
||||
return (new TestablePreferencePlayer(interop, media, settings), js);
|
||||
}
|
||||
|
||||
// ── Lossless branch: must NOT probe capability or fetch the sidecar ──
|
||||
|
||||
// A Lossless preference short-circuits to AudioFormat.Lossless with zero JS calls.
|
||||
[Test]
|
||||
public async Task ResolveStreamFormat_LosslessPreference_ReturnsLosslessWithoutProbe()
|
||||
{
|
||||
var (player, js) = Build(StreamQuality.Lossless, canDecode: true);
|
||||
|
||||
var format = await player.ResolveFormatForTest("track-1");
|
||||
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
Assert.That(format, Is.EqualTo(AudioFormat.Lossless), "Lossless preference must yield lossless");
|
||||
Assert.That(js.CanDecodeCallCount, Is.Zero, "Lossless branch must not probe Opus capability");
|
||||
Assert.That(js.SetSidecarCallCount, Is.Zero, "Lossless branch must not inject a sidecar");
|
||||
});
|
||||
}
|
||||
|
||||
// The Lossless branch is independent of browser Opus capability — even an incapable browser
|
||||
// must get lossless without a probe (the probe is unnecessary and should not fire).
|
||||
[Test]
|
||||
public async Task ResolveStreamFormat_LosslessPreference_IncapableBrowser_ReturnsLosslessWithoutProbe()
|
||||
{
|
||||
var (player, js) = Build(StreamQuality.Lossless, canDecode: false);
|
||||
|
||||
var format = await player.ResolveFormatForTest("track-2");
|
||||
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
Assert.That(format, Is.EqualTo(AudioFormat.Lossless));
|
||||
Assert.That(js.CanDecodeCallCount, Is.Zero, "no probe on the Lossless path, even for incapable browsers");
|
||||
});
|
||||
}
|
||||
|
||||
// ── LowData branch: must delegate to base (capability gate + C2 fallback are inherited) ──
|
||||
|
||||
// Happy path: capable browser + present sidecar → Opus (base logic reached and succeeded).
|
||||
[Test]
|
||||
public async Task ResolveStreamFormat_LowDataPreference_CapableBrowser_SidecarPresent_ChoosesOpus()
|
||||
{
|
||||
var (player, js) = Build(StreamQuality.LowData, canDecode: true, sidecarParseSucceeds: true,
|
||||
HttpStatusCode.OK, SidecarBytes);
|
||||
|
||||
var format = await player.ResolveFormatForTest("track-3");
|
||||
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
Assert.That(format, Is.EqualTo(AudioFormat.Opus), "capable browser + present sidecar → Opus");
|
||||
Assert.That(js.CanDecodeCallCount, Is.EqualTo(1), "LowData path must invoke the capability probe");
|
||||
});
|
||||
}
|
||||
|
||||
// AC7 inherited: LowData + incapable browser → lossless (base capability gate fires).
|
||||
[Test]
|
||||
public async Task ResolveStreamFormat_LowDataPreference_IncapableBrowser_FallsBackToLossless()
|
||||
{
|
||||
var (player, js) = Build(StreamQuality.LowData, canDecode: false);
|
||||
|
||||
var format = await player.ResolveFormatForTest("track-4");
|
||||
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
Assert.That(format, Is.EqualTo(AudioFormat.Lossless), "AC7: incapable browser must get lossless");
|
||||
Assert.That(js.CanDecodeCallCount, Is.EqualTo(1), "the capability probe was called (and returned false)");
|
||||
Assert.That(js.SetSidecarCallCount, Is.Zero, "no sidecar injected when Opus is gated out");
|
||||
});
|
||||
}
|
||||
|
||||
// C2 inherited: LowData + capable browser + no sidecar → lossless (base C2 fallback fires).
|
||||
[Test]
|
||||
public async Task ResolveStreamFormat_LowDataPreference_CapableBrowser_NoSidecar_FallsBackToLossless()
|
||||
{
|
||||
var (player, _) = Build(StreamQuality.LowData, canDecode: true,
|
||||
sidecarStatus: HttpStatusCode.NotFound, sidecarBody: null);
|
||||
|
||||
var format = await player.ResolveFormatForTest("track-5");
|
||||
|
||||
Assert.That(format, Is.EqualTo(AudioFormat.Lossless),
|
||||
"C2: no sidecar → lossless even with a capable browser");
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,73 @@
|
||||
using DeepDrftPublic.Client.Common;
|
||||
using DeepDrftPublic.Client.Services;
|
||||
|
||||
namespace DeepDrftTests;
|
||||
|
||||
/// <summary>
|
||||
/// Unit tests for the <see cref="SettingsServiceBase"/> parse/format contract (Phase 18 wave 18.6):
|
||||
/// the <c>streamQuality</c> cookie round-trips through <c>FormatStreamQuality</c> → wire string →
|
||||
/// <c>ParseStreamQuality</c>, and an absent, empty, or unrecognized cookie value defaults to
|
||||
/// <see cref="StreamQuality.LowData"/> (the OQ2 default — a missing preference should never force
|
||||
/// lossless unnecessarily on a low-bandwidth listener).
|
||||
/// </summary>
|
||||
[TestFixture]
|
||||
public class SettingsServiceBaseTests
|
||||
{
|
||||
// Expose the protected static helpers via a concrete subclass — no other members needed.
|
||||
private sealed class Exposed : SettingsServiceBase
|
||||
{
|
||||
public static StreamQuality Parse(string? v) => ParseStreamQuality(v);
|
||||
public static string Format(StreamQuality q) => FormatStreamQuality(q);
|
||||
}
|
||||
|
||||
// ── round-trip ──
|
||||
|
||||
// LowData serializes and deserializes intact.
|
||||
[Test]
|
||||
public void FormatThenParse_LowData_RoundTrips()
|
||||
{
|
||||
var wire = Exposed.Format(StreamQuality.LowData);
|
||||
Assert.That(Exposed.Parse(wire), Is.EqualTo(StreamQuality.LowData));
|
||||
}
|
||||
|
||||
// Lossless serializes and deserializes intact.
|
||||
[Test]
|
||||
public void FormatThenParse_Lossless_RoundTrips()
|
||||
{
|
||||
var wire = Exposed.Format(StreamQuality.Lossless);
|
||||
Assert.That(Exposed.Parse(wire), Is.EqualTo(StreamQuality.Lossless));
|
||||
}
|
||||
|
||||
// ── default for absent / garbled cookie ──
|
||||
|
||||
// A null cookie (no cookie present) defaults to LowData, not Lossless.
|
||||
[Test]
|
||||
public void Parse_Null_DefaultsToLowData()
|
||||
=> Assert.That(Exposed.Parse(null), Is.EqualTo(StreamQuality.LowData));
|
||||
|
||||
// An empty string (cookie present but blank) defaults to LowData.
|
||||
[Test]
|
||||
public void Parse_Empty_DefaultsToLowData()
|
||||
=> Assert.That(Exposed.Parse(string.Empty), Is.EqualTo(StreamQuality.LowData));
|
||||
|
||||
// A garbled/unknown name defaults to LowData rather than throwing.
|
||||
// Note: Enum.TryParse accepts numeric strings as valid (e.g. "0" → LowData, "1" → Lossless),
|
||||
// so only non-numeric unrecognized names are tested here.
|
||||
[TestCase("garbage")]
|
||||
[TestCase("lossless_extra")]
|
||||
public void Parse_Unrecognized_DefaultsToLowData(string value)
|
||||
=> Assert.That(Exposed.Parse(value), Is.EqualTo(StreamQuality.LowData));
|
||||
|
||||
// Case-insensitive parse: the enum parse is case-insensitive, so wire values survive case drift.
|
||||
[TestCase("lossless")]
|
||||
[TestCase("LOSSLESS")]
|
||||
[TestCase("Lossless")]
|
||||
public void Parse_LosslessCaseVariants_ParseCorrectly(string value)
|
||||
=> Assert.That(Exposed.Parse(value), Is.EqualTo(StreamQuality.Lossless));
|
||||
|
||||
[TestCase("lowdata")]
|
||||
[TestCase("LOWDATA")]
|
||||
[TestCase("LowData")]
|
||||
public void Parse_LowDataCaseVariants_ParseCorrectly(string value)
|
||||
=> Assert.That(Exposed.Parse(value), Is.EqualTo(StreamQuality.LowData));
|
||||
}
|
||||
Reference in New Issue
Block a user