Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#5855 closed (fixed)

Parameterized feeds have incorrect 404 behavior

Reported by: niran@… Owned by: nobody
Component: contrib.syndication Version: master
Severity: Keywords:
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

When using the syndication framework to generate parameterized feeds as described in the documentation, leaving off the parameter from the URL causes an exception to be raised in the Feed's link() function.

For example, if I'm trying to generate per-tag feeds for a blog at /feeds/tags/TAG_NAME, /feeds/tags/ will cause an AttributeError in my Feed subclass' link() function because the object passed to it is None. My proposed solution, which seems to work for me, is instead of checking the contents of url in get_feed(), we should check if the Feed class (self) has a get_object function, since from what I understand, any Feed subclass with a get_object() function is intended to generate a parameterized feed.

With my patch applied, /feeds/tags/ results in a 404, which seems like the appropriate behavior to me.

Attachments (2)

patch-5855.diff (764 bytes) - added by niran@… 8 years ago.
patch
patch-5855-2.diff (1.0 KB) - added by Niran Babalola <niran@…> 7 years ago.
revised patch

Download all attachments as: .zip

Change History (9)

Changed 8 years ago by niran@…

patch

comment:1 Changed 8 years ago by Simon G <dev@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Ready for checkin

comment:2 Changed 7 years ago by mtredinnick

  • Resolution set to wontfix
  • Status changed from new to closed

This change in behaviour isn't a good idea. It would prevent having feeds that handle both parameterised and non-parameterised versions, as we currently permit (and as I'm using in code of my own, so I know it's possible).

I will update the example documentation to have better error handling, though.

comment:3 Changed 7 years ago by Niran Babalola <niran@…>

  • Resolution wontfix deleted
  • Status changed from closed to reopened
  • Triage Stage changed from Ready for checkin to Unreviewed

Okay, here's a cleaner patch that maintains the ability to handle both parameterized and non-parameterized feeds in the same class.

Changed 7 years ago by Niran Babalola <niran@…>

revised patch

comment:4 Changed 7 years ago by mtredinnick

Yes, this might be a reasonably approach. I want to think about it a bit to see if there are any drawbacks, but it might well work.

comment:5 Changed 7 years ago by MichaelBishop

  • Triage Stage changed from Unreviewed to Design decision needed

Design decision needed. Are there any drawbacks to this approach?

comment:6 Changed 7 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [7295]) Added more robust processing to parameterised syndication feeds for the case
when all the "extra" URL bits are accidentally omitted. Patch from Niran
Babalola <niran@…>. Fixed #5855.

comment:7 Changed 7 years ago by mtredinnick

(In [7328]) Merged the tests and documentation from #4720 to support the changes in [7295].
Thanks, Andy Gayton. Fixed #4720. Refs #5855.

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