Skip to content

Commit 1598361

Browse files
authored
Improve code quality and package workflow coverage (#76)
* Remove obsolete developer scripts and clean up QML diagnostics * Consolidate image decoder ownership * Deduplicate audio keyframe evaluation * Harden preset and repository file handling * Update quality baselines and documentation * Fix keyframe holds and Linux CI races
1 parent 2b60409 commit 1598361

25 files changed

Lines changed: 495 additions & 489 deletions

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ set(CORE_SOURCES
9191
core/src/media_decoder.cpp
9292
core/src/media_utils.cpp
9393
core/src/package_manager.cpp
94+
core/src/package_url_utils.hpp
9495
core/src/permission_manager.cpp
9596
core/src/preset_manager.cpp
9697
core/src/project_serializer.cpp

clean.py

Lines changed: 0 additions & 140 deletions
This file was deleted.

core/include/image_decoder.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ class ImageDecoder : public MediaDecoder {
2020
void startDecoding() override;
2121

2222
void load(); // 互換性のため維持
23+
void waitForFinished();
2324

2425
signals:
2526
// MediaDecoder::ready を使用
@@ -33,4 +34,4 @@ class ImageDecoder : public MediaDecoder {
3334
QFuture<void> m_future;
3435
};
3536

36-
} // namespace AviQtl::Core
37+
} // namespace AviQtl::Core

core/src/image_decoder.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,11 @@ namespace AviQtl::Core {
1818
ImageDecoder::ImageDecoder(int clipId, const QUrl &source, VideoFrameStore *store, QObject *parent) : MediaDecoder(clipId, source, parent), m_store(store) {}
1919

2020
ImageDecoder::~ImageDecoder() {
21-
if (m_future.isRunning()) {
22-
m_future.waitForFinished();
23-
}
21+
waitForFinished();
2422
}
2523

24+
void ImageDecoder::waitForFinished() { m_future.waitForFinished(); }
25+
2626
void ImageDecoder::seek(qint64 ms) {
2727
Q_UNUSED(ms);
2828
if (m_isReady && m_cachedVideoFrame.isValid()) {

core/src/package_manager.cpp

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include "package_manager.hpp"
2+
#include "package_url_utils.hpp"
23
#include "effect_registry.hpp"
34
#include "settings_manager.hpp"
45
#include "shader_compiler.hpp"
@@ -46,10 +47,6 @@ bool isValidPackageType(const QString &packageType) {
4647
packageType == QStringLiteral("object") || packageType == QStringLiteral("transition");
4748
}
4849

49-
bool isSecureNetworkUrl(const QUrl &url) {
50-
return url.isValid() && url.scheme() == QStringLiteral("https") && !url.host().isEmpty();
51-
}
52-
5350
bool writeJsonAtomically(const QString &path, const QJsonDocument &document) {
5451
QSaveFile file(path);
5552
if (!file.open(QIODevice::WriteOnly))
@@ -272,7 +269,7 @@ void PackageManager::saveRepositories(const QVariantList &repos) {
272269
}
273270

274271
void PackageManager::addRepository(const QString &url, bool enabled, int priority) {
275-
if (!isSecureNetworkUrl(QUrl(url)))
272+
if (!Internal::isSecureNetworkUrl(QUrl(url)))
276273
return;
277274
QVariantList repos = repositories();
278275
for (const auto &r : repos) {
@@ -369,16 +366,6 @@ void PackageManager::refreshRepositories() {
369366
continue;
370367

371368
QString repoUrl = repo.value(QStringLiteral("url")).toString();
372-
QUrl baseUrl(repoUrl);
373-
QString basePath = repoUrl;
374-
if (basePath.endsWith(QStringLiteral("/repo.json")))
375-
basePath.chop(9);
376-
else if (basePath.endsWith(QStringLiteral(".json"))) {
377-
int slash = basePath.lastIndexOf('/');
378-
if (slash != -1)
379-
basePath = basePath.left(slash);
380-
}
381-
382369
struct SyncCtx {
383370
QVariantMap repoInfo;
384371
QByteArray catalogData;
@@ -391,15 +378,15 @@ void PackageManager::refreshRepositories() {
391378
if (!fetchUrl.path().endsWith(QStringLiteral("/repo.json")))
392379
fetchUrl.setPath(fetchUrl.path() + (fetchUrl.path().endsWith('/') ? QStringLiteral("repo.json") : QStringLiteral("/repo.json")));
393380

394-
if (!isSecureNetworkUrl(fetchUrl)) {
381+
if (!Internal::isSecureNetworkUrl(fetchUrl)) {
395382
m_pendingRequests--;
396383
emit errorOccurred(tr("Repository URL must use HTTPS: %1").arg(repoUrl));
397384
tryFinishSyncLegacy(installed);
398385
continue;
399386
}
400387
QNetworkReply *reply = m_networkManager->get(packageNetworkRequest(fetchUrl));
401388
enforceReplySizeLimit(reply, kMaxRepositoryResponseBytes);
402-
connect(reply, &QNetworkReply::finished, this, [this, reply, fetchUrl, repoUrl, basePath, ctx, installed]() {
389+
connect(reply, &QNetworkReply::finished, this, [this, reply, fetchUrl, repoUrl, ctx, installed]() {
403390
reply->deleteLater();
404391
m_pendingRequests--;
405392

@@ -411,13 +398,9 @@ void PackageManager::refreshRepositories() {
411398
ctx->repoInfo[QStringLiteral("name")] = repoObj.value(QStringLiteral("repo_name")).toString();
412399
QString catalogUrl = repoObj.value(QStringLiteral("catalog_url")).toString();
413400
if (!catalogUrl.isEmpty()) {
414-
QUrl absUrl;
415-
if (catalogUrl.startsWith(QStringLiteral("http://")) || catalogUrl.startsWith(QStringLiteral("https://")))
416-
absUrl = QUrl(catalogUrl);
417-
else
418-
absUrl = QUrl(basePath + QStringLiteral("/") + catalogUrl);
401+
const QUrl absUrl = Internal::resolveRepositoryReference(fetchUrl, catalogUrl);
419402

420-
if (!isSecureNetworkUrl(absUrl)) {
403+
if (!Internal::isSecureNetworkUrl(absUrl)) {
421404
emit errorOccurred(tr("Catalog URL must use HTTPS: %1").arg(absUrl.toString()));
422405
onCatalogFetched(ctx->repoInfo, {}, installed);
423406
tryFinishSyncLegacy(installed);
@@ -646,7 +629,7 @@ void PackageManager::fetchPackageMetadata(const QString &packageId, const QStrin
646629

647630
setStatus(tr("Fetching package details: %1").arg(packageId));
648631
QUrl url(metadataUrl);
649-
if (!isSecureNetworkUrl(url)) {
632+
if (!Internal::isSecureNetworkUrl(url)) {
650633
emit errorOccurred(tr("Invalid or insecure metadata URL for package: %1").arg(packageId));
651634
return;
652635
}
@@ -837,7 +820,7 @@ void PackageManager::downloadPackage(const QString &packageId, const QUrl &url,
837820
emit errorOccurred(tr("Invalid package ID or type."));
838821
return;
839822
}
840-
if (!isSecureNetworkUrl(url)) {
823+
if (!Internal::isSecureNetworkUrl(url)) {
841824
setBusy(false);
842825
emit errorOccurred(tr("Invalid or insecure package download URL."));
843826
return;
@@ -884,7 +867,11 @@ void PackageManager::downloadPackage(const QString &packageId, const QUrl &url,
884867
emit errorOccurred(tr("Package archive exceeds the maximum allowed size."));
885868
return;
886869
}
887-
file.write(data);
870+
if (file.write(data) != data.size()) {
871+
setBusy(false);
872+
emit errorOccurred(tr("Failed to write the complete downloaded package."));
873+
return;
874+
}
888875
file.close();
889876

890877
// SHA256 verification

core/src/package_url_utils.hpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
#pragma once
2+
3+
#include <QString>
4+
#include <QUrl>
5+
6+
namespace AviQtl::Core::Internal {
7+
8+
inline bool isSecureNetworkUrl(const QUrl &url) {
9+
return url.isValid() && url.scheme() == QStringLiteral("https") && !url.host().isEmpty();
10+
}
11+
12+
inline QUrl resolveRepositoryReference(const QUrl &repositoryIndexUrl, const QString &reference) {
13+
if (!repositoryIndexUrl.isValid() || reference.isEmpty()) {
14+
return {};
15+
}
16+
return repositoryIndexUrl.resolved(QUrl(reference));
17+
}
18+
19+
} // namespace AviQtl::Core::Internal

core/src/preset_manager.cpp

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include <QFile>
55
#include <QJsonDocument>
66
#include <QJsonObject>
7+
#include <QSaveFile>
78
#include <QStandardPaths>
89

910
namespace AviQtl::Core {
@@ -13,6 +14,12 @@ bool isUnsafeName(const QString &s) {
1314
return s.isEmpty() || s.contains(QLatin1Char('/')) || s.contains(QLatin1Char('\\'))
1415
|| s.contains(QLatin1String("..")) || s.startsWith(QLatin1Char('.'));
1516
}
17+
18+
bool isPathWithin(const QString &basePath, const QString &candidatePath) {
19+
const QString base = QDir::cleanPath(basePath);
20+
const QString candidate = QDir::cleanPath(candidatePath);
21+
return candidate == base || candidate.startsWith(base + QDir::separator());
22+
}
1623
} // namespace
1724

1825
PresetManager &PresetManager::instance() {
@@ -43,11 +50,19 @@ QString PresetManager::presetPath(const QString &effectId, const QString &name)
4350
const QString dir = presetDir(effectId);
4451
if (dir.isEmpty())
4552
return {};
46-
const QString path = QDir(dir).filePath(name + QStringLiteral(".json"));
47-
const QString baseCanon = QFileInfo(resolveBaseDir()).canonicalFilePath();
48-
const QString fileCanon = QFileInfo(path).canonicalFilePath();
49-
if (!baseCanon.isEmpty() && !fileCanon.isEmpty() && !fileCanon.startsWith(baseCanon))
53+
54+
const QString basePath = QDir(resolveBaseDir()).absolutePath();
55+
const QString candidateDir = QDir(dir).absolutePath();
56+
if (!isPathWithin(basePath, candidateDir))
57+
return {};
58+
59+
const QString canonicalBase = QDir(basePath).canonicalPath();
60+
const QString canonicalDir = QDir(candidateDir).canonicalPath();
61+
if (!canonicalBase.isEmpty() && !canonicalDir.isEmpty() &&
62+
!isPathWithin(canonicalBase, canonicalDir))
5063
return {};
64+
65+
const QString path = QDir(dir).filePath(name + QStringLiteral(".json"));
5166
return path;
5267
}
5368

@@ -94,11 +109,14 @@ bool PresetManager::savePreset(const QString &effectId, const QString &name, con
94109
obj[QStringLiteral("params")] = QJsonObject::fromVariantMap(params);
95110
obj[QStringLiteral("keyframes")] = QJsonObject::fromVariantMap(keyframes);
96111

97-
QFile f(path);
98-
if (!f.open(QIODevice::WriteOnly | QIODevice::Truncate))
112+
QSaveFile file(path);
113+
if (!file.open(QIODevice::WriteOnly))
114+
return false;
115+
116+
const QByteArray payload = QJsonDocument(obj).toJson(QJsonDocument::Compact);
117+
if (file.write(payload) != payload.size() || !file.commit())
99118
return false;
100119

101-
f.write(QJsonDocument(obj).toJson(QJsonDocument::Compact));
102120
emit presetsChanged(effectId);
103121
return true;
104122
}

0 commit comments

Comments
 (0)