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

Broken relative links #125

Open
lemon24 opened this issue Aug 9, 2019 · 6 comments
Open

Broken relative links #125

lemon24 opened this issue Aug 9, 2019 · 6 comments

Comments

@lemon24
Copy link
Owner

lemon24 commented Aug 9, 2019

@lemon24
Copy link
Owner Author

lemon24 commented Aug 12, 2019

This:

import reader
r = reader.Reader(':memory:')
r.add_feed('https://rachelbythebay.com/w/atom.xml')
r.update_feeds()
e, = [e for e in r.get_entries() if '2019/08/01/reliability' in e.link]
print(e.content[0].value.split('<p>')[1].splitlines()[2])

import feedparser
f = feedparser.parse('https://rachelbythebay.com/w/atom.xml')  
e, = [e for e in f.entries if '2019/08/01/reliability' in e.link]    
print(e.content[0].value.split('<p>')[1].splitlines()[2])

Outputs this:

<a href="/w/2019/07/21/reliability/">put forth</a>
<a href="/w/2019/07/21/reliability/">put forth</a>

So this is from feedparser, not reader.

Next steps:

@lemon24
Copy link
Owner Author

lemon24 commented Aug 17, 2019

Installing sgmllib3k results in:

<a href="https://rachelbythebay.com/w/2019/07/21/reliability/">put forth</a>
<a href="https://rachelbythebay.com/w/2019/07/21/reliability/">put forth</a>

@lemon24
Copy link
Owner Author

lemon24 commented Aug 18, 2019

Ideally, we should pull relative link resolution out of feedparser's control and into reader's (like we did with HTTP requests). This will also allow downloading assets (images etc.) in the future.

I assume sanitization also doesn't work (it probably relies on sgmllib). This should be documented / fixed ASAP, since it is a security issue.

Update: nope, sanitization doesn't work without sgmllib; from feedparser/sgml.py:

sgmllib is not available by default in Python 3; if the end user doesn't have it available then we'll lose illformed XML parsing and content sanitizing

Next steps:

  • Check if sanitization works (without sgmllib). If it does, see if it is possible to turn it off, and force it on (also, mark all entries as stale). If it doesn't, warn in the documentation and force it off (so we get consistent results).
  • Similarly, force relative link resolution off.
  • Check why were the relative.{rss,atom} tests passing.
  • Document what feedparser features reader is using;
    • even better, be explicit about all configurable behaviors and don't leave them up to defaults.
  • Re-implement sanitization (html5lib.filters.sanitizer.Filter or Bleach should do it).
  • Re-implement relative link resolution (we'll probably need to expose the base attribute).

hrw added a commit to hrw/very-simple-planet-aggregator that referenced this issue Jan 22, 2020
On my blog I use relative urls for links and images. This makes them
proper long links.

kurtmckee/feedparser#43
lemon24/reader#125
lemon24 added a commit that referenced this issue Apr 27, 2020
@lemon24
Copy link
Owner Author

lemon24 commented Apr 27, 2020

So in the end, I made sgmllib3k a required dependency, and forced sanitization and link resolution on (commit above).

We can consider the problem fixed; the "ideally" part of the comment above can be considered a feature request.

lemon24 added a commit that referenced this issue Apr 27, 2020
@lemon24
Copy link
Owner Author

lemon24 commented Apr 28, 2020

Deploying 1.0 doesn't seem to fix it...

Update: Turns out it's update_feeds()'s fault; see #164 for details.

@lemon24
Copy link
Owner Author

lemon24 commented Dec 3, 2023

A few quick thoughts on how re-implementing sanitization would work:

  • (optional) add a .sanitized flag to data objects, default false, feedparser true
  • (optional) add a "before entry update" hook to allow a plugin to sanitize the content before it's stored
  • add a get_entries()/get_feeds() hook/plugin that lazily sanitizes attributes at runtime (noop if .sanitized == true)

Note:

  • sanitization refers strictly to html attributes
  • ideally, relative link resolution would also be part of this, since we want to avoid parsing the html twice
    • bleach seems to have some hooks out of the box https://bleach.readthedocs.io/en/latest/linkify.html
    • I need to think about how resolution works more; per summary of this issue, "relative to original page" does not work for us, we want something like
      • link (same page) -> fragment link (same page)
      • relative link to another page -> link to corresponding entry, if we have it (how do we find out?); this only makes sense in the context of the web app
      • -> external (absolute) link otherwise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant