disable pagination

[FrancYescO]

Looking at the name of the project probably out of the scope :D

but is there a way to disable pagination (i just need to reuse the filters and maybe sorting and other applicable logic)?

[ppetzold]

just set defaultLimit and maxLimit very high 😅 so it returns always just a single page.

[FrancYescO]

yep this is my actual workaround :) i was searching just if there was some cleaner way maybe can be implemented like putting limit value to 0/-1 (if allowed by maxLimit) so i can dynamically choose if i want to get it unpaginated

[FrancYescO]

FYI, the workaround of setting an high default/maxLimit will cause a very inefficient query generated in case of relations on the table (something like https://github.com/ppetzold/nestjs-paginate/issues/329) that is not happening using the suggested PR

[ppetzold]

@FrancYescO Could elaborate in more detail about the differences? It still executes COUNT + FIND

An issue with the current implementation would be that it introduces an attack surface. Allowing every client to remove the resource limit can cause perf issues on the db with large tables (DoS attack).

[FrancYescO]

The limit (and DoS protection) should be up to the dafault/maxLimit (so if the maxlimit is i.e. 20 don't allow the 0-infinite limit).

Count + find will cause the result query to have a giant list of OR for each row on each key of joined table

...
Table WHERE (`Table`.`ID` = ? AND Table`.`secondID = ?) OR (`Table`.`ID` = ? AND Table`.`secondID = ?) OR (`Table`.`ID` = ? AND Table`.`secondID = ?) OR (`Table`.`ID` = ? AND Table`.`secondID = ?) OR (`Table`.`ID` = ? AND Table`.`secondID = ?) OR (`Table`.`ID` = ? AND Table`.`secondID = ?) OR (`Table`.`ID` = ? AND Table`.`secondID = ?) OR (`Table`.`ID` = ? AND Table`.`secondID = ?) OR (`Table`.`ID` = ? AND Table`.`secondID = ?) OR (`Table`.`ID` = ? AND Table`.`secondID = ?) -- PARAMETERS: ["169","3","170","3","171","3","172","3","173","3","174","3","175","3","176","3","177","3","178","3"]

That will cause like x100 of time to execute the query and get the results

the PR suggested does not set any LIMIT in the query that will not cause this query composition.

[ppetzold]

(1) Ok, the PR tho used an infinite limit 😅 https://github.com/ppetzold/nestjs-paginate/pull/406/files#diff-c58e144dadf00250381ed55e6ce83245cda76aca84131ad494fd4911932f656fR180

(2) So, you are saying removing .take(limit).skip((page - 1) * limit) would create a better query https://github.com/ppetzold/nestjs-paginate/pull/406/files#diff-c58e144dadf00250381ed55e6ce83245cda76aca84131ad494fd4911932f656fR247

(3) doesn't (2) imply that we query an infinite result set?

[FrancYescO]

1) yep the PR maybe should be fixed to avoid allowing 0 if we have a 20 as maxlimit

2) yep

3) is what i want, if asking for a 0 limit

[xMase]

1) Yep no problem to modify the pr to include the maxLimit check.

[ppetzold]

to (1) if we must do (2) then setting maxLimit has no effect

I think, what it comes down to is that there cannot be any DoS protection. But I understand that there are use cases where this is neglectable.

So, I am okay adding this feature but it should be whitelisted. Something like enableLimitRemoval: true

[FrancYescO]

enableLimitRemoval: true == maxLimit: 0

[ppetzold]

Okay with that as well. But then no pagination would be possible on that endpoint. So always infinite resource requests.

[ppetzold]

Btw. not sure how typeorm handles this.

But if I anyway query all items, I don't need a separate COUNT query. So you may optimize there as well

[FrancYescO]

Okay with that as well. But then no pagination would be possible on that endpoint. So always infinite resource requests.

pagination will be enabled if runtime i'll set the limit to something different than 0 (PR already act like this)

[ppetzold]

Okay with that as well. But then no pagination would be possible on that endpoint. So always infinite resource requests.

pagination will be enabled if runtime i'll set the limit to something different than 0 (PR already act like this)

once again, I am not okay with making it a default behavior due to the attack surface it opens

I understood you suggested using maxLimit: 0 instead of enableLimitRemoval: true. which is okay with me to act as whitelisting. but then there is no pagination while maxLimit: 0

[FrancYescO]

sure i was not saying to set as defaultLimit/maxLimit to 0 (neither this is done in the PR), but just to use maxLimit: 0 as a know-what-you-are-doing flag.

there are other usecase while pagination can be enabled also when maxLimit: 0:

• maxLimit: 20 (defaultLimit: 20) > actual (unchanged) behaviour, not allowed to do unpaginated (if you set 0 runtime will be forced to 20), max allowed limit 20
• maxLimit: 0 defaultLimit: 20 > allowed to do unpaginated runtime but default is LIMIT 20
• maxLimit: 0 (defaultLimit: 0) > default unpaginated, allowed to set runtime LIMIT 20 to get a paginated query

[ppetzold]

ah okay, that's cool. I was reading it still literally but you meant maxLimit: 0 as way of disabling it.

sounds good to me. please add some lines to the README then :)

[FrancYescO]

just got another needs (get the total count of filtered items avoiding to get all datas) and probably a little rework on this PR can help...

• limit = 0 can be used to get the count of elements in the table avoid getting data (reading meta.totalItems) that actually is also bugged as it return 0 in this value
• limit < 0 (or -1) unpaginated

[ppetzold]

cannot really follow. could you explain with example query + response?

[FrancYescO]

maybe easier in this way: maxLimit: 0 = allowed only to count elements in the table maxLimit: -1 = allowed to do unpaginated query (actual behaviour as 0)

http://localhost:3000/cats?limit=0&search=i&filter.age=$gte:3

{
  "data": [], << NO DATA RETRIVED
  "meta": {
    "itemsPerPage": 0,
    "totalItems": 12, << this is the only value that i need/i want to allow to retrive
    "currentPage": 1, << or maybe these can be 0..
    "totalPages": 1,
    "search": "i",
    "filter": {
      "age": "$gte:3"
    }
  },
  "links": {
    ...
  }
}

http://localhost:3000/cats?limit=-1&search=i&filter.age=$gte:3

{
  "data": [...],  <<<< ALL ITEMS OF THE TABLE
  "meta": {
    "itemsPerPage": 12,
    "totalItems": 12, <<< after your edits this is 0 when retrieving full table!!
    "currentPage": 1,
    "totalPages": 1,
    "search": "i",
    "filter": {
      "age": "$gte:3"
    }
  },
  "links": {
    ...
  }
}

[ppetzold]

that actually makes sense semantically, lol.

totalItems tho should keep the count as usual on limit=-1

currentPage can stay 1 on limit=0

no need to hijack those fields.

so, yeah would accept PR 😅

[ppetzold]

addition:

limit=0 can then be allowed always (even if maxLimit > 0) limit=-1 must be "enabled" with maxLimit=-1