Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#31330 closed Bug (fixed)

Trailing slash is missing in a "catchall" pattern in flatpages URLconf example.

Reported by: Bryant Glisson Owned by: Hasan Ramezani
Component: Documentation Version: 3.0
Severity: Normal Keywords: append_slash, flatpage, catchall
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

We would like to append slashes to all URLs that do not have them, and we would also like to have a catchall flatpage expression, so our editors can enter whatever they like in that area of the CMS. The problem is that the flatpage validator adds the slash, but then only checks to see if a Flatpage exists for the resultant path (https://github.com/django/django/blob/master/django/contrib/flatpages/views.py#L41), whereas the is_valid_path method (https://github.com/django/django/blob/b9cf764be62e77b4777b3a75ec256f6209a57671/django/urls/base.py#L150) checks the urlconf, of which the catchall is a part, returns True, because the non-slash url fits the catchall. Hence, it does not redirect to the slashed version, making it impossible to reach and resulting in a 404.

Again, here is the flow:

  1. Attempt to reach "/my-valid-page-without-a-slash"
  2. is_valid_path returns True because "/my-valid-page-without-a-slash" fits the flatpage catchall and hence does not append a slash
  3. The page is not found in Flatpage entries, resulting in 404

It seems to me that if APPEND_SLASH is set to True, then we should not be looking in the urlconf for the page without the slash, but should instead immediately append the slash, then check the urlconf, in which case the correct pattern would be found and flatpages would never be called.

Change History (15)

comment:1 by Bryant Glisson, 5 years ago

Component: Uncategorizedcontrib.flatpages
Easy pickings: set
Type: UncategorizedBug

comment:2 by Mariusz Felisiak, 5 years ago

Summary: Flatpage catchall is killing APPEND_SLASH functionality on normal pagesFlatpage catchall is killing APPEND_SLASH functionality on normal pages.

Thanks for this ticket, I think there is a regression introduced in df41b5a05d4e00e80e73afe629072e37873e767a in flatpages docs, i.e. a "catchall" pattern should contain a trailing slash when you use APPEND_SLASH:

urlpatterns += [
    path('<path:url>/', views.flatpage),
]

Can you check your app with this change?

comment:3 by Ninad Kulkarni, 5 years ago

Owner: changed from nobody to Ninad Kulkarni
Status: newassigned

I would like to work on this ticket but could someone please guide me about how to solve this issue?

comment:4 by Mariusz Felisiak, 5 years ago

Ninad, there is nothing to work on for a moment. Ticket is still not accepted, we're waiting for confirmation from the reporter that it's a documentation regression.

comment:5 by Mariusz Felisiak, 5 years ago

Component: contrib.flatpagesDocumentation
Summary: Flatpage catchall is killing APPEND_SLASH functionality on normal pages.Trailing slash is missing in a "catchall" pattern in flatpages URLconf example.
Triage Stage: UnreviewedAccepted

meesterguyman, I understand that you're not using contrib.flatpages.forms.FlatpageForm because in such case trailing slash is required when APPEND_SLASH is True. I confirmed that when you allow for URLs without a trailing slash and want to use a “catchall” pattern then a trailing slash is required in:

path('<path:url>/', views.flatpage)

It's a regression in df41b5a05d4e00e80e73afe629072e37873e767a.

comment:6 by Mariusz Felisiak, 5 years ago

Ninad, we need to add a trailing slash to flatpages URLconf example.

comment:7 by Mariusz Felisiak, 5 years ago

Owner: Ninad Kulkarni removed
Status: assignednew

comment:8 by Hasan Ramezani, 5 years ago

Owner: set to Hasan Ramezani
Status: newassigned

comment:9 by Hasan Ramezani, 5 years ago

Has patch: set
Last edited 5 years ago by Mariusz Felisiak (previous) (diff)

comment:10 by GitHub <noreply@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In a0916d7:

Fixed #31330 -- Updated flatpages URLconf example to work with APPEND_SLASH.

Regression in df41b5a05d4e00e80e73afe629072e37873e767a.

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 6831049:

[3.0.x] Fixed #31330 -- Updated flatpages URLconf example to work with APPEND_SLASH.

Regression in df41b5a05d4e00e80e73afe629072e37873e767a.
Backport of a0916d7212aaae634f4388d47d8717abc2cd9036 from master

comment:12 by Simon Barnett, 5 years ago

Resolution: fixed
Status: closednew

I think the addition of the slash causes a different problem.

Any valid page URL will match the catch all pattern, but the actual URL that is captured will not have the last slash in it. This will cause the flatpages view code to fail to find the page, add a slash to the URL and then return a permanent redirect to that URL with an extra slash on the end of it. So if the URL is '/someurl/', the user will be redirected to '/someurl'.

I think the last slash needs to be inside the captured part of the URL, which can be done with re_path, like this:-

re_path('^(?P<url>.+/)$', views.flatpage)
Version 0, edited 5 years ago by Simon Barnett (next)

comment:13 by Mariusz Felisiak, 5 years ago

Triage Stage: AcceptedReady for checkin

Simon, Thanks.

PR

comment:14 by Carlton Gibson <carlton.gibson@…>, 5 years ago

Resolution: fixed
Status: newclosed

In 8f2a6c76:

Fixed #31330 -- Corrected catchall URL pattern in flatpages docs.

Use re_path() pattern with the regex used before original regression in
df41b5a05d4e00e80e73afe629072e37873e767a.
Regression in a0916d7212aaae634f4388d47d8717abc2cd9036.

comment:15 by Carlton Gibson <carlton.gibson@…>, 5 years ago

In 3bb1e6c:

[3.0.x] Fixed #31330 -- Corrected catchall URL pattern in flatpages docs.

Use re_path() pattern with the regex used before original regression in
df41b5a05d4e00e80e73afe629072e37873e767a.
Regression in a0916d7212aaae634f4388d47d8717abc2cd9036.

Backport of 8f2a6c76d19e4010c4683b20ed7f1eb4b07c17eb from master

Note: See TracTickets for help on using tickets.
Back to Top