diff --git a/README.md b/README.md index 8437b11..6336a4f 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,7 @@ For everyone else: - **Fast gallery**: a WebP thumbnail grid plus a full-screen lightbox backed by a downscaled web-safe proxy, so viewing is sharp without sending a full-size original over the wire. Filter by uploader; photos sort by capture time (EXIF). - **HEIC / HEIF from phones**: decoded server-side, so iPhone photos get thumbnails and previews in every browser, not just Safari. - **GIFs that move**: an animated GIF (or animated WebP) plays in the lightbox and when you hover its tile. The grid itself holds still, so a box full of GIFs doesn't flicker at everyone at once. -- **Short videos, minimally**: MP4/MOV/WebM clips can be dropped in alongside the photos. Each gets a poster frame so it has a tile in the grid, and downloads as the original file. Nothing is transcoded and there is no in-browser playback. +- **Short videos, minimally**: MP4/MOV/MKV/WebM clips can be dropped in alongside the photos. Each gets a poster frame so it has a tile in the grid, and downloads as the original file. Nothing is transcoded and there is no in-browser playback. - **Flexible downloads**: a single photo, the whole box as a streamed ZIP, or "download others'": everything except your own uploads. - **Private admin link**: the creator can rename the box, change or remove the password, adjust expiry, delete individual photos, or delete the whole box. - **Auto-expiry**: a box can be set to delete itself a chosen number of days after the event. @@ -168,6 +168,11 @@ appear in the gallery everywhere. The original file is always stored unmodified Download button returns. Files of the wrong type, over `MaxFileSizeMb`, or that don't decode as a real image within the pixel limits are rejected at upload. +The browser checks a file against these limits before sending it, so one that wouldn't be +accepted is refused as soon as it's picked; the server checks again regardless. Uploads that +fail outright answer with the reason in the body, and the page shows what it was told rather +than filling in a cause of its own. + ### Animations An animated GIF (or animated WebP) keeps its animation: the display proxy is written as an @@ -194,6 +199,7 @@ same box as the photos, and no more: | MP4 | `.mp4`, `.m4v` | | QuickTime | `.mov` | | WebM | `.webm` | +| Matroska | `.mkv` | A clip is stored untouched, appears in the grid as a still frame with a **Video** badge, and is included in ZIP downloads like anything else. There is **no in-browser playback and no diff --git a/src/Shoebox.Web/Api/MediaEndpoints.cs b/src/Shoebox.Web/Api/MediaEndpoints.cs index f1c9012..8270551 100644 --- a/src/Shoebox.Web/Api/MediaEndpoints.cs +++ b/src/Shoebox.Web/Api/MediaEndpoints.cs @@ -23,41 +23,62 @@ public static void MapMediaApi(this IEndpointRouteBuilder app) api.MapGet("/p/{code}/qr", QrCodeAsync); } + /// + /// Takes one upload. Every way this can fail answers with the reason in the body: what the + /// page shows has to come from here, since a client filling in the blank itself can only + /// guess, and its guess is always that the network was at fault. + /// private static async Task UploadAsync( string code, HttpRequest request, AppDbContext db, PoolService pools, MediaService media, + MediaHandlers handlers, PoolAccessService access, UploaderIdentity identity) { var pool = await pools.FindByCodeAsync(code); if (pool is null) { - return Results.NotFound(); + return UploadFailed(StatusCodes.Status404NotFound, + "This box no longer exists — it may have expired."); } if (!access.CanView(request.HttpContext, pool)) { - return Results.Unauthorized(); + return UploadFailed(StatusCodes.Status401Unauthorized, + "This box is locked. Refresh the page and enter the password again."); } if (!request.HasFormContentType) { - return Results.BadRequest(new { error = "Expected multipart form data." }); + return UploadFailed(StatusCodes.Status400BadRequest, "Expected multipart form data."); + } + + IFormCollection form; + try + { + form = await request.ReadFormAsync(); + } + catch (Exception ex) when (ex is BadHttpRequestException or InvalidDataException) + { + // Past the request-body limit. Left alone this is an unhandled exception and an + // HTML error page, which says nothing about size. + return UploadFailed(StatusCodes.Status413PayloadTooLarge, + $"That file is larger than this server accepts — {handlers.Policy.Summary}."); } - var form = await request.ReadFormAsync(); var uploaderName = form["uploaderName"].ToString().Trim(); if (uploaderName.Length is 0 or > 80) { - return Results.BadRequest(new { error = "Please tell us who you are (1-80 characters)." }); + return UploadFailed(StatusCodes.Status400BadRequest, + "Please tell us who you are (1-80 characters)."); } if (form.Files.Count == 0) { - return Results.BadRequest(new { error = "No files in upload." }); + return UploadFailed(StatusCodes.Status400BadRequest, "No files in upload."); } var uid = identity.GetOrCreateUid(request.HttpContext); @@ -72,6 +93,9 @@ private static async Task UploadAsync( return Results.Ok(new { results }); } + private static IResult UploadFailed(int statusCode, string error) => + Results.Json(new { error }, statusCode: statusCode); + private static async Task ServeThumbAsync( Guid id, HttpContext context, AppDbContext db, PoolAccessService access, StoragePaths paths) { diff --git a/src/Shoebox.Web/Pages/Pool/Gallery.cshtml b/src/Shoebox.Web/Pages/Pool/Gallery.cshtml index e79ccb9..906707f 100644 --- a/src/Shoebox.Web/Pages/Pool/Gallery.cshtml +++ b/src/Shoebox.Web/Pages/Pool/Gallery.cshtml @@ -53,7 +53,9 @@
- +

or drop photos anywhere on this page

diff --git a/src/Shoebox.Web/Pages/Pool/Gallery.cshtml.cs b/src/Shoebox.Web/Pages/Pool/Gallery.cshtml.cs index bc24ac7..1369a88 100644 --- a/src/Shoebox.Web/Pages/Pool/Gallery.cshtml.cs +++ b/src/Shoebox.Web/Pages/Pool/Gallery.cshtml.cs @@ -1,3 +1,4 @@ +using System.Text.Json; using Shoebox.Web.Data; using Shoebox.Web.Services; using Microsoft.AspNetCore.Mvc; @@ -16,7 +17,8 @@ public class GalleryModel( PoolService pools, PoolAccessService access, UploaderIdentity identity, - ShareLinkService links) : PageModel + ShareLinkService links, + MediaHandlers handlers) : PageModel { public Data.Pool Pool { get; set; } = null!; public List Items { get; set; } = []; @@ -32,6 +34,11 @@ public class GalleryModel( // "everyone else's" download only offers itself when it would actually return files. public bool HasOthers { get; set; } + // Handed to the page so the browser can turn a file away before sending it. + public UploadPolicy UploadPolicy => handlers.Policy; + + public string UploadLimitsJson => JsonSerializer.Serialize(UploadPolicy.MaxBytesByExtension); + public async Task OnGetAsync(string code) { var pool = await pools.FindByCodeAsync(code); diff --git a/src/Shoebox.Web/Services/IMediaHandler.cs b/src/Shoebox.Web/Services/IMediaHandler.cs index 7339703..bf05be4 100644 --- a/src/Shoebox.Web/Services/IMediaHandler.cs +++ b/src/Shoebox.Web/Services/IMediaHandler.cs @@ -11,10 +11,14 @@ public interface IMediaHandler { MediaKind Kind { get; } + /// What to call this kind when telling someone their file didn't fit. + string Label { get; } + /// - /// The content type to store for this extension, or null when this handler doesn't take it. + /// Every extension this handler takes, mapped to the content type to store for it. Looked + /// up case-insensitively, so implementations build it with an ordinal-ignore-case comparer. /// - string? ContentTypeFor(string extension); + IReadOnlyDictionary ContentTypes { get; } /// Per-file upload ceiling for this kind. long MaxBytes { get; } @@ -40,6 +44,15 @@ public interface IMediaHandler string? RenderFailureReason { get; } } +/// +/// What the server takes: the ceiling per extension, for the browser to check a file against +/// before sending it, and the same thing in words, for saying why one didn't fit. +/// +public record UploadPolicy(IReadOnlyDictionary MaxBytesByExtension, string Summary) +{ + public static string DescribeSize(long bytes) => $"{bytes / (1024.0 * 1024.0):0.#} MB"; +} + /// Finds the handler for an upload, by file extension or by stored kind. public class MediaHandlers(IEnumerable handlers) { @@ -53,7 +66,7 @@ public class MediaHandlers(IEnumerable handlers) { foreach (var handler in all) { - if (handler.ContentTypeFor(extension) is { } contentType) + if (handler.ContentTypes.GetValueOrDefault(extension) is { } contentType) { return (handler, contentType); } @@ -63,4 +76,12 @@ public class MediaHandlers(IEnumerable handlers) } public IMediaHandler For(MediaKind kind) => all.First(h => h.Kind == kind); + + /// Assembled from the handlers, so nothing that quotes it can drift from them. + public UploadPolicy Policy => new( + all.SelectMany(h => h.ContentTypes.Keys.Select(e => (Extension: e.ToLowerInvariant(), h.MaxBytes))) + .ToDictionary(x => x.Extension, x => x.MaxBytes, StringComparer.OrdinalIgnoreCase), + string.Join(", ", all.Select(h => + $"{h.Label}s ({string.Join(", ", h.ContentTypes.Keys.Select(e => e.TrimStart('.')))}) " + + $"up to {UploadPolicy.DescribeSize(h.MaxBytes)}"))); } diff --git a/src/Shoebox.Web/Services/MediaService.cs b/src/Shoebox.Web/Services/MediaService.cs index efe0e1c..bbbd138 100644 --- a/src/Shoebox.Web/Services/MediaService.cs +++ b/src/Shoebox.Web/Services/MediaService.cs @@ -22,7 +22,8 @@ public async Task SaveAsync(Pool pool, IFormFile file, string uplo var match = handlers.For(extension); if (match is null) { - return UploadResult.Rejected(fileName, "Unsupported file type"); + var what = extension.Length == 0 ? "files with no extension" : $"{extension.ToLowerInvariant()} files"; + return UploadResult.Rejected(fileName, $"Can't take {what} — {handlers.Policy.Summary}"); } var (handler, contentType) = match.Value; @@ -34,7 +35,9 @@ public async Task SaveAsync(Pool pool, IFormFile file, string uplo if (file.Length > handler.MaxBytes) { - return UploadResult.Rejected(fileName, $"Larger than {handler.MaxBytes / (1024 * 1024)} MB"); + return UploadResult.Rejected(fileName, + $"Too big ({UploadPolicy.DescribeSize(file.Length)}) — {handler.Label}s can be up to " + + UploadPolicy.DescribeSize(handler.MaxBytes)); } Directory.CreateDirectory(paths.OriginalsDirectory(pool.Id)); diff --git a/src/Shoebox.Web/Services/PhotoHandler.cs b/src/Shoebox.Web/Services/PhotoHandler.cs index 54c1c51..825abe9 100644 --- a/src/Shoebox.Web/Services/PhotoHandler.cs +++ b/src/Shoebox.Web/Services/PhotoHandler.cs @@ -9,7 +9,7 @@ namespace Shoebox.Web.Services; /// public class PhotoHandler(ImageRenderer renderer, IOptions options) : IMediaHandler { - private static readonly Dictionary ContentTypes = new(StringComparer.OrdinalIgnoreCase) + public IReadOnlyDictionary ContentTypes { get; } = new Dictionary(StringComparer.OrdinalIgnoreCase) { [".jpg"] = "image/jpeg", [".jpeg"] = "image/jpeg", @@ -22,7 +22,7 @@ public class PhotoHandler(ImageRenderer renderer, IOptions optio public MediaKind Kind => MediaKind.Photo; - public string? ContentTypeFor(string extension) => ContentTypes.GetValueOrDefault(extension); + public string Label => "photo"; public long MaxBytes => options.Value.MaxFileSizeBytes; diff --git a/src/Shoebox.Web/Services/VideoHandler.cs b/src/Shoebox.Web/Services/VideoHandler.cs index 7e3acc1..1669d28 100644 --- a/src/Shoebox.Web/Services/VideoHandler.cs +++ b/src/Shoebox.Web/Services/VideoHandler.cs @@ -10,17 +10,19 @@ namespace Shoebox.Web.Services; /// public class VideoHandler(VideoRenderer renderer, IOptions options) : IMediaHandler { - private static readonly Dictionary ContentTypes = new(StringComparer.OrdinalIgnoreCase) + public IReadOnlyDictionary ContentTypes { get; } = new Dictionary(StringComparer.OrdinalIgnoreCase) { [".mp4"] = "video/mp4", [".m4v"] = "video/mp4", [".mov"] = "video/quicktime", [".webm"] = "video/webm", + // Matroska shares WebM's container, so the header check and ffmpeg already handled it. + [".mkv"] = "video/x-matroska", }; public MediaKind Kind => MediaKind.Video; - public string? ContentTypeFor(string extension) => ContentTypes.GetValueOrDefault(extension); + public string Label => "video"; public long MaxBytes => options.Value.MaxVideoFileSizeBytes; diff --git a/src/Shoebox.Web/wwwroot/js/gallery.js b/src/Shoebox.Web/wwwroot/js/gallery.js index 3e2a5a4..89e1044 100644 --- a/src/Shoebox.Web/wwwroot/js/gallery.js +++ b/src/Shoebox.Web/wwwroot/js/gallery.js @@ -12,6 +12,39 @@ // ---------- Upload ---------- + // What the server takes, extension -> byte ceiling, so a file it would refuse is refused + // here instead of after a long upload. The server checks again regardless. + const limits = parseJson(fileInput.dataset.limits) || {}; + const limitsSummary = fileInput.dataset.limitsSummary || ""; + + function parseJson(text) { + try { + return JSON.parse(text); + } catch { + return null; + } + } + + // Why this file can't be sent, or null when it's worth trying. + function whyNotUploadable(file) { + if (!Object.keys(limits).length) return null; + + const dot = file.name.lastIndexOf("."); + const extension = dot > 0 ? file.name.slice(dot).toLowerCase() : ""; + const max = limits[extension]; + if (max === undefined) { + const what = extension ? extension + " files" : "files with no extension"; + return `Can't take ${what} — ${limitsSummary}`; + } + if (file.size === 0) return "Empty file"; + if (file.size > max) return `Too big (${describeSize(file.size)}) — up to ${describeSize(max)}`; + return null; + } + + function describeSize(bytes) { + return (bytes / (1024 * 1024)).toFixed(1).replace(/\.0$/, "") + " MB"; + } + pickBtn.addEventListener("click", () => { if (!requireName()) return; fileInput.click(); @@ -63,6 +96,13 @@ progressList.appendChild(item); const status = item.querySelector(".status"); + const problem = whyNotUploadable(file); + if (problem) { + status.textContent = problem; + status.className = "status fail"; + continue; + } + try { const result = await uploadOne(file, (pct) => (status.textContent = pct + "%")); const r = result.results && result.results[0]; @@ -99,20 +139,26 @@ xhr.upload.addEventListener("progress", (e) => { if (e.lengthComputable) onProgress(Math.round((e.loaded / e.total) * 100)); }); + // The reason is the server's to give; nothing here invents one it wasn't told. xhr.addEventListener("load", () => { + const body = parseJson(xhr.responseText); if (xhr.status >= 200 && xhr.status < 300) { - resolve(JSON.parse(xhr.responseText)); - } else if (xhr.status === 401) { - reject(new Error("box is locked; refresh the page")); + if (body) resolve(body); + else reject(new Error("the server's reply wasn't in a form this page could read")); + } else if (body && body.error) { + reject(new Error(body.error)); } else if (xhr.status === 413) { - reject(new Error("file too large")); + // A 413 with no reason in it came from something in front of the app, but the + // status alone still says what happened. + reject(new Error(`larger than this server accepts — ${limitsSummary}`)); } else { - let msg = "upload failed"; - try { msg = JSON.parse(xhr.responseText).error || msg; } catch { /* keep default */ } - reject(new Error(msg)); + reject(new Error(`the server refused this upload (HTTP ${xhr.status})`)); } }); - xhr.addEventListener("error", () => reject(new Error("network error"))); + // No response arrived, which is the whole of what this event says. Say only that. + xhr.addEventListener("error", () => + reject(new Error("the upload didn't finish — the server sent no reply")) + ); xhr.send(form); }); } diff --git a/tests/Shoebox.Tests.Web/CoreFlowTests.cs b/tests/Shoebox.Tests.Web/CoreFlowTests.cs index 73d6dcd..f030e50 100644 --- a/tests/Shoebox.Tests.Web/CoreFlowTests.cs +++ b/tests/Shoebox.Tests.Web/CoreFlowTests.cs @@ -22,6 +22,14 @@ public class CoreFlowTests (byte)'i', (byte)'s', (byte)'o', (byte)'m', (byte)'m', (byte)'p', (byte)'4', (byte)'2', ]; + // An EBML header: what a Matroska (.mkv) file starts with, enough to clear the container + // check without a real clip, as with Mp4Header above. + private static readonly byte[] MatroskaHeader = + [ + 0x1A, 0x45, 0xDF, 0xA3, 0x01, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x1F, 0x42, 0x86, 0x81, 0x01, + ]; + [Fact] public async Task Animated_gif_gets_a_moving_proxy_and_a_still_thumbnail() { @@ -191,6 +199,48 @@ public async Task Real_clip_gets_a_poster_frame() return null; } + [Fact] + public async Task Matroska_clip_is_accepted_like_any_other_video() + { + using var factory = new ShoeboxWebApplicationFactory(); + using var owner = CreateClient(factory); + var code = await CreateBoxAsync(owner); + + var added = await UploadAsync(owner, code, "Alice", "clip.mkv", MatroskaHeader); + Assert.Equal("added", added.Status); + var mediaId = Assert.IsType(added.MediaId); + + var original = await owner.GetAsync($"/api/media/{mediaId}/original"); + Assert.Equal(HttpStatusCode.OK, original.StatusCode); + Assert.Equal("video/x-matroska", original.Content.Headers.ContentType?.MediaType); + Assert.Equal(MatroskaHeader, await original.Content.ReadAsByteArrayAsync()); + + var gallery = await owner.GetAsync($"/p/{code}"); + Assert.Contains("media-badge\">\u25B6 Video", await gallery.Content.ReadAsStringAsync()); + } + + [Fact] + public async Task Upload_past_the_request_body_limit_answers_413_saying_so() + { + // Ceilings small enough to reach the request-body limit without moving hundreds of + // megabytes through the test. + using var factory = new ShoeboxWebApplicationFactory(new Dictionary + { + ["Shoebox:MaxFileSizeMb"] = "1", + ["Shoebox:MaxVideoFileSizeMb"] = "1", + }); + using var owner = CreateClient(factory); + var code = await CreateBoxAsync(owner); + + var response = await PostUploadAsync(owner, code, "Alice", "huge.mp4", new byte[3 * 1024 * 1024]); + + // Uncaught, an over-limit body is a 500 and an HTML error page never mentioning size. + Assert.Equal(HttpStatusCode.RequestEntityTooLarge, response.StatusCode); + var error = (await response.Content.ReadFromJsonAsync()).GetProperty("error").GetString(); + Assert.Contains("larger than this server accepts", error); + Assert.Contains("1 MB", error); + } + [Fact] public async Task File_that_is_not_really_a_video_is_rejected() { @@ -335,6 +385,19 @@ private static async Task UploadAsync( string uploader, string fileName, byte[] bytes) + { + var response = await PostUploadAsync(client, code, uploader, fileName, bytes); + response.EnsureSuccessStatusCode(); + var body = await response.Content.ReadFromJsonAsync(); + return Assert.Single(Assert.IsType(body).Results); + } + + private static async Task PostUploadAsync( + HttpClient client, + string code, + string uploader, + string fileName, + byte[] bytes) { using var form = new MultipartFormDataContent(); form.Add(new StringContent(uploader), "uploaderName"); @@ -343,10 +406,7 @@ private static async Task UploadAsync( new System.Net.Http.Headers.MediaTypeHeaderValue("application/octet-stream"); form.Add(file, "files", fileName); - var response = await client.PostAsync($"/api/p/{code}/media", form); - response.EnsureSuccessStatusCode(); - var body = await response.Content.ReadFromJsonAsync(); - return Assert.Single(Assert.IsType(body).Results); + return await client.PostAsync($"/api/p/{code}/media", form); } private sealed record UploadEnvelope(UploadResponse[] Results); diff --git a/tests/Shoebox.Tests.Web/ShoeboxWebApplicationFactory.cs b/tests/Shoebox.Tests.Web/ShoeboxWebApplicationFactory.cs index 6e94097..a53c24e 100644 --- a/tests/Shoebox.Tests.Web/ShoeboxWebApplicationFactory.cs +++ b/tests/Shoebox.Tests.Web/ShoeboxWebApplicationFactory.cs @@ -6,8 +6,12 @@ namespace Shoebox.Tests.Web; public sealed class ShoeboxWebApplicationFactory : WebApplicationFactory { - public ShoeboxWebApplicationFactory() + private readonly IReadOnlyDictionary settings; + + /// Configuration overrides for this instance. + public ShoeboxWebApplicationFactory(IReadOnlyDictionary? settings = null) { + this.settings = settings ?? new Dictionary(); DataPath = Path.Combine( Path.GetTempPath(), $"shoebox-tests-{Guid.NewGuid():N}"); @@ -21,6 +25,10 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) builder.UseEnvironment("Testing"); builder.UseSetting("Shoebox:DataPath", DataPath); builder.UseSetting("Shoebox:CookieLifetimeDays", "1"); + foreach (var (key, value) in settings) + { + builder.UseSetting(key, value); + } } protected override void Dispose(bool disposing)