From 8d655e5c733af84f8931d704fbb00cd0ea55ffc7 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 3 Oct 2022 06:38:42 +0000 Subject: To prevent against damages caused by excessive hits/offset param being sent down, cap it against numdocs. --- .../src/tests/proton/matching/matching_test.cpp | 40 ++++++++++++++++------ .../searchcore/proton/matching/match_params.cpp | 14 ++++---- .../src/vespa/searchlib/engine/searchrequest.h | 2 +- 3 files changed, 39 insertions(+), 17 deletions(-) diff --git a/searchcore/src/tests/proton/matching/matching_test.cpp b/searchcore/src/tests/proton/matching/matching_test.cpp index df8f2a9476b..70f6cf73a72 100644 --- a/searchcore/src/tests/proton/matching/matching_test.cpp +++ b/searchcore/src/tests/proton/matching/matching_test.cpp @@ -934,8 +934,8 @@ TEST("require that getSummaryFeatures prefers cached query setup") { } TEST("require that match params are set up straight with ranking on") { - MatchParams p(1, 2, 4, 0.7, 0, 1, true, true); - ASSERT_EQUAL(1u, p.numDocs); + MatchParams p(10, 2, 4, 0.7, 0, 1, true, true); + ASSERT_EQUAL(10u, p.numDocs); ASSERT_EQUAL(2u, p.heapSize); ASSERT_EQUAL(4u, p.arraySize); ASSERT_EQUAL(0.7, p.rankDropLimit); @@ -945,8 +945,8 @@ TEST("require that match params are set up straight with ranking on") { } TEST("require that match params can turn off rank-drop-limit") { - MatchParams p(1, 2, 4, -std::numeric_limits::quiet_NaN(), 0, 1, true, true); - ASSERT_EQUAL(1u, p.numDocs); + MatchParams p(10, 2, 4, -std::numeric_limits::quiet_NaN(), 0, 1, true, true); + ASSERT_EQUAL(10u, p.numDocs); ASSERT_EQUAL(2u, p.heapSize); ASSERT_EQUAL(4u, p.arraySize); ASSERT_TRUE(std::isnan(p.rankDropLimit)); @@ -957,8 +957,8 @@ TEST("require that match params can turn off rank-drop-limit") { TEST("require that match params are set up straight with ranking on arraySize is atleast the size of heapSize") { - MatchParams p(1, 6, 4, 0.7, 1, 1, true, true); - ASSERT_EQUAL(1u, p.numDocs); + MatchParams p(10, 6, 4, 0.7, 1, 1, true, true); + ASSERT_EQUAL(10u, p.numDocs); ASSERT_EQUAL(6u, p.heapSize); ASSERT_EQUAL(6u, p.arraySize); ASSERT_EQUAL(0.7, p.rankDropLimit); @@ -967,8 +967,8 @@ TEST("require that match params are set up straight with ranking on arraySize is } TEST("require that match params are set up straight with ranking on arraySize is atleast the size of hits+offset") { - MatchParams p(1, 6, 4, 0.7, 4, 4, true, true); - ASSERT_EQUAL(1u, p.numDocs); + MatchParams p(10, 6, 4, 0.7, 4, 4, true, true); + ASSERT_EQUAL(10u, p.numDocs); ASSERT_EQUAL(6u, p.heapSize); ASSERT_EQUAL(8u, p.arraySize); ASSERT_EQUAL(0.7, p.rankDropLimit); @@ -976,9 +976,29 @@ TEST("require that match params are set up straight with ranking on arraySize is ASSERT_EQUAL(4u, p.hits); } -TEST("require that match params are set up straight with ranking off array and heap size is 0") { - MatchParams p(1, 6, 4, 0.7, 4, 4, true, false); +TEST("require that match params are capped by numDocs") { + MatchParams p(1, 6, 4, 0.7, 4, 4, true, true); ASSERT_EQUAL(1u, p.numDocs); + ASSERT_EQUAL(1u, p.heapSize); + ASSERT_EQUAL(1u, p.arraySize); + ASSERT_EQUAL(0.7, p.rankDropLimit); + ASSERT_EQUAL(1u, p.offset); + ASSERT_EQUAL(0u, p.hits); +} + +TEST("require that match params are capped by numDocs and hits adjusted down") { + MatchParams p(5, 6, 4, 0.7, 4, 4, true, true); + ASSERT_EQUAL(5u, p.numDocs); + ASSERT_EQUAL(5u, p.heapSize); + ASSERT_EQUAL(5u, p.arraySize); + ASSERT_EQUAL(0.7, p.rankDropLimit); + ASSERT_EQUAL(4u, p.offset); + ASSERT_EQUAL(1u, p.hits); +} + +TEST("require that match params are set up straight with ranking off array and heap size is 0") { + MatchParams p(10, 6, 4, 0.7, 4, 4, true, false); + ASSERT_EQUAL(10u, p.numDocs); ASSERT_EQUAL(0u, p.heapSize); ASSERT_EQUAL(0u, p.arraySize); ASSERT_EQUAL(0.7, p.rankDropLimit); diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_params.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_params.cpp index 7465088d8f0..5dc5071f434 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_params.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_params.cpp @@ -8,7 +8,8 @@ namespace proton::matching { namespace { -uint32_t computeArraySize(uint32_t hitsPlussOffset, uint32_t heapSize, uint32_t arraySize) +uint32_t +computeArraySize(uint32_t hitsPlussOffset, uint32_t heapSize, uint32_t arraySize) { return std::max(hitsPlussOffset, std::max(heapSize, arraySize)); } @@ -24,16 +25,17 @@ MatchParams::MatchParams(uint32_t numDocs_in, bool hasFinalRank, bool needRanking) : numDocs(numDocs_in), - heapSize((hasFinalRank && needRanking) ? heapSize_in : 0), + heapSize((hasFinalRank && needRanking) ? std::min(numDocs_in, heapSize_in) : 0), arraySize((needRanking && ((heapSize_in + arraySize_in) > 0)) - ? computeArraySize(hits_in + offset_in, heapSize, arraySize_in) + ? std::min(numDocs_in, computeArraySize(hits_in + offset_in, heapSize, arraySize_in)) : 0), - offset(offset_in), - hits(hits_in), + offset(std::min(numDocs_in, offset_in)), + hits(numDocs_in > offset_in ? std::min(numDocs_in - offset_in, hits_in) : 0), rankDropLimit(rankDropLimit_in) { } -bool MatchParams::has_rank_drop_limit() const { +bool +MatchParams::has_rank_drop_limit() const { return ! std::isnan(rankDropLimit); } diff --git a/searchlib/src/vespa/searchlib/engine/searchrequest.h b/searchlib/src/vespa/searchlib/engine/searchrequest.h index 7b598735b42..ec1bc7550b5 100644 --- a/searchlib/src/vespa/searchlib/engine/searchrequest.h +++ b/searchlib/src/vespa/searchlib/engine/searchrequest.h @@ -23,7 +23,7 @@ public: SearchRequest(); explicit SearchRequest(RelativeTime relativeTime); - ~SearchRequest(); + ~SearchRequest() override; }; } -- cgit v1.2.3 From e5594f3503a2ffb1fdf04d076205ccb6e194e18e Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 3 Oct 2022 19:14:36 +0000 Subject: Simplify by using already computed maximun offset. --- searchcore/src/vespa/searchcore/proton/matching/match_params.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_params.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_params.cpp index 5dc5071f434..68a6f74fa50 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_params.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_params.cpp @@ -30,7 +30,7 @@ MatchParams::MatchParams(uint32_t numDocs_in, ? std::min(numDocs_in, computeArraySize(hits_in + offset_in, heapSize, arraySize_in)) : 0), offset(std::min(numDocs_in, offset_in)), - hits(numDocs_in > offset_in ? std::min(numDocs_in - offset_in, hits_in) : 0), + hits(std::min(numDocs_in - offset, hits_in)), rankDropLimit(rankDropLimit_in) { } -- cgit v1.2.3