Skip to content

Commit

Permalink
core: fix MultipleObjectsReturned in core.views.serve.serve.docs
Browse files Browse the repository at this point in the history
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
  • Loading branch information
xrmx committed Jul 11, 2018
1 parent b5a761a commit becad35
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 8 deletions.
15 changes: 8 additions & 7 deletions readthedocs/core/views/serve.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,23 +152,24 @@ def serve_docs(
"""Exists to map existing proj, lang, version, filename views to the file format."""
if not version_slug:
version_slug = project.get_default_version()

try:
version = project.versions.public(request.user).get(slug=version_slug)
version = project.versions.get(slug=version_slug)
except Version.DoesNotExist:
# Properly raise a 404 if the version doesn't exist & a 401 if it does
if project.versions.filter(slug=version_slug).exists():
return _serve_401(request, project)
raise Http404('Version does not exist.')

# only project admins should see private versions
if (version.privacy_level == constants.PRIVATE and
not AdminPermission.is_member(user=request.user, obj=project)):
return _serve_401(request, project)

filename = resolve_path(
subproject or project, # Resolve the subproject if it exists
version_slug=version_slug,
language=lang_slug,
filename=filename,
subdomain=True, # subdomain will make it a "full" path without a URL prefix
)
if (version.privacy_level == constants.PRIVATE and
not AdminPermission.is_member(user=request.user, obj=project)):
return _serve_401(request, project)
return _serve_symlink_docs(
request,
filename=filename,
Expand Down
16 changes: 15 additions & 1 deletion readthedocs/rtd_tests/tests/test_redirects.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from __future__ import absolute_import

from django.contrib.auth.models import User
from django.core.urlresolvers import reverse
from django.http import Http404
from django.http import Http404, HttpResponse
from django.test import TestCase
from django.test.utils import override_settings

Expand Down Expand Up @@ -48,6 +50,18 @@ def test_proper_url(self):
self.assertEqual(
r['Location'], 'http://readthedocs.org/docs/pip/en/latest/')

def test_owner_with_multiple_projects_can_serve_the_requested_version(self):
# GH #4350
eric = User.objects.get(username='eric')
notpip = eric.projects.exclude(slug='pip').first()
notpip.versions.create_latest()

self.client.login(username='eric', password='test')
with patch('readthedocs.core.views.serve._serve_symlink_docs') as _serve_docs:
_serve_docs.return_value = HttpResponse()
r = self.client.get('/docs/pip/en/latest/')
self.assertTrue(_serve_docs.called)

# Specific Page Redirects
def test_proper_page_on_main_site(self):
r = self.client.get('/docs/pip/page/test.html')
Expand Down

0 comments on commit becad35

Please sign in to comment.