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/6700 #6738

Merged
merged 5 commits into from
Jan 24, 2022
Merged

Fix/6700 #6738

merged 5 commits into from
Jan 24, 2022

Conversation

drew2a
Copy link
Contributor

@drew2a drew2a commented Jan 20, 2022

This PR fixes #6700.

The problem was the interaction between GUI and Core by using REST. In particular, the problem was in the manual URI serializing by using quote_plus_unicode and manual URI deserializing by using uri_to_path.
I removed both serializing and deserializing procedures by replacing them with the native aiohttp mechanism (by using params). The current quote/unquote processing is performed by https://github.com/aio-libs/yarl/.

The funny fact is that they had a bug for yarl below v1.6.3 and our manual quote/unquote mechanism (now removed) had kinda the same bug.

During investigation procedures, two functions have been extracted from run_tribler.py:

  1. run_core
  2. run_gui

During the PR the following bugs were fixed:

  1. import encodings.idna removes by IDE as unused
  2. SentryStrategy.SEND_SUPPRESSED has been set for the core from the GUI application

@drew2a drew2a force-pushed the fix/6700 branch 2 times, most recently from 75fc363 to 2183121 Compare January 20, 2022 21:12
@drew2a drew2a marked this pull request as ready for review January 20, 2022 21:20
@drew2a drew2a requested review from a team and kozlovsky and removed request for a team January 20, 2022 21:20
Copy link
Contributor

@kozlovsky kozlovsky left a comment

Choose a reason for hiding this comment

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

Great finding, but it seems that the fix is incomplete. When I launch run_tribler.py with a torrent path as an argument and the torrent name contains URI-encoded characters, Tribler still cannot download the torrent. For example, if I try to download the file ubuntu%2021.10-desktop-amd64.iso.torrent (note a %20 sequence which represents a space character), I see the following error after Tribler is started:

image

As the bug is related to URI-escaped characters, I think it is better to add a test with a torrent file name that contains such characters.

@drew2a drew2a force-pushed the fix/6700 branch 2 times, most recently from 29d8273 to 7e6d3f3 Compare January 21, 2022 15:19
@drew2a
Copy link
Contributor Author

drew2a commented Jan 21, 2022

Our tests use yarl 1.4.2 But we need at least yarn 1.6.3 because of aio-libs/yarl#517

image

@drew2a drew2a force-pushed the fix/6700 branch 6 times, most recently from b02b7a3 to e791fbe Compare January 24, 2022 11:33
kozlovsky
kozlovsky previously approved these changes Jan 24, 2022
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@drew2a drew2a merged commit 71bb956 into Tribler:main Jan 24, 2022
@drew2a drew2a mentioned this pull request Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Error when adding a download that contains URL-escaped characters in the filename
2 participants