Skip to content

Commit bb792eb

Browse files
committed
Block unique-subfolder migration when target files exist
Refuse conversion when any destination file path already exists (libtorrent may not overwrite). Empty unique directories and unrelated files are fine. Partial retries map under the unique root without double-wrapping. Drop setName on success and remove unreleased append_hash resume compatibility.
1 parent 8964c2b commit bb792eb

8 files changed

Lines changed: 115 additions & 122 deletions

src/base/bittorrent/bencoderesumedatastorage.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -285,10 +285,6 @@ BitTorrent::LoadResumeDataResult BitTorrent::BencodeResumeDataStorage::loadTorre
285285
// fromLTString(root.dict_find_string_value("qBt-contentLayout")), TorrentContentLayout::Default);
286286
// === END REPLACEMENT CODE === //
287287

288-
// Legacy flag from intermediate builds; content_layout is source of truth now.
289-
if (resumeDataRoot.dict_find_int_value("qBt-appendHashToPayloadName"))
290-
torrentParams.contentLayout = TorrentContentLayout::UniqueSubfolder;
291-
292288
torrentParams.stopCondition = Utils::String::toEnum(
293289
fromLTString(resumeDataRoot.dict_find_string_value("qBt-stopCondition")), Torrent::StopCondition::None);
294290
torrentParams.sslParameters =

src/base/bittorrent/dbresumedatastorage.cpp

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ namespace
7272
{
7373
const QString DB_CONNECTION_NAME = u"ResumeDataStorage"_s;
7474

75-
const int DB_VERSION = 11;
75+
const int DB_VERSION = 10;
7676

7777
const QString DB_TABLE_META = u"meta"_s;
7878
const QString DB_TABLE_TORRENTS = u"torrents"_s;
@@ -147,7 +147,6 @@ namespace
147147
const Column DB_COLUMN_SHARE_LIMIT_ACTION = makeColumn(u"share_limit_action"_s);
148148
const Column DB_COLUMN_HAS_OUTER_PIECES_PRIORITY = makeColumn(u"has_outer_pieces_priority"_s);
149149
const Column DB_COLUMN_HAS_SEED_STATUS = makeColumn(u"has_seed_status"_s);
150-
const Column DB_COLUMN_APPEND_HASH_TO_PAYLOAD_NAME = makeColumn(u"append_hash_to_payload_name"_s);
151150
const Column DB_COLUMN_OPERATING_MODE = makeColumn(u"operating_mode"_s);
152151
const Column DB_COLUMN_STOPPED = makeColumn(u"stopped"_s);
153152
const Column DB_COLUMN_STOP_CONDITION = makeColumn(u"stop_condition"_s);
@@ -480,7 +479,6 @@ void BitTorrent::DBResumeDataStorage::createDB() const
480479
makeColumnDefinition(DB_COLUMN_SHARE_LIMIT_ACTION, u"TEXT NOT NULL DEFAULT `Default`"_s),
481480
makeColumnDefinition(DB_COLUMN_HAS_OUTER_PIECES_PRIORITY, u"INTEGER NOT NULL"_s),
482481
makeColumnDefinition(DB_COLUMN_HAS_SEED_STATUS, u"INTEGER NOT NULL"_s),
483-
makeColumnDefinition(DB_COLUMN_APPEND_HASH_TO_PAYLOAD_NAME, u"INTEGER NOT NULL DEFAULT 0"_s),
484482
makeColumnDefinition(DB_COLUMN_OPERATING_MODE, u"TEXT NOT NULL"_s),
485483
makeColumnDefinition(DB_COLUMN_STOPPED, u"INTEGER NOT NULL"_s),
486484
makeColumnDefinition(DB_COLUMN_STOP_CONDITION, u"TEXT NOT NULL DEFAULT `None`"_s),
@@ -595,9 +593,6 @@ void BitTorrent::DBResumeDataStorage::updateDB(const int fromVersion) const
595593
if (fromVersion <= 9)
596594
addColumn(DB_TABLE_TORRENTS, DB_COLUMN_SHARE_LIMITS_MODE, u"TEXT NOT NULL DEFAULT `Default`"_s);
597595

598-
if (fromVersion <= 10)
599-
addColumn(DB_TABLE_TORRENTS, DB_COLUMN_APPEND_HASH_TO_PAYLOAD_NAME, u"INTEGER NOT NULL DEFAULT 0"_s);
600-
601596
const QString updateMetaVersionQuery = makeUpdateStatement(DB_TABLE_META, {DB_COLUMN_NAME, DB_COLUMN_VALUE});
602597
if (!query.prepare(updateMetaVersionQuery))
603598
throw RuntimeError(query.lastError().text());
@@ -657,9 +652,6 @@ LoadResumeDataResult DBResumeDataStorage::parseQueryResultRow(const QSqlQuery &q
657652
};
658653
resumeData.contentLayout = Utils::String::toEnum<TorrentContentLayout>(
659654
query.value(DB_COLUMN_CONTENT_LAYOUT.name).toString(), TorrentContentLayout::Original);
660-
// Legacy column (pre-UniqueSubfolder layout enum); content_layout is source of truth now.
661-
if (query.value(DB_COLUMN_APPEND_HASH_TO_PAYLOAD_NAME.name).toBool())
662-
resumeData.contentLayout = TorrentContentLayout::UniqueSubfolder;
663655
resumeData.operatingMode = Utils::String::toEnum<TorrentOperatingMode>(
664656
query.value(DB_COLUMN_OPERATING_MODE.name).toString(), TorrentOperatingMode::AutoManaged);
665657
resumeData.stopped = query.value(DB_COLUMN_STOPPED.name).toBool();
@@ -881,7 +873,6 @@ StoreJob::StoreJob(const TorrentID &torrentID, LoadTorrentParams resumeData)
881873
DB_COLUMN_SHARE_LIMIT_ACTION,
882874
DB_COLUMN_HAS_OUTER_PIECES_PRIORITY,
883875
DB_COLUMN_HAS_SEED_STATUS,
884-
DB_COLUMN_APPEND_HASH_TO_PAYLOAD_NAME,
885876
DB_COLUMN_OPERATING_MODE,
886877
DB_COLUMN_STOPPED,
887878
DB_COLUMN_STOP_CONDITION,
@@ -947,8 +938,6 @@ StoreJob::StoreJob(const TorrentID &torrentID, LoadTorrentParams resumeData)
947938
query.bindValue(DB_COLUMN_SHARE_LIMIT_ACTION.placeholder, Utils::String::fromEnum(m_resumeData.shareLimits.action));
948939
query.bindValue(DB_COLUMN_HAS_OUTER_PIECES_PRIORITY.placeholder, m_resumeData.firstLastPiecePriority);
949940
query.bindValue(DB_COLUMN_HAS_SEED_STATUS.placeholder, m_resumeData.hasFinishedStatus);
950-
// Column kept for schema compatibility; always 0. Layout is in content_layout.
951-
query.bindValue(DB_COLUMN_APPEND_HASH_TO_PAYLOAD_NAME.placeholder, false);
952941
query.bindValue(DB_COLUMN_OPERATING_MODE.placeholder, Utils::String::fromEnum(m_resumeData.operatingMode));
953942
query.bindValue(DB_COLUMN_STOPPED.placeholder, m_resumeData.stopped);
954943
query.bindValue(DB_COLUMN_STOP_CONDITION.placeholder, Utils::String::fromEnum(m_resumeData.stopCondition));

src/base/bittorrent/toplevelpayload.cpp

Lines changed: 54 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,31 @@ namespace
111111

112112
return base;
113113
}
114+
115+
// Path without its first component (or the path itself when single-component).
116+
Path payloadRelativePath(const Path &path)
117+
{
118+
const QString s = path.data();
119+
const qsizetype slash = s.indexOf(u'/');
120+
if (slash < 0)
121+
return path;
122+
return Path(s.sliced(slash + 1));
123+
}
124+
125+
// Map each path under uniqueRoot without double-wrapping paths already there.
126+
PathList mapUnderUniqueRoot(const PathList &filePaths, const Path &uniqueRoot)
127+
{
128+
PathList out;
129+
out.reserve(filePaths.size());
130+
for (const Path &path : filePaths)
131+
{
132+
if ((path == uniqueRoot) || path.hasAncestor(uniqueRoot))
133+
out.append(path);
134+
else
135+
out.append(uniqueRoot / payloadRelativePath(path));
136+
}
137+
return out;
138+
}
114139
}
115140

116141
QString BitTorrent::uniqueSubfolderTag(const TorrentID &id)
@@ -130,18 +155,20 @@ PathList BitTorrent::applyUniqueSubfolderLayout(PathList filePaths, const Torren
130155

131156
const QString tag = uniqueSubfolderTag(id);
132157
const QString folderName = uniqueSubfolderName(id, originalNameForUniqueDir(filePaths, torrentName, tag));
158+
const Path uniqueRoot {folderName};
133159

160+
// Uniform layout: common root rename, or wrap rootless/single-file.
161+
// Mixed (partial migration) is handled by mapUnderUniqueRoot so paths already
162+
// under uniqueRoot are left alone.
134163
const Path rootFolder = Path::findRootFolder(filePaths);
135164
if (!rootFolder.isEmpty())
136165
{
137-
if (rootFolder.toString() == folderName)
166+
if (rootFolder == uniqueRoot)
138167
return filePaths;
139168
return renameRootFolder(std::move(filePaths), rootFolder.toString(), folderName);
140169
}
141170

142-
// Single file or rootless multi-file: wrap under the unique folder.
143-
Path::addRootFolder(filePaths, Path(folderName));
144-
return filePaths;
171+
return mapUnderUniqueRoot(filePaths, uniqueRoot);
145172
}
146173

147174
BitTorrent::UniqueSubfolderMigrationPlan BitTorrent::makeUniqueSubfolderMigrationPlan(
@@ -152,37 +179,45 @@ BitTorrent::UniqueSubfolderMigrationPlan BitTorrent::makeUniqueSubfolderMigratio
152179
if (currentPaths.isEmpty())
153180
return plan;
154181

155-
const PathList targetPaths = applyUniqueSubfolderLayout(currentPaths, id, torrentName);
156-
if (targetPaths == currentPaths)
157-
return plan;
158-
159182
if (storageRoot.isEmpty())
160183
{
161184
plan.blocked = true;
162185
plan.blockReason = QCoreApplication::translate("BitTorrent", "Storage location is unknown.");
163186
return plan;
164187
}
165188

166-
const Path uniqueDir = Path::findRootFolder(targetPaths);
167-
if (uniqueDir.isEmpty())
189+
// Always derive the unique folder from the torrent name (stable across partial states).
190+
const QString tag = uniqueSubfolderTag(id);
191+
const Path uniqueRoot {uniqueSubfolderName(id, originalNameForUniqueDir(currentPaths, torrentName, tag))};
192+
if (uniqueRoot.isEmpty())
168193
{
169194
plan.blocked = true;
170195
plan.blockReason = QCoreApplication::translate("BitTorrent", "Target unique subfolder is invalid.");
171196
return plan;
172197
}
173198

199+
const PathList targetPaths = mapUnderUniqueRoot(currentPaths, uniqueRoot);
200+
174201
for (int i = 0; i < targetPaths.size(); ++i)
175202
{
176-
if (targetPaths.at(i) != currentPaths.at(i))
177-
plan.renames.append({.fileIndex = i, .to = targetPaths.at(i)});
178-
}
179-
if (plan.renames.isEmpty())
180-
return plan;
203+
if (targetPaths.at(i) == currentPaths.at(i))
204+
continue;
181205

182-
// Existing destination: do not block. UI may confirm a merge (overwrite matching paths only).
183-
const Path destAbs = storageRoot / uniqueDir;
184-
if (destAbs.exists())
185-
plan.existingUniqueFolder = destAbs;
206+
// Refuse if the destination *file* already exists. Existing directories are fine.
207+
// Unrelated files under the unique folder that are not rename targets are left alone.
208+
const Path targetAbs = storageRoot / targetPaths.at(i);
209+
if (targetAbs.exists() && !Utils::Fs::isDir(targetAbs))
210+
{
211+
plan.blocked = true;
212+
plan.blockReason = QCoreApplication::translate("BitTorrent"
213+
, "Migration cannot continue: destination file already exists: \"%1\".")
214+
.arg(targetPaths.at(i).toString());
215+
plan.renames.clear();
216+
return plan;
217+
}
218+
219+
plan.renames.append({.fileIndex = i, .to = targetPaths.at(i)});
220+
}
186221

187222
return plan;
188223
}

src/base/bittorrent/toplevelpayload.h

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,26 +47,17 @@ namespace BitTorrent
4747
};
4848

4949
// Preflight only — no disk changes.
50-
// When existingUniqueFolder is set, the UI should confirm a merge before start.
50+
// If any absolute target *file* already exists, blocked is set (directories are OK).
5151
struct UniqueSubfolderMigrationPlan
5252
{
5353
QList<UniqueSubfolderRename> renames;
54-
55-
// Absolute path of an existing unique destination (empty if none).
56-
Path existingUniqueFolder;
57-
5854
bool blocked = false;
5955
QString blockReason;
6056

6157
bool isEmpty() const
6258
{
6359
return renames.isEmpty() && !blocked;
6460
}
65-
66-
bool needsConfirmation() const
67-
{
68-
return !existingUniqueFolder.isEmpty();
69-
}
7061
};
7162

7263
// Pure preflight used by TorrentImpl and unit tests.

src/base/bittorrent/torrentimpl.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2949,12 +2949,7 @@ void TorrentImpl::finishUniqueSubfolderMigration()
29492949

29502950
const bool success = job.failedFileIndexes.isEmpty();
29512951
if (success)
2952-
{
29532952
m_contentLayout = TorrentContentLayout::UniqueSubfolder;
2954-
const Path uniqueRoot = Path::findRootFolder(filePaths());
2955-
if (!uniqueRoot.isEmpty())
2956-
setName(uniqueRoot.toString());
2957-
}
29582953

29592954
deferredRequestResumeData();
29602955

src/gui/optionsdialog.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,7 @@ void OptionsDialog::loadDownloadsTabOptions()
588588
+ tr("Put content in a unique folder with a short hash suffix (e.g. Show a19f83c275d1). "
589589
"Avoids name collisions; breaks path-based cross-seeding. "
590590
"Existing torrents stay as-is until converted via right-click "
591-
"(if the folder already exists, you can confirm a merge that replaces matching files).")
591+
"(migration stops if matching destination files already exist).")
592592
+ u"</p><p><b>" + tr("Don't create subfolder") + u"</b> - "
593593
+ tr("Put files directly in the save path.")
594594
+ u"</p></body></html>");

src/gui/transferlistwidget.cpp

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -982,24 +982,6 @@ void TransferListWidget::createUniqueSubfolderForSelectedTorrents()
982982
if (plan.isEmpty())
983983
return;
984984

985-
// Merge if the unique folder already exists (overwrite matching paths only).
986-
if (plan.needsConfirmation())
987-
{
988-
const QString text = tr(
989-
"Torrent: \"%1\"\n\n"
990-
"The unique subfolder already exists. Continuing will merge the torrent into it "
991-
"and replace files with matching paths. Other files will not be removed.")
992-
.arg(torrent->name());
993-
994-
const QMessageBox::StandardButton answer = QMessageBox::warning(this
995-
, tr("Create unique subfolder")
996-
, text
997-
, (QMessageBox::Ok | QMessageBox::Cancel)
998-
, QMessageBox::Cancel);
999-
if (answer != QMessageBox::Ok)
1000-
return;
1001-
}
1002-
1003985
connect(torrent, &BitTorrent::Torrent::uniqueSubfolderMigrationFinished, this
1004986
, [this](const bool success, const QString &message)
1005987
{
@@ -1095,7 +1077,7 @@ void TransferListWidget::displayListMenu()
10951077
, tr("Create &unique subfolder"), listMenu);
10961078
actionCreateUniqueSubfolder->setToolTip(tr(
10971079
"Move selected torrents into a unique folder (e.g. Show a19f83c275d1). "
1098-
"If that folder already exists, matching files may be overwritten after confirmation."));
1080+
"If matching destination files already exist, migration cannot continue."));
10991081
connect(actionCreateUniqueSubfolder, &QAction::triggered, this, &TransferListWidget::createUniqueSubfolderForSelectedTorrents);
11001082
auto *actionSequentialDownload = new TriStateAction(tr("Download in sequential order"), listMenu);
11011083
connect(actionSequentialDownload, &QAction::triggered, this, &TransferListWidget::setSelectedTorrentsSequentialDownload);

0 commit comments

Comments
 (0)