mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-08 23:54:39 +00:00
Merge pull request #3848 from vitorpamplona/fix/count-ignores-default-limit
Stop COUNT from inheriting the default page size
This commit is contained in:
+27
-1
@@ -72,9 +72,13 @@ class LimitsPolicy(
|
||||
return PolicyResult.Accepted(if (clamped === cmd.filters) cmd else ReqCmd(cmd.subId, clamped))
|
||||
}
|
||||
|
||||
/**
|
||||
* A COUNT is clamped by [RelayLimits.maxLimit] but NEVER given
|
||||
* [RelayLimits.defaultLimit] — see [capLimits].
|
||||
*/
|
||||
override fun accept(cmd: CountCmd): PolicyResult<CountCmd> {
|
||||
subscriptionRejection(cmd.queryId, cmd.filters)?.let { return PolicyResult.Rejected(it) }
|
||||
val clamped = clampLimits(cmd.filters)
|
||||
val clamped = capLimits(cmd.filters)
|
||||
return PolicyResult.Accepted(if (clamped === cmd.filters) cmd else CountCmd(cmd.queryId, clamped))
|
||||
}
|
||||
|
||||
@@ -106,6 +110,28 @@ class LimitsPolicy(
|
||||
return filters.map { it.copy(limit = targetLimit(it.limit)) }
|
||||
}
|
||||
|
||||
/**
|
||||
* Cap what a COUNT asks for, without inventing a page size for it.
|
||||
*
|
||||
* `defaultLimit` answers "how many events should a REQ return when the
|
||||
* client names no limit". A COUNT returns no events, so that question has
|
||||
* no meaning for it — and applying the answer anyway turns every unbounded
|
||||
* COUNT into `min(matches, defaultLimit)`.
|
||||
*
|
||||
* Silently: a relay holding 12,289,614 profiles replied `{"count":500}`,
|
||||
* which is a plausible-looking number, so a client cannot tell it from the
|
||||
* truth. The kinds that happened to fall under the default were correct,
|
||||
* which is what made it survive.
|
||||
*
|
||||
* `maxLimit` still applies, because a client that explicitly asks to count
|
||||
* at most N is asking a question this relay may bound.
|
||||
*/
|
||||
private fun capLimits(filters: List<Filter>): List<Filter> {
|
||||
val max = limits.maxLimit ?: return filters
|
||||
if (filters.none { it.limit != null && it.limit!! > max }) return filters
|
||||
return filters.map { if (it.limit != null && it.limit!! > max) it.copy(limit = max) else it }
|
||||
}
|
||||
|
||||
private fun targetLimit(current: Int?): Int? =
|
||||
when {
|
||||
current != null && limits.maxLimit != null && current > limits.maxLimit -> limits.maxLimit
|
||||
|
||||
+34
@@ -30,6 +30,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.server.policies.PolicyResult
|
||||
import com.vitorpamplona.quartz.nip01Core.relay.server.policies.RelayLimits
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertNull
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
class RelayLimitsTest {
|
||||
@@ -157,6 +158,39 @@ class RelayLimitsTest {
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun countNeverGetsTheDefaultLimit() {
|
||||
// A COUNT returns no events, so "how many should a REQ return by
|
||||
// default" is not a question it asked. Applying the answer anyway turns
|
||||
// every unbounded COUNT into min(matches, defaultLimit) — a relay
|
||||
// holding 12,289,614 profiles replied {"count":500}, which is plausible
|
||||
// enough that no client can tell it from the truth.
|
||||
val policy = LimitsPolicy(RelayLimits(defaultLimit = 50, maxLimit = 100))
|
||||
|
||||
val result = policy.accept(CountCmd("q", listOf(Filter(kinds = listOf(1))))) as PolicyResult.Accepted
|
||||
|
||||
assertNull(
|
||||
result.cmd.filters
|
||||
.single()
|
||||
.limit,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun countStillHonoursAnExplicitMaxLimit() {
|
||||
// Asking to count at most N is a question the relay may bound.
|
||||
val policy = LimitsPolicy(RelayLimits(defaultLimit = 50, maxLimit = 100))
|
||||
|
||||
val result = policy.accept(CountCmd("q", listOf(Filter(kinds = listOf(1), limit = 900)))) as PolicyResult.Accepted
|
||||
|
||||
assertEquals(
|
||||
100,
|
||||
result.cmd.filters
|
||||
.single()
|
||||
.limit,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun leavesAcceptableRequestUnchanged() {
|
||||
val policy = LimitsPolicy(RelayLimits(maxLimit = 100))
|
||||
|
||||
Reference in New Issue
Block a user