Opened 9 years ago

Closed 9 years ago

Last modified 9 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@… 9 years ago.
patch
patch-5855-2.diff (1.0 KB) - added by Niran Babalola <niran@…> 9 years ago.
revised patch

Download all attachments as: .zip

Change History (9)

Changed 9 years ago by niran@…

Attachment: patch-5855.diff added

patch

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

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedReady for checkin

comment:2 Changed 9 years ago by Malcolm Tredinnick

Resolution: wontfix
Status: newclosed

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 9 years ago by Niran Babalola <niran@…>

Resolution: wontfix
Status: closedreopened
Triage Stage: Ready for checkinUnreviewed

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

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

Attachment: patch-5855-2.diff added

revised patch

comment:4 Changed 9 years ago by Malcolm Tredinnick

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 9 years ago by MichaelBishop

Triage Stage: UnreviewedDesign decision needed

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

comment:6 Changed 9 years ago by Malcolm Tredinnick

Resolution: fixed
Status: reopenedclosed

(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 9 years ago by Malcolm Tredinnick

(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