diff --git a/app/controllers/concerns/api/v1/paginatable.rb b/app/controllers/concerns/api/v1/paginatable.rb index 79f89e8..37d3be3 100644 --- a/app/controllers/concerns/api/v1/paginatable.rb +++ b/app/controllers/concerns/api/v1/paginatable.rb @@ -38,6 +38,12 @@ module Paginatable # preferible a que el default de 20 le esconda depósitos en silencio. WHOLE_LIST_PER_PAGE = MAX_PER_PAGE + # La página más alta que se acepta. Muy por encima de cualquier listado + # real (un millón de páginas de cien filas), y lejos del límite de + # `bigint` del OFFSET: un `?page=` de veinte dígitos lo desbordaba y la + # API respondía 500 (hallazgo de la auditoría de TESIS-89). + MAX_PAGE = 1_000_000 + private # Devuelve `[filas, meta]`. @@ -53,11 +59,13 @@ def paginate(scope, per_page: DEFAULT_PER_PAGE, total: nil) { page: page, per_page: size, total: total || scope.count }] end - # Página pedida, nunca menor que 1. `page=0` y `page=-3` se acotan en vez + # Página pedida, entre 1 y MAX_PAGE. `page=0` y `page=-3` se acotan en vez # de romper: un offset negativo es un error de SQL, y un 400 por un número - # que se puede interpretar sería antipático. + # que se puede interpretar sería antipático. Por arriba, el mismo criterio: + # una página enorme se lee como la última aceptada y responde vacía, como + # cualquier página pasada del final. def page_number - [scalar_param(:page).to_i, 1].max + scalar_param(:page).to_i.clamp(1, MAX_PAGE) end # `scalar_param` y no `params[...]`: `?per_page[]=1` entrega un Array y diff --git a/spec/requests/api/v1/pagination_spec.rb b/spec/requests/api/v1/pagination_spec.rb index 7534973..53933aa 100644 --- a/spec/requests/api/v1/pagination_spec.rb +++ b/spec/requests/api/v1/pagination_spec.rb @@ -56,6 +56,15 @@ def rows_for(params) end # Una página más allá del final no es un error: es una página vacía. + # Un número de veinte dígitos desbordaba el OFFSET (bigint) y respondía 500. + it 'answers an empty page for a page number too large for the database', :aggregate_failures do + get '/api/v1/orders', params: { page: '99999999999999999999' }, headers: headers + + expect(response).to have_http_status(:ok) + expect(response.parsed_body['data']).to be_empty + expect(response.parsed_body.dig('meta', 'page')).to eq(Api::V1::Paginatable::MAX_PAGE) + end + it 'answers an empty page past the end, with the real total', :aggregate_failures do create_products(3)