feat(293): paginate GET /api/v1/search/all-items to cap DoS exposure
Build ErsatzTV Image / CI image pin matches docker/ci (pull_request) Successful in 5s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 9s
Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 7s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 7m10s
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Successful in 5m12s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 14m53s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 12m39s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 14m31s
Build ErsatzTV Image / CI image pin matches docker/ci (pull_request) Successful in 5s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 9s
Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 7s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 7m10s
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Successful in 5m12s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 14m53s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 12m39s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 14m31s
The all-items endpoint fired ten index searches with limit:0 (every hit), so a broad authenticated query materialized the whole index into one response. Add optional pageNum/pageSize (clamped 1..1000; pageNum 0..2_000_000 so skip can't overflow int) and an additive per-kind Totals on the response; the SPA add-all flow now pages to completeness instead of a single unbounded fetch. - SearchController.SearchAllItems: clamp params (Logs §1 precedent), map Totals - QuerySearchIndexAllItemsHandler: skip=pageNum*pageSize, limit=pageSize, read SearchResult.TotalCount per kind - SearchResultAllItemsResponseModel: additive Totals (frozen-v1-safe) - web/src/api/search.ts: getSearchAllItems paging params + getAllSearchItemIds (pages until each kind hits its total; empty-page safety break) - tests: controller clamp/thread/totals, handler skip/limit/totals, SPA paging - docs: decisions.md 2026-07-18 (#293), api-conventions.md §5; regenerated OpenAPI Design: issue option (a) full pagination, operator-confirmed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
namespace ErsatzTV.Application.Search;
|
||||
namespace ErsatzTV.Application.Search;
|
||||
|
||||
public record QuerySearchIndexAllItems(string Query) : IRequest<SearchResultAllItemsViewModel>;
|
||||
public record QuerySearchIndexAllItems(string Query, int PageNum, int PageSize)
|
||||
: IRequest<SearchResultAllItemsViewModel>;
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
using ErsatzTV.Core.Interfaces.Search;
|
||||
using ErsatzTV.Core.Interfaces.Search;
|
||||
using ErsatzTV.Core.Search;
|
||||
using ErsatzTV.Infrastructure.Search;
|
||||
|
||||
namespace ErsatzTV.Application.Search;
|
||||
@@ -8,21 +9,69 @@ public class QuerySearchIndexAllItemsHandler(ISearchIndex searchIndex)
|
||||
{
|
||||
public async Task<SearchResultAllItemsViewModel> Handle(
|
||||
QuerySearchIndexAllItems request,
|
||||
CancellationToken cancellationToken) =>
|
||||
new(
|
||||
await GetIds(LuceneSearchIndex.MovieType, request.Query, cancellationToken),
|
||||
await GetIds(LuceneSearchIndex.ShowType, request.Query, cancellationToken),
|
||||
await GetIds(LuceneSearchIndex.SeasonType, request.Query, cancellationToken),
|
||||
await GetIds(LuceneSearchIndex.EpisodeType, request.Query, cancellationToken),
|
||||
await GetIds(LuceneSearchIndex.ArtistType, request.Query, cancellationToken),
|
||||
await GetIds(LuceneSearchIndex.MusicVideoType, request.Query, cancellationToken),
|
||||
await GetIds(LuceneSearchIndex.OtherVideoType, request.Query, cancellationToken),
|
||||
await GetIds(LuceneSearchIndex.SongType, request.Query, cancellationToken),
|
||||
await GetIds(LuceneSearchIndex.ImageType, request.Query, cancellationToken),
|
||||
await GetIds(LuceneSearchIndex.RemoteStreamType, request.Query, cancellationToken));
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
int skip = request.PageNum * request.PageSize;
|
||||
int limit = request.PageSize;
|
||||
|
||||
private async Task<List<int>> GetIds(string type, string query, CancellationToken cancellationToken) =>
|
||||
(await searchIndex.Search($"type:{type} AND ({query})", string.Empty, 0, 0, cancellationToken)).Items
|
||||
.Map(i => i.Id)
|
||||
.ToList();
|
||||
(List<int> Ids, int Total) movies =
|
||||
await GetIds(LuceneSearchIndex.MovieType, request.Query, skip, limit, cancellationToken);
|
||||
(List<int> Ids, int Total) shows =
|
||||
await GetIds(LuceneSearchIndex.ShowType, request.Query, skip, limit, cancellationToken);
|
||||
(List<int> Ids, int Total) seasons =
|
||||
await GetIds(LuceneSearchIndex.SeasonType, request.Query, skip, limit, cancellationToken);
|
||||
(List<int> Ids, int Total) episodes =
|
||||
await GetIds(LuceneSearchIndex.EpisodeType, request.Query, skip, limit, cancellationToken);
|
||||
(List<int> Ids, int Total) artists =
|
||||
await GetIds(LuceneSearchIndex.ArtistType, request.Query, skip, limit, cancellationToken);
|
||||
(List<int> Ids, int Total) musicVideos =
|
||||
await GetIds(LuceneSearchIndex.MusicVideoType, request.Query, skip, limit, cancellationToken);
|
||||
(List<int> Ids, int Total) otherVideos =
|
||||
await GetIds(LuceneSearchIndex.OtherVideoType, request.Query, skip, limit, cancellationToken);
|
||||
(List<int> Ids, int Total) songs =
|
||||
await GetIds(LuceneSearchIndex.SongType, request.Query, skip, limit, cancellationToken);
|
||||
(List<int> Ids, int Total) images =
|
||||
await GetIds(LuceneSearchIndex.ImageType, request.Query, skip, limit, cancellationToken);
|
||||
(List<int> Ids, int Total) remoteStreams =
|
||||
await GetIds(LuceneSearchIndex.RemoteStreamType, request.Query, skip, limit, cancellationToken);
|
||||
|
||||
return new SearchResultAllItemsViewModel(
|
||||
movies.Ids,
|
||||
shows.Ids,
|
||||
seasons.Ids,
|
||||
episodes.Ids,
|
||||
artists.Ids,
|
||||
musicVideos.Ids,
|
||||
otherVideos.Ids,
|
||||
songs.Ids,
|
||||
images.Ids,
|
||||
remoteStreams.Ids,
|
||||
new SearchResultAllItemsTotals(
|
||||
movies.Total,
|
||||
shows.Total,
|
||||
seasons.Total,
|
||||
episodes.Total,
|
||||
artists.Total,
|
||||
musicVideos.Total,
|
||||
otherVideos.Total,
|
||||
songs.Total,
|
||||
images.Total,
|
||||
remoteStreams.Total));
|
||||
}
|
||||
|
||||
private async Task<(List<int> Ids, int Total)> GetIds(
|
||||
string type,
|
||||
string query,
|
||||
int skip,
|
||||
int limit,
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
SearchResult result = await searchIndex.Search(
|
||||
$"type:{type} AND ({query})",
|
||||
string.Empty,
|
||||
skip,
|
||||
limit,
|
||||
cancellationToken);
|
||||
return (result.Items.Map(i => i.Id).ToList(), result.TotalCount);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
namespace ErsatzTV.Application.Search;
|
||||
namespace ErsatzTV.Application.Search;
|
||||
|
||||
public record SearchResultAllItemsViewModel(
|
||||
List<int> MovieIds,
|
||||
@@ -10,4 +10,17 @@ public record SearchResultAllItemsViewModel(
|
||||
List<int> OtherVideoIds,
|
||||
List<int> SongIds,
|
||||
List<int> ImageIds,
|
||||
List<int> RemoteStreamIds);
|
||||
List<int> RemoteStreamIds,
|
||||
SearchResultAllItemsTotals Totals);
|
||||
|
||||
public record SearchResultAllItemsTotals(
|
||||
int MovieCount,
|
||||
int ShowCount,
|
||||
int SeasonCount,
|
||||
int EpisodeCount,
|
||||
int ArtistCount,
|
||||
int MusicVideoCount,
|
||||
int OtherVideoCount,
|
||||
int SongCount,
|
||||
int ImageCount,
|
||||
int RemoteStreamCount);
|
||||
|
||||
@@ -11,4 +11,5 @@ public record SearchResultAllItemsResponseModel(
|
||||
List<int> OtherVideoIds,
|
||||
List<int> SongIds,
|
||||
List<int> ImageIds,
|
||||
List<int> RemoteStreamIds);
|
||||
List<int> RemoteStreamIds,
|
||||
SearchResultAllItemsTotalsResponseModel Totals);
|
||||
|
||||
@@ -0,0 +1,17 @@
|
||||
#nullable enable
|
||||
namespace ErsatzTV.Core.Api.Search;
|
||||
|
||||
// Per-kind total hit counts for a search all-items query, independent of the current page. Lets a
|
||||
// client page GET /api/v1/search/all-items to completeness (the SPA "add all" flow) instead of
|
||||
// materializing every id in one unbounded response (issue #293).
|
||||
public record SearchResultAllItemsTotalsResponseModel(
|
||||
int MovieCount,
|
||||
int ShowCount,
|
||||
int SeasonCount,
|
||||
int EpisodeCount,
|
||||
int ArtistCount,
|
||||
int MusicVideoCount,
|
||||
int OtherVideoCount,
|
||||
int SongCount,
|
||||
int ImageCount,
|
||||
int RemoteStreamCount);
|
||||
@@ -0,0 +1,78 @@
|
||||
using ErsatzTV.Application.Search;
|
||||
using ErsatzTV.Core.Interfaces.Search;
|
||||
using ErsatzTV.Core.Search;
|
||||
using NSubstitute;
|
||||
using NUnit.Framework;
|
||||
using Shouldly;
|
||||
|
||||
namespace ErsatzTV.Tests.Application.Search;
|
||||
|
||||
[TestFixture]
|
||||
public class QuerySearchIndexAllItemsHandlerTests
|
||||
{
|
||||
private ISearchIndex _searchIndex = null!;
|
||||
private QuerySearchIndexAllItemsHandler _handler = null!;
|
||||
|
||||
[SetUp]
|
||||
public void SetUp()
|
||||
{
|
||||
_searchIndex = Substitute.For<ISearchIndex>();
|
||||
|
||||
// Default every kind's search to an empty page so unconfigured kinds don't NRE.
|
||||
_searchIndex.Search(
|
||||
Arg.Any<string>(),
|
||||
Arg.Any<string>(),
|
||||
Arg.Any<int>(),
|
||||
Arg.Any<int>(),
|
||||
Arg.Any<CancellationToken>())
|
||||
.Returns(new SearchResult([], 0));
|
||||
|
||||
_handler = new QuerySearchIndexAllItemsHandler(_searchIndex);
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task Handle_Should_Translate_Page_To_Skip_And_Limit()
|
||||
{
|
||||
await _handler.Handle(new QuerySearchIndexAllItems("star", 2, 50), CancellationToken.None);
|
||||
|
||||
// Every one of the ten media kinds is queried with skip = pageNum * pageSize, limit = pageSize.
|
||||
await _searchIndex.Received(10).Search(
|
||||
Arg.Any<string>(),
|
||||
string.Empty,
|
||||
100,
|
||||
50,
|
||||
Arg.Any<CancellationToken>());
|
||||
await _searchIndex.Received(1).Search(
|
||||
Arg.Is<string>(q => q.Contains("type:movie")),
|
||||
string.Empty,
|
||||
100,
|
||||
50,
|
||||
Arg.Any<CancellationToken>());
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task Handle_Should_Return_Ids_And_Per_Kind_Totals()
|
||||
{
|
||||
_searchIndex.Search(
|
||||
Arg.Is<string>(q => q.Contains("type:movie")),
|
||||
Arg.Any<string>(),
|
||||
Arg.Any<int>(),
|
||||
Arg.Any<int>(),
|
||||
Arg.Any<CancellationToken>())
|
||||
.Returns(new SearchResult(
|
||||
[new SearchItem("movie", 1), new SearchItem("movie", 2)],
|
||||
7));
|
||||
|
||||
SearchResultAllItemsViewModel result =
|
||||
await _handler.Handle(new QuerySearchIndexAllItems("star", 0, 500), CancellationToken.None);
|
||||
|
||||
// The bucket holds this page's ids; the total reflects the full hit count (SearchResult.TotalCount),
|
||||
// which is what lets a client page to completeness.
|
||||
result.MovieIds.ShouldBe(new List<int> { 1, 2 });
|
||||
result.Totals.MovieCount.ShouldBe(7);
|
||||
|
||||
// A kind with no hits returns an empty bucket and a zero total.
|
||||
result.ShowIds.ShouldBeEmpty();
|
||||
result.Totals.ShowCount.ShouldBe(0);
|
||||
}
|
||||
}
|
||||
@@ -107,7 +107,7 @@ public class SearchControllerTests
|
||||
[Test]
|
||||
public async Task SearchAllItems_Should_Return_422_For_Empty_Query()
|
||||
{
|
||||
IActionResult result = await _controller.SearchAllItems(" ", CancellationToken.None);
|
||||
IActionResult result = await _controller.SearchAllItems(" ", 0, 500, CancellationToken.None);
|
||||
|
||||
var unprocessable = result.ShouldBeOfType<UnprocessableEntityObjectResult>();
|
||||
unprocessable.StatusCode.ShouldBe(422);
|
||||
@@ -115,7 +115,32 @@ public class SearchControllerTests
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task SearchAllItems_Should_Map_Id_Lists()
|
||||
public async Task SearchAllItems_Should_Clamp_Paging_And_Thread_PageNum()
|
||||
{
|
||||
_mediator.Send(Arg.Any<QuerySearchIndexAllItems>(), Arg.Any<CancellationToken>())
|
||||
.Returns(EmptyAllItems());
|
||||
|
||||
// A valid pageNum threads through; a pageSize above the max clamps to 1000 (issue #293).
|
||||
await _controller.SearchAllItems("star", 2, 50_000, CancellationToken.None);
|
||||
await _mediator.Received(1).Send(
|
||||
Arg.Is<QuerySearchIndexAllItems>(q => q.Query == "star" && q.PageNum == 2 && q.PageSize == 1000),
|
||||
Arg.Any<CancellationToken>());
|
||||
|
||||
// A negative pageNum clamps to 0; a pageSize below 1 clamps to 1.
|
||||
await _controller.SearchAllItems("star", -3, 0, CancellationToken.None);
|
||||
await _mediator.Received(1).Send(
|
||||
Arg.Is<QuerySearchIndexAllItems>(q => q.Query == "star" && q.PageNum == 0 && q.PageSize == 1),
|
||||
Arg.Any<CancellationToken>());
|
||||
|
||||
// An absurd pageNum clamps to the max so pageNum * pageSize can't overflow int (issue #293 hardening).
|
||||
await _controller.SearchAllItems("star", 9_999_999, 1000, CancellationToken.None);
|
||||
await _mediator.Received(1).Send(
|
||||
Arg.Is<QuerySearchIndexAllItems>(q => q.Query == "star" && q.PageNum == 2_000_000 && q.PageSize == 1000),
|
||||
Arg.Any<CancellationToken>());
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task SearchAllItems_Should_Map_Id_Lists_And_Totals()
|
||||
{
|
||||
_mediator.Send(Arg.Any<QuerySearchIndexAllItems>(), Arg.Any<CancellationToken>())
|
||||
.Returns(new SearchResultAllItemsViewModel(
|
||||
@@ -128,14 +153,17 @@ public class SearchControllerTests
|
||||
[],
|
||||
[],
|
||||
[],
|
||||
[]));
|
||||
[],
|
||||
new SearchResultAllItemsTotals(5, 1, 0, 0, 0, 0, 0, 0, 0, 0)));
|
||||
|
||||
IActionResult result = await _controller.SearchAllItems("star", CancellationToken.None);
|
||||
IActionResult result = await _controller.SearchAllItems("star", 0, 500, CancellationToken.None);
|
||||
|
||||
var body = result.ShouldBeOfType<OkObjectResult>().Value
|
||||
.ShouldBeOfType<SearchResultAllItemsResponseModel>();
|
||||
body.MovieIds.ShouldBe(new List<int> { 1, 2 });
|
||||
body.ShowIds.ShouldBe(new List<int> { 3 });
|
||||
body.Totals.MovieCount.ShouldBe(5);
|
||||
body.Totals.ShowCount.ShouldBe(1);
|
||||
await _mediator.Received(1).Send(
|
||||
Arg.Is<QuerySearchIndexAllItems>(q => q.Query == "star"),
|
||||
Arg.Any<CancellationToken>());
|
||||
@@ -193,4 +221,18 @@ public class SearchControllerTests
|
||||
var empty = new SearchResultGroupResponseModel(0, []);
|
||||
return new SearchResultsResponseModel(empty, empty, empty, empty, empty, empty, empty, empty, empty, empty);
|
||||
}
|
||||
|
||||
private static SearchResultAllItemsViewModel EmptyAllItems() =>
|
||||
new(
|
||||
[],
|
||||
[],
|
||||
[],
|
||||
[],
|
||||
[],
|
||||
[],
|
||||
[],
|
||||
[],
|
||||
[],
|
||||
[],
|
||||
new SearchResultAllItemsTotals(0, 0, 0, 0, 0, 0, 0, 0, 0, 0));
|
||||
}
|
||||
|
||||
@@ -17,6 +17,16 @@ public class SearchController(IMediator mediator) : ControllerBase
|
||||
{
|
||||
private const int MaxPageSize = 100;
|
||||
|
||||
// all-items returns bare ids (cheap), so it tolerates a larger page than the item-returning
|
||||
// /search endpoint; the clamp still bounds one response to <= 10 kinds * MaxAllItemsPageSize ids
|
||||
// so a broad query can't materialize the whole index in one request (issue #293).
|
||||
private const int DefaultAllItemsPageSize = 500;
|
||||
private const int MaxAllItemsPageSize = 1000;
|
||||
|
||||
// Upper-bound the page number so pageNum * pageSize (the search skip) can't overflow int and 500 —
|
||||
// no legitimate client pages past this, and it keeps skip + limit inside int range at MaxAllItemsPageSize.
|
||||
private const int MaxAllItemsPageNum = 2_000_000;
|
||||
|
||||
[HttpGet("/api/v1/search", Name = "Search")]
|
||||
[Tags("Search")]
|
||||
[EndpointSummary("Search library items across all media kinds")]
|
||||
@@ -46,13 +56,17 @@ public class SearchController(IMediator mediator) : ControllerBase
|
||||
[Tags("Search")]
|
||||
[EndpointSummary("Search library items across all media kinds and return raw id lists")]
|
||||
[EndpointDescription(
|
||||
"Returns every matching item's id, grouped by media kind, with no paging. Used by the SPA's " +
|
||||
"\"add all to collection/playlist\" flow to materialize ids before calling the add endpoints.")]
|
||||
"Returns matching item ids, grouped by media kind, one clamped page per kind plus per-kind " +
|
||||
"total counts. Used by the SPA's \"add all to collection/playlist\" flow, which pages to " +
|
||||
"completeness. Paging bounds a broad query so it can't materialize the whole index in one " +
|
||||
"request (issue #293).")]
|
||||
[EndpointGroupName("general")]
|
||||
[ProducesResponseType(typeof(SearchResultAllItemsResponseModel), StatusCodes.Status200OK)]
|
||||
[ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)]
|
||||
public async Task<IActionResult> SearchAllItems(
|
||||
[FromQuery] string query = "",
|
||||
[FromQuery] int pageNum = 0,
|
||||
[FromQuery] int pageSize = DefaultAllItemsPageSize,
|
||||
CancellationToken cancellationToken = default)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(query))
|
||||
@@ -60,8 +74,10 @@ public class SearchController(IMediator mediator) : ControllerBase
|
||||
return BaseError.New("A non-empty query is required").ToErrorResult();
|
||||
}
|
||||
|
||||
int clampedPageNum = Math.Clamp(pageNum, 0, MaxAllItemsPageNum);
|
||||
int clampedPageSize = Math.Clamp(pageSize, 1, MaxAllItemsPageSize);
|
||||
SearchResultAllItemsViewModel result = await mediator.Send(
|
||||
new QuerySearchIndexAllItems(query),
|
||||
new QuerySearchIndexAllItems(query, clampedPageNum, clampedPageSize),
|
||||
cancellationToken);
|
||||
return new OkObjectResult(Project(result));
|
||||
}
|
||||
@@ -184,5 +200,16 @@ public class SearchController(IMediator mediator) : ControllerBase
|
||||
vm.OtherVideoIds,
|
||||
vm.SongIds,
|
||||
vm.ImageIds,
|
||||
vm.RemoteStreamIds);
|
||||
vm.RemoteStreamIds,
|
||||
new SearchResultAllItemsTotalsResponseModel(
|
||||
vm.Totals.MovieCount,
|
||||
vm.Totals.ShowCount,
|
||||
vm.Totals.SeasonCount,
|
||||
vm.Totals.EpisodeCount,
|
||||
vm.Totals.ArtistCount,
|
||||
vm.Totals.MusicVideoCount,
|
||||
vm.Totals.OtherVideoCount,
|
||||
vm.Totals.SongCount,
|
||||
vm.Totals.ImageCount,
|
||||
vm.Totals.RemoteStreamCount));
|
||||
}
|
||||
|
||||
@@ -17419,7 +17419,7 @@
|
||||
"Search"
|
||||
],
|
||||
"summary": "Search library items across all media kinds and return raw id lists",
|
||||
"description": "Returns every matching item's id, grouped by media kind, with no paging. Used by the SPA's \"add all to collection/playlist\" flow to materialize ids before calling the add endpoints.",
|
||||
"description": "Returns matching item ids, grouped by media kind, one clamped page per kind plus per-kind total counts. Used by the SPA's \"add all to collection/playlist\" flow, which pages to completeness. Paging bounds a broad query so it can't materialize the whole index in one request (issue #293).",
|
||||
"operationId": "SearchAllItems",
|
||||
"parameters": [
|
||||
{
|
||||
@@ -17429,6 +17429,24 @@
|
||||
"type": "string",
|
||||
"default": ""
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "pageNum",
|
||||
"in": "query",
|
||||
"schema": {
|
||||
"type": "integer",
|
||||
"format": "int32",
|
||||
"default": 0
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "pageSize",
|
||||
"in": "query",
|
||||
"schema": {
|
||||
"type": "integer",
|
||||
"format": "int32",
|
||||
"default": 500
|
||||
}
|
||||
}
|
||||
],
|
||||
"responses": {
|
||||
@@ -30463,7 +30481,8 @@
|
||||
"otherVideoIds",
|
||||
"songIds",
|
||||
"imageIds",
|
||||
"remoteStreamIds"
|
||||
"remoteStreamIds",
|
||||
"totals"
|
||||
],
|
||||
"type": "object",
|
||||
"properties": {
|
||||
@@ -30536,6 +30555,66 @@
|
||||
"type": "integer",
|
||||
"format": "int32"
|
||||
}
|
||||
},
|
||||
"totals": {
|
||||
"$ref": "#/components/schemas/SearchResultAllItemsTotalsResponseModel"
|
||||
}
|
||||
}
|
||||
},
|
||||
"SearchResultAllItemsTotalsResponseModel": {
|
||||
"required": [
|
||||
"movieCount",
|
||||
"showCount",
|
||||
"seasonCount",
|
||||
"episodeCount",
|
||||
"artistCount",
|
||||
"musicVideoCount",
|
||||
"otherVideoCount",
|
||||
"songCount",
|
||||
"imageCount",
|
||||
"remoteStreamCount"
|
||||
],
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"movieCount": {
|
||||
"type": "integer",
|
||||
"format": "int32"
|
||||
},
|
||||
"showCount": {
|
||||
"type": "integer",
|
||||
"format": "int32"
|
||||
},
|
||||
"seasonCount": {
|
||||
"type": "integer",
|
||||
"format": "int32"
|
||||
},
|
||||
"episodeCount": {
|
||||
"type": "integer",
|
||||
"format": "int32"
|
||||
},
|
||||
"artistCount": {
|
||||
"type": "integer",
|
||||
"format": "int32"
|
||||
},
|
||||
"musicVideoCount": {
|
||||
"type": "integer",
|
||||
"format": "int32"
|
||||
},
|
||||
"otherVideoCount": {
|
||||
"type": "integer",
|
||||
"format": "int32"
|
||||
},
|
||||
"songCount": {
|
||||
"type": "integer",
|
||||
"format": "int32"
|
||||
},
|
||||
"imageCount": {
|
||||
"type": "integer",
|
||||
"format": "int32"
|
||||
},
|
||||
"remoteStreamCount": {
|
||||
"type": "integer",
|
||||
"format": "int32"
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
@@ -366,6 +366,17 @@ Returns the curated `SearchFieldCatalog` (name, friendly label, type, UI group,
|
||||
enum fields) as `List<SearchFieldResponseModel>`. Drives the SmartCollection rule builder and is
|
||||
introspectable by MCP; no query parameters.
|
||||
|
||||
**Param + DTO expansion (#293, cap `search/all-items`)**: no new endpoint — `GET /api/v1/search/all-items`
|
||||
gained two **optional** query params (`pageNum` 0-based, `pageSize` default 500, clamped 1–1000 via the §1
|
||||
Logs `Math.Clamp` precedent) so a broad query can't materialize the whole index in one response, and one
|
||||
**additive** response field, `Totals` (`SearchResultAllItemsTotalsResponseModel`, ten per-kind `…Count`
|
||||
ints), so a client can page to completeness. The clamp is per media kind, so one response is bounded to
|
||||
≤ 10 × `pageSize` ids. The SPA add-all flow (`getAllSearchItemIds` in `web/src/api/search.ts`) pages until
|
||||
each kind has collected its `Totals` count. Changing the no-param default from "everything" to one page is an
|
||||
intentional, security-motivated behavior change (only in-repo consumer is the SPA, updated in the same PR;
|
||||
external callers read `Totals` and page). Regenerated the OpenAPI trio. See `docs/decisions.md` 2026-07-18
|
||||
(#293).
|
||||
|
||||
**Resolved wart (#287)**: `DayOfWeek` previously serialized as an integer in the OpenAPI schema while
|
||||
the runtime JSON payload is the enum's **name string** ("Sunday".."Saturday"). It is now added to
|
||||
`Startup.UseStringEnumSchemas`'s hand-list, so the "v1" schema emits it as a **string enum** matching
|
||||
|
||||
@@ -95,6 +95,7 @@ in-file entries.
|
||||
- [2026-07-18 — Auto-Tune DetailPanel SPA: reusable `SlideOver` + shared advanced-options model; decorative panes dropped to match the backend (#386)](#2026-07-18--auto-tune-detailpanel-spa-reusable-slideover--shared-advanced-options-model-decorative-panes-dropped-to-match-the-backend-386)
|
||||
- [2026-07-18 — SmartCollection rule builder: compile-only closed subset, no stored AST, one-level nesting (#176)](#2026-07-18--smartcollection-rule-builder-compile-only-closed-subset-no-stored-ast-one-level-nesting-176)
|
||||
- [2026-07-18 — Auto-Tune per-source weights ride #70's MultiCollection machinery; created at tune time, not a post-hoc PUT (#425)](#2026-07-18--auto-tune-per-source-weights-ride-70s-multicollection-machinery-created-at-tune-time-not-a-post-hoc-put-425)
|
||||
- [2026-07-18 — Search all-items is paged to cap DoS exposure; SPA add-all pages to completeness (#293)](#2026-07-18--search-all-items-is-paged-to-cap-dos-exposure-spa-add-all-pages-to-completeness-293)
|
||||
|
||||
---
|
||||
|
||||
@@ -1764,3 +1765,42 @@ isn't known until `CreateChannelFromLineup` runs; ownership is stamped immediate
|
||||
channels set no ownership, so their pre-existing orphan-on-delete behavior is unchanged. The create is
|
||||
non-atomic across the two handlers (mirrors #69) with best-effort rollback of the artifacts on channel-create
|
||||
failure.
|
||||
|
||||
## 2026-07-18 — Search all-items is paged to cap DoS exposure; SPA add-all pages to completeness (#293)
|
||||
|
||||
`GET /api/v1/search/all-items` (`SearchController.SearchAllItems` → `QuerySearchIndexAllItemsHandler`) fired
|
||||
ten index searches with **`limit: 0`** (= "return every hit", `LuceneSearchIndex` line ~244), so a single
|
||||
broad query (e.g. one matching the whole library) materialized *every* matching doc across all ten media
|
||||
kinds into ten `List<int>` buckets and serialized them in one response — unbounded work per request. #285
|
||||
closed the original *unauthenticated* exposure (the endpoint is now behind `Api:RequireKeyForReads`, default
|
||||
true); the residual was DoS-hardening against an **authenticated** caller with a very broad query. Deferred
|
||||
from #285 because the SPA "add all to collection/playlist" flow materializes the full id set before the add
|
||||
POST, so a naive hard cap would silently truncate "add all".
|
||||
|
||||
**Decision (issue option (a), operator-confirmed): paginate the endpoint and teach the SPA add-all flow to
|
||||
page to completeness** — rather than option (b) (a generous cap + truncation signal). Chosen because
|
||||
"add all" must stay complete for real use, and it matches the sibling `GET /api/v1/search` /
|
||||
`GET /api/v1/channels/auto-tune/members` (#384) paging convention already in the codebase.
|
||||
|
||||
- **Endpoint (additive).** `SearchAllItems` gains optional `pageNum` (0-based) + `pageSize`, clamped exactly
|
||||
like the §1 Logs / sibling `Search` precedent: `pageNum = Math.Max(0, pageNum)`,
|
||||
`pageSize = Math.Clamp(pageSize, 1, MaxAllItemsPageSize)` with `MaxAllItemsPageSize = 1000`,
|
||||
`DefaultAllItemsPageSize = 500`. The clamp is applied per media kind (a page returns ≤ `pageSize` ids of
|
||||
*each* of the ten kinds), so one response is bounded to ≤ 10 × `pageSize` ids. `QuerySearchIndexAllItems`
|
||||
carries `PageNum`/`PageSize`; the handler passes `skip = PageNum × PageSize`, `limit = PageSize` into
|
||||
`ISearchIndex.Search` (native skip/limit) and reads `SearchResult.TotalCount` (the true total, free) per
|
||||
kind.
|
||||
- **Response (additive, frozen-v1-safe).** The ten `…Ids` buckets are unchanged; a new non-null nested
|
||||
`Totals` (`SearchResultAllItemsTotalsResponseModel`, ten `…Count` ints) is added so a client knows how many
|
||||
ids exist per kind and can page to completeness. Nothing is removed or retyped (#286 additive-only holds).
|
||||
- **Deliberate default-behavior change.** A caller that sends no `pageSize` now gets one page (default 500 /
|
||||
kind) plus `Totals`, not the entire id set. This is the security change the issue asks for; it is safe here
|
||||
because the only in-repo consumer is the SPA (updated in the same PR) and any external/MCP caller can read
|
||||
`Totals` and page. Recorded as intentional, not a regression.
|
||||
- **SPA pages to completeness.** `web/src/api/search.ts` `getSearchAllItems(query, pageNum, pageSize)` gains
|
||||
the params; a new `getAllSearchItemIds(query)` loops pages (requesting `pageSize = 1000`, the server max),
|
||||
accumulating every bucket until each kind has collected its `Totals` count (with an empty-page safety break
|
||||
against total-count drift), and returns the merged `SearchAllItemIds`. `SearchScreen.addAll` calls it
|
||||
instead of the single-shot fetch; the #221 stale-query guard and the single add POST are unchanged.
|
||||
- **Out of scope (unchanged):** the add POST itself still accepts the full merged id set in one request body
|
||||
— bounding *that* surface is a separate concern (see #308 for the add path); #293 is the GET.
|
||||
|
||||
Vendored
+13
@@ -1454,6 +1454,19 @@ export interface components {
|
||||
"songIds": Array<number>;
|
||||
"imageIds": Array<number>;
|
||||
"remoteStreamIds": Array<number>;
|
||||
"totals": components["schemas"]["SearchResultAllItemsTotalsResponseModel"];
|
||||
};
|
||||
"SearchResultAllItemsTotalsResponseModel": {
|
||||
"movieCount": number;
|
||||
"showCount": number;
|
||||
"seasonCount": number;
|
||||
"episodeCount": number;
|
||||
"artistCount": number;
|
||||
"musicVideoCount": number;
|
||||
"otherVideoCount": number;
|
||||
"songCount": number;
|
||||
"imageCount": number;
|
||||
"remoteStreamCount": number;
|
||||
};
|
||||
"SearchResultGroupResponseModel": {
|
||||
"totalCount": number;
|
||||
|
||||
+113
-1
@@ -1,5 +1,41 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { getSearchAllItems, getSearchFields, getSearchResults, toAddItemsRequestFromSearch } from './search';
|
||||
import {
|
||||
getAllSearchItemIds,
|
||||
getSearchAllItems,
|
||||
getSearchFields,
|
||||
getSearchResults,
|
||||
toAddItemsRequestFromSearch
|
||||
} from './search';
|
||||
|
||||
// Builds an all-items page response body. `totals` fills unspecified counts with 0.
|
||||
function allItemsPage(
|
||||
buckets: Record<string, number[]>,
|
||||
totals: Record<string, number> = {}
|
||||
): Record<string, unknown> {
|
||||
return {
|
||||
...buckets,
|
||||
totals: {
|
||||
movieCount: 0,
|
||||
showCount: 0,
|
||||
seasonCount: 0,
|
||||
episodeCount: 0,
|
||||
artistCount: 0,
|
||||
musicVideoCount: 0,
|
||||
otherVideoCount: 0,
|
||||
songCount: 0,
|
||||
imageCount: 0,
|
||||
remoteStreamCount: 0,
|
||||
...totals
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
function jsonResponse(body: unknown): Response {
|
||||
return new Response(JSON.stringify(body), {
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
status: 200
|
||||
});
|
||||
}
|
||||
|
||||
const emptyGroup = { totalCount: 0, items: [] };
|
||||
const sampleResults = {
|
||||
@@ -111,6 +147,82 @@ describe('getSearchFields', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('getSearchAllItems paging params', () => {
|
||||
beforeEach(() => {
|
||||
window.localStorage.clear();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('includes pageNum and pageSize when provided', async () => {
|
||||
const fetchSpy = vi.spyOn(window, 'fetch').mockResolvedValue(jsonResponse(allItemsPage({})));
|
||||
|
||||
await getSearchAllItems('star', 2, 1000);
|
||||
|
||||
const [url] = fetchSpy.mock.calls[0];
|
||||
expect(url).toBe('/api/v1/search/all-items?query=star&pageNum=2&pageSize=1000');
|
||||
});
|
||||
});
|
||||
|
||||
describe('getAllSearchItemIds', () => {
|
||||
beforeEach(() => {
|
||||
window.localStorage.clear();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('returns a single page when every bucket is already complete', async () => {
|
||||
const fetchSpy = vi
|
||||
.spyOn(window, 'fetch')
|
||||
.mockResolvedValue(jsonResponse(allItemsPage({ movieIds: [1, 2], showIds: [9] }, { movieCount: 2, showCount: 1 })));
|
||||
|
||||
const result = await getAllSearchItemIds('star');
|
||||
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(1);
|
||||
expect(result.movieIds).toEqual([1, 2]);
|
||||
expect(result.showIds).toEqual([9]);
|
||||
expect(result.seasonIds).toEqual([]);
|
||||
});
|
||||
|
||||
it('pages to completeness and merges every bucket', async () => {
|
||||
const fetchSpy = vi
|
||||
.spyOn(window, 'fetch')
|
||||
.mockResolvedValueOnce(jsonResponse(allItemsPage({ movieIds: [1, 2] }, { movieCount: 3 })))
|
||||
.mockResolvedValueOnce(jsonResponse(allItemsPage({ movieIds: [3] }, { movieCount: 3 })));
|
||||
|
||||
const result = await getAllSearchItemIds('star');
|
||||
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(2);
|
||||
expect(result.movieIds).toEqual([1, 2, 3]);
|
||||
// The second request advances the page.
|
||||
expect(fetchSpy.mock.calls[1][0]).toBe('/api/v1/search/all-items?query=star&pageNum=1&pageSize=1000');
|
||||
});
|
||||
|
||||
it('pages until an empty page when the server omits totals', async () => {
|
||||
// Without `totals` the fast completeness check can't fire, so it must keep paging until empty
|
||||
// rather than silently truncating to the first page.
|
||||
const fetchSpy = vi
|
||||
.spyOn(window, 'fetch')
|
||||
.mockResolvedValueOnce(jsonResponse({ movieIds: [1, 2] }))
|
||||
.mockResolvedValueOnce(jsonResponse({ movieIds: [3] }))
|
||||
.mockResolvedValueOnce(jsonResponse({ movieIds: [] }));
|
||||
|
||||
const result = await getAllSearchItemIds('star');
|
||||
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(3);
|
||||
expect(result.movieIds).toEqual([1, 2, 3]);
|
||||
});
|
||||
|
||||
it('stops on an empty page even if a total claims more (drift safety)', async () => {
|
||||
const fetchSpy = vi
|
||||
.spyOn(window, 'fetch')
|
||||
.mockResolvedValue(jsonResponse(allItemsPage({}, { movieCount: 5 })));
|
||||
|
||||
const result = await getAllSearchItemIds('star');
|
||||
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(1);
|
||||
expect(result.movieIds).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('toAddItemsRequestFromSearch', () => {
|
||||
it('fills every bucket, defaulting null arrays to []', () => {
|
||||
expect(
|
||||
|
||||
+105
-4
@@ -19,6 +19,37 @@ export function getSearchFields(): Promise<SearchField[]> {
|
||||
// match AddItemsToCollectionRequest exactly, so a result pipes straight into addItemsToCollection /
|
||||
// addItemsToPlaylist via toAddItemsRequestFromSearch below.
|
||||
export type SearchAllItemIds = components['schemas']['SearchResultAllItemsResponseModel'];
|
||||
type SearchAllItemTotals = SearchAllItemIds['totals'];
|
||||
|
||||
// The ten id-bucket keys, paired with their per-kind total-count key in `totals`. Used to page the
|
||||
// all-items endpoint to completeness (issue #293) without hard-coding the loop ten times.
|
||||
const ALL_ITEM_KINDS = [
|
||||
'movieIds',
|
||||
'showIds',
|
||||
'seasonIds',
|
||||
'episodeIds',
|
||||
'artistIds',
|
||||
'musicVideoIds',
|
||||
'otherVideoIds',
|
||||
'songIds',
|
||||
'imageIds',
|
||||
'remoteStreamIds'
|
||||
] as const;
|
||||
|
||||
type AllItemKind = (typeof ALL_ITEM_KINDS)[number];
|
||||
|
||||
const TOTAL_KEY: Record<AllItemKind, keyof SearchAllItemTotals> = {
|
||||
movieIds: 'movieCount',
|
||||
showIds: 'showCount',
|
||||
seasonIds: 'seasonCount',
|
||||
episodeIds: 'episodeCount',
|
||||
artistIds: 'artistCount',
|
||||
musicVideoIds: 'musicVideoCount',
|
||||
otherVideoIds: 'otherVideoCount',
|
||||
songIds: 'songCount',
|
||||
imageIds: 'imageCount',
|
||||
remoteStreamIds: 'remoteStreamCount'
|
||||
};
|
||||
|
||||
export interface GetSearchResultsParams {
|
||||
query: string;
|
||||
@@ -36,17 +67,87 @@ export function getSearchResults(params: GetSearchResultsParams): Promise<Search
|
||||
return request<SearchResults>(`/api/v1/search?${searchParams.toString()}`);
|
||||
}
|
||||
|
||||
// Resolves a search query to the full set of matching media-item ids, bucketed by kind. Backs the
|
||||
// "Add all results" flow so the caller never has to page through every result to add them.
|
||||
export function getSearchAllItems(query: string): Promise<SearchAllItemIds> {
|
||||
// Fetches one clamped page of matching media-item ids, bucketed by kind, plus per-kind totals. The
|
||||
// endpoint pages so a broad query can't materialize the whole index in one response (issue #293);
|
||||
// callers that need every id use getAllSearchItemIds below.
|
||||
export function getSearchAllItems(query: string, pageNum?: number, pageSize?: number): Promise<SearchAllItemIds> {
|
||||
const searchParams = new URLSearchParams();
|
||||
searchParams.set('query', query);
|
||||
if (pageNum != null) {
|
||||
searchParams.set('pageNum', String(pageNum));
|
||||
}
|
||||
if (pageSize != null) {
|
||||
searchParams.set('pageSize', String(pageSize));
|
||||
}
|
||||
return request<SearchAllItemIds>(`/api/v1/search/all-items?${searchParams.toString()}`);
|
||||
}
|
||||
|
||||
// Resolves a search query to the FULL set of matching media-item ids by paging the all-items endpoint
|
||||
// to completeness. Backs the "Add all results" flow. Requests the server's max page size and stops once
|
||||
// every kind has collected its reported total (or a page comes back empty — a safety break against the
|
||||
// total drifting if the index is written mid-page).
|
||||
export async function getAllSearchItemIds(query: string): Promise<SearchAllItemIds> {
|
||||
const pageSize = 1000; // the server clamps to its own max; ask for the largest page
|
||||
const merged: Record<AllItemKind, number[]> = {
|
||||
movieIds: [],
|
||||
showIds: [],
|
||||
seasonIds: [],
|
||||
episodeIds: [],
|
||||
artistIds: [],
|
||||
musicVideoIds: [],
|
||||
otherVideoIds: [],
|
||||
songIds: [],
|
||||
imageIds: [],
|
||||
remoteStreamIds: []
|
||||
};
|
||||
|
||||
let totals: SearchAllItemTotals | undefined;
|
||||
let pageNum = 0;
|
||||
|
||||
for (;;) {
|
||||
const page = await getSearchAllItems(query, pageNum, pageSize);
|
||||
totals = page.totals;
|
||||
|
||||
let pageCount = 0;
|
||||
for (const kind of ALL_ITEM_KINDS) {
|
||||
const ids = page[kind] ?? [];
|
||||
pageCount += ids.length;
|
||||
merged[kind].push(...ids);
|
||||
}
|
||||
pageNum += 1;
|
||||
|
||||
// Only trust the fast completeness check when the server sent totals; otherwise keep paging until an
|
||||
// empty page so a missing `totals` can't silently truncate to the first page. (Capture in a const so
|
||||
// the null-narrowing survives into the closure.)
|
||||
const pageTotals = totals;
|
||||
const complete =
|
||||
pageTotals != null &&
|
||||
ALL_ITEM_KINDS.every((kind) => merged[kind].length >= (pageTotals[TOTAL_KEY[kind]] ?? 0));
|
||||
if (complete || pageCount === 0) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
movieIds: merged.movieIds,
|
||||
showIds: merged.showIds,
|
||||
seasonIds: merged.seasonIds,
|
||||
episodeIds: merged.episodeIds,
|
||||
artistIds: merged.artistIds,
|
||||
musicVideoIds: merged.musicVideoIds,
|
||||
otherVideoIds: merged.otherVideoIds,
|
||||
songIds: merged.songIds,
|
||||
imageIds: merged.imageIds,
|
||||
remoteStreamIds: merged.remoteStreamIds,
|
||||
totals: totals as SearchAllItemTotals
|
||||
};
|
||||
}
|
||||
|
||||
// Normalizes a SearchAllItemIds result (nullable arrays) into a full AddItemsToCollectionRequest
|
||||
// so it can be piped straight into addItemsToCollection / addItemsToPlaylist.
|
||||
export function toAddItemsRequestFromSearch(result: SearchAllItemIds): AddItemsToCollectionRequest {
|
||||
export function toAddItemsRequestFromSearch(
|
||||
result: Omit<SearchAllItemIds, 'totals'>
|
||||
): AddItemsToCollectionRequest {
|
||||
return {
|
||||
artistIds: result.artistIds ?? [],
|
||||
episodeIds: result.episodeIds ?? [],
|
||||
|
||||
@@ -37,7 +37,20 @@ const allItems = {
|
||||
otherVideoIds: null,
|
||||
songIds: null,
|
||||
imageIds: null,
|
||||
remoteStreamIds: null
|
||||
remoteStreamIds: null,
|
||||
// Totals equal to this single page's ids, so getAllSearchItemIds treats it as complete after one fetch.
|
||||
totals: {
|
||||
movieCount: 2,
|
||||
showCount: 0,
|
||||
seasonCount: 0,
|
||||
episodeCount: 0,
|
||||
artistCount: 0,
|
||||
musicVideoCount: 0,
|
||||
otherVideoCount: 0,
|
||||
songCount: 0,
|
||||
imageCount: 0,
|
||||
remoteStreamCount: 0
|
||||
}
|
||||
};
|
||||
|
||||
const emptyBuckets = {
|
||||
|
||||
@@ -2,7 +2,7 @@ import { useCallback, useEffect, useRef, useState } from 'react';
|
||||
import { CheckSquare, FolderPlus, ListVideo, Save, Search, TriangleAlert, X } from 'lucide-react';
|
||||
import { Button, Card, Input, Spinner, Toast } from '../components';
|
||||
import {
|
||||
getSearchAllItems,
|
||||
getAllSearchItemIds,
|
||||
getSearchResults,
|
||||
messageFromSearchError,
|
||||
toAddItemsRequestFromSearch,
|
||||
@@ -169,7 +169,7 @@ export function SearchScreen() {
|
||||
}
|
||||
|
||||
setPendingAll(kind);
|
||||
getSearchAllItems(trimmed)
|
||||
getAllSearchItemIds(trimmed)
|
||||
.then((result) => {
|
||||
if (!activeRef.current) {
|
||||
return;
|
||||
|
||||
Reference in New Issue
Block a user