Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix case-sensitive OPML feed parser #46

Closed
wants to merge 2 commits into from

Conversation

thcipriani
Copy link
Contributor

The OPML spec seems to be ambiguous on the subject of case-sensitivity.
As a result, OPML feeds that are exported from podcatchers may have
uppercase attributes (i.e., URL vs url). This patch will find
attributes of xml.etree.ElementTree.Element objects in a
case-insensitive way which side-steps the problem.

I ran into this importing my podcast opml from mysqueezebox.com. Adding this OPML as the browse_root caused a stack-trace in my mopidy log:

Jan 06 12:31:03 radio mopidy[458]: ERROR    PodcastBackend backend caused an exception.                                          
Jan 06 12:31:03 radio mopidy[458]: Traceback (most recent call last):                                       
Jan 06 12:31:03 radio mopidy[458]:   File "/usr/lib/python2.7/dist-packages/mopidy/core/library.py", line 19, in _backend_error_handling                                  
Jan 06 12:31:03 radio mopidy[458]:     yield                                                                            
Jan 06 12:31:03 radio mopidy[458]:   File "/usr/lib/python2.7/dist-packages/mopidy/core/library.py", line 112, in _browse
Jan 06 12:31:03 radio mopidy[458]:     result = backend.library.browse(uri).get()                                                       
Jan 06 12:31:03 radio mopidy[458]:   File "/usr/lib/python2.7/dist-packages/pykka/threading.py", line 52, in get                                                         
Jan 06 12:31:03 radio mopidy[458]:     compat.reraise(*self._data['exc_info'])                                           
Jan 06 12:31:03 radio mopidy[458]:   File "/usr/lib/python2.7/dist-packages/pykka/compat.py", line 12, in reraise        
Jan 06 12:31:03 radio mopidy[458]:     exec('raise tp, value, tb')                                              
Jan 06 12:31:03 radio mopidy[458]:   File "/usr/lib/python2.7/dist-packages/pykka/actor.py", line 201, in _actor_loop                       
Jan 06 12:31:03 radio mopidy[458]:     response = self._handle_receive(message)                                  
Jan 06 12:31:03 radio mopidy[458]:   File "/usr/lib/python2.7/dist-packages/pykka/actor.py", line 295, in _handle_receive
Jan 06 12:31:03 radio mopidy[458]:     return callee(*message['args'], **message['kwargs'])                          
Jan 06 12:31:03 radio mopidy[458]:   File "/usr/local/lib/python2.7/dist-packages/mopidy_podcast/library.py", line 68, in browse                                         
Jan 06 12:31:03 radio mopidy[458]:     return list(feed.items(self.__browse_order == 'desc'))                            
Jan 06 12:31:03 radio mopidy[458]:   File "/usr/local/lib/python2.7/dist-packages/mopidy_podcast/feeds.py", line 226, in items              
Jan 06 12:31:03 radio mopidy[458]:     yield ref(e)                                                                             
Jan 06 12:31:03 radio mopidy[458]:   File "/usr/local/lib/python2.7/dist-packages/mopidy_podcast/feeds.py", line 203, in <lambda>
Jan 06 12:31:03 radio mopidy[458]:     if e.get('url').endswith('.opml')
Jan 06 12:31:03 radio mopidy[458]: AttributeError: 'NoneType' object has no attribute 'endswith'
Jan 06 12:31:05 radio mopidy[458]: ERROR    PodcastBackend backend caused an exception.

The reason, of course, is that the url attribute in my OPML feed is actually the URL attribute.

The OPML spec seems to be ambiguous on the subject of case-sensitivity.
As a result, OPML feeds that are exported from podcatchers may have
uppercase attributes (i.e., `URL` vs `url`). This patch will find
attributes of `xml.etree.ElementTree.Element` objects in a
case-insensitive way which side-steps the problem.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 88.926% when pulling d8b5e02 on thcipriani:case-insensitive into f126d6e on tkem:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 88.926% when pulling df7fc01 on thcipriani:case-insensitive into f126d6e on tkem:master.

@tkem
Copy link
Owner

tkem commented Jan 7, 2018

Nice catch, and special thanks for adding a unit test ;-)
Could you also post your squeezebox OPML for reference? At least an excerpt with a few entries if it's too long?

@thcipriani
Copy link
Contributor Author

Sure, here's my OPML as exported from the podcast app on mysqueezebox.com

<?xml version="1.0" encoding="utf-8" ?>
<opml version="1.1">
  <head title="Podcasts">
    <expansionState></expansionState>
  </head>
  <body>
    <outline URL="http://feeds.99percentinvisible.org/99percentinvisible" text="99% Invisible" type="link" />
    <outline URL="http://www.npr.org/rss/podcast.php?id=381444908" text="Fresh Air" type="link" />
    <outline URL="http://radioproject.libsyn.com/rss" text="Making Contact" type="link" />
    <outline URL="http://feeds.wnyc.org/radiolab" text="Radiolab" type="link" />
    <outline URL="http://feeds.stownpodcast.org/stownpodcast" text="S-Town" type="link" />
    <outline URL="http://feeds.serialpodcast.org/serialpodcast" text="Serial" type="link" />
    <outline URL="http://feeds.hearstartup.com/hearstartup" text="StartUp Podcast" type="link" />
    <outline URL="http://www.npr.org/rss/rss.php?id=37" text="All Songs Considered" type="link" />
    <outline URL="http://www.econlib.org/library/EconTalk.xml" text="EconTalk" type="link" />
    <outline URL="http://feeds.thisamericanlife.org/talpodcast" text="This American Life" type="link" />
    <outline URL="http://www.thebrewingnetwork.com/feed/cybi/" text="Can You Brew It" type="link" />
    <outline URL="http://www.thebrewingnetwork.com/feed/bws/" text="Brewing With Style" type="link" />
    <outline URL="http://feeds.wnyc.org/tnypoliticalscene" text="The New Yorker: Politics and More" type="link" />
    <outline URL="http://www.npr.org/rss/podcast.php?id=510289" text="Planet Money" type="link" />
    <outline URL="http://www.bbc.co.uk/programmes/b006qykl/episodes/downloads.rss" text="In Our Time" type="link" />
    <outline URL="http://www.bbc.co.uk/programmes/b00nrtd2/episodes/downloads.rss" text="A History of the World in 100 Objects" type="link" />
    <outline URL="http://feeds.propublica.org/propublica/podcast" text="ProPublica Podcast" type="link" />
    <outline URL="http://feeds.soundcloud.com/users/soundcloud:users:70607212/sounds.rss" text="This is Hell!" type="link" />
    <outline URL="http://feeds.trumpconlaw.com/TrumpConLaw" text="What Trump Can Teach Us About Con Law" type="link" />
    <outline URL="http://feeds.gimletmedia.com/mysteryshow" text="Mystery Show" type="link" />
    <outline URL="http://feeds.earhustlesq.com/earhustlesq" text="Ear Hustle" type="link" />
    <outline URL="https://feeds.feedburner.com/RevisionistHistory" text="Revisionist History" type="link" />
    <outline URL="http://rss.art19.com/levar-burton-reads" text="LeVar Burton Reads" type="link" />
    <outline URL="http://thrillingadventurehour.libsyn.com/rss" text="Thrilling Adventure Hour" type="link" />
    <outline URL="https://feeds.podtrac.com/zKq6WZZLTlbM" text="The Daily" type="link" />
    <outline URL="http://adventurezone.libsyn.com/rss" text="The Adventure Zone" type="link" />
    <outline URL="http://feeds.megaphone.fm/watergate" text="Slow Burn: A Podcast About Watergate" type="link" />
    <outline URL="http://feeds.wnyc.org/moreperfect?format=xml" text="Radiolab Presents: More Perfect" type="link" />
  </body>
</opml>

@tkem
Copy link
Owner

tkem commented Jan 8, 2018

Thanks! I'm wondering if it wouldn't be preferable to limit the case-insensitive handling to url/URL only.
I mean, OPML is an XML format, and XML is case sensitive - no ambiguity here, IMHO ;-)
Although OPML maybe isn't specific about it, the presence of the mixed-case "xmlUrl" attribute also seems to support this.
So if you don't object, I'd just add special handling for "url" for squeezebox compatibility.

@thcipriani
Copy link
Contributor Author

Suggestion works for me. I'll close this one in favor or #47 rather than add another commit here.

@thcipriani thcipriani closed this Jan 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants