fix(archive): tar.7z use QProcess pipeline instead of shell script#370
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom Mar 13, 2026
Merged
Conversation
- Replace temp script with two QProcess (tar | 7z) to avoid command injection and special chars in password breaking the command - Add CliProperties::getTar7zTarArgs/getTar7z7zArgs for safe argv - Pass 7z password as single "-p"+password to avoid stdin prompt freeze - Add killTar7zPipelineIfActive() and writeToProcess guard in pipeline - cli7zplugin: call killTar7zPipelineIfActive() in killProcess() Log: 修复 tar.7z 使用脚本导致的命令注入与密码特殊字符问题，改为双 QProcess 管道并避免加密时卡死 Bug:https://pms.uniontech.com/bug-view-343119.html
Reviewer's GuideRefactors tar.7z creation to replace the temporary shell script with a safer two-QProcess tar | 7z pipeline, introduces dedicated argument builders for tar/7z, hardens password handling, and adds specific lifecycle and kill logic for the new pipeline while adapting progress/output parsing accordingly. Sequence diagram for new tar_7z QProcess pipeline in addFilessequenceDiagram
actor User
participant Cli7zPlugin
participant CliInterface
participant CliProperties
participant QProcessTar as QProcess_tar
participant QProcess7z as QProcess_7z
User->>Cli7zPlugin: requestAddFiles(options.bTar_7z = true)
Cli7zPlugin->>CliInterface: addFiles(files, options, password)
activate CliInterface
CliInterface->>CliInterface: m_isTar7z = true
CliInterface->>CliInterface: m_filesSize = options.qTotalSize
CliInterface->>CliProperties: getTar7zTarArgs(fileList, renameList)
CliProperties-->>CliInterface: tarArgs(QStringList)
CliInterface->>CliProperties: getTar7z7zArgs(archivePath, password, options.bHeaderEncryption, options.iCompressionLevel, options.strCompressionMethod, options.strEncryptionMethod, options.iVolumeSize)
CliProperties-->>CliInterface: sevenZArgs(QStringList)
CliInterface->>CliInterface: tarPath = findExecutable(tar)
CliInterface->>CliInterface: sevenZPath = findExecutable(addProgram)
CliInterface->>CliInterface: create m_tarProcess, m_tar7z_7z
CliInterface->>QProcessTar: setStandardOutputProcess(m_tar7z_7z)
CliInterface->>QProcess7z: setProcessChannelMode(MergedChannels)
CliInterface->>CliInterface: connect readyReadStandardOutput -> readStdout
CliInterface->>CliInterface: connect finished -> processFinished
CliInterface->>QProcessTar: start(tarPath, tarArgs)
QProcessTar-->>CliInterface: started
CliInterface->>CliInterface: waitForStarted(5000)
alt tar started
CliInterface->>QProcess7z: start(sevenZPath, sevenZArgs)
CliInterface->>CliInterface: m_processId = m_tar7z_7z->processId()
CliInterface->>CliInterface: m_childProcessId = [m_tarProcess->processId()]
CliInterface->>CliInterface: connect errorOccurred -> lambda(error)
else tar failed
CliInterface->>CliInterface: ret = false
CliInterface->>CliInterface: deleteProcess()
end
loop compressionProgress
QProcess7z-->>CliInterface: stdout (progress, messages)
CliInterface->>CliInterface: readStdout(handleAll)
CliInterface->>CliInterface: handleProgress(line)
CliInterface-->>Cli7zPlugin: signalprogress(percentage)
CliInterface-->>Cli7zPlugin: signalCurFileName(name)
end
QProcess7z-->>CliInterface: finished(exitCode, exitStatus)
CliInterface->>CliInterface: processFinished(exitCode, exitStatus)
CliInterface-->>Cli7zPlugin: emitFinished
Cli7zPlugin-->>User: addFiles result
deactivate CliInterface
Sequence diagram for killProcess with tar_7z pipelinesequenceDiagram
actor User
participant Cli7zPlugin
participant CliInterface
participant QProcessTar as QProcess_tar
participant QProcess7z as QProcess_7z
User->>Cli7zPlugin: cancelOperation()
Cli7zPlugin->>Cli7zPlugin: killProcess(emitFinished)
Cli7zPlugin->>CliInterface: killTar7zPipelineIfActive()
alt tar_7z pipeline active
CliInterface->>CliInterface: m_tar7z_7z != nullptr
opt tar still running
CliInterface->>QProcessTar: kill()
end
opt 7z still running
CliInterface->>QProcess7z: kill()
end
CliInterface->>CliInterface: m_isProcessKilled = true
CliInterface-->>Cli7zPlugin: return true
Cli7zPlugin-->>User: cancellationDone
else no tar_7z pipeline
CliInterface-->>Cli7zPlugin: return false
Cli7zPlugin->>Cli7zPlugin: fallback kill using m_process
Cli7zPlugin-->>User: cancellationDone
end
Updated class diagram for CliInterface, CliProperties, and Cli7zPluginclassDiagram
class CliProperties {
+QStringList addArgs(archive, files, password, headerEncryption, compressionLevel, compressionMethod, encryptionMethod, volumeSize, isTar7z, globalWorkDir, renameList)
+QStringList getTar7zTarArgs(files, renameList)
+QStringList getTar7z7zArgs(archive, password, headerEncryption, compressionLevel, compressionMethod, encryptionMethod, volumeSize)
+QStringList commentArgs(archive, commentfile)
+QStringList deleteArgs(archive, files, password)
+QStringList extractArgs(archive, files, preservePaths, password)
-QString m_progressarg
-QString substituteCompressionLevelSwitch(level)
-QString substituteCompressionMethodSwitch(method)
-QString substituteEncryptionMethodSwitch(method)
-QString substituteMultiVolumeSwitch(volumeSize)
}
class CliInterface {
<<abstract>>
+PluginFinishType addFiles(files, options, password)
+bool runProcess(programName, args)
+virtual void killProcess(emitFinished) = 0
+bool killTar7zPipelineIfActive()
-void deleteProcess()
-void handleProgress(line)
-void writeToProcess(data)
-void readStdout(handleAll)
-void processFinished(exitCode, exitStatus)
-CliProperties* m_cliProps
-KPtyProcess* m_process
-QProcess* m_tarProcess
-QProcess* m_tar7z_7z
-QVector~qint64~ m_childProcessId
-qint64 m_filesSize
-bool m_isTar7z
-bool m_isProcessKilled
-QByteArray m_stdOutData
}
class Cli7zPlugin {
+bool isOpenFileFailed(line)
+void killProcess(emitFinished)
-CliInterface* m_interface
}
CliInterface --> CliProperties : uses
Cli7zPlugin ..|> CliInterface : implements via composition
class QProcess {
+void start(program, arguments)
+bool waitForStarted(msecs)
+void setStandardOutputProcess(process)
+void setProcessChannelMode(mode)
+qint64 processId()
+ProcessState state()
+void kill()
+void waitForFinished(msecs)
}
CliInterface --> QProcess : manages
class KPtyProcess {
+QIODevice* pty()
+QStringList program()
+qint64 processId()
+void write(data)
}
CliInterface --> KPtyProcess : manages existing process
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这份代码改动主要针对 以下是对代码的详细审查意见： 1. 语法与逻辑审查1.1 内存管理风险 (严重)在 m_tar7z_7z = new QProcess();
m_tarProcess = new QProcess();问题：这里直接使用了裸指针 1.2 进程启动顺序与状态检查m_tarProcess->start(tarPath, tarArgs);
if (m_tarProcess->waitForStarted(5000)) {
m_tar7z_7z->start(sevenZPath, sevenZArgs);
// ...
ret = m_tar7z_7z->state() == QProcess::Starting || m_tar7z_7z->state() == QProcess::Running;
// ...
}问题：逻辑基本正确，但 1.3 管道断开处理connect(m_tar7z_7z, &QProcess::errorOccurred, this, [this](QProcess::ProcessError) {
if (m_tar7z_7z && m_tar7z_7z->state() == QProcess::NotRunning) {
processFinished(-1, QProcess::CrashExit);
}
});问题：只连接了 7z 进程的 2. 代码质量审查2.1 代码复用与清晰度
2.2 注释新增的注释（如 3. 代码性能审查3.1 字符串操作在 for (QString file : files) {
// ...
if (file.endsWith(QLatin1Char('/'))) {
file.chop(1);
}
// ...
}问题：这里对文件列表进行了遍历和字符串操作。 4. 代码安全审查4.1 密码处理 (优秀)if (!password.isEmpty()) {
// 单参数 -p+密码，避免 7z 再从 stdin 提示输入导致管道卡死
args << (QStringLiteral("-p") + password);
}评价：这是本次改动最大的安全亮点。之前的 Shell 脚本方式容易导致密码泄露到进程列表或环境变量中，或者被 Shell 历史记录捕获。现在直接作为参数传递给 7z，虽然仍可能被 4.2 临时文件处理旧代码生成了临时脚本文件 5. 具体改进建议
总结这次改动显著提高了代码的安全性和健壮性，消除了 Shell 脚本带来的注入风险和转义字符噩梦。 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
CliProperties::getTar7z7zArgstheheaderEncryptionparameter is currently unused even though callers (e.g.CliInterface::addFiles) passoptions.bHeaderEncryption; either incorporate this flag into the generated 7z arguments or remove the parameter to avoid confusion and divergence from previous behavior. Cli7zPlugin::killProcessreturns early afterkillTar7zPipelineIfActive()without callingdeleteProcess(), som_tarProcess/m_tar7z_7zand their signal connections are not cleaned up there; consider centralizing process teardown (including deleting these QProcess instances and disconnecting signals) to prevent leaks and unexpected callbacks after kill.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `CliProperties::getTar7z7zArgs` the `headerEncryption` parameter is currently unused even though callers (e.g. `CliInterface::addFiles`) pass `options.bHeaderEncryption`; either incorporate this flag into the generated 7z arguments or remove the parameter to avoid confusion and divergence from previous behavior.
- `Cli7zPlugin::killProcess` returns early after `killTar7zPipelineIfActive()` without calling `deleteProcess()`, so `m_tarProcess`/`m_tar7z_7z` and their signal connections are not cleaned up there; consider centralizing process teardown (including deleting these QProcess instances and disconnecting signals) to prevent leaks and unexpected callbacks after kill.
## Individual Comments
### Comment 1
<location path="3rdparty/interface/archiveinterface/cliproperties.cpp" line_range="119-121" />
<code_context>
+ return args;
+}
+
+QStringList CliProperties::getTar7z7zArgs(const QString &archive, const QString &password, bool headerEncryption,
+ int compressionLevel, const QString &compressionMethod,
+ const QString &encryptionMethod, int volumeSize)
+{
+ QStringList args;
</code_context>
<issue_to_address>
**issue (bug_risk):** Header-encryption flag is ignored in tar.7z pipeline, potentially changing behavior vs non-tar.7z paths.
`headerEncryption` is accepted but never used here. In the previous flow, `substitutePasswordSwitch(password, headerEncryption)` applied the appropriate 7z switches (e.g. `-mhe=on`). In the tar.7z pipeline that behavior is lost, so headers might not be encrypted when requested. If header encryption is required for tar.7z, this method should add the appropriate 7z switch based on `headerEncryption` to match the non-tar.7z behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+119
to
+121
| QStringList CliProperties::getTar7z7zArgs(const QString &archive, const QString &password, bool headerEncryption, | ||
| int compressionLevel, const QString &compressionMethod, | ||
| const QString &encryptionMethod, int volumeSize) |
There was a problem hiding this comment.
issue (bug_risk): Header-encryption flag is ignored in tar.7z pipeline, potentially changing behavior vs non-tar.7z paths.
headerEncryption is accepted but never used here. In the previous flow, substitutePasswordSwitch(password, headerEncryption) applied the appropriate 7z switches (e.g. -mhe=on). In the tar.7z pipeline that behavior is lost, so headers might not be encrypted when requested. If header encryption is required for tar.7z, this method should add the appropriate 7z switch based on headerEncryption to match the non-tar.7z behavior.
max-lvs
approved these changes
Mar 13, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: LiHua000, max-lvs The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Log: 修复 tar.7z 使用脚本导致的命令注入与密码特殊字符问题，改为双 QProcess 管道并避免加密时卡死
Bug:https://pms.uniontech.com/bug-view-343119.html
Summary by Sourcery
Replace tar.7z compression via temporary shell script with a safer QProcess-based tar→7z pipeline and adjust process handling accordingly.
New Features:
Bug Fixes:
Enhancements: