MultipleObjectsReturned in core.views.serve.serve.docs #4350

xrmx · 2018-07-10T10:57:10Z

We get this exception while trying to serve some docs:

MultipleObjectsReturned: get() returned more than one Version -- it returned 2!
  File "django/core/handlers/base.py", line 149, in get_response
    response = self.process_exception_by_middleware(e, request)
  File "django/core/handlers/base.py", line 147, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "readthedocs/core/views/serve.py", line 96, in inner_view
    return view_func(request, project=project, *args, **kwargs)
  File "readthedocs/core/views/serve.py", line 74, in inner_view
    return view_func(request, subproject=subproject, *args, **kwargs)
  File "readthedocs/core/views/serve.py", line 156, in serve_docs
    version = project.versions.public(request.user).get(slug=version_slug)
  File "django/db/models/query.py", line 391, in get
    (self.model._meta.object_name, num)

The view code looks legit at a first look but there's a huge side effect of the user filtering in the public() method. It does not filter the projects by user but it adds to the queryset all the other user projects which is not what we need here:

# project of the version we are looking for
>>> versione.project
<Project: Documentazione API anagrafe del Comune di Tre Civette sul Comò>
# this queryset is fine
>>> versione.project.versions.all()
[<Version: Version master of Documentazione API anagrafe del Comune di Tre Civette sul Comò (861)>, <Version: Version bozza of Documentazione API anagrafe del Comune di Tre Civette sul Comò (860)>]
# but here we have other projects' version too
>>> versione.project.versions.public(user)
[<Version: Version stabile of design-docs (859)>, <Version: Version master of Documentazione API anagrafe del Comune di Tre Civette sul Comò (861)>, <Version: Version bozza of design-docs (852)>, <Version: Version bozza of Documentazione API anagrafe del Comune di Tre Civette sul Comò (860)>]

The text was updated successfully, but these errors were encountered:

xrmx · 2018-07-10T12:31:07Z

Wondering if not passing the user to public is the correct fix.

stsewd · 2018-07-10T17:03:27Z

I have encountered this sometimes locally, but I wasn't able to replicate, and I think this has never happened in production.

stsewd · 2018-07-10T17:05:41Z

Also, this had already been reported before #2613

xrmx · 2018-07-10T17:24:10Z

Thanks for the heads up. I'll try to cook a test and a PR then.

We get this exception while trying to serve some docs: MultipleObjectsReturned: get() returned more than one Version -- it returned 2! File "django/core/handlers/base.py", line 149, in get_response response = self.process_exception_by_middleware(e, request) File "django/core/handlers/base.py", line 147, in get_response response = wrapped_callback(request, *callback_args, **callback_kwargs) File "readthedocs/core/views/serve.py", line 96, in inner_view return view_func(request, project=project, *args, **kwargs) File "readthedocs/core/views/serve.py", line 74, in inner_view return view_func(request, subproject=subproject, *args, **kwargs) File "readthedocs/core/views/serve.py", line 156, in serve_docs version = project.versions.public(request.user).get(slug=version_slug) File "django/db/models/query.py", line 391, in get (self.model._meta.object_name, num) The view code looks legit at a first look but there's a huge side effect of the user filtering in the public() method. It does not filter the projects by user but it adds to the queryset all the other user projects which is not what we need here. So just don't pass the user to the public() method fixes to get the code work. Fix readthedocs#4350

We get this exception while trying to serve some docs: MultipleObjectsReturned: get() returned more than one Version -- it returned 2! File "django/core/handlers/base.py", line 149, in get_response response = self.process_exception_by_middleware(e, request) File "django/core/handlers/base.py", line 147, in get_response response = wrapped_callback(request, *callback_args, **callback_kwargs) File "readthedocs/core/views/serve.py", line 96, in inner_view return view_func(request, project=project, *args, **kwargs) File "readthedocs/core/views/serve.py", line 74, in inner_view return view_func(request, subproject=subproject, *args, **kwargs) File "readthedocs/core/views/serve.py", line 156, in serve_docs version = project.versions.public(request.user).get(slug=version_slug) File "django/db/models/query.py", line 391, in get (self.model._meta.object_name, num) The view code looks legit at a first look but there's a huge side effect of the user filtering in the public() method. It does not filter the projects by user but it adds to the queryset all the other user projects which is not what we need here. Instead simplify the code to: - return 404 if the requested version does not exist - return 401 if the version is not private and the user is not admin - search the file if the version is private and the user is the admin Fix readthedocs#4350

p3trus · 2018-10-30T09:42:12Z

Just a short question, why is this issue closed? To me it appears that the fix has not been applied to readthedocs but to the fork only.

xrmx · 2018-10-30T10:04:34Z

@p3trus thanks for checking, it has been closed automatically by github :)

We get this exception while trying to serve some docs: MultipleObjectsReturned: get() returned more than one Version -- it returned 2! File "django/core/handlers/base.py", line 149, in get_response response = self.process_exception_by_middleware(e, request) File "django/core/handlers/base.py", line 147, in get_response response = wrapped_callback(request, *callback_args, **callback_kwargs) File "readthedocs/core/views/serve.py", line 96, in inner_view return view_func(request, project=project, *args, **kwargs) File "readthedocs/core/views/serve.py", line 74, in inner_view return view_func(request, subproject=subproject, *args, **kwargs) File "readthedocs/core/views/serve.py", line 156, in serve_docs version = project.versions.public(request.user).get(slug=version_slug) File "django/db/models/query.py", line 391, in get (self.model._meta.object_name, num) The view code looks legit at a first look but there's a huge side effect of the user filtering in the public() method. It does not filter the projects by user but it adds to the queryset all the other user projects which is not what we need here. Instead simplify the code to: - return 404 if the requested version does not exist - return 401 if the version is private and the user is not admin - serve the file if the version is private and the user is the admin Fix readthedocs#4350

stale · 2019-01-10T15:30:42Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

p3trus · 2019-01-15T13:06:58Z

Any plans to merge this bugfix in the near future?

xrmx · 2019-01-15T14:04:46Z

@p3trus this is the bug not the PR

p3trus · 2019-01-15T17:13:24Z

I know but the docsitalia team already fixed it.

xrmx · 2019-01-15T17:16:42Z

@p3trus you probably missed I'm the one who is trying to get this merged :)

p3trus · 2019-01-15T17:23:08Z

@xrmx I didn't mean to sound nagging. Just wanted to know the timeframe if there is any.

xrmx · 2019-01-15T17:47:02Z

@p3trus asking for ETA is nagging :) If you look at the PR you'll see clearly stated that it's not merged because coverage tests fail. They fail because code does not do what the tests expect and I suspect even before the proposed changes.

We get this exception while trying to serve some docs: MultipleObjectsReturned: get() returned more than one Version -- it returned 2! File "django/core/handlers/base.py", line 149, in get_response response = self.process_exception_by_middleware(e, request) File "django/core/handlers/base.py", line 147, in get_response response = wrapped_callback(request, *callback_args, **callback_kwargs) File "readthedocs/core/views/serve.py", line 96, in inner_view return view_func(request, project=project, *args, **kwargs) File "readthedocs/core/views/serve.py", line 74, in inner_view return view_func(request, subproject=subproject, *args, **kwargs) File "readthedocs/core/views/serve.py", line 156, in serve_docs version = project.versions.public(request.user).get(slug=version_slug) File "django/db/models/query.py", line 391, in get (self.model._meta.object_name, num) The view code looks legit at a first look but there's a huge side effect of the user filtering in the public() method. It does not filter the projects by user but it adds to the queryset all the other user projects which is not what we need here. Instead simplify the code to: - return 404 if the requested version does not exist - return 401 if the version is private and the user is not admin - serve the file if the version is private and the user is the admin Fix readthedocs#4350

When filtering using `public` and using a user, the queryset hit this https://github.com/rtfd/readthedocs.org/blob/45df7fd0da44be9eab3c0cb2888f6a9a15421fc5/readthedocs/builds/querysets.py#L22-L24 When the user is authenticated, we call to `get_objects_for_user` which gets all the versions from all the user's projects. Overriding any previous filter (`project.versions` in this case) We don't see this in production because we serve from another domain. And we don't see this in the corporate site because we override the serve_docs view. Fix readthedocs#4350 Closes readthedocs#4356

xrmx mentioned this issue Jul 11, 2018

core: fix MultipleObjectsReturned in views.serve.serve.docs #4356

Closed

xrmx mentioned this issue Jul 11, 2018

core: fix MultipleObjectsReturned in core.views.serve.serve.docs italia/docs.italia.it#74

Merged

xrmx closed this as completed in italia/docs.italia.it#74 Jul 18, 2018

xrmx reopened this Oct 30, 2018

stale bot added the Status: stale Issue will be considered inactive soon label Jan 10, 2019

stsewd added Bug A bug Accepted Accepted issue on our roadmap and removed Status: stale Issue will be considered inactive soon labels Jan 10, 2019

stsewd mentioned this issue Jun 11, 2019

Use querysets from the class not from an instance #5783

Merged

stsewd closed this as completed in #5783 Jun 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MultipleObjectsReturned in core.views.serve.serve.docs #4350

MultipleObjectsReturned in core.views.serve.serve.docs #4350

xrmx commented Jul 10, 2018

xrmx commented Jul 10, 2018 •

edited

Loading

stsewd commented Jul 10, 2018

stsewd commented Jul 10, 2018

xrmx commented Jul 10, 2018

p3trus commented Oct 30, 2018

xrmx commented Oct 30, 2018

stale bot commented Jan 10, 2019

p3trus commented Jan 15, 2019

xrmx commented Jan 15, 2019

p3trus commented Jan 15, 2019

xrmx commented Jan 15, 2019

p3trus commented Jan 15, 2019

xrmx commented Jan 15, 2019

MultipleObjectsReturned in core.views.serve.serve.docs #4350

MultipleObjectsReturned in core.views.serve.serve.docs #4350

Comments

xrmx commented Jul 10, 2018

xrmx commented Jul 10, 2018 • edited Loading

stsewd commented Jul 10, 2018

stsewd commented Jul 10, 2018

xrmx commented Jul 10, 2018

p3trus commented Oct 30, 2018

xrmx commented Oct 30, 2018

stale bot commented Jan 10, 2019

p3trus commented Jan 15, 2019

xrmx commented Jan 15, 2019

p3trus commented Jan 15, 2019

xrmx commented Jan 15, 2019

p3trus commented Jan 15, 2019

xrmx commented Jan 15, 2019

xrmx commented Jul 10, 2018 •

edited

Loading