Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LimitOffset pagination crashes Browseable API when limit=0 #4079

Closed
awbacker opened this issue Apr 28, 2016 · 3 comments · Fixed by #4098
Closed

LimitOffset pagination crashes Browseable API when limit=0 #4079

awbacker opened this issue Apr 28, 2016 · 3 comments · Fixed by #4098
Labels
Milestone

Comments

@awbacker
Copy link

awbacker commented Apr 28, 2016

We are using LimitOffsetPagination on all our endpoints, and noticed that sending limit=0 to the browsable api causes a crash. Getting json output (either from an xmlhttprequest or from format=json) does not crash.

Steps to reproduce

  1. Install LimitOffsetPagination as the default pagination
  2. Open a list (ours are ModelViewSet) in the browser
  3. Set the query string to ?limit=0&offset=20
  4. boom

Notes
We use a custom LimitOffset pagination, which just overrides the max and default limit
Using DRF 3.3.2

Expected behavior

Should show the browsable api for the endpoint, with the following data:

{
    "count":148,
    "next":"https://api.co/page/?limit=0&offset=20",
    "previous":"https://api.co/page/?limit=0&offset=20",
    "results":[]
}

Actual behavior

Exception Type: ZeroDivisionError
Exception Value:    
integer division or modulo by zero
Exception Location: .../venv/local/lib/python2.7/site-packages/rest_framework/pagination.py in _divide_with_ceil, line 41
Python Executable:  /usr/local/bin/uwsgi

.../venv/local/lib/python2.7/site-packages/rest_framework/pagination.py in get_html_context
    current = _divide_with_ceil(self.offset, self.limit) + 1 ...

    ▼ Local vars
    self      <mysite.LargePageination object at 0x7f614e677bd0>
    base_url  'https://api.co/page/?limit=0&offset=20'

.../venv/local/lib/python2.7/site-packages/rest_framework/pagination.py in _divide_with_ceil
    if a % b: ...

    ▼ Local vars
    a       20
    b       0
@xordoquy xordoquy added the Bug label Apr 28, 2016
@anx-ckreuzberger
Copy link
Contributor

I am able to reproduce this behaviour. Also this is not really a crash, it just displays you an error message as you have Debug Mode set to True in your Django Project.

Though I guess there is no harm done in catching this and just returning an empty result.

@tomchristie
Copy link
Member

tomchristie commented Apr 28, 2016

Thanks for confirming @anx-ckreuzberger!
Agree - this ought to be resolved.

@awbacker
Copy link
Author

awbacker commented Apr 29, 2016

Thanks for confirming too. It's nice to know I'm not crazy (though I did try it on a fresh project). Not sure how it's not a crash, it's a div by 0. The stack is well protected enough that this sends back a 500, so I guess it's not a core dump ;) Well, no core dump, just an uncaught exception popping up in Sentry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants