0
0
mirror of https://github.com/wagtail/wagtail.git synced 2024-11-21 18:09:02 +01:00

Stop invalid Site hostname records from breaking preview

As made famous by https://youtu.be/v3KEaMTfKg0?t=319 :-)

By design, Wagtail tolerates the default Site record being left at its default value of 'localhost' up to a certain point. Ideally, that point should be when it becomes strictly necessary for Wagtail to care about hostnames (such as when setting up the second site of a multi-site installation) and it should be clear to the developer what has happened and how to fix it.

In practice, that point often comes when the developer deploys their site to production, sets `DEBUG=False`, and is then required by Django to set `ALLOWED_HOSTS` to their real domain name. At this point, front-end page requests work (because the initial site record is default=True, matching any domain including the live one) but previews are broken (because the dummy request object is still formed using localhost as per the site's hostname field, which is disallowed by ALLOWED_HOSTS).

This is unnecessary, and can be avoided by validating the hostname against ALLOWED_HOSTS and substituting one that _is_ allowed if necessary, as we already do for pages that don't have an associated site record.
This commit is contained in:
Matt Westcott 2024-10-24 13:59:02 +01:00 committed by Sage Abdullah
parent 6315d3c1cc
commit aa31d329ab
No known key found for this signature in database
GPG Key ID: EB1A33CC51CC0217
4 changed files with 97 additions and 11 deletions

View File

@ -5,6 +5,8 @@ Changelog
~~~~~~~~~~~~~~~~
* Add the ability to apply basic Page QuerySet optimizations to `specific()` sub-queries using `select_related` & `prefetch_related` (Andy Babic)
* Increase `DATA_UPLOAD_MAX_NUMBER_FIELDS` in project template (Matt Westcott)
* Stop invalid Site hostname records from breaking preview (Matt Westcott)
* Fix: Improve handling of translations for bulk page action confirmation messages (Matt Westcott)
* Fix: Ensure custom rich text feature icons are correctly handled when provided as a list of SVG paths (Temidayo Azeez, Joel William, LB (Ben) Johnston)
* Fix: Ensure manual edits to `StreamField` values do not throw an error (Stefan Hammer)

View File

@ -16,6 +16,7 @@ depth: 1
* Add the ability to apply basic Page QuerySet optimizations to `specific()` sub-queries using `select_related` & `prefetch_related`, see [](../reference/pages/queryset_reference.md) (Andy Babic)
* Increase `DATA_UPLOAD_MAX_NUMBER_FIELDS` in project template (Matt Westcott)
* Stop invalid Site hostname records from breaking preview (Matt Westcott)
### Bug fixes

View File

@ -7,7 +7,7 @@ from django.utils import timezone
from freezegun import freeze_time
from wagtail.admin.views.pages.preview import PreviewOnEdit
from wagtail.models import Page
from wagtail.models import Page, Site
from wagtail.test.testapp.models import (
CustomPreviewSizesPage,
EventCategory,
@ -228,6 +228,39 @@ class TestPreview(WagtailTestUtils, TestCase):
self.assertContains(response, "<li>Parties</li>")
self.assertContains(response, "<li>Holidays</li>")
def test_preview_on_create_with_incorrect_site_hostname(self):
# Failing to set a valid hostname in the Site record (as determined by ALLOWED_HOSTS)
# should not prevent the preview from being generated.
Site.objects.filter(is_default_site=True).update(hostname="bad.example.com")
preview_url = reverse(
"wagtailadmin_pages:preview_on_add",
args=("tests", "eventpage", self.home_page.id),
)
response = self.client.post(preview_url, self.post_data)
# Check the JSON response
self.assertEqual(response.status_code, 200)
self.assertJSONEqual(
response.content.decode(),
{"is_valid": True, "is_available": True},
)
# Check the user can refresh the preview
preview_session_key = "wagtail-preview-tests-eventpage-{}".format(
self.home_page.id
)
self.assertIn(preview_session_key, self.client.session)
response = self.client.get(preview_url)
# Check the HTML response
self.assertEqual(response.status_code, 200)
self.assertTemplateUsed(response, "tests/event_page.html")
self.assertContains(response, "Beach party")
self.assertContains(response, "<li>Parties</li>")
self.assertContains(response, "<li>Holidays</li>")
def test_preview_on_edit_with_m2m_field(self):
preview_url = reverse(
"wagtailadmin_pages:preview_on_edit", args=(self.event_page.id,)
@ -254,6 +287,36 @@ class TestPreview(WagtailTestUtils, TestCase):
self.assertContains(response, "<li>Parties</li>")
self.assertContains(response, "<li>Holidays</li>")
def test_preview_on_edit_with_incorrect_site_hostname(self):
# Failing to set a valid hostname in the Site record (as determined by ALLOWED_HOSTS)
# should not prevent the preview from being generated.
Site.objects.filter(is_default_site=True).update(hostname="bad.example.com")
preview_url = reverse(
"wagtailadmin_pages:preview_on_edit", args=(self.event_page.id,)
)
response = self.client.post(preview_url, self.post_data)
# Check the JSON response
self.assertEqual(response.status_code, 200)
self.assertJSONEqual(
response.content.decode(),
{"is_valid": True, "is_available": True},
)
# Check the user can refresh the preview
preview_session_key = f"wagtail-preview-{self.event_page.id}"
self.assertIn(preview_session_key, self.client.session)
response = self.client.get(preview_url)
# Check the HTML response
self.assertEqual(response.status_code, 200)
self.assertTemplateUsed(response, "tests/event_page.html")
self.assertContains(response, "Beach party")
self.assertContains(response, "<li>Parties</li>")
self.assertContains(response, "<li>Holidays</li>")
def test_preview_on_edit_with_valid_then_invalid_data(self):
preview_url = reverse(
"wagtailadmin_pages:preview_on_edit", args=(self.event_page.id,)

View File

@ -41,6 +41,7 @@ from django.db.models.expressions import OuterRef, Subquery
from django.db.models.functions import Concat, Substr
from django.dispatch import receiver
from django.http import Http404
from django.http.request import validate_host
from django.template.response import TemplateResponse
from django.urls import NoReverseMatch, reverse
from django.utils import timezone
@ -725,6 +726,26 @@ class PreviewableMixin:
handler.load_middleware()
return handler.get_response(request)
def _get_fallback_hostname(self):
"""
Return a hostname that can be used on preview requests when the object has no
routable URL, or the real hostname is not valid according to ALLOWED_HOSTS.
"""
try:
hostname = settings.ALLOWED_HOSTS[0]
except IndexError:
# Django disallows empty ALLOWED_HOSTS outright when DEBUG=False, so we must
# have DEBUG=True. In this mode Django allows localhost amongst others.
return "localhost"
if hostname == "*":
# Any hostname is allowed
return "localhost"
# Hostnames beginning with a dot are domain wildcards such as ".example.com" -
# these allow example.com itself, so just strip the dot
return hostname.lstrip(".")
def _get_dummy_headers(self, original_request=None):
"""
Return a dict of META information to be included in a faked HttpRequest object to pass to
@ -734,20 +755,19 @@ class PreviewableMixin:
if url:
url_info = urlsplit(url)
hostname = url_info.hostname
if not validate_host(
hostname,
settings.ALLOWED_HOSTS or [".localhost", "127.0.0.1", "[::1]"],
):
# The hostname is not valid according to ALLOWED_HOSTS - use a fallback
hostname = self._get_fallback_hostname()
path = url_info.path
port = url_info.port or (443 if url_info.scheme == "https" else 80)
scheme = url_info.scheme
else:
# Cannot determine a URL to this object - cobble one together based on
# whatever we find in ALLOWED_HOSTS
try:
hostname = settings.ALLOWED_HOSTS[0]
if hostname == "*":
# '*' is a valid value to find in ALLOWED_HOSTS[0], but it's not a valid domain name.
# So we pretend it isn't there.
raise IndexError
except IndexError:
hostname = "localhost"
# Cannot determine a URL to this object - cobble together an arbitrary valid one
hostname = self._get_fallback_hostname()
path = "/"
port = 80
scheme = "http"