starred/qBittorrent

Fork 0

mirror of https://github.com/qbittorrent/qBittorrent.git synced 2026-03-02 22:57:32 -05:00

Incorrect using TorrentHandle::rootPath() as torrent save path. #3120

New issue

Closed

opened 2026-02-21 16:35:35 -05:00 by deekerman · 6 comments

deekerman commented

2026-02-21 16:35:35 -05:00

Owner

Originally created by @glassez on GitHub (Sep 23, 2015).

In most cases, the use rootPath() wrong.

Explanation and examples:
PropertiesWidget,TorrentModel and WebAPI data: We should show real torrent save path if we show it under "Save path" label. Otherwise, there is some discrepancy: the user sees one path, and when, for example, he wants to change it, he sees another path (in some cases). And yet, rootPath() returns the actual path, and we must show the target path.
rootPath() (by implication, torrent content path) should be used only for folder opening.

@Chocobo1, @sledgehammer999, @qbittorrent/qbittorrent-frequent-contributors Your comments?..

P.S. TorrentHandle::rootPath() is incorrect itself. But I'll fix it soon.
Here some examples (save path = /user/torrents):

Torrent has 1 file with path subdir/filename.ext.
rootPath() returns /user/torrents but correct result is /user/torrents/subdir
Torrent has multiply files with paths:
subdir1/filename1.ext
subdir1/filename2.ext
subdir2/filename1.ext
rootPath() returns /user/torrents/subdir1 but correct result is /user/torrents
And so on...

Originally created by @glassez on GitHub (Sep 23, 2015). In most cases, the use rootPath() wrong. Explanation and examples: PropertiesWidget,TorrentModel and WebAPI data: We should show **real** torrent save path if we show it under "Save path" label. Otherwise, there is some discrepancy: the user sees one path, and when, for example, he wants to change it, he sees another path (in some cases). And yet, rootPath() returns the **actual** path, and we must show the **target** path. rootPath() (by implication, **torrent content path**) should be used only for folder opening. @Chocobo1, @sledgehammer999, @qbittorrent/qbittorrent-frequent-contributors Your comments?.. P.S. TorrentHandle::rootPath() is incorrect itself. But I'll fix it soon. Here some examples (save path = `/user/torrents`): 1. Torrent has 1 file with path `subdir/filename.ext`. rootPath() returns `/user/torrents` but correct result is `/user/torrents/subdir` 2. Torrent has multiply files with paths: `subdir1/filename1.ext` `subdir1/filename2.ext` `subdir2/filename1.ext` rootPath() returns `/user/torrents/subdir1` but correct result is `/user/torrents` 3. And so on...

deekerman closed this issue

2026-02-21 16:35:38 -05:00

deekerman commented

2026-02-21 16:35:40 -05:00

Author

Owner

@Chocobo1 commented on GitHub (Sep 23, 2015):

PropertiesWidget,TorrentModel and WebAPI data

Notice that there is also a parameter in Run external program on torrent completion that is called Save path which also invoke rootPath().

Torrent has multiply files with paths:

I've thought of this when working on #3604, but I am confused when looking at the bittorrent spec,
it says:

For the case of the multi-file mode, the info dictionary contains the following structure:
name: the file path of the directory in which to store all the files. This is purely advisory. (string)

What does "purely advisory" means? The name field can be omitted? Is the root directory a must or not?
In fact, I've never seen a torrent have the structure like in the example. Our Torrent creator also doesn't allow that kind of structure.

1.Torrent has 1 file with path subdir/filename.ext.

I think /user/torrents is correct, since subdir is hardcoded in the torrent, I don't think we need to be smart here, I would like to hear more discussion about this too.
Another acceptable answer would be /user/torrents/subdir/filename.ext, but then, it could cause problem in the Run external program... case.

2.Torrent has multiply files with paths:

but correct result is /user/torrents

Agree.

@Chocobo1 commented on GitHub (Sep 23, 2015): > PropertiesWidget,TorrentModel and WebAPI data Notice that there is also a parameter in `Run external program on torrent completion` that is called `Save path` which also invoke `rootPath()`. --- > Torrent has multiply files with paths: I've thought of this when working on #3604, but I am confused when looking at the [bittorrent spec](https://wiki.theory.org/BitTorrentSpecification#Info_in_Multiple_File_Mode), it says: > For the case of the multi-file mode, the info dictionary contains the following structure: > name: the file path of the directory in which to store all the files. This is purely advisory. (string) What does "purely advisory" means? The `name` field can be omitted? Is the root directory a must or not? In fact, I've never seen a torrent have the structure like in the example. Our `Torrent creator` also doesn't allow that kind of structure. --- > 1.Torrent has 1 file with path subdir/filename.ext. I think `/user/torrents` is correct, since `subdir` is hardcoded in the torrent, I don't think we need to be smart here, I would like to hear more discussion about this too. Another acceptable answer would be `/user/torrents/subdir/filename.ext`, but then, it could cause problem in the `Run external program...` case. > 2.Torrent has multiply files with paths: > > > but correct result is /user/torrents Agree.

deekerman commented

2026-02-21 16:35:40 -05:00

Author

Owner

@glassez commented on GitHub (Sep 23, 2015):

In fact, I've never seen a torrent have the structure like in the example. Our Torrent creator also doesn't allow that kind of structure.

I agree. On the basis of the specifications it is not possible. But we want to implement a strip first subdir (or even first few subdirs) from the file names feature (#588). The applying of this operation can lead to creation of such structure.
Here is another example:
Torrent has multiply files with paths:
subdir1/subdir2/filename1.ext
subdir1/subdir2/filename2.ext
subdir1/subdir2/filename3.ext
rootPath() returns /user/torrents/subdir1 but correct result is /user/torrents/subdir1/subdir2

I think /user/torrents is correct, since subdir is hardcoded in the torrent, I don't think we need to be smart here.

I don't agree. Regardless of how many files the torrent contains, the function should behave the same way. As I wrote here, the meaning of this method is " torrent content path" (by the way, I want to rename rootPath() with contentPath()). This means the directory where the useful content of the torrent (i.e. files in most cases) is placed.

@glassez commented on GitHub (Sep 23, 2015): > In fact, I've never seen a torrent have the structure like in the example. Our Torrent creator also doesn't allow that kind of structure. I agree. On the basis of the specifications it is not possible. But we want to implement a strip first subdir (or even first few subdirs) from the file names feature (#588). The applying of this operation can lead to creation of such structure. Here is another example: Torrent has multiply files with paths: `subdir1/subdir2/filename1.ext` `subdir1/subdir2/filename2.ext` `subdir1/subdir2/filename3.ext` rootPath() returns `/user/torrents/subdir1` but correct result is `/user/torrents/subdir1/subdir2` > I think /user/torrents is correct, since subdir is hardcoded in the torrent, I don't think we need to be smart here. I don't agree. Regardless of how many files the torrent contains, the function should behave the same way. As I wrote here, the meaning of this method is " torrent content path" (by the way, I want to rename rootPath() with contentPath()). This means the directory where the useful content of the torrent (i.e. files in most cases) is placed.

deekerman commented

2026-02-21 16:35:40 -05:00

Author

Owner

@sledgehammer999 commented on GitHub (Sep 23, 2015):

I agree. On the basis of the specifications it is not possible. But we want to implement a strip first subdir (or even first few subdirs) from the file names feature (#588).

Without derailing the discussion: I have been testing this. The only way to strip the root is to remap files from the torrent_info object. So yes, multifile torrents always have a "name" which acts as root folder.

As to which is correct: Well it depends on what you want to do. I think we need at least a function that will return the current and real save path. Especially for the "Save Path:" label in "General". And probably this will be usefull for "run external program". I mean /user/torrents in @glassez example.

Question: If the temp folder is enabled should "Save path:" also show it? (since it is the current and real save path) or always show the target path(== final folder)?

@sledgehammer999 commented on GitHub (Sep 23, 2015): > I agree. On the basis of the specifications it is not possible. But we want to implement a strip first subdir (or even first few subdirs) from the file names feature (#588). Without derailing the discussion: I have been testing this. The only way to strip the root is to remap files from the torrent_info object. So yes, multifile torrents always have a "name" which acts as root folder. As to which is correct: Well it depends on what you want to do. I think we need at least a function that will return the current and real save path. Especially for the "Save Path:" label in "General". And probably this will be usefull for "run external program". I mean `/user/torrents` in @glassez example. Question: If the temp folder is enabled should "Save path:" also show it? (since it is the current and real save path) or always show the target path(== final folder)?

deekerman commented

2026-02-21 16:35:40 -05:00

Author

Owner

@glassez commented on GitHub (Sep 24, 2015):

Question: If the temp folder is enabled should "Save path:" also show it? (since it is the current and real save path) or always show the target path(== final folder)?

Save path must show target path. If it needs to show actual torrent location we should use addition field (aka "Actual save path" or something else).

Especially for the "Save Path:" label in "General". And probably this will be usefull for "run external program".

For "run external program", on the contrary, it is useful to use torrent content location (at least in addition to save path)since "save path" itself bears little value to an external program.
We can only decide what we mean by "torrent content location". It should be based on the usefulness of this concept. (Forget about my previous suggestions here. It is necessary to define from scratch.)

With regard to the use of target or actual path. This should follow from the context. For example, the open the file/folder function should use the actual path. And so on.))

@glassez commented on GitHub (Sep 24, 2015): > Question: If the temp folder is enabled should "Save path:" also show it? (since it is the current and real save path) or always show the target path(== final folder)? Save path must show **target** path. If it needs to show actual torrent location we should use addition field (aka "Actual save path" or something else). > Especially for the "Save Path:" label in "General". And probably this will be usefull for "run external program". For "run external program", on the contrary, it is useful to use torrent content location (at least in addition to save path)since "save path" itself bears little value to an external program. We can only decide what we mean by "torrent content location". It should be based on the usefulness of this concept. (Forget about my previous suggestions here. It is necessary to define from scratch.) With regard to the use of target or actual path. This should follow from the context. For example, the open the file/folder function should use the actual path. And so on.))

deekerman commented

2026-02-21 16:35:40 -05:00

Author

Owner

@sledgehammer999 commented on GitHub (Sep 24, 2015):

(Forget about my previous suggestions here. It is necessary to define from scratch.)

I was going to say the same thing.
First let's forget what is currently implemented by our TorrentHandle class.
I think we have two needs in the application, depending on what functionality is used.
Let me define 2 terms to make this discussion easier:
target path: Path where the files will be moved after they finish downloading.
current path: Path where the files are written to disk. eg the tmp folder
Obviously when temp folder is disabled current and target paths are the same.

So we need 2 functions returning those 2 values.
Target path: useful for "Save path:" field in General tab. Also when "Set location..." is chosen.
Current path: Useful for "run external program". Also for "open destionation folder" or "open file".

(possibly other usages too)

Nitpick: For "run external program" current path will be always the same as target path regardless if tmp folders are enabled, because that command is run after the content has been moved to target path.

@sledgehammer999 commented on GitHub (Sep 24, 2015): > (Forget about my previous suggestions here. It is necessary to define from scratch.) I was going to say the same thing. First let's forget what is currently implemented by our TorrentHandle class. I think we have two needs in the application, depending on what functionality is used. Let me define 2 terms to make this discussion easier: **target path**: Path where the files will be moved after they finish downloading. **current path**: Path where the files are written to disk. eg the tmp folder Obviously when temp folder is disabled current and target paths are the same. So we need 2 functions returning those 2 values. Target path: useful for "Save path:" field in General tab. Also when "Set location..." is chosen. Current path: Useful for "run external program". Also for "open destionation folder" or "open file". (possibly other usages too) Nitpick: For "run external program" current path will be always the same as target path regardless if tmp folders are enabled, because that command is run after the content has been moved to target path.

deekerman commented

2026-02-21 16:35:40 -05:00

Author

Owner

@glassez commented on GitHub (Sep 24, 2015):

@sledgehammer999 Still, the solution must be two-dimensional:

	Target	Current
Save path	eg. /user/downloads	eg. /user/downloads/tmp
Content path	eg. /user/downloads/torrentname	eg. /user/downloads/tmp/torrentname

As I said, the save path itself is marginally useful for some tasks. For example, what use is it to an external program? It is also useless for "Open containing folder" feature and so on.

@glassez commented on GitHub (Sep 24, 2015): @sledgehammer999 Still, the solution must be two-dimensional: | | Target | Current | | --- | --- | --- | | **Save path** | eg. /user/downloads | eg. /user/downloads/tmp | | **Content path** | eg. /user/downloads/torrentname | eg. /user/downloads/tmp/torrentname | As I said, the save path itself is marginally useful for some tasks. For example, what use is it to an external program? It is also useless for "Open containing folder" feature and so on.