fix: Fix Chinese password compression failure#351
Merged
pengfeixx merged 1 commit intolinuxdeepin:develop/snipefrom Jan 20, 2026
Merged
fix: Fix Chinese password compression failure#351pengfeixx merged 1 commit intolinuxdeepin:develop/snipefrom
pengfeixx merged 1 commit intolinuxdeepin:develop/snipefrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates zip password handling to use a custom Unicode-to-byte conversion for Chinese characters, ensures the encoded password data remains valid during compression, and replaces regex-based Chinese detection with a direct character-range check, adding logging to help diagnose encoding behavior. Sequence diagram for Chinese password compression and extractionsequenceDiagram
actor User
participant MainWindow
participant LibzipPlugin
participant Libzip
User->>MainWindow: select files and enter password
MainWindow->>MainWindow: slotCompress(val)
MainWindow->>MainWindow: check each QChar in m_stCompressParameter.strPassword
MainWindow->>MainWindow: set zipPasswordIsChinese flag
MainWindow->>LibzipPlugin: start compression with options
LibzipPlugin->>LibzipPlugin: passwordUnicode(options.strPassword, 0)
LibzipPlugin->>Libzip: zip_file_set_encryption(..., passwordBytes.constData)
LibzipPlugin->>Libzip: zip_set_default_password(..., passwordBytes.constData)
Libzip-->>LibzipPlugin: return status
LibzipPlugin-->>MainWindow: compression result
MainWindow-->>User: show compression success or error
Class diagram for updated LibzipPlugin and MainWindow password handlingclassDiagram
class LibzipPlugin {
+QMap<QString,int> m_mapRealDirValue
+QSet<QString> m_setLongName
+bool m_bLnfs
+QByteArray m_passwordData
+PluginFinishType extractFiles(files, options)
+bool writeEntry(archive, entry, options)
+QByteArray passwordUnicode(strPassword, iIndex)
+void setPassword(password)
}
class MainWindow {
+CompressParameters m_stCompressParameter
+void slotCompress(val)
}
class CompressParameters {
+QString strMimeType
+QString strPassword
}
MainWindow "1" *-- "1" CompressParameters
MainWindow ..> LibzipPlugin : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 2 issues, and left some high level feedback:
- The newly added
m_passwordDatamember is never used; either remove it or wire it into the password handling so its purpose (extending password lifetime) is actually fulfilled. - The new
qInfo()calls inwriteEntryandpasswordUnicodelog password length and encoding details, which may leak sensitive information; consider removing or guarding these logs in production builds.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The newly added `m_passwordData` member is never used; either remove it or wire it into the password handling so its purpose (extending password lifetime) is actually fulfilled.
- The new `qInfo()` calls in `writeEntry` and `passwordUnicode` log password length and encoding details, which may leak sensitive information; consider removing or guarding these logs in production builds.
## Individual Comments
### Comment 1
<location> `3rdparty/libzipplugin/libzipplugin.cpp:673-677` </location>
<code_context>
// 设置压缩的加密算法
if (options.bEncryption && !options.strEncryptionMethod.isEmpty()) { //ReadOnlyArchiveInterface::password()
int ret = 0;
+ QByteArray passwordBytes = passwordUnicode(options.strPassword, 0);
+ qInfo() << "Setting encryption for file, method:" << options.strEncryptionMethod
+ << "password length:" << options.strPassword.length()
+ << "encoded bytes length:" << passwordBytes.length();
if (QLatin1String("AES128") == options.strEncryptionMethod) {
- ret = zip_file_set_encryption(archive, uindex, ZIP_EM_AES_128, options.strPassword.toUtf8().constData());
+ ret = zip_file_set_encryption(archive, uindex, ZIP_EM_AES_128, passwordBytes.constData());
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Avoid logging detailed password metadata (lengths) in qInfo for security/privacy reasons.
These `qInfo()` calls log password-related metadata (length and encoded length), and similar logging was added in `passwordUnicode` for Chinese detection. Even without the actual password, this information can aid password guessing and will likely persist in logs. Please remove these logs, or restrict them to a debug-only/very-verbose path that is disabled in production.
Suggested implementation:
```cpp
QByteArray passwordBytes = passwordUnicode(options.strPassword, 0);
if (QLatin1String("AES128") == options.strEncryptionMethod) {
```
The review comment also refers to similar logging added inside `passwordUnicode` for Chinese detection. For full compliance with the security/privacy concern, you should locate that logging in `passwordUnicode` (likely in the same file or related source file) and either:
1. Remove it entirely, or
2. Wrap it in a very-verbose/debug-only mechanism that is disabled in production (e.g., behind a compile-time flag or an environment-variable-guarded `qDebug()`).
Apply the same principle: avoid logging password content or metadata (lengths, encodings) that can persist in logs.
</issue_to_address>
### Comment 2
<location> `3rdparty/libzipplugin/libzipplugin.cpp:1223-1228` </location>
<code_context>
// chinese
if (b) {
+ qInfo() << "Password contains Chinese characters, using codec:" << m_listCodecs[iIndex];
QTextCodec *utf8 = QTextCodec::codecForName("UTF-8");
QTextCodec *gbk = QTextCodec::codecForName(m_listCodecs[iIndex].toUtf8().data());
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Reduce per-password qInfo logging to avoid noisy logs and potential password side-channel leakage.
These `qInfo()` calls in `passwordUnicode` run for every password and log whether it contains Chinese characters and which codec is used. Beyond the security concerns about leaking password characteristics to logs, this will generate a lot of noise when many archives are processed. Please either remove these logs after you finish diagnosing the issue or move them behind a debug-only mechanism that is disabled in normal operation.
```suggestion
// chinese
if (b) {
#ifdef QT_DEBUG
qDebug() << "Password contains Chinese characters, using codec:" << m_listCodecs[iIndex];
#endif
QTextCodec *utf8 = QTextCodec::codecForName("UTF-8");
QTextCodec *gbk = QTextCodec::codecForName(m_listCodecs[iIndex].toUtf8().data());
// QTextCodec *gbk = QTextCodec::codecForName("UTF-8");
QByteArray gb_bytes = gbk->fromUnicode(strUnicode);
return gb_bytes; //获取其char *115645 【专业版】【1060】【归档管理器】【5.12.0.2】无法解压中文密码的zip压缩包（含有长名称）
} else {
#ifdef QT_DEBUG
qDebug() << "Password is non-Chinese, using UTF-8 encoding";
#endif
return strPassword.toUtf8();
}
```
</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
673
to
674
| QByteArray passwordBytes = passwordUnicode(options.strPassword, 0); | ||
| qInfo() << "Setting encryption for file, method:" << options.strEncryptionMethod | ||
| << "password length:" << options.strPassword.length() | ||
| << "encoded bytes length:" << passwordBytes.length(); | ||
| if (QLatin1String("AES128") == options.strEncryptionMethod) { |
There was a problem hiding this comment.
🚨 suggestion (security): Avoid logging detailed password metadata (lengths) in qInfo for security/privacy reasons.
These qInfo() calls log password-related metadata (length and encoded length), and similar logging was added in passwordUnicode for Chinese detection. Even without the actual password, this information can aid password guessing and will likely persist in logs. Please remove these logs, or restrict them to a debug-only/very-verbose path that is disabled in production.
Suggested implementation:
QByteArray passwordBytes = passwordUnicode(options.strPassword, 0);
if (QLatin1String("AES128") == options.strEncryptionMethod) {
The review comment also refers to similar logging added inside passwordUnicode for Chinese detection. For full compliance with the security/privacy concern, you should locate that logging in passwordUnicode (likely in the same file or related source file) and either:
- Remove it entirely, or
- Wrap it in a very-verbose/debug-only mechanism that is disabled in production (e.g., behind a compile-time flag or an environment-variable-guarded
qDebug()).
Apply the same principle: avoid logging password content or metadata (lengths, encodings) that can persist in logs.
Comment on lines
1223
to
1224
| // chinese | ||
| if (b) { | ||
| qInfo() << "Password contains Chinese characters, using codec:" << m_listCodecs[iIndex]; | ||
| QTextCodec *utf8 = QTextCodec::codecForName("UTF-8"); | ||
| QTextCodec *gbk = QTextCodec::codecForName(m_listCodecs[iIndex].toUtf8().data()); | ||
| // QTextCodec *gbk = QTextCodec::codecForName("UTF-8"); |
There was a problem hiding this comment.
🚨 suggestion (security): Reduce per-password qInfo logging to avoid noisy logs and potential password side-channel leakage.
These qInfo() calls in passwordUnicode run for every password and log whether it contains Chinese characters and which codec is used. Beyond the security concerns about leaking password characteristics to logs, this will generate a lot of noise when many archives are processed. Please either remove these logs after you finish diagnosing the issue or move them behind a debug-only mechanism that is disabled in normal operation.
Suggested change
| // chinese | |
| if (b) { | |
| qInfo() << "Password contains Chinese characters, using codec:" << m_listCodecs[iIndex]; | |
| QTextCodec *utf8 = QTextCodec::codecForName("UTF-8"); | |
| QTextCodec *gbk = QTextCodec::codecForName(m_listCodecs[iIndex].toUtf8().data()); | |
| // QTextCodec *gbk = QTextCodec::codecForName("UTF-8"); | |
| // chinese | |
| if (b) { | |
| #ifdef QT_DEBUG | |
| qDebug() << "Password contains Chinese characters, using codec:" << m_listCodecs[iIndex]; | |
| #endif | |
| QTextCodec *utf8 = QTextCodec::codecForName("UTF-8"); | |
| QTextCodec *gbk = QTextCodec::codecForName(m_listCodecs[iIndex].toUtf8().data()); | |
| // QTextCodec *gbk = QTextCodec::codecForName("UTF-8"); | |
| QByteArray gb_bytes = gbk->fromUnicode(strUnicode); | |
| return gb_bytes; //获取其char *115645 【专业版】【1060】【归档管理器】【5.12.0.2】无法解压中文密码的zip压缩包（含有长名称） | |
| } else { | |
| #ifdef QT_DEBUG | |
| qDebug() << "Password is non-Chinese, using UTF-8 encoding"; | |
| #endif | |
| return strPassword.toUtf8(); | |
| } |
Fix Chinese password compression failure Log: Fix Chinese password compression failure
deepin pr auto review这段代码主要针对压缩/解压过程中的密码编码处理进行了改进，特别是针对非ASCII字符（如中文）的处理。以下是对代码的详细审查和改进建议： 1. 语法逻辑审查优点：
问题与建议：
2. 代码质量审查优点：
问题与建议：
3. 代码性能审查优点：
问题与建议：
4. 代码安全审查优点：
问题与建议：
改进后的代码示例
|
lzwind
approved these changes
Jan 20, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, pengfeixx 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 |
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.
Fix Chinese password compression failure
Log: Fix Chinese password compression failure
Summary by Sourcery
Fix ZIP password handling to correctly support Chinese-character passwords during compression and extraction.
Bug Fixes:
Enhancements: