Proxito: redirect to main project from subprojects #8187
Conversation
| self.assertIsNone(res) | ||
| self.assertEqual(getattr(request, 'canonicalize', None), 'subproject-main-domain') | ||
|
|
||
| # Using a custom domain in a subproject isn't supported (or shouldn't be!). |
There was a problem hiding this comment.
Added a test for this to check isn't failing since it could happen, but we can also move the subdomain check first. Think we have talked about checking this case in the domain creation form.
There was a problem hiding this comment.
Yea, we shouldn't allow this.
| self.assertIsNone(res) | ||
| self.assertEqual(getattr(request, 'canonicalize', None), 'subproject-main-domain') | ||
|
|
||
| # Using a custom domain in a subproject isn't supported (or shouldn't be!). |
There was a problem hiding this comment.
Yea, we shouldn't allow this.
| @@ -55,6 +55,57 @@ def test_single_version_subproject_root_url_no_slash(self): | |||
| r['Location'], 'https://project.dev.readthedocs.io/projects/subproject/', | |||
| ) | |||
|
|
|||
| def test_subproject_redirect(self): | |||
| r = self.client.get('/', HTTP_HOST='subproject.dev.readthedocs.io') | |||
There was a problem hiding this comment.
Does this break with the canonicalization around http -> https not doing the redirect?
There was a problem hiding this comment.
do you mean http://suproject.rtd.io/foo? well, we do http -> https redirects from cloudflare/nginx, so it gets redirected to https://suproject.rtd.io/foo. We don't support custom domains on the domain of the subproject, but if a user sets one, we would redirect directly to the main domain, instead of passing for https first, yeah. And we don't allow redirects from subprojects (well we do, but they don't work #5889).
I may start adding some validation in our forms to avoid having to deal with that at all p:
Good idea, I'll see if that can be done, proxied APIs usually pass the project and version as query args. |
This introduces one more query for normal serving, except when a custom domain is used, there the queries are the same, I think this is fine.
Closes #7881
We can revert #7880
after releasing this.