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

plugins.rules: URLCallback shall handle invalid URLs by ignoring them #2086

Merged
merged 3 commits into from
Jun 11, 2021

Conversation

dgw
Copy link
Member

@dgw dgw commented Jun 5, 2021

Description

Invalid URLs can crash Sopel's URL handling, as reported on IRC by @RhinosF1. The traceback is included below for reference.

We handle it now by simply skipping any URL that causes the underlying library (urllib.parse from Python's standard library) to throw a ValueError when plugins.rules.URLCallback.match() tries to determine its scheme.

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
  • I have tested the functionality of the things this change touches

Notes

Includes a new test verifying the behavior—and more importantly, the lack of an exception being thrown.

Also fixes an incorrect classname in one of the other URLCallback tests, in a separate commit.

Motivating traceback, reported on IRC

[2021-06-05 10:03:52,811] sopel.exceptions     ERROR    - Fatal error in core, handle_error() was called.
Buffer:

Last Line:
@account=mirahezebots :MirahezeRC!~MirahezeR@miraheze/bots PRIVMSG #miraheze-feed :famepediawiki 5* 14[[07Yasir Tech14]]4 !N10 02https://en.famepedia.org/w/index.php?oldid=35752&rcid=23962 5* 03YasirTech 5* (+6564) 10Created page with "{{Short description}}'''Yasir Tech''' '''(یاسر ٹیک)(यासिर टेक) [https://yasirtech.com]is an Blogger, He was born on 01 January 2009 in Firozpur in Sambh..."
[2021-06-05 10:03:52,812] sopel.exceptions     ERROR    - Fatal error traceback
Traceback (most recent call last):
  File "/usr/lib/python3.7/asyncore.py", line 83, in read
    obj.handle_read_event()
  File "/usr/lib/python3.7/asyncore.py", line 422, in handle_read_event
    self.handle_read()
  File "/usr/lib/python3.7/asynchat.py", line 171, in handle_read
    self.found_terminator()
  File "/srv/sopelbots/prodvenv/lib/python3.7/site-packages/sopel/irc/backends.py", line 241, in found_terminator
    self.bot.on_message(line)
  File "/srv/sopelbots/prodvenv/lib/python3.7/site-packages/sopel/irc/__init__.py", line 228, in on_message
    self.dispatch(pretrigger)
  File "/srv/sopelbots/prodvenv/lib/python3.7/site-packages/sopel/bot.py", line 901, in dispatch
    for rule, match in self._rules_manager.get_triggered_rules(self, pretrigger):
  File "/srv/sopelbots/prodvenv/lib/python3.7/site-packages/sopel/plugins/rules.py", line 440, in get_triggered_rules
    return tuple(sorted(matches, key=lambda x: x[0].priority_scale))
  File "/srv/sopelbots/prodvenv/lib/python3.7/site-packages/sopel/plugins/rules.py", line 431, in <genexpr>
    for match in rule.match(bot, pretrigger)
  File "/srv/sopelbots/prodvenv/lib/python3.7/site-packages/sopel/plugins/rules.py", line 1688, in match
    for url in urls:
  File "/srv/sopelbots/prodvenv/lib/python3.7/site-packages/sopel/plugins/rules.py", line 1684, in <genexpr>
    if urlparse(url).scheme in self._schemes
  File "/usr/lib/python3.7/urllib/parse.py", line 368, in urlparse
    splitresult = urlsplit(url, scheme, allow_fragments)
  File "/usr/lib/python3.7/urllib/parse.py", line 459, in urlsplit
    raise ValueError("Invalid IPv6 URL")
ValueError: Invalid IPv6 URL
[2021-06-05 10:03:52,897] sopel.exceptions     ERROR    - ----------------------------------------

dgw added 2 commits June 5, 2021 17:17
Ignoring invalid URLs is much better than spitting out "Fatal error in
core" and dumping a traceback.

It's not our job to accommodate servers that accept invalid URLs. If it
doesn't parse, it doesn't get handled by Sopel.
@dgw dgw added the Bugfix Generally, PRs that reference (and fix) one or more issue(s) label Jun 5, 2021
@dgw dgw added this to the 7.1.1 milestone Jun 5, 2021
@dgw dgw requested a review from Exirel June 5, 2021 22:32
Copy link
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice test you got here.

@dgw dgw merged commit 0cbc386 into master Jun 11, 2021
@dgw dgw deleted the urlcallback-exception-fix branch June 11, 2021 22:27
dgw added a commit that referenced this pull request Jun 11, 2021
Includes commits:
* test: fix incorrect class name in test_url_callback_parse
* plugins.rules: handle `ValueError` from `urllib.parse.urlparse`
* plugins.rules: simplify `URLCallback.match()` method

Corresponds to this merge commit on the master branch:
0cbc386
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix Generally, PRs that reference (and fix) one or more issue(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants