StoreFileName: close review findings (collisions, case, device stems, length, renderId)
- Pass-through ids may not end in "-<16 hex>", so a rewritten name fed back as an id cannot alias it; hash widened 32 -> 64 bits. - Ids with uppercase are rewritten (case-insensitive filesystems). - Windows device stems (CON PRN AUX NUL COM0-9 LPT0-9, any extension) are rewritten. - Stem capped at 100 chars; longer ids carry the hash. - Daemon samples/ write now goes through StoreFileName.From(renderId). - Tests: rewritten-as-id, case-only, device stems, overlong; daemon test uses a compiling patch so the samples/ write is reached when Faust is present. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
396da106cd
commit
abfc816804
3 changed files with 109 additions and 16 deletions
|
|
@ -5,12 +5,19 @@ namespace AquaSynth.Dsl;
|
|||
|
||||
/// <summary>
|
||||
/// The one mapping from a caller-chosen id to a single file-name component in a store.
|
||||
/// Ids already within [A-Za-z0-9._-] pass through unchanged. Anything else is sanitised and
|
||||
/// suffixed with the first 8 hex of SHA-256 of the original id, so distinct ids stay distinct.
|
||||
/// The result is never a path: no separators, never "." or "..".
|
||||
/// An id passes through unchanged only if it is already a fixed point: ASCII [A-Za-z0-9._-],
|
||||
/// all lowercase (stores live on case-insensitive filesystems), at most <see cref="MaxStem"/> long,
|
||||
/// no trailing dot, not a Windows device stem (CON, NUL, COM1, ...), and not ending in a
|
||||
/// "-" plus 16 hex digits. Every other id is sanitised and suffixed with "-" plus the first
|
||||
/// 16 hex of SHA-256 of the original id. Passed-through names never end in the hash shape and
|
||||
/// rewritten names always do, so the two sets cannot meet; distinct ids stay distinct up to a 64-bit
|
||||
/// hash. The result is never a path: no separators, never "." or "..".
|
||||
/// </summary>
|
||||
public static class StoreFileName
|
||||
{
|
||||
private const int MaxStem = 100;
|
||||
private const int HashLength = 16;
|
||||
|
||||
public static string From(string id)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(id);
|
||||
|
|
@ -19,20 +26,56 @@ public static class StoreFileName
|
|||
throw new ArgumentException($"Id '{id}' cannot name a store file.", nameof(id));
|
||||
}
|
||||
|
||||
var builder = new StringBuilder(id.Length);
|
||||
foreach (var c in id)
|
||||
var builder = new StringBuilder(Math.Min(id.Length, MaxStem));
|
||||
foreach (var c in id.Take(MaxStem))
|
||||
{
|
||||
builder.Append(c is >= 'a' and <= 'z' or >= 'A' and <= 'Z' or >= '0' and <= '9' or '-' or '_' or '.' ? c : '_');
|
||||
}
|
||||
|
||||
var safe = builder.ToString();
|
||||
// A trailing dot is dropped by Windows, so "a." would alias "a"; hash it like any other rewrite.
|
||||
if (safe == id && !id.EndsWith('.'))
|
||||
if (IsFixedPoint(id, safe))
|
||||
{
|
||||
return safe;
|
||||
}
|
||||
|
||||
var hash = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(id)))[..8].ToLowerInvariant();
|
||||
var hash = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(id)))[..HashLength].ToLowerInvariant();
|
||||
return $"{safe}-{hash}";
|
||||
}
|
||||
|
||||
private static bool IsFixedPoint(string id, string safe)
|
||||
{
|
||||
if (safe != id || id.EndsWith('.') || id.Any(char.IsAsciiLetterUpper) || IsDeviceStem(id) || EndsWithHashShape(id))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
// Windows resolves CON, PRN, AUX, NUL, COM0-9, LPT0-9 as devices whatever the extension.
|
||||
private static bool IsDeviceStem(string id)
|
||||
{
|
||||
var dot = id.IndexOf('.');
|
||||
var stem = (dot < 0 ? id : id[..dot]).TrimEnd(' ');
|
||||
return stem.Length switch
|
||||
{
|
||||
3 => stem.Equals("con", StringComparison.OrdinalIgnoreCase)
|
||||
|| stem.Equals("prn", StringComparison.OrdinalIgnoreCase)
|
||||
|| stem.Equals("aux", StringComparison.OrdinalIgnoreCase)
|
||||
|| stem.Equals("nul", StringComparison.OrdinalIgnoreCase),
|
||||
4 => char.IsAsciiDigit(stem[3])
|
||||
&& (stem.StartsWith("com", StringComparison.OrdinalIgnoreCase) || stem.StartsWith("lpt", StringComparison.OrdinalIgnoreCase)),
|
||||
_ => false,
|
||||
};
|
||||
}
|
||||
|
||||
private static bool EndsWithHashShape(string id)
|
||||
{
|
||||
if (id.Length <= HashLength || id[^(HashLength + 1)] != '-')
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
return id[^HashLength..].All(c => c is >= '0' and <= '9' or >= 'a' and <= 'f');
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -661,8 +661,9 @@ public sealed class AquaSynthDaemonService : IDisposable
|
|||
var renderId = RenderId(command.CommandId);
|
||||
var sampleDir = Path.Combine(options.StoreRoot, "samples");
|
||||
Directory.CreateDirectory(sampleDir);
|
||||
var floatPath = Path.Combine(sampleDir, $"{renderId}.f32");
|
||||
var wavPath = Path.Combine(sampleDir, $"{renderId}.wav");
|
||||
var sampleStem = StoreFileName.From(renderId);
|
||||
var floatPath = Path.Combine(sampleDir, $"{sampleStem}.f32");
|
||||
var wavPath = Path.Combine(sampleDir, $"{sampleStem}.wav");
|
||||
await WriteFloat32Async(floatPath, samples).ConfigureAwait(false);
|
||||
await WriteWavAsync(wavPath, samples, patch.Manifest.SampleRate).ConfigureAwait(false);
|
||||
|
||||
|
|
|
|||
|
|
@ -15,14 +15,55 @@ public sealed class StoreFileNameTests
|
|||
[Theory]
|
||||
[InlineData("dings.warm-bell")]
|
||||
[InlineData("demo-stream-block")]
|
||||
[InlineData("A_b.c-1")]
|
||||
[InlineData("a_b.c-1")]
|
||||
[InlineData("conx")]
|
||||
[InlineData("com10")]
|
||||
public void CleanIdsPassThroughUnchanged(string id) => Assert.Equal(id, StoreFileName.From(id));
|
||||
|
||||
[Fact]
|
||||
public void RewrittenIdOutputAsAnIdDoesNotCollideWithTheOriginal()
|
||||
{
|
||||
var rewritten = StoreFileName.From("a b");
|
||||
Assert.NotEqual(rewritten, StoreFileName.From(rewritten));
|
||||
Assert.NotEqual(StoreFileName.From("a_b"), rewritten);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void IdsDifferingOnlyByCaseGetDistinctNames()
|
||||
{
|
||||
var names = new[] { "A", "a", "Ab", "aB", "ab" }.Select(StoreFileName.From).ToArray();
|
||||
Assert.Equal(names.Length, names.Distinct(StringComparer.OrdinalIgnoreCase).Count());
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("CON")]
|
||||
[InlineData("nul")]
|
||||
[InlineData("Aux.txt")]
|
||||
[InlineData("com1")]
|
||||
[InlineData("COM9.f32")]
|
||||
[InlineData("lpt0")]
|
||||
[InlineData("prn.cc")]
|
||||
public void WindowsDeviceStemsAreRewritten(string id)
|
||||
{
|
||||
var name = StoreFileName.From(id);
|
||||
Assert.NotEqual(id.ToLowerInvariant(), name.ToLowerInvariant());
|
||||
Assert.Matches("-[0-9a-f]{16}$", name);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void OverlongIdsAreBoundedAndStayDistinct()
|
||||
{
|
||||
var a = StoreFileName.From(new string('x', 400));
|
||||
var b = StoreFileName.From(new string('x', 399) + "y");
|
||||
Assert.True(a.Length + ".cc".Length <= 255);
|
||||
Assert.NotEqual(a, b);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void RewrittenIdsCarryHashOfTheOriginal()
|
||||
{
|
||||
// sha256("a b") begins c2a5..., checked independently of the implementation.
|
||||
var expected = Convert.ToHexString(System.Security.Cryptography.SHA256.HashData("a b"u8.ToArray()))[..8].ToLowerInvariant();
|
||||
var expected = Convert.ToHexString(System.Security.Cryptography.SHA256.HashData("a b"u8.ToArray()))[..16].ToLowerInvariant();
|
||||
Assert.Equal($"a_b-{expected}", StoreFileName.From("a b"));
|
||||
}
|
||||
|
||||
|
|
@ -52,17 +93,25 @@ public sealed class StoreFileNameTests
|
|||
}
|
||||
|
||||
[Fact]
|
||||
public async Task DaemonReceiptForTraversalCommandIdStaysInsideStore()
|
||||
public async Task DaemonSampleFilesForTraversalCommandIdStayInsideStore()
|
||||
{
|
||||
var parent = Path.Combine(Path.GetTempPath(), "aquasynth-sfn-" + Guid.NewGuid().ToString("N"));
|
||||
var storeRoot = Path.Combine(parent, "store");
|
||||
using var service = new AquaSynthDaemonService(new AquaSynthDaemonOptions(storeRoot));
|
||||
|
||||
await service.SampleAsync(new AquaSynthInstrumentSampleCommand(
|
||||
"../../escape", "test.invalid", "test_invalid", "this is not aqua syntax", DurationSeconds: 0.1f));
|
||||
// A patch that compiles reaches the samples/ write; without native Faust only the compile-failed branch runs.
|
||||
var result = await service.SampleAsync(new AquaSynthInstrumentSampleCommand(
|
||||
"../../escape", "test.sine", "test_sine",
|
||||
"voice wave=sine freq=440 gain=.2 attack=.001 sustain=.05 decay=.05", DurationSeconds: 0.1f));
|
||||
|
||||
Assert.Empty(Directory.GetFiles(parent));
|
||||
Assert.Empty(Directory.GetDirectories(parent).Where(d => Path.GetFileName(d) != "store"));
|
||||
Assert.NotEmpty(Directory.GetFiles(Path.Combine(storeRoot, "compile")));
|
||||
if (string.Equals(result.RenderReceipt.Status, "succeeded", StringComparison.Ordinal))
|
||||
{
|
||||
Assert.Equal(storeRoot, Path.GetDirectoryName(Path.GetDirectoryName(UriPath(result.RenderReceipt.Float32Uri))));
|
||||
Assert.Equal(storeRoot, Path.GetDirectoryName(Path.GetDirectoryName(UriPath(result.RenderReceipt.WavUri))));
|
||||
}
|
||||
}
|
||||
|
||||
private static string UriPath(string uri) => new Uri(uri).LocalPath;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue