From 52d201eee9113f2bb1c225aa0bf92248a794e735 Mon Sep 17 00:00:00 2001 From: thepra Date: Wed, 7 Oct 2026 10:23:09 +0200 Subject: [PATCH] A focal point is two finite numbers float.TryParse takes "NaN" and "Infinity", and Math.Clamp keeps a NaN, so `focus=NaN,NaN` on an upload or a media PUT was stored as given. Once the media was posted, every status and timeline response holding the post, and the Note and its delivery, failed to serialise, for every viewer. FocalPoint.Parse takes finite numbers only (anything else is ignored, as an unreadable focus was); the mapper and the renderer leave out a focus that isn't sound, and migration _016 removes the ones stored before, from the uploads and from the copy each post keeps. What PrivaPub sends is unchanged for any focus that could be sent before. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01LsXgEaXee4GCU1hwYgPJXw --- CLAUDE.md | 4 ++- PrivaPub.Tests/Http/MastodonMediaTests.cs | 19 ++++++++++++++ .../Infrastructure/MigrationTests.cs | 26 +++++++++++++++++++ .../Mastodon/Controllers/MediaController.cs | 5 ++-- .../Api/Mastodon/Mappers/MastodonMapper.cs | 2 +- PrivaPub/Domain/Media/FocalPoint.cs | 21 +++++++++++++++ PrivaPub/Domain/Media/MediaService.cs | 12 +-------- .../Rendering/ActivityPubRenderer.cs | 3 ++- .../_016_focal_points_are_finite.cs | 25 ++++++++++++++++++ 9 files changed, 100 insertions(+), 17 deletions(-) create mode 100644 PrivaPub/Domain/Media/FocalPoint.cs create mode 100644 PrivaPub/Infrastructure/Data/Migrations/_016_focal_points_are_finite.cs diff --git a/CLAUDE.md b/CLAUDE.md index 2289cde..3e5fb40 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -307,7 +307,9 @@ group www-data and reaches the private mongod; `sudo -u www-data` works too. and never cached. That is how remote video plays. nginx has a `/media/proxy/` location with `proxy_buffering off` and a 600 s read timeout for those streams. -5. **Remote video and audio become one playable attachment** in the Mastodon API (`MastodonMapper.Playable`): the best MP4 +5. **A focal point is two finite numbers** within -1..1 (`FocalPoint.Parse`); anything else is ignored. A stored NaN made + every status, timeline and Note holding its post fail to serialise; migration `_016` removed the ones stored before. +6. **Remote video and audio become one playable attachment** in the Mastodon API (`MastodonMapper.Playable`): the best MP4 up to 720p that carries both sound and picture, including PeerTube's fragmented files inside an HLS entry. HLS playlists themselves are not rewritten. diff --git a/PrivaPub.Tests/Http/MastodonMediaTests.cs b/PrivaPub.Tests/Http/MastodonMediaTests.cs index 1523442..cbd03b8 100644 --- a/PrivaPub.Tests/Http/MastodonMediaTests.cs +++ b/PrivaPub.Tests/Http/MastodonMediaTests.cs @@ -136,6 +136,25 @@ namespace PrivaPub.Tests.Http Assert.Null((await alice.Client.Put($"/api/v1/media/{id}", ("description", ""))).Ok().Body.Text("description")); } + // a focal point is two finite numbers: NaN or Infinity is ignored, never stored (it broke every timeline holding the post) + [Fact] + public async Task A_focal_point_that_is_not_a_number_is_ignored() + { + var alice = await _host.Mastodon("alice"); + var uploaded = (await Upload(alice, "/api/v2/media", MastodonHelpers.JpegWithMetadata(32, 32), "image/jpeg", "a.jpg", ("focus", "NaN,NaN"))).Ok(); + Assert.Null(uploaded.Body["meta"]?["focus"]); + var id = uploaded.Body.Text("id"); + + (await alice.Client.Put($"/api/v1/media/{id}", ("focus", "0.5,0.25"))).Ok(); + foreach (var unsound in new[] { "NaN,0", "Infinity,1", "0,-Infinity" }) + { + var focus = (await alice.Client.Put($"/api/v1/media/{id}", ("focus", unsound))).Ok().Body["meta"]!["focus"]!; + Assert.Equal((0.5, 0.25), (focus["x"]!.GetValue(), focus["y"]!.GetValue())); + } + var status = (await alice.Client.Post("/api/v1/statuses", ("status", "focused"), ("media_ids[]", id))).Ok(); + Assert.Equal(0.5, status.Body["media_attachments"]![0]!["meta"]!["focus"]!["x"]!.GetValue()); + } + [Fact] public async Task Unsupported_unreadable_and_missing_files_are_refused_with_422() { diff --git a/PrivaPub.Tests/Infrastructure/MigrationTests.cs b/PrivaPub.Tests/Infrastructure/MigrationTests.cs index 93f2932..7899587 100644 --- a/PrivaPub.Tests/Infrastructure/MigrationTests.cs +++ b/PrivaPub.Tests/Infrastructure/MigrationTests.cs @@ -1,3 +1,4 @@ +using PrivaPub.Models.Media; using MongoDB.Bson; using MongoDB.Driver; using MongoDB.Entities; @@ -22,6 +23,31 @@ namespace PrivaPub.Tests.Infrastructure [Trait("Category", "Integration")] public class MigrationTests { + [Fact] + public async Task A_focal_point_that_is_not_a_number_is_removed() + { + Assert.SkipUnless(MongoFixture.Enabled, MongoFixture.Skip); + var token = TestContext.Current.CancellationToken; + var broken = new MediaAttachment { OwnerAvatarId = "a1", Focus = [float.NaN, 0f] }; + var sound = new MediaAttachment { OwnerAvatarId = "a1", Focus = [0.5f, -0.5f] }; + var post = new Post + { + ObjectURI = $"https://privapub.test/{Guid.NewGuid():N}", + Media = [new PostMedia { URL = "https://privapub.test/media/files/a.jpg", Focus = [float.PositiveInfinity, 0f] }, new PostMedia { URL = "https://privapub.test/media/files/b.jpg", Focus = [0.1f, 0.2f] }] + }; + await DB.Default.SaveAsync(broken, token); + await DB.Default.SaveAsync(sound, token); + await DB.Default.SaveAsync(post, token); + + await new _016_focal_points_are_finite().UpgradeAsync(); + + Assert.Null((await DB.Default.Find().OneAsync(broken.ID, token)).Focus); + Assert.Equal(new[] { 0.5f, -0.5f }, (await DB.Default.Find().OneAsync(sound.ID, token)).Focus); + var migrated = await DB.Default.Find().OneAsync(post.ID, token); + Assert.Null(migrated.Media[0].Focus); + Assert.Equal(new[] { 0.1f, 0.2f }, migrated.Media[1].Focus); + } + [Fact] public async Task Stored_remote_content_is_sanitized_and_local_content_rendered() { diff --git a/PrivaPub/Api/Mastodon/Controllers/MediaController.cs b/PrivaPub/Api/Mastodon/Controllers/MediaController.cs index 101e99b..0eec267 100644 --- a/PrivaPub/Api/Mastodon/Controllers/MediaController.cs +++ b/PrivaPub/Api/Mastodon/Controllers/MediaController.cs @@ -54,9 +54,8 @@ namespace PrivaPub.Api.Mastodon.Controllers return NotFoundError(); if (Params.Has("description")) attachment.Description = Params.Get("description")?.Trim() is { Length: > 0 } description ? description[..Math.Min(description.Length, 1500)] : default; - if (Params.Get("focus")?.Split(',') is [var x, var y] - && float.TryParse(x, NumberStyles.Float, CultureInfo.InvariantCulture, out var fx) && float.TryParse(y, NumberStyles.Float, CultureInfo.InvariantCulture, out var fy)) - attachment.Focus = new[] { Math.Clamp(fx, -1, 1), Math.Clamp(fy, -1, 1) }; + if (FocalPoint.Parse(Params.Get("focus")) is { } focus) + attachment.Focus = focus; await DB.Default.SaveAsync(attachment, token); return Json(View(attachment)); } diff --git a/PrivaPub/Api/Mastodon/Mappers/MastodonMapper.cs b/PrivaPub/Api/Mastodon/Mappers/MastodonMapper.cs index 7646ea3..eefc19f 100644 --- a/PrivaPub/Api/Mastodon/Mappers/MastodonMapper.cs +++ b/PrivaPub/Api/Mastodon/Mappers/MastodonMapper.cs @@ -642,7 +642,7 @@ namespace PrivaPub.Api.Mastodon.Mappers meta["original"] = original; if (media.DurationSeconds is { } length) meta["length"] = TimeSpan.FromSeconds(length).ToString(@"h\:mm\:ss\.ff", CultureInfo.InvariantCulture); - if (media.Focus is { Length: 2 }) + if (FocalPoint.IsSound(media.Focus)) meta["focus"] = new { x = media.Focus[0], y = media.Focus[1] }; return meta; } diff --git a/PrivaPub/Domain/Media/FocalPoint.cs b/PrivaPub/Domain/Media/FocalPoint.cs new file mode 100644 index 0000000..aa6b76f --- /dev/null +++ b/PrivaPub/Domain/Media/FocalPoint.cs @@ -0,0 +1,21 @@ +using System.Globalization; + +namespace PrivaPub.Domain.Media +{ + // A focal point as Mastodon's API takes it ("x,y", each within -1..1): finite numbers only, clamped to the frame. A NaN + // once stored here made every status, timeline and Note holding the post fail to serialise, for every viewer. + public static class FocalPoint + { + public static float[] Parse(string value) + { + if (value?.Split(',') is not [var x, var y] + || !float.TryParse(x, NumberStyles.Float, CultureInfo.InvariantCulture, out var fx) || !float.IsFinite(fx) + || !float.TryParse(y, NumberStyles.Float, CultureInfo.InvariantCulture, out var fy) || !float.IsFinite(fy)) + return default; + return [Math.Clamp(fx, -1, 1), Math.Clamp(fy, -1, 1)]; + } + + /// Whether a stored focal point can be written out: two finite numbers. + public static bool IsSound(float[] focus) => focus is [var x, var y] && float.IsFinite(x) && float.IsFinite(y); + } +} diff --git a/PrivaPub/Domain/Media/MediaService.cs b/PrivaPub/Domain/Media/MediaService.cs index ee9bff3..5542de3 100644 --- a/PrivaPub/Domain/Media/MediaService.cs +++ b/PrivaPub/Domain/Media/MediaService.cs @@ -75,7 +75,7 @@ namespace PrivaPub.Domain.Media { OwnerAvatarId = owner.Id, Description = Clean(description, 1500), - Focus = Focus(focus) + Focus = FocalPoint.Parse(focus) }; if (ImageTypes.Contains(contentType)) { @@ -252,15 +252,5 @@ namespace PrivaPub.Domain.Media static string Clean(string value, int max) => string.IsNullOrWhiteSpace(value) ? default : value.Trim().Length <= max ? value.Trim() : value.Trim()[..max]; - - static float[] Focus(string focus) - { - var parts = focus?.Split(','); - if (parts is not { Length: 2 } - || !float.TryParse(parts[0], NumberStyles.Float, CultureInfo.InvariantCulture, out var x) - || !float.TryParse(parts[1], NumberStyles.Float, CultureInfo.InvariantCulture, out var y)) - return default; - return new[] { Math.Clamp(x, -1, 1), Math.Clamp(y, -1, 1) }; - } } } diff --git a/PrivaPub/Federation/Rendering/ActivityPubRenderer.cs b/PrivaPub/Federation/Rendering/ActivityPubRenderer.cs index 16872b6..95ed4e5 100644 --- a/PrivaPub/Federation/Rendering/ActivityPubRenderer.cs +++ b/PrivaPub/Federation/Rendering/ActivityPubRenderer.cs @@ -4,6 +4,7 @@ using PrivaPub.Federation.Actors; using PrivaPub.Federation.Objects; using PrivaPub.Models.Federation; using PrivaPub.Models.Post; +using PrivaPub.Domain.Media; using System.Globalization; using System.Net; @@ -386,7 +387,7 @@ namespace PrivaPub.Federation.Rendering }; if (!string.IsNullOrEmpty(m.Blurhash)) attachment["blurhash"] = m.Blurhash; - if (m.Focus is { Length: 2 }) + if (FocalPoint.IsSound(m.Focus)) attachment["focalPoint"] = new JsonArray(m.Focus[0], m.Focus[1]); if (m.Width.HasValue && m.Height.HasValue) { diff --git a/PrivaPub/Infrastructure/Data/Migrations/_016_focal_points_are_finite.cs b/PrivaPub/Infrastructure/Data/Migrations/_016_focal_points_are_finite.cs new file mode 100644 index 0000000..070ee49 --- /dev/null +++ b/PrivaPub/Infrastructure/Data/Migrations/_016_focal_points_are_finite.cs @@ -0,0 +1,25 @@ +using MongoDB.Bson; +using MongoDB.Driver; +using MongoDB.Entities; + +using PrivaPub.Models.Media; + +using PostEntity = PrivaPub.Models.Post.Post; + +namespace PrivaPub.Infrastructure.Data.Migrations +{ + // A focal point of NaN or Infinity (taken as given before FocalPoint) made every status, timeline and Note holding its + // post fail to serialise: it goes, from the upload and from the copy each post keeps. + public class _016_focal_points_are_finite : IMigration + { + public async Task UpgradeAsync() + { + var unsound = new BsonDocument("$in", new BsonArray { double.NaN, double.PositiveInfinity, double.NegativeInfinity }); + await DB.Default.Collection().UpdateManyAsync(new BsonDocument("Focus", unsound), + new BsonDocument("$set", new BsonDocument("Focus", BsonNull.Value))); + await DB.Default.Collection().UpdateManyAsync(new BsonDocument("Media.Focus", unsound), + new BsonDocument("$set", new BsonDocument("Media.$[m].Focus", BsonNull.Value)), + new UpdateOptions { ArrayFilters = [new BsonDocumentArrayFilterDefinition(new BsonDocument("m.Focus", unsound))] }); + } + } +}