nestjs-paginate icon indicating copy to clipboard operation
nestjs-paginate copied to clipboard

Page can be NaN

Open seeren opened this issue 1 year ago • 3 comments

https://github.com/ppetzold/nestjs-paginate/blob/3c96e99836f25d4f9143adcb32d07b44ef93bfbf/src/decorator.ts#L81

Using a string value for page that is not a numer string, the value is parsed as number but with value NaN.

Ex: http://localhost/?page=foo

{
  page: NaN,
  limit: undefined,
  sortBy: undefined,
  search: undefined,
  searchBy: undefined,
  filter: undefined,
  select: undefined,
  path: 'http://localhost:4200/tags'
 }

After that the paginate function throw a Provided "skip" value is not a number. Please provide a numeric value.

I fix the problem with a dummy interceptor that cast the page value correctly but the problem is the parseInt usage described in the initial link.

seeren avatar Oct 13 '23 16:10 seeren

Probably we can use string and regexp to check numeric values, but ideally, I think we need to have some validator in general for fields instead of silently skipping things (but it's a bigger change)

And it may be hard to choose a validator library to cover everyone's needs ...

vsamofal avatar Oct 14 '23 22:10 vsamofal

Thanks for reply,

I think we can close this issue if the perspective is not to validate or transform query params. To validate or transform I think every one have in dependency class-validator class-transformer.

Solutions to patch this problem:

  • We can validate and transform with our proper DTO, I think it's overkill as we expect this package to do that.
  • We can remove invalid query params as a patch, easy solution that I'm going to merge an our projects:
@Injectable()
export class PaginateInterceptor implements NestInterceptor {
  intercept(context: ExecutionContext, next: CallHandler): Observable<unknown> {
    const { query } = context.switchToHttp().getRequest();
    if (query && 'page' in query && !isNumberString(query.page)) {
      delete query.page;
    }
    return next.handle();
  }
}

seeren avatar Oct 16 '23 07:10 seeren

I think it is a valid issue, and we'd better to fix it on our end as well. let's keep it open for now

vsamofal avatar Oct 16 '23 21:10 vsamofal