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

Allow custom 404.html on projects #5130

Merged
merged 17 commits into from
Feb 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,18 @@
# Activate autosectionlabel plugin
autosectionlabel_prefix_document = True

# sphinx-notfound-page
# https://github.com/rtfd/sphinx-notfound-page
notfound_context = {
'body': '''
<h1>Page not found</h1>

<p>Sorry, we couldn't find that page.</p>

<p>Try using the search box or go to the homepage.</p>
''',
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just for the future, right? We need to add the extension if we want it to do anything.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just of our docs to start testing it.

The extension is installed in our requirements.

I can remove it if you consider that it's best to start testing this in another dumb project, though.


def setup(app):
app.add_stylesheet('css/sphinx_prompt_css.css')
91 changes: 90 additions & 1 deletion readthedocs/core/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,23 @@
from __future__ import division
import os
import logging
from urllib.parse import urlparse

from django.conf import settings
from django.http import HttpResponseRedirect, Http404, JsonResponse
from django.shortcuts import render, get_object_or_404, redirect
from django.views.generic import TemplateView


from readthedocs.builds.models import Version
from readthedocs.core.resolver import resolve_path
from readthedocs.core.symlink import PrivateSymlink, PublicSymlink
from readthedocs.core.utils import broadcast
from readthedocs.core.views.serve import _serve_file
from readthedocs.projects.constants import PRIVATE
from readthedocs.projects.models import Project, ImportedFile
from readthedocs.projects.tasks import remove_dirs
from readthedocs.redirects.utils import get_redirect_response
from readthedocs.redirects.utils import get_redirect_response, project_and_path_from_request, language_and_version_from_path

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -115,6 +121,7 @@ def server_error_404(request, exception=None, template_name='404.html'): # pyli
"""
response = get_redirect_response(request, full_path=request.get_full_path())

# Return a redirect response if there is one
if response:
if response.url == request.build_absolute_uri():
# check that we do have a response and avoid infinite redirect
Expand All @@ -124,6 +131,88 @@ def server_error_404(request, exception=None, template_name='404.html'): # pyli
)
else:
return response

# Try to serve custom 404 pages if it's a subdomain/cname
if getattr(request, 'subdomain', False) or getattr(request, 'cname', False):
return server_error_404_subdomain(request, template_name)

# Return the default 404 page generated by Read the Docs
r = render(request, template_name)
r.status_code = 404
return r


def server_error_404_subdomain(request, template_name='404.html'):
"""
Handler for 404 pages on subdomains.

Check if the project associated has a custom ``404.html`` and serve this
page. First search for a 404 page in the current version, then continues
with the default version and finally, if none of them are found, the Read
the Docs default page (Maze Found) is rendered by Django and served.
"""

def resolve_404_path(project, version_slug=None, language=None):
"""
Helper to resolve the path of ``404.html`` for project.

The resolution is based on ``project`` object, version slug and
language.

:returns: tuple containing the (basepath, filename)
:rtype: tuple
"""
filename = resolve_path(
project,
version_slug=version_slug,
language=language,
filename='404.html',
subdomain=True, # subdomain will make it a "full" path without a URL prefix
)

# This breaks path joining, by ignoring the root when given an "absolute" path
if filename[0] == '/':
filename = filename[1:]

version = None
if version_slug:
version = project.versions.get(slug=version_slug)

private = any([
version and version.privacy_level == PRIVATE,
not version and project.privacy_level == PRIVATE,
])
if private:
symlink = PrivateSymlink(project)
else:
symlink = PublicSymlink(project)
basepath = symlink.project_root
fullpath = os.path.join(basepath, filename)
return (basepath, filename, fullpath)

project, full_path = project_and_path_from_request(request, request.get_full_path())

language = None
version_slug = None
schema, netloc, path, params, query, fragments = urlparse(full_path)
if not project.single_version:
language, version_slug, path = language_and_version_from_path(path)

# Firstly, attempt to serve the 404 of the current version (version_slug)
# Secondly, try to serve the 404 page for the default version (project.get_default_version())
for slug in (version_slug, project.get_default_version()):
basepath, filename, fullpath = resolve_404_path(project, slug, language)
if os.path.exists(fullpath):
log.debug(
'serving 404.html page current version: [project: %s] [version: %s]',
project.slug,
slug,
)
r = _serve_file(request, filename, basepath)
r.status_code = 404
return r

# Finally, return the default 404 page generated by Read the Docs
r = render(request, template_name)
r.status_code = 404
return r
Expand Down
17 changes: 17 additions & 0 deletions readthedocs/core/views/serve.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,23 @@ def _serve_401(request, project):


def _serve_file(request, filename, basepath):
"""
Serve media file via Django or NGINX based on ``PYTHON_MEDIA``.

When using ``PYTHON_MEDIA=True`` (or when ``DEBUG=True``) the file is served
by ``django.views.static.serve`` function.

On the other hand, when ``PYTHON_MEDIA=False`` the file is served by using
``X-Accel-Redirect`` header for NGINX to take care of it and serve the file.

:param request: Django HTTP request
:param filename: path to the filename to be served relative to ``basepath``
:param basepath: base path to prepend to the filename

:returns: Django HTTP response object

:raises: ``Http404`` on ``UnicodeEncodeError``
"""
# Serve the file from the proper location
if settings.DEBUG or getattr(settings, 'PYTHON_MEDIA', False):
# Serve from Python
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/doc_builder/python_environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ def install_core_requirements(self):
cmd.extend(requirements)
self.build_env.run(
*cmd,
cwd=self.checkout_path # noqa - no comma here in py27 :/
cwd=self.checkout_path,
)

pip_cmd = [
Expand Down
33 changes: 32 additions & 1 deletion readthedocs/rtd_tests/tests/test_doc_serving.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,17 @@

import django_dynamic_fixture as fixture
import mock
import os
from django.conf import settings
from django.contrib.auth.models import User
from django.http import Http404
from django.test import TestCase
from django.test import TestCase, RequestFactory
from django.test.utils import override_settings
from django.urls import reverse
from mock import mock_open, patch

from readthedocs.core.middleware import SubdomainMiddleware
from readthedocs.core.views import server_error_404_subdomain
from readthedocs.core.views.serve import _serve_symlink_docs
from readthedocs.projects import constants
from readthedocs.projects.models import Project
Expand Down Expand Up @@ -169,3 +172,31 @@ def test_custom_robots_txt(self, os_mock, open_mock):
)
self.assertEqual(response.status_code, 200)
self.assertEqual(response.content, b'My own robots.txt')

@override_settings(
PYTHON_MEDIA=False,
USE_SUBDOMAIN=True,
PUBLIC_DOMAIN='readthedocs.io',
ROOT_URLCONF=settings.SUBDOMAIN_URLCONF,
)
@patch('readthedocs.core.views.serve.os')
@patch('readthedocs.core.views.os')
def test_custom_404_page(self, os_view_mock, os_serve_mock):
os_view_mock.path.exists.return_value = True

os_serve_mock.path.join.side_effect = os.path.join
os_serve_mock.path.exists.return_value = True

self.public.versions.update(active=True, built=True)

factory = RequestFactory()
request = factory.get(
'/en/latest/notfoundpage.html',
HTTP_HOST='public.readthedocs.io',
)

middleware = SubdomainMiddleware()
middleware.process_request(request)
response = server_error_404_subdomain(request)
self.assertEqual(response.status_code, 404)
self.assertTrue(response['X-Accel-Redirect'].endswith('/public/en/latest/404.html'))
3 changes: 2 additions & 1 deletion requirements/local-docs-build.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
-r pip.txt

# Base packages
# Base packages
docutils==0.14
Sphinx==1.8.3
sphinx_rtd_theme==0.4.2
Expand All @@ -17,5 +17,6 @@ Markdown==3.0.1
# Docs
sphinxcontrib-httpdomain==1.7.0
sphinx-prompt==1.0.0
sphinx-notfound-page==0.1
commonmark==0.8.1
recommonmark==0.5.0