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 ascii decoding bug in rss #11712

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

sawyersteven
Copy link
Contributor

  • [ *] PR is based on the DEVELOP branch
  • [ *] Don't send big changes all at once. Split up big PRs into multiple smaller PRs that are easier to manage and review
  • [ *] Read the contribution guide

Some episode descriptions have non-ascii characters in it. This ignores those characters when reading the rss file from disk. Magnet links and urls must be ascii only so ignoring non-ascii will not cause problems.

sawyersteven and others added 17 commits February 11, 2023 21:44
The previous test compared the xml as a string to an expected string. This worked occasionally because the xml writer will sometimes place attributes in a random order.

Also a bunch of formatting changes while I tried to eliminate warnings
Change import from ET to ElemTree and change whitespace location in expected string to satisfy Flake8. This doesn't affect the test, but unclogs the test output a little.

Recursive test fails on assertion rather than returning bool to bubble up through the nested generators.

Strip newlines from expected string before parsing in ElementTree
Strips all non-ascii characters when writing the xml document.

Writes a new xml document if the existing document is empty or just whitespace
Some episode descriptions contain non-ascii characters, and this can cause errors. This commit simply ignores those characters, which is safe because hashes and urls must be ascii by definition.
qBittorrent (and possibly others) do not support xml with ascii encoding, so this commit forces utf-8/unicode in ElementTree and when reading/writing.
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.

2 participants