From 9130f3875281367e5a17a57e8516b2ffda615fa6 Mon Sep 17 00:00:00 2001 From: Matt Wheeler Date: Mon, 10 Apr 2017 15:02:53 +0100 Subject: [PATCH 1/4] check for matching alias before subproject slug fixes #2731 --- readthedocs/core/views/serve.py | 20 +++++++------ readthedocs/rtd_tests/tests/test_resolver.py | 30 +++++++++++++++++++ .../rtd_tests/tests/test_subprojects.py | 17 +++++++++-- 3 files changed, 55 insertions(+), 12 deletions(-) diff --git a/readthedocs/core/views/serve.py b/readthedocs/core/views/serve.py index 09f663015c7..bf6d2e9e385 100644 --- a/readthedocs/core/views/serve.py +++ b/readthedocs/core/views/serve.py @@ -56,17 +56,19 @@ def map_subproject_slug(view_func): @wraps(view_func) def inner_view(request, subproject=None, subproject_slug=None, *args, **kwargs): if subproject is None and subproject_slug: + # Try to fetch by subproject alias first, otherwise we might end up + # redirected to an unrelated project. try: - subproject = Project.objects.get(slug=subproject_slug) - except Project.DoesNotExist: + # Depends on a project passed into kwargs + rel = ProjectRelationship.objects.get( + parent=kwargs['project'], + alias=subproject_slug, + ) + subproject = rel.child + except (ProjectRelationship.DoesNotExist, KeyError): try: - # Depends on a project passed into kwargs - rel = ProjectRelationship.objects.get( - parent=kwargs['project'], - alias=subproject_slug, - ) - subproject = rel.child - except (ProjectRelationship.DoesNotExist, KeyError): + subproject = Project.objects.get(slug=subproject_slug) + except Project.DoesNotExist: raise Http404 return view_func(request, subproject=subproject, *args, **kwargs) return inner_view diff --git a/readthedocs/rtd_tests/tests/test_resolver.py b/readthedocs/rtd_tests/tests/test_resolver.py index 1c67cf11735..df1fd7b536f 100644 --- a/readthedocs/rtd_tests/tests/test_resolver.py +++ b/readthedocs/rtd_tests/tests/test_resolver.py @@ -385,3 +385,33 @@ def test_resolver_public_domain_overrides(self): self.assertEqual(url, 'http://docs.foobar.com/en/latest/') url = resolve(project=self.pip, private=False) self.assertEqual(url, 'http://docs.foobar.com/en/latest/') + + +class ResolverAltSetUp(object): + + def setUp(self): + with mock.patch('readthedocs.projects.models.broadcast'): + with mock.patch('readthedocs.projects.models.update_static_metadata'): + self.owner = create_user(username='owner', password='test') + self.tester = create_user(username='tester', password='test') + self.pip = get(Project, slug='pip', users=[self.owner], main_language_project=None) + self.seed = get(Project, slug='sub', users=[self.owner], main_language_project=None) + self.subproject = get(Project, slug='subproject', language='ja', users=[self.owner], main_language_project=None) + self.translation = get(Project, slug='trans', language='ja', users=[self.owner], main_language_project=None) + self.pip.add_subproject(self.subproject, alias='sub') + self.pip.translations.add(self.translation) + + +@override_settings(PUBLIC_DOMAIN='readthedocs.org') +class ResolverDomainTestsAlt(ResolverAltSetUp, ResolverDomainTests): + pass + + +@override_settings(PUBLIC_DOMAIN='readthedocs.org') +class SmartResolverPathTestsAlt(ResolverAltSetUp, SmartResolverPathTests): + pass + + +@override_settings(PUBLIC_DOMAIN='readthedocs.org') +class ResolverTestsAlt(ResolverAltSetUp, ResolverTests): + pass diff --git a/readthedocs/rtd_tests/tests/test_subprojects.py b/readthedocs/rtd_tests/tests/test_subprojects.py index d2e63dcd88b..0bf378a237f 100644 --- a/readthedocs/rtd_tests/tests/test_subprojects.py +++ b/readthedocs/rtd_tests/tests/test_subprojects.py @@ -77,12 +77,14 @@ def setUp(self): self.owner], main_language_project=None) self.pip.add_subproject(self.subproject) self.pip.translations.add(self.translation) - - @override_settings(PRODUCTION_DOMAIN='readthedocs.org') - def test_resolver_subproject_alias(self): relation = self.pip.subprojects.first() relation.alias = 'sub_alias' relation.save() + get(Project, slug='sub_alias', language='ya') + + + @override_settings(PRODUCTION_DOMAIN='readthedocs.org') + def test_resolver_subproject_alias(self): with override_settings(USE_SUBDOMAIN=False): resp = self.client.get('/docs/pip/projects/sub_alias/') self.assertEqual(resp.status_code, 302) @@ -90,3 +92,12 @@ def test_resolver_subproject_alias(self): resp._headers['location'][1], 'http://readthedocs.org/docs/pip/projects/sub_alias/ja/latest/' ) + + def test_resolver_subproject_subdomain_alias(self): + with override_settings(USE_SUBDOMAIN=True): + resp = self.client.get('/projects/sub_alias/', HTTP_HOST='pip.readthedocs.org') + self.assertEqual(resp.status_code, 302) + self.assertEqual( + resp._headers['location'][1], + 'http://pip.readthedocs.org/projects/sub_alias/ja/latest/' + ) From c3d2ddc2847e4c17b27269db5605731f510af8ee Mon Sep 17 00:00:00 2001 From: Matt Wheeler Date: Thu, 26 Oct 2017 12:18:34 +0100 Subject: [PATCH 2/4] Address PR comments --- .../rtd_tests/tests/test_subprojects.py | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_subprojects.py b/readthedocs/rtd_tests/tests/test_subprojects.py index f3e83513510..59f4b2b3633 100644 --- a/readthedocs/rtd_tests/tests/test_subprojects.py +++ b/readthedocs/rtd_tests/tests/test_subprojects.py @@ -165,21 +165,23 @@ def setUp(self): fixture.get(Project, slug='sub_alias', language='ya') - @override_settings(PRODUCTION_DOMAIN='readthedocs.org') - def test_resolver_subproject_alias(self): - with override_settings(USE_SUBDOMAIN=False): - resp = self.client.get('/docs/pip/projects/sub_alias/') - self.assertEqual(resp.status_code, 302) - self.assertEqual( - resp._headers['location'][1], - 'http://readthedocs.org/docs/pip/projects/sub_alias/ja/latest/' + @override_settings( + PRODUCTION_DOMAIN='readthedocs.org', + USE_SUBDOMAIN=False, ) + def test_resolver_subproject_alias(self): + resp = self.client.get('/docs/pip/projects/sub_alias/') + self.assertEqual(resp.status_code, 302) + self.assertEqual( + resp._headers['location'][1], + 'http://readthedocs.org/docs/pip/projects/sub_alias/ja/latest/' + ) + @override_settings(USE_SUBDOMAIN=True) def test_resolver_subproject_subdomain_alias(self): - with override_settings(USE_SUBDOMAIN=True): - resp = self.client.get('/projects/sub_alias/', HTTP_HOST='pip.readthedocs.org') - self.assertEqual(resp.status_code, 302) - self.assertEqual( - resp._headers['location'][1], - 'http://pip.readthedocs.org/projects/sub_alias/ja/latest/' - ) + resp = self.client.get('/projects/sub_alias/', HTTP_HOST='pip.readthedocs.org') + self.assertEqual(resp.status_code, 302) + self.assertEqual( + resp._headers['location'][1], + 'http://pip.readthedocs.org/projects/sub_alias/ja/latest/' + ) From a4bb8e3db4e73b4a40cd9ea8da90e1303cb8d771 Mon Sep 17 00:00:00 2001 From: Matt Wheeler Date: Fri, 12 Jan 2018 19:17:35 +0000 Subject: [PATCH 3/4] use get_object_or_404 shortcut --- readthedocs/core/views/serve.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/readthedocs/core/views/serve.py b/readthedocs/core/views/serve.py index db0ef091d66..becd40953d4 100644 --- a/readthedocs/core/views/serve.py +++ b/readthedocs/core/views/serve.py @@ -27,7 +27,7 @@ from __future__ import absolute_import from django.conf import settings from django.http import HttpResponse, HttpResponseRedirect, Http404 -from django.shortcuts import render_to_response +from django.shortcuts import get_object_or_404, render_to_response from django.template import RequestContext from django.views.static import serve @@ -67,10 +67,7 @@ def inner_view(request, subproject=None, subproject_slug=None, *args, **kwargs): ) subproject = rel.child except (ProjectRelationship.DoesNotExist, KeyError): - try: - subproject = Project.objects.get(slug=subproject_slug) - except Project.DoesNotExist: - raise Http404 + get_object_or_404(Project, slug=subproject_slug) return view_func(request, subproject=subproject, *args, **kwargs) return inner_view From 9bb0f28d778e6adcd3a70cd04f5a56c45e4ae83e Mon Sep 17 00:00:00 2001 From: Matt Wheeler Date: Thu, 18 Jan 2018 19:28:20 +0000 Subject: [PATCH 4/4] revert style changes (where did they even come from?) --- readthedocs/rtd_tests/tests/test_subprojects.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_subprojects.py b/readthedocs/rtd_tests/tests/test_subprojects.py index 59f4b2b3633..485eb9b9228 100644 --- a/readthedocs/rtd_tests/tests/test_subprojects.py +++ b/readthedocs/rtd_tests/tests/test_subprojects.py @@ -152,10 +152,12 @@ def setUp(self): self.owner = create_user(username='owner', password='test') self.tester = create_user(username='tester', password='test') self.pip = fixture.get(Project, slug='pip', users=[self.owner], main_language_project=None) - self.subproject = fixture.get(Project, slug='sub', language='ja', users=[ - self.owner], main_language_project=None) - self.translation = fixture.get(Project, slug='trans', language='ja', users=[ - self.owner], main_language_project=None) + self.subproject = fixture.get(Project, slug='sub', language='ja', + users=[ self.owner], + main_language_project=None) + self.translation = fixture.get(Project, slug='trans', language='ja', + users=[ self.owner], + main_language_project=None) self.pip.add_subproject(self.subproject) self.pip.translations.add(self.translation)