Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return 404 at SubdomainMiddleware level if project doesn't exist #3801

Closed
humitos opened this issue Mar 14, 2018 · 2 comments
Closed

Return 404 at SubdomainMiddleware level if project doesn't exist #3801

humitos opened this issue Mar 14, 2018 · 2 comments
Labels
Bug A bug
Milestone

Comments

@humitos
Copy link
Member

humitos commented Mar 14, 2018

While deploying .com we found that SubdomainMiddleware doesn't check the existence of the project that it's injecting at request.slug.

So, any middleware that relies on this to do some extra work, will probably do something like Project.objects.get(slug=request.slug) and will fail.

We workarounded it by adding try/except block in the middleware that uses this, but we should probably just raise a 404 inside the SubdomainMiddleware itself instead of continue with the flow.

Code: https://github.com/rtfd/readthedocs.org/blob/74bbb3ac23b6b06a70517bc1b301961db6821e32/readthedocs/core/middleware.py#L72

@humitos humitos added the Bug A bug label Mar 14, 2018
@agjohnson agjohnson added this to the Cleanup milestone Mar 23, 2018
@dojutsu-user
Copy link
Member

dojutsu-user commented Oct 18, 2018

@humitos I would like to take this issue up.
If I'm not wrong, you probably want something like this

# Checking if the project exist or not
from django.shortcuts import get_object_or_404
get_object_or_404(Project, slug=subdomain)

request.subdomain = True
request.slug = subdomain
request.urlconf = SUBDOMAIN_URLCONF
return None

With get_object_or_404, it will show error and does not go further.

@humitos
Copy link
Member Author

humitos commented Oct 23, 2018

@dojutsu-user since this is a code that it's executed a lot, retrieving the whole Project object it may be overkilling since we not going to use that data anyway. You may just need to check if the project does exist with .exists() and using an if in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug
Projects
None yet
Development

No branches or pull requests

3 participants