diff --git a/media/css/core.css b/media/css/core.css
index 7d40d19b3dc..fa58300a696 100644
--- a/media/css/core.css
+++ b/media/css/core.css
@@ -1331,6 +1331,9 @@ div.module.project-subprojects li.subproject a.subproject-edit:before {
content: "\f044";
}
+#content ul.normal_list {list-style: disc; margin-left: 20px;}
+#content code {background: #eee; border: 1px solid #ccc; padding: 3px; display: inline-block;}
+
/* Pygments */
div.highlight pre .hll { background-color: #ffffcc }
div.highlight pre .c { color: #60a0b0; font-style: italic } /* Comment */
diff --git a/readthedocs/proxito/exceptions.py b/readthedocs/proxito/exceptions.py
new file mode 100644
index 00000000000..813037be633
--- /dev/null
+++ b/readthedocs/proxito/exceptions.py
@@ -0,0 +1,176 @@
+from django.http import Http404
+from django.utils.translation import pgettext_lazy
+
+_not_found_subject_translation_context = (
+ "Names a subject that was not found in a 404 error message. Used like "
+ "'The {{ not_found_subject }} you are looking for at {{ path_not_found }}
"
+ "was not found.'"
+)
+
+
+class ContextualizedHttp404(Http404):
+
+ """
+ Base class for contextualized HTTP 404 handling.
+
+ Subclasses may define their own template name,
+ HTTP status and object that was not found.
+
+ The contextualized exception is handled by proxito's 404 handler
+ """
+
+ template_name = "errors/404/base.html"
+ not_found_subject = pgettext_lazy(_not_found_subject_translation_context, "page")
+
+ def __init__(self, http_status=404, path_not_found=None, **kwargs):
+ """
+ Constructor that all subclasses should call.
+
+ :param kwargs: all kwargs are added as page context for rendering the 404 template
+ :param http_status: 404 view should respect this and set the HTTP status.
+ :param path_not_found: Inform the template and 404 view about a different path from
+ request.path
+ """
+ self.http_status = http_status
+ self.path_not_found = path_not_found
+ self.kwargs = kwargs
+
+ def get_context(self):
+ c = {
+ "not_found_subject": self.not_found_subject,
+ "path_not_found": self.path_not_found,
+ }
+ c.update(self.kwargs)
+ return c
+
+
+class DomainDNSHttp404(ContextualizedHttp404):
+
+ """Raised if a DNS record points to us and we don't know the domain."""
+
+ template_name = "errors/404/dns.html"
+ not_found_subject = pgettext_lazy(
+ _not_found_subject_translation_context, "matching DNS record"
+ )
+
+ def __init__(self, domain, **kwargs):
+ """
+ Raised when DNS for a custom domain is bad.
+
+ :param domain: The domain for which DNS is misconfigured.
+ :param kwargs:
+ """
+ kwargs["domain"] = domain
+ super().__init__(**kwargs)
+
+
+class ProjectHttp404(ContextualizedHttp404):
+
+ """
+ Raised if a domain did not resolve to a project.
+
+ This is currently used very broadly.
+ It indicates a number of reasons for the user.
+ """
+
+ template_name = "errors/404/no_project.html"
+ not_found_subject = pgettext_lazy(_not_found_subject_translation_context, "project")
+
+ def __init__(self, domain, **kwargs):
+ """
+ Raised when a project wasn't found for a given domain.
+
+ :param domain: The domain (custom and hosted) that was not found.
+ :param kwargs:
+ """
+ kwargs["domain"] = domain
+ super().__init__(**kwargs)
+
+
+class SubprojectHttp404(ContextualizedHttp404):
+
+ """Raised if a subproject was not found."""
+
+ template_name = "errors/404/no_subproject.html"
+ not_found_subject = pgettext_lazy(
+ "Names an object not found in a 404 error", "subproject"
+ )
+
+ def __init__(self, project, **kwargs):
+ """
+ Raised if a subproject was not found.
+
+ :param project: The project in which the subproject could not be found
+ :param kwargs: Context dictionary of the rendered template
+ """
+ kwargs["project"] = project
+ super().__init__(**kwargs)
+
+
+class ProjectFilenameHttp404(ContextualizedHttp404):
+
+ """Raised if a page inside an existing project was not found."""
+
+ template_name = "errors/404/no_project_page.html"
+ not_found_subject = pgettext_lazy(
+ _not_found_subject_translation_context, "documentation page"
+ )
+
+ def __init__(self, project, **kwargs):
+ """
+ Raised if a page inside an existing project was not found.
+
+ :param project: The project in which the file could not be found
+ :param kwargs: Context dictionary of the rendered template
+ """
+ kwargs["project"] = project
+ super().__init__(**kwargs)
+
+
+class ProjectTranslationHttp404(ContextualizedHttp404):
+
+ """
+ Raised if a translation of a project was not found.
+
+ This means that the project does not exist for requested language.
+ If a page isn't found, raise a ProjectPageHttp404.
+ """
+
+ template_name = "errors/404/no_language.html"
+ not_found_subject = pgettext_lazy(
+ "Names an object not found in a 404 error", "translation"
+ )
+
+ def __init__(self, project, **kwargs):
+ """
+ Raised if a translation of a project was not found.
+
+ :param project: The project in which the translation could not be found
+ :param kwargs: Context dictionary of the rendered template
+ """
+ kwargs["project"] = project
+ super().__init__(**kwargs)
+
+
+class ProjectVersionHttp404(ContextualizedHttp404):
+
+ """
+ Raised if a version was not found.
+
+ Note: The containing project can be a subproject.
+ """
+
+ template_name = "errors/404/no_version.html"
+ not_found_subject = pgettext_lazy(
+ _not_found_subject_translation_context, "documentation version"
+ )
+
+ def __init__(self, project, **kwargs):
+ """
+ Raised if a version was not found.
+
+ :param project: The project in which the version could not be found
+ :param kwargs: Context dictionary of the rendered template
+ """
+ kwargs["project"] = project
+ super().__init__(**kwargs)
diff --git a/readthedocs/proxito/middleware.py b/readthedocs/proxito/middleware.py
index d41b1b7bc72..85881d0fbe7 100644
--- a/readthedocs/proxito/middleware.py
+++ b/readthedocs/proxito/middleware.py
@@ -12,8 +12,7 @@
import structlog
from django.conf import settings
from django.core.exceptions import SuspiciousOperation
-from django.http import Http404
-from django.shortcuts import redirect, render
+from django.shortcuts import redirect
from django.urls import reverse
from django.utils.deprecation import MiddlewareMixin
@@ -30,6 +29,8 @@
from readthedocs.proxito.cache import add_cache_tags, cache_response, private_response
from readthedocs.proxito.redirects import redirect_to_https
+from .exceptions import DomainDNSHttp404, ProjectHttp404
+
log = structlog.get_logger(__name__)
@@ -212,20 +213,23 @@ def process_request(self, request): # noqa
unresolved_domain = unresolver.unresolve_domain_from_request(request)
except SuspiciousHostnameError as exc:
log.warning("Weird variation on our hostname.", domain=exc.domain)
- return render(
- request,
- "core/dns-404.html",
- context={"host": exc.domain},
- status=400,
+ # Raise a contextualized 404 that will be handled by proxito's 404 handler
+ raise DomainDNSHttp404(
+ http_status=400,
+ domain=exc.domain,
)
- except (InvalidSubdomainError, InvalidExternalDomainError):
+ except (InvalidSubdomainError, InvalidExternalDomainError) as exc:
log.debug("Invalid project set on the subdomain.")
- raise Http404
+ # Raise a contextualized 404 that will be handled by proxito's 404 handler
+ raise ProjectHttp404(
+ domain=exc.domain,
+ )
except InvalidCustomDomainError as exc:
# Some person is CNAMEing to us without configuring a domain - 404.
log.debug("CNAME 404.", domain=exc.domain)
- return render(
- request, "core/dns-404.html", context={"host": exc.domain}, status=404
+ # Raise a contextualized 404 that will be handled by proxito's 404 handler
+ raise DomainDNSHttp404(
+ domain=exc.domain,
)
except InvalidXRTDSlugHeaderError:
raise SuspiciousOperation("Invalid X-RTD-Slug header.")
diff --git a/readthedocs/proxito/tests/test_middleware.py b/readthedocs/proxito/tests/test_middleware.py
index c43547d07c9..b637782283b 100644
--- a/readthedocs/proxito/tests/test_middleware.py
+++ b/readthedocs/proxito/tests/test_middleware.py
@@ -4,7 +4,7 @@
import pytest
from django.core.exceptions import SuspiciousOperation
from django.http import HttpRequest
-from django.test import TestCase
+from django.test import Client, TestCase
from django.test.utils import override_settings
from django_dynamic_fixture import get
@@ -12,6 +12,7 @@
from readthedocs.projects.constants import PUBLIC
from readthedocs.projects.models import Domain, Feature, Project, ProjectRelationship
from readthedocs.proxito.constants import RedirectType
+from readthedocs.proxito.exceptions import DomainDNSHttp404
from readthedocs.proxito.middleware import ProxitoMiddleware
from readthedocs.rtd_tests.base import RequestFactoryTestMixin
from readthedocs.rtd_tests.storage import BuildMediaFileSystemStorageTest
@@ -141,9 +142,18 @@ def test_proper_cname_uppercase(self):
def test_invalid_cname(self):
self.assertFalse(Domain.objects.filter(domain='my.host.com').exists())
request = self.request(method='get', path=self.url, HTTP_HOST='my.host.com')
- r = self.run_middleware(request)
- # We show the 404 error page
- self.assertContains(r, 'my.host.com', status_code=404)
+
+ with self.assertRaises(DomainDNSHttp404) as cm:
+ self.run_middleware(request)
+
+ assert cm.exception.http_status == 404
+
+ # test all the exception handling combined
+ client_without_exception_handling = Client(raise_request_exception=False)
+ r = client_without_exception_handling.request(
+ method="get", path=self.url, HTTP_HOST="my.host.com"
+ )
+ assert r.status_code == 404
def test_proper_subdomain(self):
request = self.request(method='get', path=self.url, HTTP_HOST='pip.dev.readthedocs.io')
@@ -196,8 +206,17 @@ def test_request_header_not_allowed(self):
def test_long_bad_subdomain(self):
domain = 'www.pip.dev.readthedocs.io'
request = self.request(method='get', path=self.url, HTTP_HOST=domain)
- res = self.run_middleware(request)
- self.assertEqual(res.status_code, 400)
+ with self.assertRaises(DomainDNSHttp404) as cm:
+ self.run_middleware(request)
+
+ assert cm.exception.http_status == 400
+
+ # test all the exception handling combined
+ client_without_exception_handling = Client(rause_request_exception=False)
+ r = client_without_exception_handling.request(
+ method="get", path=self.url, HTTP_HOST=domain
+ )
+ assert r.status_code == 400
def test_front_slash(self):
domain = 'pip.dev.readthedocs.io'
diff --git a/readthedocs/proxito/views/decorators.py b/readthedocs/proxito/views/decorators.py
index 41472b64f6a..c44051cd0e9 100644
--- a/readthedocs/proxito/views/decorators.py
+++ b/readthedocs/proxito/views/decorators.py
@@ -20,7 +20,7 @@ def map_subproject_slug(view_func):
@wraps(view_func)
def inner_view( # noqa
- request, subproject=None, subproject_slug=None, *args, **kwargs
+ 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
diff --git a/readthedocs/proxito/views/serve.py b/readthedocs/proxito/views/serve.py
index 8e42cc0314b..c14a5b0b400 100644
--- a/readthedocs/proxito/views/serve.py
+++ b/readthedocs/proxito/views/serve.py
@@ -27,16 +27,22 @@
from readthedocs.projects.models import Domain, Feature
from readthedocs.projects.templatetags.projects_tags import sort_version_aware
from readthedocs.proxito.constants import RedirectType
+from readthedocs.proxito.exceptions import (
+ ContextualizedHttp404,
+ ProjectFilenameHttp404,
+ ProjectTranslationHttp404,
+ ProjectVersionHttp404,
+)
from readthedocs.proxito.redirects import canonical_redirect
-from readthedocs.redirects.exceptions import InfiniteRedirectException
-from readthedocs.storage import build_media_storage
-
-from .mixins import (
+from readthedocs.proxito.views.mixins import (
InvalidPathError,
ServeDocsMixin,
ServeRedirectMixin,
StorageFileNotFound,
)
+from readthedocs.redirects.exceptions import InfiniteRedirectException
+from readthedocs.storage import build_media_storage
+
from .utils import _get_project_data_from_request
log = structlog.get_logger(__name__) # noqa
@@ -138,7 +144,13 @@ def get(
original_version_slug = version_slug
version_slug = self.get_version_from_host(request, version_slug)
- final_project, lang_slug, version_slug, filename = _get_project_data_from_request( # noqa
+
+ (
+ final_project,
+ lang_slug,
+ version_slug,
+ filename,
+ ) = _get_project_data_from_request( # noqa
request,
project_slug=project_slug,
subproject_slug=subproject_slug,
@@ -240,7 +252,7 @@ def get(
'Invalid URL for project with versions.',
filename=filename,
)
- raise Http404('Invalid URL for project with versions')
+ raise Http404("Invalid URL for project with versions")
redirect_path, http_status = self.get_redirect(
project=final_project,
@@ -322,6 +334,9 @@ def get_using_unresolver(self, request):
if unresolved_domain.is_from_external_domain:
self.version_type = EXTERNAL
+ # 404 errors aren't contextualized here because all 404s use the internal nginx redirect,
+ # where the path will be 'unresolved' again when handling the 404 error
+ # See: ServeError404Base
try:
unresolved = unresolver.unresolve_path(
unresolved_domain=unresolved_domain,
@@ -497,13 +512,18 @@ def get(self, request, proxito_path, template_name="404.html"):
version_slug = kwargs.get('version_slug')
version_slug = self.get_version_from_host(request, version_slug)
- final_project, lang_slug, version_slug, filename = _get_project_data_from_request( # noqa
+ (
+ final_project,
+ lang_slug,
+ version_slug,
+ filename,
+ ) = _get_project_data_from_request( # noqa
request,
- project_slug=kwargs.get('project_slug'),
- subproject_slug=kwargs.get('subproject_slug'),
- lang_slug=kwargs.get('lang_slug'),
+ project_slug=kwargs.get("project_slug"),
+ subproject_slug=kwargs.get("subproject_slug"),
+ lang_slug=kwargs.get("lang_slug"),
version_slug=version_slug,
- filename=kwargs.get('filename', ''),
+ filename=kwargs.get("filename", ""),
)
log.bind(
@@ -566,7 +586,7 @@ def get(self, request, proxito_path, template_name="404.html"):
if response:
return response
- raise Http404('No custom 404 page found.')
+ raise Http404("No custom 404 page found.")
def _register_broken_link(self, project, version, path, full_path):
try:
@@ -731,6 +751,9 @@ def get_using_unresolver(self, request, path):
# Try to map the current path to a project/version/filename.
# If that fails, we fill the variables with the information we have
# available in the exceptions.
+
+ contextualized_404_class = ContextualizedHttp404
+
try:
unresolved = unresolver.unresolve_path(
unresolved_domain=unresolved_domain,
@@ -742,21 +765,26 @@ def get_using_unresolver(self, request, path):
filename = unresolved.filename
lang_slug = project.language
version_slug = version.slug
+ contextualized_404_class = ProjectFilenameHttp404
except VersionNotFoundError as exc:
project = exc.project
lang_slug = project.language
version_slug = exc.version_slug
filename = exc.filename
+ contextualized_404_class = ProjectVersionHttp404
except TranslationNotFoundError as exc:
project = exc.project
lang_slug = exc.language
version_slug = exc.version_slug
filename = exc.filename
+ contextualized_404_class = ProjectTranslationHttp404
except InvalidExternalVersionError as exc:
project = exc.project
+ # TODO: Use a contextualized 404
except InvalidPathForVersionedProjectError as exc:
project = exc.project
filename = exc.path
+ # TODO: Use a contextualized 404
log.bind(
project_slug=project.slug,
@@ -819,7 +847,14 @@ def get_using_unresolver(self, request, path):
)
if response:
return response
- raise Http404("No custom 404 page found.")
+
+ # No custom 404 page, use our contextualized 404 response
+ # Several additional context variables can be added if the templates
+ # or other error handling is developed (version, language, filename).
+ raise contextualized_404_class(
+ project=project,
+ path_not_found=path,
+ )
class ServeError404(SettingsOverrideObject):
@@ -1015,7 +1050,7 @@ def changefreqs_generator():
only_active=True,
)
if not public_versions.exists():
- raise Http404
+ raise Http404()
sorted_versions = sort_version_aware(public_versions)
diff --git a/readthedocs/proxito/views/utils.py b/readthedocs/proxito/views/utils.py
index 77cb58d7fdb..2185719e6e0 100644
--- a/readthedocs/proxito/views/utils.py
+++ b/readthedocs/proxito/views/utils.py
@@ -4,6 +4,7 @@
from django.http import HttpResponse
from django.shortcuts import get_object_or_404, render
+from ..exceptions import ContextualizedHttp404
from .decorators import map_project_slug, map_subproject_slug
log = structlog.get_logger(__name__) # noqa
@@ -19,33 +20,53 @@ def fast_404(request, *args, **kwargs):
return HttpResponse("Not Found.", status=404)
-def proxito_404_page_handler(request, exception=None, template_name='404.html'):
+def proxito_404_page_handler(
+ request, template_name="errors/404/base.html", exception=None
+):
"""
- Decide what 404 page return depending if it's an internal NGINX redirect.
+ Serves a 404 error message, handling 404 exception types raised throughout the app.
We want to return fast when the 404 is used as an internal NGINX redirect to
reach our ``ServeError404`` view. However, if the 404 exception was risen
- inside ``ServeError404`` view, we want to render the default Read the Docs
- Maze page.
+ inside ``ServeError404`` view, we want to render a useful HTML response.
"""
+ # 404 exceptions that don't originate from our proxito 404 handler should have a fast response
+ # with no HTML rendered, since they will be forwarded to our 404 handler again.
if request.resolver_match and request.resolver_match.url_name != 'proxito_404_handler':
return fast_404(request, exception, template_name)
- resp = render(request, template_name)
- resp.status_code = 404
- return resp
+ context = {}
+ http_status = 404
+
+ # Contextualized 404 exceptions:
+ # Context is defined by the views that raise these exceptions and handled
+ # in their templates.
+ if isinstance(exception, ContextualizedHttp404):
+ context.update(exception.get_context())
+ template_name = exception.template_name
+ http_status = exception.http_status
+
+ context["path_not_found"] = context.get("path_not_found") or request.path
+
+ r = render(
+ request,
+ template_name,
+ context=context,
+ )
+ r.status_code = http_status
+ return r
@map_project_slug
@map_subproject_slug
def _get_project_data_from_request(
- request,
- project,
- subproject,
- lang_slug=None,
- version_slug=None,
- filename='',
+ request,
+ project,
+ subproject,
+ lang_slug=None,
+ version_slug=None,
+ filename="",
):
"""
Get the proper project based on the request and URL.
diff --git a/readthedocs/templates/401.html b/readthedocs/templates/401.html
index d9d818be0da..b5db786a6df 100644
--- a/readthedocs/templates/401.html
+++ b/readthedocs/templates/401.html
@@ -1,4 +1,4 @@
-{% extends "base.html" %}
+{% extends "errors/base.html" %}
{% load core_tags %}
{% load i18n %}
@@ -6,15 +6,6 @@
{% trans "Maze Found" %}
{% endblock %}
-{% block header-wrapper %}
- {% include "error_header.html" %}
-{% endblock %}
-
-{% block notify %}{% endblock %}
-
-{# Hide the language select form so we don't set a CSRF cookie #}
-{% block language-select-form %}{% endblock %}
-
{% block content %}
diff --git a/readthedocs/templates/404.html b/readthedocs/templates/404.html index 3dfa2fd6268..81bb1261bc4 100644 --- a/readthedocs/templates/404.html +++ b/readthedocs/templates/404.html @@ -1,48 +1,2 @@ -{% extends "base.html" %} -{% load core_tags %} -{% load i18n %} - -{% block title %} - {% trans "Maze Found" %} -{% endblock %} - -{% block header-wrapper %} - {% include "error_header.html" %} -{% endblock %} - -{% block notify %}{% endblock %} - -{# Hide the language select form so we don't set a CSRF cookie #} -{% block language-select-form %}{% endblock %} - -{% block content %} -
{% blocktrans context "404 page" %}This is a 404 error page. The link you have followed or the URL that you entered does not exist. It might be that the documentation has been updated, or this page has been removed.{% endblocktrans %}
- -{% blocktrans with site_name=request.get_host|default:_("this site") context "404 page" %}Back to the front page of {{ site_name }}{% endblocktrans %} - -- - \ SORRY / - \ / - \ This page does / - ] not exist yet. [ ,'| - ] [ / | - ]___ ___[ ,' | - ] ]\ /[ [ |: | - ] ] \ / [ [ |: | - ] ] ] [ [ [ |: | - ] ] ]__ __[ [ [ |: | - ] ] ] ]\ _ /[ [ [ [ |: | - ] ] ] ] (#) [ [ [ [ :====' - ] ] ]_].nHn.[_[ [ [ - ] ] ] HHHHH. [ [ [ - ] ] / `HH("N \ [ [ - ]__]/ HHH " \[__[ - ] NNN [ - ] N/" [ - ] N H [ - / N \ - / q, \ - / \ --{% endblock %} +{% extends "errors/404/base.html" %} +{# this is the default template, used by Django's default 404 handler. Proxito has its own 404 view, but the main application doesn't (yet) #} diff --git a/readthedocs/templates/429.html b/readthedocs/templates/429.html index b7c79fd8c16..600a2b3ff7a 100644 --- a/readthedocs/templates/429.html +++ b/readthedocs/templates/429.html @@ -1,4 +1,4 @@ -{% extends "base.html" %} +{% extends "errors/base.html" %} {% load core_tags %} {% load i18n %} @@ -6,15 +6,6 @@ {% trans "Too many requests" %} {% endblock %} -{% block header-wrapper %} - {% include "error_header.html" %} -{% endblock %} - -{% block notify %}{% endblock %} - -{# Hide the language select form so we don't set a CSRF cookie #} -{% block language-select-form %}{% endblock %} - {% block content %}
.--~~,__ diff --git a/readthedocs/templates/500.html b/readthedocs/templates/500.html index 70693b5ea3a..381ca625572 100644 --- a/readthedocs/templates/500.html +++ b/readthedocs/templates/500.html @@ -1,19 +1,10 @@ -{% extends "base.html" %} +{% extends "errors/base.html" %} {% load i18n %} {% block title %} {% trans "Server Error" %} {% endblock %} -{% block header-wrapper %} - {% include "error_header.html" %} -{% endblock %} - -{% block notify %}{% endblock %} - -{# Hide the language select form so we don't set a CSRF cookie #} -{% block language-select-form %}{% endblock %} - {% block content %}. diff --git a/readthedocs/templates/core/widesearchbar.html b/readthedocs/templates/core/widesearchbar.html index d310e94b5e9..7950c8096cd 100644 --- a/readthedocs/templates/core/widesearchbar.html +++ b/readthedocs/templates/core/widesearchbar.html @@ -1,13 +1,14 @@ {% load i18n %} + {% if not term|default_if_none:False %}{% trans "Search all the docs" %}
- + {% endif %}