From b99cfdd6a9cb9acf9db83f5e3833484dbb8fcbbc Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 28 Sep 2026 17:00:10 -0400 Subject: [PATCH 1/4] Add storage limit, daily upload budget and one client key to PlanShare Sharing is refused with 507 once the database uses 10 GB, counted as (page_count - freelist_count) * page_size. /api/event skips the insert in that state and still answers 200, so analytics never show an error. Each client key can store 100 MB of plan data per UTC day. The budget is in memory like the rate limiters and CleanupService sweeps it. Every per-client limit (share, analytics, read, budget) now uses one key: an IPv4 address, an IPv4-mapped IPv6 address as its IPv4 address, and any other IPv6 address as its /64. The visitor hash still uses the full address. /api/share and /api/event answer 400 instead of 500 for a JSON root that is not an object and for fields of the wrong type. /api/event refuses a path over 512 characters. DELETE /api/plans/{id} takes the token in the X-Delete-Token header and still accepts ?token=. Refusals carry an "error" text in a JSON body. The new classes are internal with InternalsVisibleTo, and the test project now references server/PlanShare so CI builds and tests it. The ci.yml path filter includes server/PlanShare. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_019n3G844aTidqrD6A6iMgza --- .github/workflows/ci.yml | 4 +- server/PlanShare/ClientKey.cs | 33 +++++ server/PlanShare/PlanShare.csproj | 5 + server/PlanShare/Program.cs | 127 ++++++++++++++---- server/PlanShare/StorageCheck.cs | 41 ++++++ server/PlanShare/UploadBudget.cs | 88 ++++++++++++ .../PlanShareClientKeyTests.cs | 65 +++++++++ .../PlanShareStorageCheckTests.cs | 95 +++++++++++++ .../PlanShareUploadBudgetTests.cs | 127 ++++++++++++++++++ .../PlanViewer.Core.Tests.csproj | 3 + 10 files changed, 561 insertions(+), 27 deletions(-) create mode 100644 server/PlanShare/ClientKey.cs create mode 100644 server/PlanShare/StorageCheck.cs create mode 100644 server/PlanShare/UploadBudget.cs create mode 100644 tests/PlanViewer.Core.Tests/PlanShareClientKeyTests.cs create mode 100644 tests/PlanViewer.Core.Tests/PlanShareStorageCheckTests.cs create mode 100644 tests/PlanViewer.Core.Tests/PlanShareUploadBudgetTests.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d2b044f6..1936c1ed 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,13 +31,15 @@ jobs: # stack of '!' patterns matches whenever a file fails ANY one of them, which for # a docs-only change is always true. Listing what IS code keeps the OR honest. # PlanViewer.Ssms and PlanViewer.Ssms.Installer stay out: they are not in the - # solution and ci.yml never built them. + # solution and ci.yml never built them. server/PlanShare is in: the test project + # references it, so this job builds and tests it. filters: | code: - 'src/PlanViewer.App/**' - 'src/PlanViewer.Cli/**' - 'src/PlanViewer.Core/**' - 'src/PlanViewer.Web/**' + - 'server/PlanShare/**' - 'src/Directory.Build.props' - 'tests/**' - 'PlanViewer.sln' diff --git a/server/PlanShare/ClientKey.cs b/server/PlanShare/ClientKey.cs new file mode 100644 index 00000000..4457b777 --- /dev/null +++ b/server/PlanShare/ClientKey.cs @@ -0,0 +1,33 @@ +using System.Net; +using System.Net.Sockets; + +namespace PlanShare; + +/// +/// The one key every per-client limit (share, analytics, read, upload budget) is counted under. +/// An IPv4 address is its own key. An IPv4-mapped IPv6 address (how a dual-stack socket reports +/// an IPv4 caller) becomes that IPv4 address, so one caller cannot hold two keys. Any other IPv6 +/// address becomes its /64 prefix: a single subscriber is normally handed a whole /64, so a +/// limit per full IPv6 address would never be reached by a caller who rotates through it. +/// +internal static class ClientKey +{ + public const string Unknown = "unknown"; + + public static string From(IPAddress? address) + { + if (address is null) + return Unknown; + + if (address.IsIPv4MappedToIPv6) + address = address.MapToIPv4(); + + if (address.AddressFamily != AddressFamily.InterNetworkV6) + return address.ToString(); + + Span bytes = stackalloc byte[16]; + address.TryWriteBytes(bytes, out _); + bytes[8..].Clear(); + return $"{new IPAddress(bytes)}/64"; + } +} diff --git a/server/PlanShare/PlanShare.csproj b/server/PlanShare/PlanShare.csproj index 0d1f7756..92abd428 100644 --- a/server/PlanShare/PlanShare.csproj +++ b/server/PlanShare/PlanShare.csproj @@ -18,4 +18,9 @@ + + + + + diff --git a/server/PlanShare/Program.cs b/server/PlanShare/Program.cs index a144de34..1ddf429b 100644 --- a/server/PlanShare/Program.cs +++ b/server/PlanShare/Program.cs @@ -4,6 +4,7 @@ using System.Text.Json; using Microsoft.AspNetCore.HttpOverrides; using Microsoft.Data.Sqlite; +using PlanShare; var builder = WebApplication.CreateBuilder(args); @@ -14,8 +15,9 @@ policy.AllowAnyOrigin().AllowAnyMethod().AllowAnyHeader()); }); -// Database path — data/ subdirectory relative to the binary -var dataDir = Path.Combine(AppContext.BaseDirectory, "data"); +// Database path — data/ subdirectory relative to the binary. PlanShare:DataDir moves it +// (the endpoint tests and local runs point it at a temp folder). +var dataDir = builder.Configuration["PlanShare:DataDir"] ?? Path.Combine(AppContext.BaseDirectory, "data"); Directory.CreateDirectory(dataDir); var dbPath = Path.Combine(dataDir, "plans.db"); var connectionString = $"Data Source={dbPath}"; @@ -71,9 +73,24 @@ created_at TEXT NOT NULL // store; 120/min per IP is far above any human's browsing rate. var readRateLimiter = new RateLimiter(maxRequests: 120, windowSeconds: 60); +// --- Storage limit and daily upload budget --- +// The production disk is 40 GB. Stopping at 10 GB of used database pages leaves room for the OS, +// logs, the SQLite journal and a VACUUM, and a full store turns shares away instead of filling +// the disk. The budget caps one client key's stored plan data per UTC day; it is per key and not +// global, so one heavy uploader cannot use up the space for everyone else in a single day. +// PlanShare:MaxDatabaseBytes and PlanShare:DailyUploadBytes override the limits (tests and local runs). +const long MaxDatabaseBytes = 10L * 1024 * 1024 * 1024; +const long DailyUploadBytes = 100L * 1024 * 1024; +var storageCheck = new StorageCheck( + connectionString, + builder.Configuration.GetValue("PlanShare:MaxDatabaseBytes") ?? MaxDatabaseBytes); +var uploadBudget = new UploadBudget( + builder.Configuration.GetValue("PlanShare:DailyUploadBytes") ?? DailyUploadBytes); + // Register the cleanup background service builder.Services.AddSingleton(new PlanDbConfig(connectionString)); builder.Services.AddSingleton(new RateLimiters(rateLimiter, analyticsRateLimiter, readRateLimiter)); +builder.Services.AddSingleton(uploadBudget); builder.Services.AddHostedService(); // Request size limit (10 MB) @@ -105,6 +122,9 @@ created_at TEXT NOT NULL const int MaxTtlDays = 365; +// Longest page path an analytics event may carry. Real paths are a few characters. +const int MaxEventPathLength = 512; + // Depth ceiling for parsing an uploaded share, mirroring PlanViewer.Core's // AnalysisJson.MaxDepth — that class is the source of truth for how deep a serialized // AnalysisResult can go (#431: an operator costs two JSON levels, so the JsonDocument @@ -121,11 +141,11 @@ created_at TEXT NOT NULL app.MapPost("/api/share", async (HttpContext ctx) => { - // Rate limit by IP - var ip = ctx.Connection.RemoteIpAddress?.ToString() ?? "unknown"; - if (!rateLimiter.IsAllowed(ip)) + // Rate limit by client key (IPv4 address, or the /64 of an IPv6 address) + var client = ClientKey.From(ctx.Connection.RemoteIpAddress); + if (!rateLimiter.IsAllowed(client)) { - return Results.StatusCode(429); + return Error(429, "Too many shares from your network. Please wait a minute and try again."); } // Read raw body @@ -134,7 +154,7 @@ created_at TEXT NOT NULL if (string.IsNullOrWhiteSpace(body)) { - return Results.BadRequest("Empty body"); + return Error(400, "Empty body"); } // Parse and extract ttl_days from the JSON. shareDocumentOptions, not defaults: this body @@ -144,12 +164,31 @@ created_at TEXT NOT NULL try { using var doc = JsonDocument.Parse(body, shareDocumentOptions); - if (doc.RootElement.TryGetProperty("ttl_days", out var ttlProp) && ttlProp.TryGetInt32(out var t)) - ttlDays = Math.Clamp(t, 1, MaxTtlDays); + // TryGetProperty and TryGetInt32 throw (a 500) on the wrong kind of element, so check the kind first + if (doc.RootElement.ValueKind != JsonValueKind.Object) + return Error(400, "The request body must be a JSON object."); + if (doc.RootElement.TryGetProperty("ttl_days", out var ttlProp) && ttlProp.ValueKind != JsonValueKind.Null) + { + if (ttlProp.ValueKind != JsonValueKind.Number) + return Error(400, "ttl_days must be a number."); + if (ttlProp.TryGetInt32(out var t)) + ttlDays = Math.Clamp(t, 1, MaxTtlDays); + } } catch (JsonException) { - return Results.BadRequest("Invalid JSON"); + return Error(400, "Invalid JSON"); + } + + if (storageCheck.IsFull()) + { + return Error(507, "Plan sharing is full right now. Please try again later."); + } + + // The stored size is the UTF-8 length of the body; ContentLength can be absent + if (!uploadBudget.TryCharge(client, Encoding.UTF8.GetByteCount(body))) + { + return Error(429, "Daily sharing limit reached for your network. Please try again tomorrow."); } var id = GenerateId(); @@ -175,8 +214,8 @@ created_at TEXT NOT NULL app.MapGet("/api/plans/{id}", (string id, HttpContext ctx) => { - var readIp = ctx.Connection.RemoteIpAddress?.ToString() ?? "unknown"; - if (!readRateLimiter.IsAllowed(readIp)) + var readClient = ClientKey.From(ctx.Connection.RemoteIpAddress); + if (!readRateLimiter.IsAllowed(readClient)) return Results.StatusCode(429); using var conn = new SqliteConnection(connectionString); @@ -199,9 +238,8 @@ created_at TEXT NOT NULL app.MapPost("/api/event", async (HttpContext ctx) => { - // Rate limit: 30 events/min per IP (generous — covers page nav + shares) - var ip = ctx.Connection.RemoteIpAddress?.ToString() ?? "unknown"; - if (!analyticsRateLimiter.IsAllowed(ip)) + // Rate limit: 30 events/min per client key (generous — covers page nav + shares) + if (!analyticsRateLimiter.IsAllowed(ClientKey.From(ctx.Connection.RemoteIpAddress))) return Results.StatusCode(429); using var reader = new StreamReader(ctx.Request.Body); @@ -212,16 +250,32 @@ created_at TEXT NOT NULL try { using var doc = JsonDocument.Parse(body); - if (doc.RootElement.TryGetProperty("path", out var p)) + // GetString() throws (a 500) on a non-string element, so check the kind first. + // JSON null counts as "not sent", like a missing property. + if (doc.RootElement.ValueKind != JsonValueKind.Object) + return Error(400, "The request body must be a JSON object."); + if (doc.RootElement.TryGetProperty("path", out var p) && p.ValueKind != JsonValueKind.Null) + { + if (p.ValueKind != JsonValueKind.String) + return Error(400, "path must be a string."); path = p.GetString() ?? "/"; - if (doc.RootElement.TryGetProperty("referrer", out var r)) + } + if (doc.RootElement.TryGetProperty("referrer", out var r) && r.ValueKind != JsonValueKind.Null) + { + if (r.ValueKind != JsonValueKind.String) + return Error(400, "referrer must be a string."); referrer = r.GetString(); + } } - catch (JsonException) + catch (Exception ex) when (ex is JsonException or InvalidOperationException) { - return Results.BadRequest("Invalid JSON"); + // InvalidOperationException: GetString() on a string with a lone surrogate escape + return Error(400, "Invalid JSON"); } + if (path.Length > MaxEventPathLength) + return Error(400, $"path must be {MaxEventPathLength} characters or fewer."); + // Strip referrer to domain only (no full URLs with query params). // If it doesn't parse as an absolute URL, drop it — never persist raw // client-supplied strings, since the dashboard renders referrers in HTML. @@ -237,11 +291,18 @@ created_at TEXT NOT NULL // space (IPv4 = 2^32, guessable UA, known date) is small enough to brute // force straight back to the source IP, so an unsalted digest would still be // personal data despite looking like a hash. + // The hash takes the full address, not the client key: a /64 would count every host in + // it as one visitor. + var ip = ctx.Connection.RemoteIpAddress?.ToString() ?? "unknown"; var ua = ctx.Request.Headers.UserAgent.FirstOrDefault() ?? ""; var day = DateTime.UtcNow.ToString("yyyy-MM-dd"); var visitorHash = Convert.ToHexString( HMACSHA256.HashData(visitorSalt, Encoding.UTF8.GetBytes($"{ip}|{ua}|{day}"))).ToLower()[..16]; + // A full store drops the event and still answers 200: analytics must never show an error + if (storageCheck.IsFull()) + return Results.Ok(); + using var conn = new SqliteConnection(connectionString); conn.Open(); using var cmd = conn.CreateCommand(); @@ -409,10 +470,14 @@ GROUP BY referrer ORDER BY count DESC LIMIT 10 app.MapDelete("/api/plans/{id}", (string id, HttpContext ctx) => { - var token = ctx.Request.Query["token"].FirstOrDefault(); + // Header first: a ?token= query string is written to the nginx access log. The query form + // stays for clients built before the header existed. + var token = ctx.Request.Headers["X-Delete-Token"].FirstOrDefault(); + if (string.IsNullOrEmpty(token)) + token = ctx.Request.Query["token"].FirstOrDefault(); if (string.IsNullOrEmpty(token)) { - return Results.BadRequest("Missing delete token"); + return Error(400, "Missing delete token"); } using var conn = new SqliteConnection(connectionString); @@ -443,6 +508,12 @@ static string GenerateDeleteToken() return Convert.ToHexString(RandomNumberGenerator.GetBytes(16)).ToLower(); } +// A refusal the web client can show: it reads the "error" text out of the body. +static IResult Error(int statusCode, string message) +{ + return Results.Json(new { error = message }, statusCode: statusCode); +} + // --- Supporting types --- record PlanDbConfig(string ConnectionString); @@ -453,12 +524,14 @@ sealed class CleanupService : BackgroundService { private readonly PlanDbConfig _config; private readonly RateLimiters _rateLimiters; + private readonly UploadBudget _uploadBudget; private readonly ILogger _logger; - public CleanupService(PlanDbConfig config, RateLimiters rateLimiters, ILogger logger) + public CleanupService(PlanDbConfig config, RateLimiters rateLimiters, UploadBudget uploadBudget, ILogger logger) { _config = config; _rateLimiters = rateLimiters; + _uploadBudget = uploadBudget; _logger = logger; } @@ -503,14 +576,16 @@ private void Cleanup() // Evict stale rate-limiter keys so the dictionaries don't grow forever. // Every limiter must be swept here: an unswept one keeps a permanent - // entry per unique client IP for the lifetime of the process. + // entry per unique client IP for the lifetime of the process. The upload + // budget is swept the same way (its entries go stale at the UTC day change). var shareEvicted = _rateLimiters.Share.Sweep(); var analyticsEvicted = _rateLimiters.Analytics.Sweep(); var readEvicted = _rateLimiters.Read.Sweep(); - if (shareEvicted + analyticsEvicted + readEvicted > 0) + var budgetEvicted = _uploadBudget.Sweep(); + if (shareEvicted + analyticsEvicted + readEvicted + budgetEvicted > 0) _logger.LogInformation( - "Evicted {Share} share + {Analytics} analytics + {Read} read rate-limit keys", - shareEvicted, analyticsEvicted, readEvicted); + "Evicted {Share} share + {Analytics} analytics + {Read} read rate-limit keys and {Budget} upload-budget keys", + shareEvicted, analyticsEvicted, readEvicted, budgetEvicted); } catch (Exception ex) { diff --git a/server/PlanShare/StorageCheck.cs b/server/PlanShare/StorageCheck.cs new file mode 100644 index 00000000..7e66cede --- /dev/null +++ b/server/PlanShare/StorageCheck.cs @@ -0,0 +1,41 @@ +using Microsoft.Data.Sqlite; + +namespace PlanShare; + +/// +/// Tells whether the plan database has reached its size limit. Size means the pages in use: +/// (page_count - freelist_count) * page_size. The freelist is left out on purpose, because +/// deleting expired plans returns their pages to it and the file itself does not shrink, so +/// the file length would stay above the limit after cleanup had freed the room. +/// +internal sealed class StorageCheck +{ + private readonly string _connectionString; + private readonly long _maxBytes; + + public StorageCheck(string connectionString, long maxBytes) + { + _connectionString = connectionString; + _maxBytes = maxBytes; + } + + public long UsedBytes() + { + using var conn = new SqliteConnection(_connectionString); + conn.Open(); + var pageCount = Pragma(conn, "page_count"); + var freePages = Pragma(conn, "freelist_count"); + var pageSize = Pragma(conn, "page_size"); + return (pageCount - freePages) * pageSize; + } + + /// True at or above the limit. + public bool IsFull() => UsedBytes() >= _maxBytes; + + private static long Pragma(SqliteConnection conn, string name) + { + using var cmd = conn.CreateCommand(); + cmd.CommandText = $"PRAGMA {name};"; + return (long)cmd.ExecuteScalar()!; + } +} diff --git a/server/PlanShare/UploadBudget.cs b/server/PlanShare/UploadBudget.cs new file mode 100644 index 00000000..10a9f3d4 --- /dev/null +++ b/server/PlanShare/UploadBudget.cs @@ -0,0 +1,88 @@ +using System.Collections.Concurrent; + +namespace PlanShare; + +/// +/// Caps how many bytes of plan data one client key can store per UTC day. In memory like +/// RateLimiter, so a restart forgets it. A charge is made before the insert and is not refunded +/// if the insert then fails: refunding would need a second code path for a rare error, and the +/// most a client can lose that way is one upload's worth of its allowance. +/// +internal sealed class UploadBudget +{ + private sealed class Usage + { + public DateOnly Day; + public long Bytes; + public bool Evicted; + } + + private readonly long _dailyLimitBytes; + private readonly TimeProvider _clock; + private readonly ConcurrentDictionary _usage = new(); + + public UploadBudget(long dailyLimitBytes, TimeProvider? clock = null) + { + _dailyLimitBytes = dailyLimitBytes; + _clock = clock ?? TimeProvider.System; + } + + private DateOnly Today() => DateOnly.FromDateTime(_clock.GetUtcNow().UtcDateTime); + + /// + /// Adds to the key's total for today and returns true, or returns + /// false and adds nothing when that would take the total over the daily limit. + /// + public bool TryCharge(string key, long bytes) + { + var today = Today(); + + while (true) + { + var usage = _usage.GetOrAdd(key, _ => new Usage { Day = today }); + + lock (usage) + { + // Sweep() removed this entry between GetOrAdd and the lock; charging it now + // would be lost, so fetch or create the live one. + if (usage.Evicted) + continue; + + if (usage.Day != today) + { + usage.Day = today; + usage.Bytes = 0; + } + + if (usage.Bytes + bytes > _dailyLimitBytes) + return false; + + usage.Bytes += bytes; + return true; + } + } + } + + /// + /// Evicts keys whose last charge was on an earlier UTC day. Call periodically so the + /// dictionary doesn't grow forever across unique clients. + /// Returns the number of keys evicted. + /// + public int Sweep() + { + var today = Today(); + var evicted = 0; + foreach (var kvp in _usage) + { + lock (kvp.Value) + { + if (kvp.Value.Day != today && _usage.TryRemove(kvp)) + { + kvp.Value.Evicted = true; + evicted++; + } + } + } + return evicted; + } +} diff --git a/tests/PlanViewer.Core.Tests/PlanShareClientKeyTests.cs b/tests/PlanViewer.Core.Tests/PlanShareClientKeyTests.cs new file mode 100644 index 00000000..64466bf2 --- /dev/null +++ b/tests/PlanViewer.Core.Tests/PlanShareClientKeyTests.cs @@ -0,0 +1,65 @@ +using System.Net; +using PlanShare; + +namespace PlanViewer.Core.Tests; + +/// +/// Every per-client limit on the share server counts under one key. An IPv4 address is its own key, +/// an IPv4-mapped IPv6 address is the IPv4 address it wraps, and any other IPv6 address is its /64. +/// +public class PlanShareClientKeyTests +{ + [Fact] + public void IPv4Address_IsItsOwnKey() + { + Assert.Equal("203.0.113.7", ClientKey.From(IPAddress.Parse("203.0.113.7"))); + } + + [Fact] + public void IPv4MappedIPv6Address_HasTheSameKeyAsTheIPv4Address() + { + var mapped = ClientKey.From(IPAddress.Parse("::ffff:203.0.113.7")); + + Assert.Equal("203.0.113.7", mapped); + Assert.Equal(ClientKey.From(IPAddress.Parse("203.0.113.7")), mapped); + } + + [Fact] + public void IPv6Addresses_InOneSlash64_ShareAKey() + { + var first = ClientKey.From(IPAddress.Parse("2001:db8:1:2::1")); + var second = ClientKey.From(IPAddress.Parse("2001:db8:1:2:ffff:eeee:dddd:cccc")); + + Assert.Equal(first, second); + Assert.Equal("2001:db8:1:2::/64", first); + } + + [Fact] + public void IPv6Addresses_InDifferentSlash64s_HaveDifferentKeys() + { + // Differ only in the last bit of the 64-bit prefix + var first = ClientKey.From(IPAddress.Parse("2001:db8:1:2::1")); + var second = ClientKey.From(IPAddress.Parse("2001:db8:1:3::1")); + + Assert.NotEqual(first, second); + } + + [Fact] + public void IPv6ZoneId_IsNotPartOfTheKey() + { + Assert.Equal("fe80::/64", ClientKey.From(IPAddress.Parse("fe80::1%3"))); + } + + [Fact] + public void IPv6Key_EndsInSlash64_SoItCannotEqualAnIPv4Key() + { + Assert.EndsWith("/64", ClientKey.From(IPAddress.Parse("2001:db8::1"))); + Assert.DoesNotContain('/', ClientKey.From(IPAddress.Parse("203.0.113.7"))); + } + + [Fact] + public void NoAddress_GivesTheUnknownKey() + { + Assert.Equal("unknown", ClientKey.From(null)); + } +} diff --git a/tests/PlanViewer.Core.Tests/PlanShareStorageCheckTests.cs b/tests/PlanViewer.Core.Tests/PlanShareStorageCheckTests.cs new file mode 100644 index 00000000..d37b7418 --- /dev/null +++ b/tests/PlanViewer.Core.Tests/PlanShareStorageCheckTests.cs @@ -0,0 +1,95 @@ +using Microsoft.Data.Sqlite; +using PlanShare; + +namespace PlanViewer.Core.Tests; + +/// +/// The share server stops taking uploads when the plan database reaches its size limit. Size is the +/// pages in use, (page_count - freelist_count) * page_size, so these tests run against a real SQLite +/// file with a small limit instead of a mock. +/// +public class PlanShareStorageCheckTests : IDisposable +{ + private readonly string _directory = Path.Combine(Path.GetTempPath(), "planshare-storage-" + Guid.NewGuid().ToString("N")); + private readonly string _dbPath; + private readonly string _connectionString; + + public PlanShareStorageCheckTests() + { + Directory.CreateDirectory(_directory); + _dbPath = Path.Combine(_directory, "plans.db"); + // No pooling, so the file is closed and can be deleted as soon as a command finishes + _connectionString = $"Data Source={_dbPath};Pooling=False"; + Execute("CREATE TABLE plans (id INTEGER PRIMARY KEY, data BLOB NOT NULL);"); + } + + public void Dispose() + { + try { Directory.Delete(_directory, recursive: true); } + catch (IOException) { } + } + + [Fact] + public void UsedBytes_IsTheFileLength_WhenNoPagesAreFree() + { + AddPlans(rows: 30, bytesPerRow: 8000); + + var used = new StorageCheck(_connectionString, long.MaxValue).UsedBytes(); + + Assert.True(used > 30 * 8000); + Assert.Equal(new FileInfo(_dbPath).Length, used); + } + + [Fact] + public void UsedBytes_LeavesOutFreePages_AfterRowsAreDeleted() + { + AddPlans(rows: 30, bytesPerRow: 8000); + var length = new FileInfo(_dbPath).Length; + + Execute("DELETE FROM plans;"); + + var used = new StorageCheck(_connectionString, long.MaxValue).UsedBytes(); + Assert.Equal(length, new FileInfo(_dbPath).Length); + Assert.True(used < length / 4, $"used {used} of {length}"); + } + + [Fact] + public void IsFull_IsTrueAtTheLimit_AndFalseJustBelowIt() + { + AddPlans(rows: 30, bytesPerRow: 8000); + var used = new StorageCheck(_connectionString, long.MaxValue).UsedBytes(); + + Assert.True(new StorageCheck(_connectionString, used).IsFull()); + Assert.True(new StorageCheck(_connectionString, used - 1).IsFull()); + Assert.False(new StorageCheck(_connectionString, used + 1).IsFull()); + } + + [Fact] + public void IsFull_GoesBackToFalse_WhenDeletedPlansFreeTheirPagesAndTheFileDoesNotShrink() + { + AddPlans(rows: 30, bytesPerRow: 8000); + var limit = 100 * 1024; + var check = new StorageCheck(_connectionString, limit); + Assert.True(check.IsFull()); + + Execute("DELETE FROM plans;"); + + Assert.True(new FileInfo(_dbPath).Length >= limit); + Assert.False(check.IsFull()); + } + + private void AddPlans(int rows, int bytesPerRow) + { + Execute($"WITH RECURSIVE n(i) AS (SELECT 1 UNION ALL SELECT i + 1 FROM n WHERE i < {rows}) " + + $"INSERT INTO plans (data) SELECT zeroblob({bytesPerRow}) FROM n;"); + } + + private void Execute(string sql) + { + using var conn = new SqliteConnection(_connectionString); + conn.Open(); + using var cmd = conn.CreateCommand(); + cmd.CommandText = sql; + cmd.ExecuteNonQuery(); + } +} diff --git a/tests/PlanViewer.Core.Tests/PlanShareUploadBudgetTests.cs b/tests/PlanViewer.Core.Tests/PlanShareUploadBudgetTests.cs new file mode 100644 index 00000000..48bed972 --- /dev/null +++ b/tests/PlanViewer.Core.Tests/PlanShareUploadBudgetTests.cs @@ -0,0 +1,127 @@ +using PlanShare; + +namespace PlanViewer.Core.Tests; + +/// +/// The share server lets one client key store a fixed number of bytes per UTC day. The clock is +/// injected so the day change can be tested without waiting for it. +/// +public class PlanShareUploadBudgetTests +{ + private const long Limit = 100; + + private sealed class FakeClock : TimeProvider + { + public DateTimeOffset Now { get; set; } = new(2026, 9, 28, 12, 0, 0, TimeSpan.Zero); + + public override DateTimeOffset GetUtcNow() => Now; + } + + [Fact] + public void Charges_UpToTheLimit_AreAllowed() + { + var budget = new UploadBudget(Limit, new FakeClock()); + + Assert.True(budget.TryCharge("client", 60)); + Assert.True(budget.TryCharge("client", 40)); + } + + [Fact] + public void ChargePastTheLimit_IsRefused() + { + var budget = new UploadBudget(Limit, new FakeClock()); + Assert.True(budget.TryCharge("client", 100)); + + Assert.False(budget.TryCharge("client", 1)); + } + + [Fact] + public void SingleChargeLargerThanTheLimit_IsRefused() + { + var budget = new UploadBudget(Limit, new FakeClock()); + + Assert.False(budget.TryCharge("client", Limit + 1)); + } + + [Fact] + public void RefusedCharge_AddsNothingToTheTotal() + { + var budget = new UploadBudget(Limit, new FakeClock()); + Assert.True(budget.TryCharge("client", 90)); + + Assert.False(budget.TryCharge("client", 20)); + Assert.True(budget.TryCharge("client", 10)); + } + + [Fact] + public void EachClientKey_HasItsOwnBudget() + { + var budget = new UploadBudget(Limit, new FakeClock()); + Assert.True(budget.TryCharge("first", 100)); + + Assert.False(budget.TryCharge("first", 1)); + Assert.True(budget.TryCharge("second", 100)); + } + + [Fact] + public void Budget_ResetsAtTheStartOfTheNextUtcDay() + { + var clock = new FakeClock { Now = new DateTimeOffset(2026, 9, 28, 23, 59, 59, TimeSpan.Zero) }; + var budget = new UploadBudget(Limit, clock); + Assert.True(budget.TryCharge("client", 100)); + Assert.False(budget.TryCharge("client", 1)); + + clock.Now = new DateTimeOffset(2026, 9, 29, 0, 0, 0, TimeSpan.Zero); + + Assert.True(budget.TryCharge("client", 100)); + Assert.False(budget.TryCharge("client", 1)); + } + + [Fact] + public void Day_IsTheUtcDay_NotTheLocalDay() + { + // Both times are on 28 September at UTC-5, but 18:30 is 23:30 UTC and 19:30 is 00:30 UTC + // on the next day, so the second charge lands in a new budget. + var clock = new FakeClock { Now = new DateTimeOffset(2026, 9, 28, 18, 30, 0, TimeSpan.FromHours(-5)) }; + var budget = new UploadBudget(Limit, clock); + Assert.True(budget.TryCharge("client", 100)); + Assert.False(budget.TryCharge("client", 1)); + + clock.Now = new DateTimeOffset(2026, 9, 28, 19, 30, 0, TimeSpan.FromHours(-5)); + + Assert.True(budget.TryCharge("client", 100)); + } + + [Fact] + public void Sweep_RemovesKeysFromEarlierDays_AndKeepsTodaysCharges() + { + var clock = new FakeClock(); + var budget = new UploadBudget(Limit, clock); + Assert.True(budget.TryCharge("old-1", 10)); + Assert.True(budget.TryCharge("old-2", 10)); + + clock.Now = clock.Now.AddDays(1); + Assert.True(budget.TryCharge("current", 60)); + + Assert.Equal(2, budget.Sweep()); + Assert.Equal(0, budget.Sweep()); + Assert.False(budget.TryCharge("current", 50)); + Assert.True(budget.TryCharge("current", 40)); + } + + [Fact] + public void ConcurrentCharges_NeverPassTheLimit() + { + const int limit = 1000; + var budget = new UploadBudget(limit, new FakeClock()); + var allowed = 0; + + Parallel.For(0, 4000, _ => + { + if (budget.TryCharge("client", 1)) + Interlocked.Increment(ref allowed); + }); + + Assert.Equal(limit, allowed); + } +} diff --git a/tests/PlanViewer.Core.Tests/PlanViewer.Core.Tests.csproj b/tests/PlanViewer.Core.Tests/PlanViewer.Core.Tests.csproj index ea2973a7..487b64e8 100644 --- a/tests/PlanViewer.Core.Tests/PlanViewer.Core.Tests.csproj +++ b/tests/PlanViewer.Core.Tests/PlanViewer.Core.Tests.csproj @@ -44,6 +44,9 @@ + + From 13ec8a120ca138aae0ba43899ffd0010494fd98c Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Mon, 28 Sep 2026 17:07:58 -0400 Subject: [PATCH 2/4] Show share errors, send the delete token in a header, fix the dashboard The web client reads the "error" text from a failed share (507, 429, 400) and shows it. A reply without that text, such as an HTML page from the proxy, keeps the generic message with the status code. DeleteAsync sends the token in X-Delete-Token instead of ?token=, which the proxy writes to its access log. The share dialog lists what the upload holds: the plan file name, the query text, operator details and warnings, compiled and runtime parameter values, missing index suggestions with database, schema and table names, and the full text report. The payload is unchanged. dashboard.html called history.replaceState, but a local array named history shadows window.history, so the call threw and left #token= in the address bar. It now calls window.history.replaceState. Tests: the endpoints run through WebApplicationFactory with the database in a temp folder and the limits passed as PlanShare:* settings, and the web share service runs against a stub HTTP handler. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_019n3G844aTidqrD6A6iMgza --- server/PlanShare/dashboard.html | 4 +- src/PlanViewer.Web/Pages/Index.razor | 11 +- .../Services/PlanShareService.cs | 32 +- src/PlanViewer.Web/wwwroot/css/app.css | 7 + tests/PlanViewer.Core.Tests/PlanShareApp.cs | 84 ++++ .../PlanShareEndpointTests.cs | 383 ++++++++++++++++++ .../PlanShareServiceTests.cs | 112 +++++ .../PlanViewer.Core.Tests.csproj | 7 + 8 files changed, 636 insertions(+), 4 deletions(-) create mode 100644 tests/PlanViewer.Core.Tests/PlanShareApp.cs create mode 100644 tests/PlanViewer.Core.Tests/PlanShareEndpointTests.cs create mode 100644 tests/PlanViewer.Core.Tests/PlanShareServiceTests.cs diff --git a/server/PlanShare/dashboard.html b/server/PlanShare/dashboard.html index 3b24ac0a..3b458157 100644 --- a/server/PlanShare/dashboard.html +++ b/server/PlanShare/dashboard.html @@ -235,7 +235,9 @@

Top Referrers (30 days)

if (m) { var t = decodeURIComponent(m[1]); try { localStorage.setItem("planshare_stats_token", t); } catch (e) { /* private mode */ } - history.replaceState(null, "", window.location.pathname + window.location.search); + // window.history: a bare "history" here is the stats array declared at the top of this + // script, which has no replaceState, so the call threw and left #token= in the address bar. + window.history.replaceState(null, "", window.location.pathname + window.location.search); return t; } try { return localStorage.getItem("planshare_stats_token") || ""; } catch (e) { return ""; } diff --git a/src/PlanViewer.Web/Pages/Index.razor b/src/PlanViewer.Web/Pages/Index.razor index e9b6b09d..eec1ec21 100644 --- a/src/PlanViewer.Web/Pages/Index.razor +++ b/src/PlanViewer.Web/Pages/Index.razor @@ -76,7 +76,16 @@ else

Share Plan Analysis

-

Your plan analysis will be uploaded and stored on a server. Anyone with the link can view the full analysis, including query text, operator details, and warnings.

+

Your plan analysis will be uploaded and stored on a server. Anyone with the link can read all of it. The upload includes:

+
    +
  • The file name of the plan
    • +
  • The query text
    • +
  • The operator details and the warnings
    • +
  • The compiled and runtime values of the parameters
    • +
  • The missing index suggestions, with database, schema, and table names
    • +
  • The full text report
    • +
+

Query text and parameter values can contain private data. Check the analysis on this page before you share it.