Code

Opened 4 years ago

Closed 2 years ago

#12065 closed Bug (fixed)

URL clash between FlatPages causes middleware error

Reported by: emes Owned by:
Component: contrib.flatpages Version: 1.1
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Steps to reproduce:

  1. Install flatpages app, as described in the documentation.
  2. Create flatpage with URL /bar/ and assign it to site foo.
  3. Create another flatpage with URL /bar/ and also assign it to site foo.
  4. Go to http://foo/bar/

It looks FUBAR, I mean 500. This is not a good way to handle it, even if admins receive an email about exception. Why?

Usually content editors are not admins. After adding such page, they can only guess what happened and why the content doesn't appear.

The attached patch fixes it. It does two things:

  1. Validates flatpage when saved through admin panel. If there is an URL clash for some of the sites, validation errors are generated for 'sites' field.
  2. For installations where a clash has already occured and is present in the database, it just serves the first page found by query. It may be misleading to hide the error, but I assume that (i) it's better to serve content than 500 to end user and (ii) any doubt would eventually cause content editor to go to the admin panel and try to edit the page, running the new validation.

Attachments (2)

django-flatpages-url_clash.patch (2.2 KB) - added by emes 4 years ago.
admin.patch (1.0 KB) - added by ryazwinski 4 years ago.
patch to admin only to disallow url collisions - view continues to throw exception

Download all attachments as: .zip

Change History (13)

Changed 4 years ago by emes

comment:1 Changed 4 years ago by russellm

  • Has patch set
  • milestone set to 1.2
  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

It would be good if this was validated at the model level, rather than just in the admin form.

comment:2 Changed 4 years ago by ryazwinski

  • Owner changed from nobody to ryazwinski

comment:3 Changed 4 years ago by ryazwinski

This ticket has spawned ticket 12938 to for general validation of models with many-to-many fields.

Changed 4 years ago by ryazwinski

patch to admin only to disallow url collisions - view continues to throw exception

comment:4 Changed 4 years ago by ryazwinski

In discussions today with Joseph and others decision was made to leave the view throwing a 500 when the database is in an inconsistent state that allows for URL collisions, but to apply the portion of the patch that stops users from corrupting the data from the admin panel.

Hoping to get tests sorted out soon. Currently no tests exist at all for flatpages middleware.

comment:5 Changed 4 years ago by ryazwinski

  • Owner ryazwinski deleted

After spending way too many hours beating on testing for flatpages, and not making very much progress, I'm going to give up. Hopefully someone else can sort out how to test the admin panel and the view. Sorry :(

comment:6 Changed 4 years ago by russellm

  • milestone 1.2 deleted

Not critical for 1.2.

comment:7 Changed 4 years ago by russellm

  • milestone set to 1.3

comment:8 Changed 3 years ago by gabrielhurley

  • Component changed from Contrib apps to contrib.flatpages

comment:9 Changed 3 years ago by julien

  • Severity set to Normal
  • Type set to Bug

comment:10 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

comment:11 Changed 2 years ago by claudep

  • Easy pickings unset
  • Resolution set to fixed
  • Status changed from new to closed
  • UI/UX unset

Same issue now resolved in #14678.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.