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

Disable the ability to create torrents with a piece size of 256MiB #21295

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

stalkerok
Copy link
Contributor

@stalkerok stalkerok commented Sep 4, 2024

Disabling will reduce the number of users experiencing this issue.

@Chocobo1
Copy link
Member

Chocobo1 commented Sep 6, 2024

Please add description of "why" is it done.

@stalkerok
Copy link
Contributor Author

Done.

@Chocobo1 Chocobo1 added the Core label Sep 6, 2024
@Chocobo1 Chocobo1 requested a review from a team September 6, 2024 18:28
@Chocobo1 Chocobo1 added this to the 5.1 milestone Sep 17, 2024
@xavier2k6
Copy link
Member

What needs to be done really IMO is for libtorrent 1.2.x to reject those torrents from actually being loaded.

@stalkerok
Copy link
Contributor Author

What will happen to the torrents already added? And like I said, 2.0 also has an issue (at least with creating torrents, but I think with rehash too) with the 512MB and 1GB piece size. Maybe for 2.0 this has already been fixed, I don't know.

@stalkerok
Copy link
Contributor Author

Can this be merged and added to the backport to 5.0?

@xavier2k6
Copy link
Member

What will happen to the torrents already added?

  • Will load/re-check fine with libtorrent 2.x based builds (currently)
  • Will load fine but will crash when re-checked with libtorrent 1.x based builds (currently)
  • Advise users to migrate to libtorrent 2.x based builds

And like I said, 2.0 also has an issue (at least with creating torrents, but I think with rehash too) with the 512MB and 1GB piece size.

Creation of torrents >256MiB in qBittorrent (LT20) was blocked/limited intentionally.

#21011 & other tickets showed that even if we reduced the ability in qBittorrent to create torrents with a piece size of 256MiB, there are programs/options to force creation of such large piece size.


py3createtorrent was used to create & indeed highlight the issue experienced in previously mentioned crash reports.

Plan should be:

  • libtorrent 1.2.x to reject unsupported torrents from actually being loaded. ie it can create MAX 128MiB, so should limit to loading <=128MiB
  • libtorrent 2.x to reject unsupported torrents from actually being loaded. ie it can create MAX 256MiB, so should limit to loading <=256MiB

@stalkerok
Copy link
Contributor Author

Wouldn't it be better to just temporarily limit torrent creation to 256MiB torrents in 2.0 until lt 1.2 is fixed?

@glassez
Copy link
Member

glassez commented Sep 21, 2024

  1. I agree with @xavier2k6. It doesn't make much sense to only prevent the creation of such torrents without preventing such torrents from being added to session.
  2. Since libtorrent-2.0 works correctly with 256MB piece size we can safely allow qBittorrent to create v2 torrents of this piece size because they won't be used by libtorrent-1.2 anyway.

@stalkerok
Copy link
Contributor Author

  1. It doesn't make much sense to only prevent the creation of such torrents without preventing such torrents from being added to session.

In any case, users are already using torrents with a 256MiB piece size for lt1.2 and won't be happy with that.

2. Since libtorrent-2.0 works correctly with 256MB piece size we can safely allow qBittorrent to create v2 torrents of this piece size because they won't be used by libtorrent-1.2 anyway.

py3createtorrent is a little known torrent creation tool. Allowing lt2.0 to create torrents V1 and hybrids will cause a really big wave of users who will face this issue on lt1.2.

But these restrictive measures still won't fix the issue for lt1.2...

@stalkerok
Copy link
Contributor Author

As I said, it can be used as a temporary solution for 5.0, which won't break anything for sure. Anyway, I don't have the skills to make something bigger.

@stalkerok
Copy link
Contributor Author

As far as I understand this PR is no longer relevant since a fix has been submitted to libtorrent, what if I make it possible to create 256MiB torrents for lt1.2 as well?

@xavier2k6
Copy link
Member

I believe the limit is now 128MiB in libtorrent 2.x too, haven't tested any new patches..

@glassez Did you?

@glassez
Copy link
Member

glassez commented Oct 4, 2024

I believe the limit is now 128MiB in libtorrent 2.x too

Yes, it is.

@xavier2k6
Copy link
Member

I believe the limit is now 128MiB in libtorrent 2.x too

Yes, it is.

@stalkerok pre-approved, will do some testing tomorrow (Saturday)

@xavier2k6
Copy link
Member

Have tested, latest libtorrent RC_1_2 commit now rejects loading & removes any previous torrent with a invalid piece size of 256MiB.

RC_2_0 is now on par with RC_1_2 with the torrent creation limit of 128MiB piece size.

However, RC_2_0 needs to have a patch rejecting the invalid piece size torrents as it still allows adding new torrents of 256MiB piece size/loading previously added to session......

Outside of that...I approve of this PR.

@Chocobo1 Chocobo1 merged commit 3ea2be4 into qbittorrent:master Oct 7, 2024
14 checks passed
@Chocobo1
Copy link
Member

Chocobo1 commented Oct 7, 2024

@stalkerok
Thank you!

@stalkerok stalkerok deleted the piece-size branch October 7, 2024 20:35
@qBittUser
Copy link

Can this be backported to v5_0_x?

@glassez glassez modified the milestones: 5.1, 5.0.1 Oct 10, 2024
@glassez
Copy link
Member

glassez commented Oct 10, 2024

Can this be backported to v5_0_x?

I believe Yes.

glassez pushed a commit to glassez/qBittorrent that referenced this pull request Oct 10, 2024
Disabling will reduce the number of users experiencing this issue.
qbittorrent#21011

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

Successfully merging this pull request may close these issues.

5 participants