Merge pull request #2787 from funkyHat/subproject_check_alias_before_…

…slug check for matching alias before subproject slug
readthedocs · Mar 7, 2018 · d127ea5 · d127ea5
2 parents 78b5781 + 6a66934
commit d127ea5
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 20 deletions.
diff --git a/readthedocs/core/views/serve.py b/readthedocs/core/views/serve.py
@@ -34,7 +34,8 @@
 from functools import wraps
 
 from django.conf import settings
-from django.http import Http404, HttpResponse, HttpResponseRedirect
+from django.http import HttpResponse, HttpResponseRedirect, Http404
+from django.shortcuts import get_object_or_404
 from django.shortcuts import render
 from django.views.static import serve
 
@@ -59,18 +60,17 @@ def map_subproject_slug(view_func):
     @wraps(view_func)
     def inner_view(request, subproject=None, subproject_slug=None, *args, **kwargs):  # noqa
         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:
-                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):
-                    raise Http404
+                # Depends on a project passed into kwargs
+                rel = ProjectRelationship.objects.get(
+                    parent=kwargs['project'],
+                    alias=subproject_slug,
+                )
+                subproject = rel.child
+            except (ProjectRelationship.DoesNotExist, KeyError):
+                get_object_or_404(Project, slug=subproject_slug)
         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
@@ -385,3 +385,32 @@ 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'):
+            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
@@ -176,15 +176,29 @@ def setUp(self):
             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()
-        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/'
+        fixture.get(Project, slug='sub_alias', language='ya')
+
+
+    @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):
+        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/'
+        )